developer | 70180b0 | 2023-11-14 17:01:47 +0800 | [diff] [blame] | 1 | The code for hostapd-mbedtls did not work when used for OWE association. |
| 2 | |
| 3 | When handling association requests, the buffer offsets and length assumptions were incorrect, leading to never calculating the y point, thus denying association. |
| 4 | |
| 5 | Also when crafting the association response, the buffer contained the trailing key-type. |
| 6 | |
| 7 | Fix up both issues to adhere to the specification and make hostapd-mbedtls work with the OWE security type. |
| 8 | |
| 9 | --- a/src/crypto/crypto_mbedtls.c |
| 10 | +++ b/src/crypto/crypto_mbedtls.c |
| 11 | @@ -2299,25 +2299,30 @@ struct crypto_ecdh * crypto_ecdh_init2(i |
| 12 | struct wpabuf * crypto_ecdh_get_pubkey(struct crypto_ecdh *ecdh, int inc_y) |
| 13 | { |
| 14 | mbedtls_ecp_group *grp = &ecdh->grp; |
| 15 | - size_t len = CRYPTO_EC_plen(grp); |
| 16 | + size_t prime_len = CRYPTO_EC_plen(grp); |
| 17 | + size_t output_len = prime_len; |
| 18 | + u8 output_offset = 0; |
| 19 | + u8 buf[256]; |
| 20 | + |
| 21 | #ifdef MBEDTLS_ECP_MONTGOMERY_ENABLED |
| 22 | /* len */ |
| 23 | #endif |
| 24 | #ifdef MBEDTLS_ECP_SHORT_WEIERSTRASS_ENABLED |
| 25 | - if (mbedtls_ecp_get_type(grp) == MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS) |
| 26 | - len = inc_y ? len*2+1 : len+1; |
| 27 | + if (mbedtls_ecp_get_type(grp) == MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS) { |
| 28 | + output_len = inc_y ? prime_len * 2 + 1 : prime_len + 1; |
| 29 | + output_offset = 1; |
| 30 | + } |
| 31 | #endif |
| 32 | - struct wpabuf *buf = wpabuf_alloc(len); |
| 33 | - if (buf == NULL) |
| 34 | + |
| 35 | + if (output_len > sizeof(buf)) |
| 36 | return NULL; |
| 37 | + |
| 38 | inc_y = inc_y ? MBEDTLS_ECP_PF_UNCOMPRESSED : MBEDTLS_ECP_PF_COMPRESSED; |
| 39 | - if (mbedtls_ecp_point_write_binary(grp, &ecdh->Q, inc_y, &len, |
| 40 | - wpabuf_mhead_u8(buf), len) == 0) { |
| 41 | - wpabuf_put(buf, len); |
| 42 | - return buf; |
| 43 | + if (mbedtls_ecp_point_write_binary(grp, &ecdh->Q, inc_y, &output_len, |
| 44 | + buf, output_len) == 0) { |
| 45 | + return wpabuf_alloc_copy(buf + output_offset, output_len - output_offset); |
| 46 | } |
| 47 | |
| 48 | - wpabuf_free(buf); |
| 49 | return NULL; |
| 50 | } |
| 51 | |
| 52 | @@ -2379,10 +2384,7 @@ struct wpabuf * crypto_ecdh_set_peerkey( |
| 53 | os_memcpy(buf+2, key, len); |
| 54 | } |
| 55 | len >>= 1; /*(repurpose len to prime_len)*/ |
| 56 | - } |
| 57 | - else if (key[0] == 0x02 || key[0] == 0x03) { /* (inc_y == 0) */ |
| 58 | - --len; /*(repurpose len to prime_len)*/ |
| 59 | - |
| 60 | + } else { /* (inc_y == 0) */ |
| 61 | /* mbedtls_ecp_point_read_binary() does not currently support |
| 62 | * MBEDTLS_ECP_PF_COMPRESSED format (buf[1] = 0x02 or 0x03) |
| 63 | * (returns MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE) */ |
| 64 | @@ -2390,22 +2392,21 @@ struct wpabuf * crypto_ecdh_set_peerkey( |
| 65 | /* derive y, amend buf[] with y for UNCOMPRESSED format */ |
| 66 | if (sizeof(buf)-2 < len*2 || len == 0) |
| 67 | return NULL; |
| 68 | + |
| 69 | buf[0] = (u8)(1+len*2); |
| 70 | buf[1] = 0x04; |
| 71 | + os_memcpy(buf+2, key, len); |
| 72 | + |
| 73 | mbedtls_mpi bn; |
| 74 | mbedtls_mpi_init(&bn); |
| 75 | - int ret = mbedtls_mpi_read_binary(&bn, key+1, len) |
| 76 | - || crypto_mbedtls_short_weierstrass_derive_y(grp, &bn, |
| 77 | - key[0] & 1) |
| 78 | + int ret = mbedtls_mpi_read_binary(&bn, key, len) |
| 79 | + || crypto_mbedtls_short_weierstrass_derive_y(grp, &bn, 0) |
| 80 | || mbedtls_mpi_write_binary(&bn, buf+2+len, len); |
| 81 | mbedtls_mpi_free(&bn); |
| 82 | if (ret != 0) |
| 83 | return NULL; |
| 84 | } |
| 85 | |
| 86 | - if (key[0] == 0) /*(repurpose len to prime_len)*/ |
| 87 | - len = CRYPTO_EC_plen(grp); |
| 88 | - |
| 89 | if (mbedtls_ecdh_read_public(&ecdh->ctx, buf, buf[0]+1)) |
| 90 | return NULL; |
| 91 | } |