iked(8): use msg_valid to handle duplicate fragment sequence numbers

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

iked(8): use msg_valid to handle duplicate fragment sequence numbers

Tobias Heider-2
With IKEv2 message fragmentation a message is split into several fragments,
which all have the same sequence number. In the original commit this was
handled with an explicit exception to accept the same sequence number as
before, in case the number of stored fragments is not 0.

Another recent fix introduced the msg_valid variable which is used to
determine whether a received message was valid and the sequence number
should be increased.
This diff gets rid of the exception and sets the valid flag/bumps up the
sequence number only when the final fragment was received and the
reconstructed message was succesfully handled.

ok?

diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index 08bf1ca0a0c..f6148be352b 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -518,8 +518,7 @@ ikev2_recv(struct iked *env, struct iked_message *msg)
  sa_free(env, sa);
  }
  return;
- } else if (sa->sa_msgid_set && msg->msg_msgid == sa->sa_msgid &&
-    !(sa->sa_fragments.frag_count)) {
+ } else if (sa->sa_msgid_set && msg->msg_msgid == sa->sa_msgid) {
  /*
  * Response is being worked on, most likely we're
  * waiting for the CA process to get back to us
@@ -2349,11 +2348,11 @@ ikev2_resp_recv(struct iked *env, struct iked_message *msg,
  if ((sa = msg->msg_sa) == NULL)
  return;
 
- msg->msg_valid = 1;
-
  if (sa->sa_fragments.frag_count !=0)
  return;
 
+ msg->msg_valid = 1;
+
  if (msg->msg_natt && sa->sa_natt == 0) {
  log_debug("%s: NAT-T message received, updated SA", __func__);
  sa->sa_natt = 1;

Reply | Threaded
Open this post in threaded view
|

Re: iked(8): use msg_valid to handle duplicate fragment sequence numbers

Alexander Bluhm
On Mon, Nov 11, 2019 at 06:47:35PM +0100, Tobias Heider wrote:

> With IKEv2 message fragmentation a message is split into several fragments,
> which all have the same sequence number. In the original commit this was
> handled with an explicit exception to accept the same sequence number as
> before, in case the number of stored fragments is not 0.
>
> Another recent fix introduced the msg_valid variable which is used to
> determine whether a received message was valid and the sequence number
> should be increased.
> This diff gets rid of the exception and sets the valid flag/bumps up the
> sequence number only when the final fragment was received and the
> reconstructed message was succesfully handled.
>
> ok?

OK bluhm@

> @@ -2349,11 +2348,11 @@ ikev2_resp_recv(struct iked *env, struct iked_message *msg,
>   if ((sa = msg->msg_sa) == NULL)
>   return;
>
> - msg->msg_valid = 1;
> -
>   if (sa->sa_fragments.frag_count !=0)

could you put a space between != and 0