diff mbox series

mka: Revert commits that break backwards compatibility

Message ID 20210921084651.1521-1-samuel.varley@alliedtelesis.co.nz
State New
Headers show
Series mka: Revert commits that break backwards compatibility | expand

Commit Message

Samuel Varley Sept. 21, 2021, 8:46 a.m. UTC
There is a problem in IEEE Std 802.1X-2010 with how it says the key
server should check for pending packet number exhaustion. It says, in
section 9.8, that the key server should check the LLPN (Latest Lowest
Acceptable Packet Number) that is sent by each device to see if it's
getting too high. However, when the CP state machine is in RETIRE, the
key in use is considered the "old" key and there is no "latest" key.
Therefore, only looking at LLPN won't work.

As described in [1], there is more than one way that a software
developer might deal with that problem. It says the standard should
have specified that the key server look at OLPN, rather than specifying
that it look at LLPN. However, it also describes some alternative
approaches that ensure the most recent key stays in the "latest" slot.

The proposed resolution in [1] aims to increase interoperability between
implementations that have already taken different approaches. It
proposed changes to the standard's description of how the key server
should look at packet numbers in "SAK Use". The new text lays out a
procedure that will work regardless of whether the most recent key is
in "latest" or "old". Those proposed changes were included in
802.1Xck-2018 (meaning 802.1X-2020 also contains the new procedure).

In version 2.9 of this code, the CP state machine transferred the key
from "latest" to "old" in a different place to that specified in
802.1X-2010. The standard says to do it in RETIRE but this code did it
in RECEIVE. This is similar to one of the alternative approaches
mentioned in [1].

Since then, patches have been delivered that do the following things:
* Make the CP state machine match the standard with regard to where it
  transfers the key from "latest" to "old" [2].
* Ensure packet numbers on the local device are still monitored
  correctly after the change to the state machine [3].
* Implement the new procedure, that was specified in [1], for looking
  at packet numbers in "SAK Use" from peers [4].

I expect that the change to the state machine will break backwards
compatibility with v2.9. If the key server runs v2.9 then it will only
look at LLPN and if its peer uses the current code then it will not
have a "latest" key when in the RETIRE state.

Therefore, this commit reverts [2] and [3]. Commit [4] is retained.

[1] M. Seaman. (2017, September 16) "MKA pending PN exhaustion" (revision 1.0) [Online].
    Available: https://grouper.ieee.org/groups/802/1/files/public/docs2017/xck-seaman-mka-pn-exhaustion-0917-v1.pdf

[2] commit 0fedfba2e209 ("mka: Change RECEIVE and RETIRE states to match the standard")
[3] commit 84851007d9b5 ("mka: Check OLPN for exhaustion on SAKuse encode")
[4] commit 6d3dc9ba1ee0 ("mka: Check OLPN for exhaustion on SAKuse decode")

Signed-off-by: Samuel Varley <samuel.varley@alliedtelesis.co.nz>
---
 src/pae/ieee802_1x_cp.c  | 26 ++++++++++++++++----------
 src/pae/ieee802_1x_kay.c | 35 +++++++++++------------------------
 2 files changed, 27 insertions(+), 34 deletions(-)
diff mbox series

Patch

