From: tobhe <tobhe@openbsd.org>
Date: Fri, 12 Nov 2021 14:18:54 +0000 (+0000)
Subject: Refactor order of checks when handling IKEv2 message fragments.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=b0cc51058c8e5609a47ff3b17c48c8d495a8643f;p=openbsd

Refactor order of checks when handling IKEv2 message fragments.
Only modify SA after sucessful ikev2_msg_decrypt().

ok patrick@
---

diff --git a/sbin/iked/ikev2_pld.c b/sbin/iked/ikev2_pld.c
index d1a8fee33f5..1c2ed558c07 100644
--- a/sbin/iked/ikev2_pld.c
+++ b/sbin/iked/ikev2_pld.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ikev2_pld.c,v 1.118 2021/09/01 15:30:06 tobhe Exp $	*/
+/*	$OpenBSD: ikev2_pld.c,v 1.119 2021/11/12 14:18:54 tobhe Exp $	*/
 
 /*
  * Copyright (c) 2019 Tobias Heider <tobias.heider@stusta.de>
@@ -1629,6 +1629,22 @@ ikev2_pld_ef(struct iked *env, struct ikev2_payload *pld,
 	log_debug("%s: Received fragment: %zu of %zu",
 	     __func__, frag_num, frag_total);
 
+	/* Drop fragment if frag_num and frag_total don't match */
+	if (frag_num > frag_total)
+		goto done;
+
+        /* Decrypt fragment */
+	if ((e = ibuf_new(buf, len)) == NULL)
+		goto done;
+
+	if ((e = ikev2_msg_decrypt(env, msg->msg_sa, msg->msg_data, e))
+	    == NULL ) {
+		log_debug("%s: Failed to decrypt fragment: %zu of %zu",
+		    __func__, frag_num, frag_total);
+		goto done;
+	}
+	elen = ibuf_length(e);
+
 	/* Check new fragmented message */
 	if (sa_frag->frag_arr == NULL) {
 		sa_frag->frag_arr = recallocarray(NULL, 0, frag_total,
@@ -1638,32 +1654,20 @@ ikev2_pld_ef(struct iked *env, struct ikev2_payload *pld,
 			goto done;
 		}
 		sa_frag->frag_total = frag_total;
-	}
-
-	/* Drop all fragments if frag_num or frag_total don't match */
-	if (frag_num > sa_frag->frag_total || frag_total != sa_frag->frag_total)
-		goto dropall;
+	} else {
+		/* Drop all fragments if frag_total doesn't match previous */
+		if (frag_total != sa_frag->frag_total)
+			goto dropall;
 
-	/* Silent drop if fragment already stored */
-	if (sa_frag->frag_arr[frag_num-1] != NULL)
-		goto done;
+		/* Silent drop if fragment already stored */
+		if (sa_frag->frag_arr[frag_num-1] != NULL)
+			goto done;
+	}
 
 	/* The first fragments IKE header determines pld_nextpayload */
 	if (frag_num == 1)
 		sa_frag->frag_nextpayload = pld->pld_nextpayload;
 
-        /* Decrypt fragment */
-	if ((e = ibuf_new(buf, len)) == NULL)
-		goto done;
-
-	if ((e = ikev2_msg_decrypt(env, msg->msg_sa, msg->msg_data, e))
-	    == NULL ) {
-		log_debug("%s: Failed to decrypt fragment: %zu of %zu",
-		    __func__, frag_num, frag_total);
-		goto done;
-	}
-	elen = ibuf_length(e);
-
 	/* Insert new list element */
 	el = calloc(1, sizeof(struct iked_frag_entry));
 	if (el == NULL) {