dalek-cryptography / merlin Goto Github PK

The lib.rs shouldn't have the bulk of the implementation; it should be split into modules.

If protocol implementation authors follow the advice on how to use Merlin, it will be exposed to their API clients. This means that the docs for the Transcript struct need to be aimed both at the end-users of cryptographic protocols, as well as implementors.

So the Transcript docs should start by having end-user docs (how to create a transcript and pass it to some protocol) and leave the documentation on how to use it to implement protocols for later.

In the current doc on Transcript Protocol:

"A sufficient condition for the transcript to be parseable is that the labels should be distinct and none should be a prefix of any other."

"parseable" is only relevant with debug feature on, right?

I don't quite follow why those labels needs to be prefix-free of each other.

Appreciate the clarification in advance!

Merlin should provide a convenience API for writing integers, which handles the work of doing little-endian encodings, so that API consumers don't need to do that themselves.

The label parameters are supposed to be hardcoded tags, not runtime data (since their purpose is to ensure that the transcript is parseable). So they should have &'static lifetimes, to enforce this using the type system, and prevent misusing the API by feeding runtime-variable data into the tag field.

I think 0.3 is basically a 1.0 prerelease.

Stabilization checklist:

• have some people look at the design
• use it for some stuff to make sure there's not a bad API
• bikeshed the function naming a little bit
• fix #22

Hello, I noticed that the CI checks failed on a PR I made to add a CONTRIBUTING.md. Since I had made no changes to the code, I looked into it to see what was going on.

The stable and nightly cargo test and cargo fmt checks passed, but the no_std compatibility check failed:

$ cargo nono check --no-default-features --features nightly

error: no such subcommand: `nono`

	Did you mean `doc`?

The command "cargo nono check --no-default-features --features nightly" exited with 101.

I believe that the issue is that cargo-nono failed to compile. I opened a PR to fix this #50 .

Rather than just checking the STROBE implementation using the TestTranscript code, there should be conformance tests in the strobe module.

Currently the protocol label is passed to the STROBE constructor, so the STROBE state is initialized with the STROBE domain separator, and then the user-supplied protocol label.

But this means there's no domain separator for the Merlin version.

Hey!

We're currently running into the following (as expected) on big-endian machines:

   |
10 | / compile_error!(
11 | |     r#"
12 | | This crate doesn't support big-endian targets, since I didn't
13 | | have one to test correctness on.  If you're seeing this message,
14 | | please file an issue!
15 | | "#
16 | | );
   | |__^

error: aborting due to previous error

error: could not compile `merlin`.

This would make it compatible with curve25519-dalek-2.0.0-alpha.1.

It would be nice to have a build-time feature that dumped the transcript.

I was confused when I encountered

/// Strobe R value; security level 128 is hardcoded
const STROBE_R: u8 = 166;

R=166 means that 166 bytes=1328 bits can be extracted (the rate) from the Keccak state. That leaves 34 bytes=272 bits of sponge capacity. These numbers seem weird to me; especially since intuition would tell me that 128 bits security would mean 32 bytes of capacity and thus a rate of 168 bytes (or even c=16 bytes and r=184 bytes).

Am I forgetting something? I'm not a Keccak expert!

Because the keccak crate takes a state as [u64; 25] (cf RustCrypto/sponges#3), the code currently does a transmute to convert from bytes.

This should be replaced with a pointer cast on little-endian machines.

On big-endian machines, a byteswap is probably necessary, but since I don't have one to test on and I'm not sure that the keccak crate works correctly there, it seems better to just fail to compile rather than provide possibly-wrong code.

The keccakf crate doesn't operate on a state composed of bytes, it operates on a state composed of words. (cf #11, #20). This is currently handled by doing

fn transmute_state(st: &mut [u8; 200]) -> &mut [u64; 25] {
    unsafe { &mut *(st as *mut [u8; 200] as *mut [u64; 25]) }
}

Unfortunately this isn't quite right because the second pointer has tighter alignment requirements than the first pointer, which isn't guaranteed to have u64 alignment.

In retrospect, the transcript functions could have been better-named. For 1.1, I'm planning to rename them and leave the old ones in place as #[deprecated] functions.

Renaming:

• commit_bytes -> append_message (avoids concept collision between the transcript layer and some protocol's commitments)
• commit_witness_bytes -> rekey_with_witness_bytes (makes rekeying explicit)

version 3 of the merlin crate is merlin-ng effectively. I think this crate should be called merlin-og? or we could ask him nicely to yank and republish under merlin-ng... this is the joys of gits decentralisation meeting crates.io's centralisation...

The current documentation is oriented at explaining both how the API works, as well as how the construction is designed. Now that the design documents are split out into https://merlin.cool, the Rust docs can just be API docs.

In your excellent Medium article you speak about "Interactive composition of noninteractive proofs".

I was thinking about a scenario where Client and Server are communicating with a significant delay (days to weeks), in which case in might make sense to "suspend" the current protocol and application to storage, effectively serializing the internal state in between communication.

Does this make sense from the security standpoint? I would assume it comes with some disadvantages (like reuse of the suspended state?)

If it does make sense to you, would you accept a pull request for (de)serializing the state? I assume that STROBE also needs modification to support this.