[PATCH net] wireguard: noise: reject responses for replaced initiations

[PATCH net] wireguard: noise: reject responses for replaced initiations

[Jérémy Jean]

WireGuard can accept an old handshake response after starting a new
handshake. This reinstalls old keys and resets transport counters and
replay state, enabling nonce reuse, replay and packet forgery. This
breaks confidentiality and integrity guarantees.

Compare ephemeral secrets under the write lock to reject responses for
replaced initiations.

Fixes: e7096c131e51 ("net: WireGuard secure network tunnel")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 drivers/net/wireguard/noise.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireguard/noise.c b/drivers/net/wireguard/noise.c
index 9c0a09bf6c95..88cc9acc7dc7 100644
--- a/drivers/net/wireguard/noise.c
+++ b/drivers/net/wireguard/noise.c
@@ -784,10 +784,12 @@ wg_noise_handshake_consume_response(struct message_handshake_response *src,
 
 	/* Success! Copy everything to peer */
 	down_write(&handshake->lock);
-	/* It's important to check that the state is still the same, while we
-	 * have an exclusive lock.
+	/* Check that this is still the initiation we authenticated against,
+	 * while we have an exclusive lock.
 	 */
-	if (handshake->state != state) {
+	if (handshake->state != state ||
+	    crypto_memneq(handshake->ephemeral_private, ephemeral_private,
+			  NOISE_PUBLIC_KEY_LEN)) {
 		up_write(&handshake->lock);
 		goto fail;
 	}
-- 
2.47.3

Re: [PATCH net] wireguard: noise: reject responses for replaced initiations

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] wireguard: noise: reject responses for replaced initiations

[Jason A. Donenfeld]

Hi,

On Mon, Oct 05, 2026 at 08:35:55PM +0000, Jérémy Jean wrote:
> WireGuard can accept an old handshake response after starting a new
> handshake. This reinstalls old keys and resets transport counters and
> replay state, enabling nonce reuse, replay and packet forgery. This
> breaks confidentiality and integrity guarantees.
> 
> Compare ephemeral secrets under the write lock to reject responses for
> replaced initiations.
> 
> Fixes: e7096c131e51 ("net: WireGuard secure network tunnel")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> ---
>  drivers/net/wireguard/noise.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireguard/noise.c b/drivers/net/wireguard/noise.c
> index 9c0a09bf6c95..88cc9acc7dc7 100644
> --- a/drivers/net/wireguard/noise.c
> +++ b/drivers/net/wireguard/noise.c
> @@ -784,10 +784,12 @@ wg_noise_handshake_consume_response(struct message_handshake_response *src,
>  
>  	/* Success! Copy everything to peer */
>  	down_write(&handshake->lock);
> -	/* It's important to check that the state is still the same, while we
> -	 * have an exclusive lock.
> +	/* Check that this is still the initiation we authenticated against,
> +	 * while we have an exclusive lock.
>  	 */
> -	if (handshake->state != state) {
> +	if (handshake->state != state ||
> +	    crypto_memneq(handshake->ephemeral_private, ephemeral_private,
> +			  NOISE_PUBLIC_KEY_LEN)) {
>  		up_write(&handshake->lock);
>  		goto fail;
>  	}

Could you describe the flow that you think causes a bug? Trying
to recreate your mental model. Something like this?

== thread 1 ==

	down_read(&handshake->lock);
	state = handshake->state;
	memcpy(hash, handshake->hash, NOISE_HASH_LEN);
	memcpy(chaining_key, handshake->chaining_key, NOISE_HASH_LEN);
	memcpy(ephemeral_private, handshake->ephemeral_private,
	       NOISE_PUBLIC_KEY_LEN);
	memcpy(preshared_key, handshake->preshared_key,
	       NOISE_SYMMETRIC_KEY_LEN);
	up_read(&handshake->lock);

	if (state != HANDSHAKE_CREATED_INITIATION)
		goto fail;

	/* e */
	message_ephemeral(e, src->unencrypted_ephemeral, chaining_key, hash);

	/* ee */
	if (!mix_dh(chaining_key, NULL, ephemeral_private, e))
		goto fail;

	/* se */
	if (!mix_dh(chaining_key, NULL, wg->static_identity.static_private, e))
		goto fail;

	/* psk */
	mix_psk(chaining_key, hash, key, preshared_key);

	/* {} */
	if (!message_decrypt(NULL, src->encrypted_nothing,
			     sizeof(src->encrypted_nothing), key, hash))
		goto fail;

== thread 2 ==

	down_read(&handshake->static_identity->lock);
	down_write(&handshake->lock);

	if (unlikely(!handshake->static_identity->has_identity))
		goto out;

	dst->header.type = cpu_to_le32(MESSAGE_HANDSHAKE_INITIATION);

	handshake_init(handshake->chaining_key, handshake->hash,
		       handshake->remote_static);

	/* e */
	curve25519_generate_secret(handshake->ephemeral_private);
	if (!curve25519_generate_public(dst->unencrypted_ephemeral,
					handshake->ephemeral_private))
		goto out;
	message_ephemeral(dst->unencrypted_ephemeral,
			  dst->unencrypted_ephemeral, handshake->chaining_key,
			  handshake->hash);

	/* es */
	if (!mix_dh(handshake->chaining_key, key, handshake->ephemeral_private,
		    handshake->remote_static))
		goto out;

	/* s */
	message_encrypt(dst->encrypted_static,
			handshake->static_identity->static_public,
			NOISE_PUBLIC_KEY_LEN, key, handshake->hash);

	/* ss */
	if (!mix_precomputed_dh(handshake->chaining_key, key,
				handshake->precomputed_static_static))
		goto out;

	/* {t} */
	tai64n_now(timestamp);
	message_encrypt(dst->encrypted_timestamp, timestamp,
			NOISE_TIMESTAMP_LEN, key, handshake->hash);

	dst->sender_index = wg_index_hashtable_insert(
		handshake->entry.peer->device->index_hashtable,
		&handshake->entry);

	handshake->state = HANDSHAKE_CREATED_INITIATION;
	ret = true;

out:
	up_write(&handshake->lock);
	up_read(&handshake->static_identity->lock);

== thread 1 ==

	down_write(&handshake->lock);
	/* It's important to check that the state is still the same, while we
	 * have an exclusive lock.
	 */
	if (handshake->state != state) {
		up_write(&handshake->lock);
		goto fail;
	}
	memcpy(handshake->remote_ephemeral, e, NOISE_PUBLIC_KEY_LEN);
	memcpy(handshake->hash, hash, NOISE_HASH_LEN);
	memcpy(handshake->chaining_key, chaining_key, NOISE_HASH_LEN);
	handshake->remote_index = src->sender_index;
	handshake->state = HANDSHAKE_CONSUMED_RESPONSE;
	up_write(&handshake->lock);
	ret_peer = peer;
	goto out;

And now begin_session is called on the older completed handshake rather
than the half-completed newer handshake?

Or did you see some other flow?

Let me know.

Thanks,
Jason

Re: [PATCH net] wireguard: noise: reject responses for replaced initiations

[Jason A. Donenfeld]

On Mon, Oct 05, 2026 at 11:18:33PM +0200, Jason A. Donenfeld wrote:
> Hi,
> 
> On Mon, Oct 05, 2026 at 08:35:55PM +0000, Jérémy Jean wrote:
> > WireGuard can accept an old handshake response after starting a new
> > handshake. This reinstalls old keys and resets transport counters and
> > replay state, enabling nonce reuse, replay and packet forgery. This
> > breaks confidentiality and integrity guarantees.
> > 
> > Compare ephemeral secrets under the write lock to reject responses for
> > replaced initiations.
> > 
> > Fixes: e7096c131e51 ("net: WireGuard secure network tunnel")
> > Cc: stable@vger.kernel.org
> > Assisted-by: LLM
> > Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> > ---
> >  drivers/net/wireguard/noise.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/wireguard/noise.c b/drivers/net/wireguard/noise.c
> > index 9c0a09bf6c95..88cc9acc7dc7 100644
> > --- a/drivers/net/wireguard/noise.c
> > +++ b/drivers/net/wireguard/noise.c
> > @@ -784,10 +784,12 @@ wg_noise_handshake_consume_response(struct message_handshake_response *src,
> >  
> >  	/* Success! Copy everything to peer */
> >  	down_write(&handshake->lock);
> > -	/* It's important to check that the state is still the same, while we
> > -	 * have an exclusive lock.
> > +	/* Check that this is still the initiation we authenticated against,
> > +	 * while we have an exclusive lock.
> >  	 */
> > -	if (handshake->state != state) {
> > +	if (handshake->state != state ||
> > +	    crypto_memneq(handshake->ephemeral_private, ephemeral_private,
> > +			  NOISE_PUBLIC_KEY_LEN)) {
> >  		up_write(&handshake->lock);
> >  		goto fail;
> >  	}
> 
> Could you describe the flow that you think causes a bug? Trying
> to recreate your mental model. Something like this?
> 
> == thread 1 ==
> 
> 	down_read(&handshake->lock);
> 	state = handshake->state;
> 	memcpy(hash, handshake->hash, NOISE_HASH_LEN);
> 	memcpy(chaining_key, handshake->chaining_key, NOISE_HASH_LEN);
> 	memcpy(ephemeral_private, handshake->ephemeral_private,
> 	       NOISE_PUBLIC_KEY_LEN);
> 	memcpy(preshared_key, handshake->preshared_key,
> 	       NOISE_SYMMETRIC_KEY_LEN);
> 	up_read(&handshake->lock);
> 
> 	if (state != HANDSHAKE_CREATED_INITIATION)
> 		goto fail;
> 
> 	/* e */
> 	message_ephemeral(e, src->unencrypted_ephemeral, chaining_key, hash);
> 
> 	/* ee */
> 	if (!mix_dh(chaining_key, NULL, ephemeral_private, e))
> 		goto fail;
> 
> 	/* se */
> 	if (!mix_dh(chaining_key, NULL, wg->static_identity.static_private, e))
> 		goto fail;
> 
> 	/* psk */
> 	mix_psk(chaining_key, hash, key, preshared_key);
> 
> 	/* {} */
> 	if (!message_decrypt(NULL, src->encrypted_nothing,
> 			     sizeof(src->encrypted_nothing), key, hash))
> 		goto fail;
> 
> == thread 2 ==
> 
> 	down_read(&handshake->static_identity->lock);
> 	down_write(&handshake->lock);
> 
> 	if (unlikely(!handshake->static_identity->has_identity))
> 		goto out;
> 
> 	dst->header.type = cpu_to_le32(MESSAGE_HANDSHAKE_INITIATION);
> 
> 	handshake_init(handshake->chaining_key, handshake->hash,
> 		       handshake->remote_static);
> 
> 	/* e */
> 	curve25519_generate_secret(handshake->ephemeral_private);
> 	if (!curve25519_generate_public(dst->unencrypted_ephemeral,
> 					handshake->ephemeral_private))
> 		goto out;
> 	message_ephemeral(dst->unencrypted_ephemeral,
> 			  dst->unencrypted_ephemeral, handshake->chaining_key,
> 			  handshake->hash);
> 
> 	/* es */
> 	if (!mix_dh(handshake->chaining_key, key, handshake->ephemeral_private,
> 		    handshake->remote_static))
> 		goto out;
> 
> 	/* s */
> 	message_encrypt(dst->encrypted_static,
> 			handshake->static_identity->static_public,
> 			NOISE_PUBLIC_KEY_LEN, key, handshake->hash);
> 
> 	/* ss */
> 	if (!mix_precomputed_dh(handshake->chaining_key, key,
> 				handshake->precomputed_static_static))
> 		goto out;
> 
> 	/* {t} */
> 	tai64n_now(timestamp);
> 	message_encrypt(dst->encrypted_timestamp, timestamp,
> 			NOISE_TIMESTAMP_LEN, key, handshake->hash);
> 
> 	dst->sender_index = wg_index_hashtable_insert(
> 		handshake->entry.peer->device->index_hashtable,
> 		&handshake->entry);
> 
> 	handshake->state = HANDSHAKE_CREATED_INITIATION;
> 	ret = true;
> 
> out:
> 	up_write(&handshake->lock);
> 	up_read(&handshake->static_identity->lock);
> 
> == thread 1 ==
> 
> 	down_write(&handshake->lock);
> 	/* It's important to check that the state is still the same, while we
> 	 * have an exclusive lock.
> 	 */
> 	if (handshake->state != state) {
> 		up_write(&handshake->lock);
> 		goto fail;
> 	}
> 	memcpy(handshake->remote_ephemeral, e, NOISE_PUBLIC_KEY_LEN);
> 	memcpy(handshake->hash, hash, NOISE_HASH_LEN);
> 	memcpy(handshake->chaining_key, chaining_key, NOISE_HASH_LEN);
> 	handshake->remote_index = src->sender_index;
> 	handshake->state = HANDSHAKE_CONSUMED_RESPONSE;
> 	up_write(&handshake->lock);
> 	ret_peer = peer;
> 	goto out;
> 
> And now begin_session is called on the older completed handshake rather
> than the half-completed newer handshake?
> 
> Or did you see some other flow?

Okay I think I worked something plausible out:

- two threads begin processing the same response
- the first succeeds. the second gets halfway, when it blocks on taking
  a lock that a queued initiation has taken
- the queued initiation does its thing and resets the state
- the second thread resumes and completes the old handshake and
  reinstalls keys

I'll continue analyzing real world feasibility, but in all cases, thank
you for the patch.

Jason