Merge branch 'master' into kazuho/psk2

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;
 }
 

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) {

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) {

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);
 }