- 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.
Details
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
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? | |
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. | |
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; |
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! |