diff --git a/src/pae/ieee802_1x_cp.c b/src/pae/ieee802_1x_cp.c
index cf41d8dbf2f9..91f5d877036f 100644
--- a/src/pae/ieee802_1x_cp.c
+++ b/src/pae/ieee802_1x_cp.c
@@ -230,6 +230,18 @@  SM_STATE(CP, SECURED)
 SM_STATE(CP, RECEIVE)
 {
 	SM_ENTRY(CP, RECEIVE);
+	/* RECEIVE state machine not keep with Figure 12-2 in
+	 * IEEE Std 802.1X-2010 */
+	if (sm->oki) {
+		ieee802_1x_kay_delete_sas(sm->kay, sm->oki);
+		os_free(sm->oki);
+	}
+	sm->oki = sm->lki;
+	sm->oan = sm->lan;
+	sm->otx = sm->ltx;
+	sm->orx = sm->lrx;
+	ieee802_1x_kay_set_old_sa_attr(sm->kay, sm->oki, sm->oan,
+				       sm->otx, sm->orx);
 
 	sm->lki = os_malloc(sizeof(*sm->lki));
 	if (!sm->lki) {
@@ -325,23 +337,17 @@  SM_STATE(CP, ABANDON)
 SM_STATE(CP, RETIRE)
 {
 	SM_ENTRY(CP, RETIRE);
+	/* RETIRE state machine not keep with Figure 12-2 in
+	 * IEEE Std 802.1X-2010 */
 	if (sm->oki) {
 		ieee802_1x_kay_delete_sas(sm->kay, sm->oki);
 		os_free(sm->oki);
 		sm->oki = NULL;
 	}
-	sm->oki = sm->lki;
-	sm->otx = sm->ltx;
-	sm->orx = sm->lrx;
-	sm->oan = sm->lan;
+	sm->orx = false;
+	sm->otx = false;
 	ieee802_1x_kay_set_old_sa_attr(sm->kay, sm->oki, sm->oan,
 				       sm->otx, sm->orx);
-	sm->lki = NULL;
-	sm->ltx = false;
-	sm->lrx = false;
-	sm->lan = 0;
-	ieee802_1x_kay_set_latest_sa_attr(sm->kay, sm->lki, sm->lan,
-					  sm->ltx, sm->lrx);
 }
 
 
diff --git a/src/pae/ieee802_1x_kay.c b/src/pae/ieee802_1x_kay.c
index 2fe88ac0c5f2..542f71d1e33b 100644
--- a/src/pae/ieee802_1x_kay.c
+++ b/src/pae/ieee802_1x_kay.c
@@ -1287,7 +1287,7 @@  ieee802_1x_mka_encode_sak_use_body(
 	struct ieee802_1x_mka_sak_use_body *body;
 	struct ieee802_1x_kay *kay = participant->kay;
 	unsigned int length;
-	u32 olpn, llpn;
+	u32 pn = 1;
 
 	length = ieee802_1x_mka_get_sak_use_length(participant);
 	body = wpabuf_put(buf, length);
@@ -1307,31 +1307,18 @@  ieee802_1x_mka_encode_sak_use_body(
 
 	/* data delay protect */
 	body->delay_protect = kay->mka_hello_time <= MKA_BOUNDED_HELLO_TIME;
-	/* lowest accept packet numbers */
-	olpn = ieee802_1x_mka_get_lpn(participant, &participant->oki);
-	body->olpn = host_to_be32(olpn);
-	llpn = ieee802_1x_mka_get_lpn(participant, &participant->lki);
-	body->llpn = host_to_be32(llpn);
-	if (participant->is_key_server) {
-		/* The CP will spend most of it's time in RETIRE where only
-		 * the old key is populated. Therefore we should be checking
-		 * the OLPN most of the time.
-		 */
-		if (participant->lrx) {
-			if (llpn > kay->pn_exhaustion) {
-				wpa_printf(MSG_WARNING,
-					   "KaY: My LLPN exhaustion");
-				participant->new_sak = true;
-			}
-		} else {
-			if (olpn > kay->pn_exhaustion) {
-				wpa_printf(MSG_WARNING,
-					   "KaY: My OLPN exhaustion");
-				participant->new_sak = true;
-			}
-		}
+	/* lowest accept packet number */
+	pn = ieee802_1x_mka_get_lpn(participant, &participant->lki);
+	if (pn > kay->pn_exhaustion) {
+		wpa_printf(MSG_WARNING, "KaY: My LPN exhaustion");
+		if (participant->is_key_server)
+			participant->new_sak = true;
 	}
 
+	body->llpn = host_to_be32(pn);
+	pn = ieee802_1x_mka_get_lpn(participant, &participant->oki);
+	body->olpn = host_to_be32(pn);
+
 	/* plain tx, plain rx */
 	body->ptx = !kay->macsec_protect;
 	body->prx = kay->macsec_validate != Strict;