Merge commit from fork

upon failure to generate X25519 secret, do not return address of freed memory

include/picotls.h

@@ -348,6 +348,7 @@ typedef struct st_ptls_key_exchange_context_t {
      * When `secret` is non-NULL, this callback derives the shared secret using the private key of the context and the peer key being
      * given, and sets the value in `secret`. The memory pointed to by `secret->base` must be freed by the caller by calling `free`.
      * When `release` is set, the callee frees resources allocated to the context and set *keyex to NULL.
+     * Upon failure (i.e., when an PTLS error code is returned), `*pubkey` and `*secret` either remain unchanged or are zero-cleared.
      */
     int (*on_exchange)(struct st_ptls_key_exchange_context_t **keyex, int release, ptls_iovec_t *secret, ptls_iovec_t peerkey);
 } ptls_key_exchange_context_t;
@@ -369,6 +370,7 @@ typedef const struct st_ptls_key_exchange_algorithm_t {
      * Implements synchronous key exchange. Called when ServerHello is generated.
      * Given a public key provided by the peer (`peerkey`), this callback generates an ephemeral private and public key, and returns
      * the public key (`pubkey`) and a secret (`secret`) derived from the peerkey and private key.
+     * Upon failure (i.e., when an PTLS error code is returned), `*pubkey` and `*secret` either remain unchanged or are zero-cleared.
      */
     int (*exchange)(const struct st_ptls_key_exchange_algorithm_t *algo, ptls_iovec_t *pubkey, ptls_iovec_t *secret,
                     ptls_iovec_t peerkey);

lib/cifra/x25519.c

@@ -49,6 +49,7 @@ static int x25519_derive_secret(ptls_iovec_t *secret, const uint8_t *clientpriv,
     static const uint8_t zeros[X25519_KEY_SIZE] = {0};
     if (ptls_mem_equal(secret->base, zeros, sizeof(zeros))) {
         free(secret->base);
+        secret->base = NULL;
         return PTLS_ERROR_INCOMPATIBLE_KEY;
     }
 

lib/hpke.c

@@ -129,8 +129,11 @@ static int dh_encap(ptls_hpke_kem_t *kem, void *secret, ptls_iovec_t *pk_s, ptls
 
     *pk_s = ptls_iovec_init(NULL, 0);
 
-    if ((ret = kem->keyex->exchange(kem->keyex, pk_s, &dh, pk_r)) != 0)
+    if ((ret = kem->keyex->exchange(kem->keyex, pk_s, &dh, pk_r)) != 0) {
+        assert(pk_s->base == NULL);
+        assert(dh.base == NULL);
         goto Exit;
+    }
 
     if ((ret = dh_derive(kem, secret, *pk_s, pk_r, dh)) != 0)
         goto Exit;
@@ -152,8 +155,10 @@ static int dh_decap(ptls_hpke_kem_t *kem, void *secret, ptls_key_exchange_contex
     ptls_iovec_t dh = {NULL};
     int ret;
 
-    if ((ret = keyex->on_exchange(&keyex, 0, &dh, pk_s)) != 0)
+    if ((ret = keyex->on_exchange(&keyex, 0, &dh, pk_s)) != 0) {
+        assert(dh.base == NULL);
         goto Exit;
+    }
 
     if ((ret = dh_derive(kem, secret, pk_s, pk_r, dh)) != 0)
         goto Exit;

lib/openssl.c

@@ -516,7 +516,7 @@ static int evp_keyex_on_exchange(ptls_key_exchange_context_t **_ctx, int release
         goto Exit;
     }
 
-    secret->base = NULL;
+    *secret = ptls_iovec_init(NULL, 0);
 
     if (peerkey.len != ctx->super.pubkey.len) {
         ret = PTLS_ALERT_DECRYPT_ERROR;
@@ -598,8 +598,10 @@ static int evp_keyex_on_exchange(ptls_key_exchange_context_t **_ctx, int release
         EVP_PKEY_CTX_free(evpctx);
     if (evppeer != NULL)
         EVP_PKEY_free(evppeer);
-    if (ret != 0)
+    if (ret != 0) {
         free(secret->base);
+        *secret = ptls_iovec_init(NULL, 0);
+    }
     if (release) {
         evp_keyex_free(ctx);
         *_ctx = NULL;
@@ -679,7 +681,7 @@ static int evp_keyex_exchange(ptls_key_exchange_algorithm_t *algo, ptls_iovec_t
     ptls_key_exchange_context_t *ctx = NULL;
     int ret;
 
-    outpubkey->base = NULL;
+    *outpubkey = ptls_iovec_init(NULL, 0);
 
     if ((ret = evp_keyex_create(algo, &ctx)) != 0)
         goto Exit;
@@ -695,8 +697,10 @@ static int evp_keyex_exchange(ptls_key_exchange_algorithm_t *algo, ptls_iovec_t
 Exit:
     if (ctx != NULL)
         evp_keyex_on_exchange(&ctx, 1, NULL, ptls_iovec_init(NULL, 0));
-    if (ret != 0)
+    if (ret != 0) {
         free(outpubkey->base);
+        *outpubkey = ptls_iovec_init(NULL, 0);
+    }
     return ret;
 }
 

lib/picotls.c

@@ -2848,8 +2848,10 @@ static int client_handle_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptl
     ptls__key_schedule_update_hash(tls->key_schedule, message.base, message.len, 0);
 
     if (sh.peerkey.base != NULL) {
-        if ((ret = tls->client.key_share_ctx->on_exchange(&tls->client.key_share_ctx, 1, &ecdh_secret, sh.peerkey)) != 0)
+        if ((ret = tls->client.key_share_ctx->on_exchange(&tls->client.key_share_ctx, 1, &ecdh_secret, sh.peerkey)) != 0) {
+            assert(ecdh_secret.base == NULL);
             goto Exit;
+        }
     }
 
     if ((ret = key_schedule_extract(tls->key_schedule, ecdh_secret)) != 0)
@@ -4677,8 +4679,11 @@ static int server_handle_hello(ptls_t *tls, ptls_message_emitter_t *emitter, ptl
             ret = ch->key_shares.base != NULL ? PTLS_ALERT_HANDSHAKE_FAILURE : PTLS_ALERT_MISSING_EXTENSION;
             goto Exit;
         }
-        if ((ret = key_share.algorithm->exchange(key_share.algorithm, &pubkey, &ecdh_secret, key_share.peer_key)) != 0)
+        if ((ret = key_share.algorithm->exchange(key_share.algorithm, &pubkey, &ecdh_secret, key_share.peer_key)) != 0) {
+            assert(pubkey.base == NULL);
+            assert(ecdh_secret.base == NULL);
             goto Exit;
+        }
         tls->key_share = key_share.algorithm;
     }