The root cause of most of these issues is assuming knowledge of the WireGuard whitepaper¹. This is an oversight; if we assume the reader knows the WireGuard whitepaper, we should say so and we shouldn't require users to read the WG whitepaper in the first place.

The other thing to know is that Rosenpass is structured like WireGuard -- it is designed to act like a VPN protocol but in practice we just use it to send a key to WireGuard; this leads to some oddities like the present but barely-used live transport data feature.

• mac, cookie -- Message integrity checking and proof-of-ip-ownership. See the WireGuard whitepaper section 5.4.4; Rosenpass whitepaper² figure 3 and Envelope::seal in the code. Rosenpass currently does not implement the proof-of-ip-ownership mechanism and always leaves the cookie-field empty
• ini_enc, res_enc -- These should be called txki and txkr (transmission key receiver/responder), which is another oversight. They are used to send transport data. The rosenpass application just outputs a key so there is no payload; these are still used to abort retransmissions in case of packages loss.
• Data & CookieReply -- These would be used if we implemented payload transmission and proof-of-ip-ownership. Leaving them orphaned like this was a bad practice.

Section 2.4.4 covers some variables which are not described and used in the handshake?

That actually was a deliberate decision. These variables are implied by the naming scheme; maybe we should make it more obvious that they are defined but unused.

What is the purpose of the enter_live helper function mentioned in section 2.5?
It appears also not to be used?

On the contrary; it is used to enter the live transport session; Rosenpass does not implement live transport sessions but this is still needed to send the EmptyData packet that terminates the initiators attempts to retransmit the InitConf package in case of package loss.

See Rosenpass Whitepaper Fig. 4 instructions ICI7 and ICR7.

How are the roles of initiator / responder assigned?

They are not assigned; both peers can be initiator and responder at the same time; such a situation results in two handshakes being executed at the same time. This is actually necessary to avoid denial of service attacks on the protocol level since without having executed the handshake neither party can be sure the other is legitimate. This is tricky to get right; the current rust implementation has a race condition due to this; I need to spend some time figuring out how to fix this.

Hi @koraa, and @Mullana

thanks for the very detailed explanations 😃

I've seen that you already updated the figures of the whitepaper. I have two questions about the changes in Figure 2:

• Was the removal of the PRF label biscuit_ad intended?
• Also the PRF label mix has been removed for the hashing of sidr / sidi in the beginning of RespHello / InitConf phases.

Thanks a lot :)

ini_enc, res_enc -- These should be called txki and txkr (transmission key receiver/responder), which is another oversight.

Should we maybe rename those as well in the figure?

Was the removal of the PRF label biscuit_ad intended?
Also the PRF label mix has been removed for the hashing of sidr / sidi in the beginning of RespHello / InitConf phases.

Hi @stv0g
That was my mistake. I accidentally used an old version. But it's already fixed!

Oh you are right. Sorry, I was confused by GitHubs image-diffing feature..

ini_enc, res_enc -- These should be called txki and txkr (transmission key receiver/responder), which is another oversight.

Should we maybe rename those as well in the figure?

I think we should!

@Mullana Its not urgent at all :)

Hi @koraa,

I added a little status section to the description of this ticket so we can track the progress.

There is another thing, which should probably be added to the whitepaper:

Endianess of biscuit counter

It is currently not defined but is essential for a working implementation.
Unfortunately, I am lacking the Rust skills to figure out myself the endianess.

@stv0g Thank you!

Everything is currently in little endian. And wonderful idea to add the status section.

Figure 4: Wrong cipher-text variable

In Figure 4, Step IHR5 it should probably say decaps_and_mix<SKEM>(sskr, spkr, sctr) instead of decaps_and_mix<SKEM>(sskr, spkr, ct1)

@koraa Sorry for all the spam here. These are just peer-review comments about the whitepaper. Nothing critical or urgent. I just want to keep track of my comments so we can improve the next version of the white paper 😃

