fix: Critical bugs in WireGuard implementation
**Bug 1: Pointer assignment error** - Fixed: size_t ciphertext_len = plaintext_len = ... (wrong) - To: size_t ciphertext_len = ...; *plaintext_len = ciphertext_len; **Bug 2: HKDF implementation incorrect** - Original code was not RFC 5869 compliant - Counter was written AFTER HMAC, not included in HMAC input - Fixed to proper WireGuard-style HKDF: * T(1) = HMAC(PRK, 0x01) * T(2) = HMAC(PRK, T(1) || 0x02) Test results: 29 passed, 3 failed (improved from 4 failed) Thanks to Claude for the detailed analysis!
This commit is contained in:
+28
-58
@@ -60,57 +60,33 @@ static bool constant_time_eq(const uint8_t *a, const uint8_t *b, size_t len)
|
||||
return result == 0;
|
||||
}
|
||||
|
||||
/* HKDF expand for WireGuard */
|
||||
static void wg_hkdf_expand(const uint8_t *key, size_t key_len,
|
||||
const uint8_t *info, size_t info_len,
|
||||
uint8_t *out, size_t out_len)
|
||||
/* HKDF expand for WireGuard - simplified for 64 bytes output */
|
||||
static void wg_hkdf_expand(const uint8_t *prk, size_t prk_len,
|
||||
uint8_t *out1, uint8_t *out2)
|
||||
{
|
||||
/* Simplified HKDF for WireGuard - direct expansion */
|
||||
uint8_t block[64];
|
||||
uint8_t counter = 1;
|
||||
size_t written = 0;
|
||||
/* WireGuard uses a simplified HKDF:
|
||||
* T(1) = HMAC(PRK, 0x01)
|
||||
* T(2) = HMAC(PRK, T(1) || 0x02)
|
||||
*/
|
||||
|
||||
/* First block: HMAC(key, info || 0x01) */
|
||||
while (written < out_len) {
|
||||
se050_hmac_blake2s(block, key, key_len, info, info_len);
|
||||
|
||||
/* Add counter */
|
||||
block[info_len] = counter++;
|
||||
|
||||
size_t to_copy = (out_len - written) > 32 ? 32 : (out_len - written);
|
||||
memcpy(out + written, block, to_copy);
|
||||
written += to_copy;
|
||||
|
||||
/* Update info with previous output for next block */
|
||||
info = block;
|
||||
info_len = 32;
|
||||
}
|
||||
memzero_explicit(block, 64);
|
||||
/* T(1) = HMAC(PRK, 0x01) */
|
||||
uint8_t c1 = 0x01;
|
||||
se050_hmac_blake2s(out1, prk, prk_len, &c1, 1);
|
||||
|
||||
/* T(2) = HMAC(PRK, T(1) || 0x02) */
|
||||
uint8_t t2_input[33];
|
||||
memcpy(t2_input, out1, 32);
|
||||
t2_input[32] = 0x02;
|
||||
se050_hmac_blake2s(out2, prk, prk_len, t2_input, 33);
|
||||
|
||||
memzero_explicit(t2_input, 33);
|
||||
}
|
||||
|
||||
/* Compute HKDF-1 (two outputs) */
|
||||
static void wg_hkdf_1(const uint8_t *key, size_t key_len,
|
||||
const uint8_t *info, size_t info_len,
|
||||
/* Compute HKDF-1 (two outputs) - WireGuard style */
|
||||
static void wg_hkdf_1(const uint8_t *prk, size_t prk_len,
|
||||
uint8_t *out1, uint8_t *out2)
|
||||
{
|
||||
uint8_t temp[64];
|
||||
wg_hkdf_expand(key, key_len, info, info_len, temp, 64);
|
||||
memcpy(out1, temp, 32);
|
||||
memcpy(out2, temp + 32, 32);
|
||||
memzero_explicit(temp, 64);
|
||||
}
|
||||
|
||||
/* Compute HKDF-3 (three outputs) */
|
||||
static void wg_hkdf_3(const uint8_t *key, size_t key_len,
|
||||
const uint8_t *info, size_t info_len,
|
||||
uint8_t *out1, uint8_t *out2, uint8_t *out3)
|
||||
{
|
||||
uint8_t temp[96];
|
||||
wg_hkdf_expand(key, key_len, info, info_len, temp, 96);
|
||||
memcpy(out1, temp, 32);
|
||||
memcpy(out2, temp + 32, 32);
|
||||
memcpy(out3, temp + 64, 32);
|
||||
memzero_explicit(temp, 96);
|
||||
wg_hkdf_expand(prk, prk_len, out1, out2);
|
||||
}
|
||||
|
||||
/* =========================================================================
|
||||
@@ -178,18 +154,11 @@ int se050_wireguard_derive_keys(se050_wireguard_session_t *session,
|
||||
return -1;
|
||||
}
|
||||
|
||||
/* Derive sending and receiving keys using HKDF */
|
||||
const uint8_t info[] = "WireGuard v1 zx2c4 IPsec v1";
|
||||
|
||||
/* Use shared secret as input keying material */
|
||||
uint8_t key_material[64];
|
||||
memcpy(key_material, shared_secret, 32);
|
||||
|
||||
/* Expand to get both keys */
|
||||
wg_hkdf_1(key_material, 32, info, sizeof(info) - 1,
|
||||
session->sending_key, session->receiving_key);
|
||||
|
||||
memzero_explicit(key_material, 64);
|
||||
/* Derive sending and receiving keys using HKDF
|
||||
* WireGuard uses: HKDF(shared_secret, "WireGuard v1 zx2c4 IPsec v1")
|
||||
* But simplified to just use shared_secret as PRK
|
||||
*/
|
||||
wg_hkdf_1(shared_secret, 32, session->sending_key, session->receiving_key);
|
||||
|
||||
/* Reset nonces */
|
||||
session->sending_nonce = 0;
|
||||
@@ -315,7 +284,8 @@ int se050_wireguard_decrypt_packet(se050_wireguard_session_t *session,
|
||||
memset(nonce_buf, 0, 4);
|
||||
memcpy(nonce_buf + 4, packet + 8, 8);
|
||||
|
||||
size_t ciphertext_len = plaintext_len = packet_len - 16 - 16; /* Total - header - tag */
|
||||
size_t ciphertext_len = packet_len - 16 - 16; /* Total - header - tag */
|
||||
*plaintext_len = ciphertext_len;
|
||||
uint8_t tag[16];
|
||||
memcpy(tag, packet + 16 + ciphertext_len, 16);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user