diff --git a/lib/cifra/x25519.c b/lib/cifra/x25519.c index 58f464f8c..3697d4ddf 100644 --- a/lib/cifra/x25519.c +++ b/lib/cifra/x25519.c @@ -45,6 +45,13 @@ static int x25519_derive_secret(ptls_iovec_t *secret, const uint8_t *clientpriv, return PTLS_ERROR_NO_MEMORY; cf_curve25519_mul(secret->base, clientpriv != NULL ? clientpriv : serverpriv, clientpriv != NULL ? serverpub : clientpub); + + static const uint8_t zeros[X25519_KEY_SIZE] = {0}; + if (ptls_mem_equal(secret->base, zeros, sizeof(zeros))) { + free(secret->base); + return PTLS_ERROR_INCOMPATIBLE_KEY; + } + secret->len = X25519_KEY_SIZE; return 0; } @@ -111,8 +118,10 @@ static int x25519_key_exchange(ptls_key_exchange_algorithm_t *algo, ptls_iovec_t Exit: ptls_clear_memory(priv, sizeof(priv)); - if (pub != NULL && ret != 0) + if (pub != NULL && ret != 0) { ptls_clear_memory(pub, X25519_KEY_SIZE); + free(pub); + } return ret; } diff --git a/lib/openssl.c b/lib/openssl.c index b03533909..294fb6019 100644 --- a/lib/openssl.c +++ b/lib/openssl.c @@ -525,22 +525,35 @@ static int evp_keyex_on_exchange(ptls_key_exchange_context_t **_ctx, int release } #ifdef OPENSSL_IS_BORINGSSL +#define X25519_KEY_SIZE 32 if (ctx->super.algo->id == PTLS_GROUP_X25519) { - secret->len = peerkey.len; - if ((secret->base = malloc(secret->len)) == NULL) { + /* allocate memory to return secret */ + if ((secret->base = malloc(X25519_KEY_SIZE)) == NULL) { ret = PTLS_ERROR_NO_MEMORY; goto Exit; } - uint8_t sk_raw[32]; + secret->len = X25519_KEY_SIZE; + /* fetch raw key and derive the secret */ + uint8_t sk_raw[X25519_KEY_SIZE]; size_t sk_raw_len = sizeof(sk_raw); if (EVP_PKEY_get_raw_private_key(ctx->privkey, sk_raw, &sk_raw_len) != 1) { ret = PTLS_ERROR_LIBRARY; goto Exit; } + assert(sk_raw_len == sizeof(sk_raw)); X25519(secret->base, sk_raw, peerkey.base); + ptls_clear_memory(sk_raw, sizeof(sk_raw)); + /* check bad key */ + static const uint8_t zeros[X25519_KEY_SIZE] = {0}; + if (ptls_mem_equal(secret->base, zeros, X25519_KEY_SIZE)) { + ret = PTLS_ERROR_INCOMPATIBLE_KEY; + goto Exit; + } + /* success */ ret = 0; goto Exit; } +#undef X25519_KEY_SIZE #endif if ((evppeer = EVP_PKEY_new()) == NULL) { diff --git a/lib/picotls.c b/lib/picotls.c index 56a92766a..77e96c60b 100644 --- a/lib/picotls.c +++ b/lib/picotls.c @@ -5958,9 +5958,11 @@ int ptls_send(ptls_t *tls, ptls_buffer_t *sendbuf, const void *input, size_t inl assert(tls->traffic_protection.enc.aead != NULL); /* "For AES-GCM, up to 2^24.5 full-size records (about 24 million) may be encrypted on a given connection while keeping a - * safety margin of approximately 2^-57 for Authenticated Encryption (AE) security." (RFC 8446 section 5.5) + * safety margin of approximately 2^-57 for Authenticated Encryption (AE) security." (RFC 8446 section 5.5). + * + * Key updates do not happen with tls 1.2, check `key_schedule` to see if we are using tls/1.3 */ - if (tls->traffic_protection.enc.seq >= 16777216) + if (tls->traffic_protection.enc.seq >= 16777216 && tls->key_schedule != NULL) tls->needs_key_update = 1; if (tls->needs_key_update) { diff --git a/t/picotls.c b/t/picotls.c index f29813b31..558c470ad 100644 --- a/t/picotls.c +++ b/t/picotls.c @@ -2302,4 +2302,10 @@ void test_key_exchange(ptls_key_exchange_algorithm_t *client, ptls_key_exchange_ ret = ctx->on_exchange(&ctx, 1, NULL, ptls_iovec_init(NULL, 0)); ok(ret == 0); ok(ctx == NULL); + + /* test derivation failure. In case of X25519, the outcome is derived key becoming all-zero and rejected. In case of others, it + * is most likely that the provided key would be rejected. */ + static uint8_t zeros[32] = {0}; + ret = server->exchange(server, &server_pubkey, &server_secret, ptls_iovec_init(zeros, sizeof(zeros))); + ok(ret != 0); }