You are correct. My patch 6/15 and the commit you reference below resolve the same issue with CKN alignment in slightly different ways. There is no need to apply patch 6/15, so please discard it. My apologies for the conflict; I had resolved the issue on an older private branch and didn't notice the issue had been independently resolved before submitting 6/15. BTW, I just tested Michael Braun's fix (commit 61127f16) and it does handle unaligned CKNs correctly. Sincerely, - Mike Siedzik -----Original Message----- From: Jouni Malinen <j@xxxxx> Sent: Wednesday, December 26, 2018 6:27 PM To: Michael Siedzik <msiedzik@xxxxxxxxxxxxxxxxxxx> Cc: hostap@xxxxxxxxxxxxxxxxxxx Subject: Re: [PATCH 06/15] mka: KaY setting Parameter Set Body Length incorrectly On Fri, Mar 02, 2018 at 03:10:54PM -0500, msiedzik@xxxxxxxxxxxxxxxxxxx wrote: > Per IEEE802.1X-2010 Clause 11.11 EAPOL-MKA, each parameter set must be > padded to a multiple of 4 octets. In order for the length of variable > length parameters, such as CAK Name, to be correctly decoded the > parameter set body length must not include the length of null padding > octets. > > When allocating buffer space for the parameter set (e.g., > wpabuf_put()) the padded length must be used. When setting the > 'Parameter set body length' within the parameter set the unpadded length must be used. > > Consider the case were the length of a PSK's CKN is not a multiple of > 4 octets. Currently ieee802_1x_mka_encode_basic_body() will correctly > reserve the padded number buffer bytes. However it will incorrectly > set ieee8021_x_mka_hdr->length and ->length1 to the padded number of > bytes. The receiver will not be able to recover the original CKN > length. > > Note that the hostap will successfully interoperate with itself > because both sides incorrectly calculate CKN length. The problem is > only seen when interoperating with non-hostap KaY's. This conflicts with an earlier proposed patch that covers at least some of these cases: https://w1.fi/cgit/hostap/commit/?id=61127f162a3fdbe3b284d4c32a8352493e43036b Some of the changes are identical, but I'm not sure how exactly this patch 6/15 or the followup patches 1/2 (mka: ICV not encoded correctly, causing receive side to drop MKPDU) and 2/2 (mka: Optional ICV indicator encoded when not necessary) should be handled. I do now have automated test cases for testing MACsec with MKA PSK mode using the hwsim test framework (tests/hwsim/test_macsec.py). This allows testing between two wpa_supplicant instances (including option to test two different hostap.git snaphots for checking against some regressions). However, I do not have any other implementations available to test for interoperability, so I'm a bit hesitant on doing this type of merges of patches without someone else clearly indicating that the changes look fine and that they have been tested with other implementations. In other words, I'd prefer to get an updated version of this patch and the followup 1/2 that fixes an issue here and all the changes rebased on top of the current hostap.git master branch snapshot. -- Jouni Malinen PGP id EFC895FA _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