Section 2.4.1: Better naming for session/index table

Section 2.4.1 mentions a global table for keeping track of ongoing session:

index – A lookup table mapping the session ID to the ongoing initiator handshake
or live session

I am wondering where this naming comes from. I think something like sessions would be more appropriate here.

load_biscuit: Inverted assertions for replay detection

The pseudo code for load_biscuit() includes the following assert statement:

assert(pt.biscuit_no ≤ peer.biscuit_used);

Shouldnt it be the opposite? E.g.:

assert(pt.biscuit_no > peer.biscuit_used);

The number in the biscuit must be larger than any previously seen number.

Step ICR5 of Figure 4 actually makes the correct assertion.
Maybe I am missing a point here. But why are the assertion made twice?

Edit: Looking at the implementation, the assertions seem to be correct. It works with them as in the whitepaper. However, I will need to put in some more effort to understand this. I guess its related to the replay vs. retransmission detection.

Figure 3: Wrong PRF labels for session encryption keys

The whitepaper uses the following PRF labels for deriving the session encryption keys:

• ini_enc: initiator session encryption
• res_enc: responder session encryption

However, the Rust implementation uses:

• ini_enc: initiator handshake encryption
• res_enc: responder handshake encryption

See:

Section 2.3: Wrong protocol identifier

The whitepaper uses the following string as the protocol identifier

PROTOCOL = "rosenpass 1 rosenpass.eu aead=chachapoly1305 hash=blake2s
↪ ekem=kyber512 skem=mceliece460896 xaead=xchachapoly1305"

However, the Rust implementation uses:

Section 2.1.1: Wrong hash function?

Section 2.1.1 says:

As keyed hash function we use the HMAC construction [12] with BLAKE2s [14] as the inner hash function.

However the Rust implementation used BLAKE2b:

Section 2.1.1: Describe non-standard HMAC-Blake2 variant

I fallen into the trap to assume that the key of the inner Blake2 hash-function for the HMAC calculation will be set to zero like Wireguard is doing it:

https://github.com/WireGuard/wireguard-go/blob/052af4a8072bbbd3bfe7edf46fe3c1b350f71f08/device/noise-helpers.go#L24-L31

However, Rosenpass seems to take another approach and uses the ipad/opad xor-ed key (temp_key in the code) as the key for the inner hash functions:

The white paper does not motivate why this is done. I would argue that the HMAC implementation of Rosenpass is not following RFC 2104 as referenced in ¹.

Interestingly, also Jason Donenfeld is arguing that HMAC-Blake2s is not that useful?

https://lore.kernel.org/lkml/20220111181037.632969-2-Jason@zx2c4.com/

Figure 3 / Section 2.5: Wrong order of mixing in en/decaps_and_mix()

The white paper mixes in the following order:

1. pk
2. ct
3. shk

The Rust implementation mixes in the following:

1. pk
2. shk
3. ct

See:

Figure 2: Wrong field ordering in RespHello message

The white paper defines the order as follows:

1. sidr
2. sidi
3. ecti
4. scti
5. biscuit
6. auth

The Rust code implements it as

1. sidr
2. sidi
3. ecti
4. scti
5. biscuit
6. auth

Note that 5. and 6. are swapped:

Section 2.1.1: Describe non-standard HMAC-Blake2 variant

I fallen into the trap to assume that the key of the inner Blake2 hash-function for the HMAC calculation will be set to zero like Wireguard is doing it:

https://github.com/WireGuard/wireguard-go/blob/052af4a8072bbbd3bfe7edf46fe3c1b350f71f08/device/noise-helpers.go#L24-L31

However, Rosenpass seems to take another approach and uses the ipad/opad xor-ed key (temp_key in the code) as the key for the inner hash functions:

rosenpass/src/sodium.rs

