Page MenuHomeFreeBSD

net80211: add 256 bit CCMP support
Needs RevisionPublic

Authored by adrian on Tue, Apr 23, 11:29 PM.
Referenced Files
Unknown Object (File)
Wed, May 8, 5:54 PM
Unknown Object (File)
Mon, Apr 29, 8:03 PM
Unknown Object (File)
Fri, Apr 26, 4:53 PM
Unknown Object (File)
Fri, Apr 26, 12:02 PM
Unknown Object (File)
Fri, Apr 26, 3:03 AM
Unknown Object (File)
Fri, Apr 26, 2:59 AM
Unknown Object (File)
Fri, Apr 26, 2:59 AM
Unknown Object (File)
Fri, Apr 26, 2:59 AM

Details

Reviewers
bz
cc
Group Reviewers
wireless
Summary
  • Split the ccmp support into ccmp_128 and ccmp_256 configs
  • use methods to reference the header/trailer size based on the key being used
  • Modify the CCM header to use 8 or 16 byte MIC appropriately.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 57437
Build 54325: arc lint + arc unit

Event Timeline

add missing RSN parsing stuff for hostapd

bz requested changes to this revision.Thu, Apr 25, 3:22 AM
bz added a subscriber: bz.

I haven't reviewed the actual function changes yet; just scrolled through

sys/net80211/ieee80211_crypto_ccmp.c
160

Use the #define please and not magic numbers in this function.

412

Unbalanced ()

This revision now requires changes to proceed.Thu, Apr 25, 3:22 AM

update to compile w/ stack changes

cc requested changes to this revision.Mon, Apr 29, 9:00 PM
cc added a subscriber: cc.
cc added inline comments.
sys/net80211/ieee80211_crypto_ccmp.c
156–157

The "ccm_m" meaning is opaque. Please add comment about this function's usage. And why is it returning int instead of uint32_t?

After reading rfc3610 section-2, M means Number of octets in authentication field, and the values are enumerated as 4, 6, 8, 10, 12, 14, and 16 octets. So I think better to use enum values instead of defined values. However, how to pick up a valid value is not clear to me. Would you please explain how/why the values are chosen?

https://datatracker.ietf.org/doc/html/rfc3610#section-2

249

Because there are multiple/repeated use of the ccmp_get_header_len(key) per-key, suggest use two constant variables in the beginning to make it clear.

Like this:

	int is_mgmt;
	const int h_len = ccmp_get_header_len(key);
	const int t_len = ccmp_get_trailer_len(key);
403–404

Don't need a new line for uint32_t m. Can put it ahead of the next two like this:

	uint32_t m, u_int64_t pn, size_t dlen,
417

I think "now M=3 after the above line calculation for 8-octet MIC". So M=3.

421

This comment looks to be confusing as it missed the description of the values after Encoding. I don't understand where does Adata come from or what does it mean of these 0x01 = L=2, 0x18 = M=8. Please add more explanation about if "the value L=1 is reserved" and if "0x18 = (m << 3) after m = (m - 2) / 2 when the input M is 8".

Also either add spaces between = or no spaces like above.
But don't mix spaces like this 0x01 = L=2.

529

Because there are multiple/repeated use of the ccmp_get_header_len(key) and the ccmp_get_trailer_len(key) per-key, suggest use two constant variables in the beginning.

Like this:

	uint8_t *pos;
	const int h_len = ccmp_get_header_len(key);
	const int t_len = ccmp_get_trailer_len(key);
530–533

Need re-alignment to use less new lines.

Like this:

	ccmp_init_blocks(&ctx->cc_aes, wh, ccmp_get_ccm_m(key),
			 key->wk_keytsc, data_len, b0, aad, b, s0);
679–680

Because there are multiple/repeated use of the ccmp_get_header_len(key) and the ccmp_get_trailer_len(key) per-key, suggest use two constant variables in the beginning.

Like this:

	u_int space;
	const int h_len = ccmp_get_header_len(key);
	const int t_len = ccmp_get_trailer_len(key);
681–683

Need re-alignment to use less new lines.

sys/net80211/ieee80211_hostap.c
1506–1509

Suggest keep the order of CCM-128 first, like this:

	if (w & (1 << IEEE80211_CIPHER_AES_CCM))
		rsn->rsn_ucastcipher = IEEE80211_CIPHER_AES_CCM;
	else if (w & (1 << IEEE80211_CIPHER_AES_CCM_256))
		rsn->rsn_ucastcipher = IEEE80211_CIPHER_AES_CCM_256;
This revision now requires changes to proceed.Mon, Apr 29, 9:00 PM
sys/net80211/ieee80211_hostap.c
1506–1509

This is in priority of preference, so having the "better" ciphers up top is preferred.

sys/net80211/ieee80211_crypto_ccmp.c
421

Oh yes, this is a left-over comment from when i was initially trying to figure out how the CCM blocks were created. Good point, let me go clean this all up and use some less magic numbers here.

Also whilst here, I discovered that the AAD assembly? It has a 2 byte length field in it - but that's not part of the 802.11-2016 or later spec at all. It's actually the two byte length field required by the CCM AAD block!

Also - the AAD format between CCMP and GCMP is shared (and so I may eventually move this routine into ieee80211_crypto.c.) The only difference is whether that two byte length header is used, but as long as what's going on is documented it should be OK!