Lines 251 to 266 in b29720b

	pub fn hmac_into(out: &mut [u8], key: &[u8], data: &[u8]) -> Result<()> {
	// Not bothering with padding; the implementation
	// uses appropriately sized keys.
	ensure!(key.len() == KEY_SIZE);
	const IPAD: [u8; KEY_SIZE] = [0x36u8; KEY_SIZE];
	let mut temp_key = [0u8; KEY_SIZE];
	temp_key.copy_from_slice(key);
	xor_into(&mut temp_key, &IPAD);
	let outer_data = mac(&temp_key, data)?;
	const OPAD: [u8; KEY_SIZE] = [0x5Cu8; KEY_SIZE];
	temp_key.copy_from_slice(key);
	xor_into(&mut temp_key, &OPAD);
	mac_into(out, &temp_key, &outer_data)
	}

The white paper does not motivate why this is done. I would argue that the HMAC implementation of Rosenpass is not following RFC 2104 as referenced in 1.

Interestingly, also Jason Donenfeld is arguing that HMAC-Blake2s is not that useful?

https://lore.kernel.org/lkml/20220111181037.632969-2-Jason@zx2c4.com/

Footnotes

1. Dr. Hugo Krawczyk, Mihir Bellare, and Ran Canetti. HMAC: Keyed-Hashing for
   Message Authentication. RFC 2104. Feb. 1997. DOI: 10.17487/RFC2104. https:
   //www.rfc-editor.org/info/rfc2104 (cit. on p. 4). [leftwards_arrow_with_hook](#user-content-fnref-12-fb4d902f8b48d5eac583aa77e22018e2)

The entire situation surrounding HMAC is a mess…I'll tell you some time in detail; it's better as an anecdote than as a technical comment. Right now the assumption is that rosenpass does the same thing WireGuard does; although I haven't consulted the WireGuard source code myself. I'll do a deep dive when I apply your comments to check this.

Good catch!

Hi @koraa,

do you mind if I split up this issue into multipLe issues for addressing, discussing each individual inconsistency?
Or do you prefer to have it all in one?

Hi @koraa,

do you mind if I split up this issue into multipLe issues for addressing, discussing each individual inconsistency? Or do you prefer to have it all in one?

I kind of like having it in one issue, I'd wait until this commit settles and then address all issues in one go xD.
We could also convert this into a project though.

Section 2.4.3: Mismatching biscuit key epoch

The white paper says:

The biscuit_key used to encrypt biscuits should be rotated every two minutes.

The Rust implementation uses an epoch of 5 minutes:

Section 2.1.4 / 2.1.5: Add references to exact versions of KEM specifications which are used by Rosenpass

Currently, the whitepaper does not say which version of Classic McEliece nor Kyber is used.

In the case of Classic McElience, the Rust implementation of Rosenpass currently uses liboqs which implements and older version of the KEM (Round 3) while the latest version of the KEM is specified by the NIST PQC round 4 submission.

I assume that there are also different version of Kyber in existance. So we should be more precise in saying which version of the KEMs must be used.

Figure 3: Indicate when responder/initiator are authenticated

I think Figure 3 would be a great place to show from which step onwards responder/initiators are authenticated. This is currently only visualized in Figure 1 in which we are lacking the implementation details.

Some nits from @wucke13 and @emilengler

From #108

@wucke13 and I just went through the whitepaper together and found some slight issues, which are as follows:

• s/private/secret/g
• s/server/responder/g
• Renaming pidiC to pidict to imply its ciphertext nature
• What do the results in the Global Domain diagram mean? Are they hash functions, are they hash inputs?
  • Consider expanding the legend; explain what > and >> mean a bit further

@Mullana I am adding some TODOs for you in the issue description; please give me some time to finalize the changes I am making before actually starting on any of these TODOs

@stv0g I applied most of your suggestions; I decided not to apply the rest of the suggestions or to do something else entirely (such as migrating to SHA3).

Further work is tracked in #136