Convert legacy server to tls_key_share.
authorjsing <jsing@openbsd.org>
Fri, 7 Jan 2022 15:46:30 +0000 (15:46 +0000)
committerjsing <jsing@openbsd.org>
Fri, 7 Jan 2022 15:46:30 +0000 (15:46 +0000)
This requires a few more additions to the DHE key share code - we need to
be able to either set the DHE parameters or specify the number of key bits
for use with auto DHE parameters. Additionally, we need to be able to
serialise the DHE parameters to send to the client.

This removes the infamous 'tmp' struct from ssl3_state_internal_st.

ok inoguchi@ tb@

lib/libssl/s3_lib.c
lib/libssl/ssl_locl.h
lib/libssl/ssl_srvr.c
lib/libssl/tls13_server.c
lib/libssl/tls_internal.h
lib/libssl/tls_key_share.c

index 54261c5..899432e 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: s3_lib.c,v 1.221 2022/01/06 18:23:56 jsing Exp $ */
+/* $OpenBSD: s3_lib.c,v 1.222 2022/01/07 15:46:30 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1565,10 +1565,6 @@ ssl3_free(SSL *s)
        ssl3_release_write_buffer(s);
        freezero(S3I(s)->hs.sigalgs, S3I(s)->hs.sigalgs_len);
 
-       DH_free(S3I(s)->tmp.dh);
-       EC_KEY_free(S3I(s)->tmp.ecdh);
-       freezero(S3I(s)->tmp.x25519, X25519_KEY_LENGTH);
-
        tls_key_share_free(S3I(s)->hs.key_share);
 
        tls13_secrets_destroy(S3I(s)->hs.tls13.secrets);
@@ -1601,14 +1597,6 @@ ssl3_clear(SSL *s)
        sk_X509_pop_free(s->internal->verified_chain, X509_free);
        s->internal->verified_chain = NULL;
 
-       DH_free(S3I(s)->tmp.dh);
-       S3I(s)->tmp.dh = NULL;
-       EC_KEY_free(S3I(s)->tmp.ecdh);
-       S3I(s)->tmp.ecdh = NULL;
-       S3I(s)->tmp.ecdh_nid = NID_undef;
-       freezero(S3I(s)->tmp.x25519, X25519_KEY_LENGTH);
-       S3I(s)->tmp.x25519 = NULL;
-
        freezero(S3I(s)->hs.sigalgs, S3I(s)->hs.sigalgs_len);
        S3I(s)->hs.sigalgs = NULL;
        S3I(s)->hs.sigalgs_len = 0;
index 83b40d2..cc7b342 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.374 2022/01/06 18:23:56 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.375 2022/01/07 15:46:30 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1168,15 +1168,6 @@ typedef struct ssl3_state_internal_st {
 
        SSL_HANDSHAKE hs;
 
-       struct  {
-               DH *dh;
-
-               EC_KEY *ecdh; /* holds short lived ECDH key */
-               int ecdh_nid;
-
-               uint8_t *x25519;
-       } tmp;
-
        /* Connection binding to prevent renegotiation attacks */
        unsigned char previous_client_finished[EVP_MAX_MD_SIZE];
        unsigned char previous_client_finished_len;
index 0496985..b66a2c1 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.130 2022/01/04 12:53:31 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.131 2022/01/07 15:46:30 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1309,23 +1309,23 @@ ssl3_send_server_done(SSL *s)
 static int
 ssl3_send_server_kex_dhe(SSL *s, CBB *cbb)
 {
-       DH *dh = NULL;
-       int al;
+       int nid = NID_dhKeyAgreement;
 
-       if ((dh = DH_new()) == NULL)
+       tls_key_share_free(S3I(s)->hs.key_share);
+       if ((S3I(s)->hs.key_share = tls_key_share_new_nid(nid)) == NULL)
                goto err;
 
        if (s->cert->dh_tmp_auto != 0) {
                size_t key_bits;
 
                if ((key_bits = ssl_dhe_params_auto_key_bits(s)) == 0) {
-                       al = SSL_AD_INTERNAL_ERROR;
                        SSLerror(s, ERR_R_INTERNAL_ERROR);
-                       goto fatal_err;
-               }
-
-               if (!ssl_kex_generate_dhe_params_auto(dh, key_bits))
+                       ssl3_send_alert(s, SSL3_AL_FATAL,
+                           SSL_AD_INTERNAL_ERROR);
                        goto err;
+               }
+               tls_key_share_set_key_bits(S3I(s)->hs.key_share,
+                   key_bits);
        } else {
                DH *dh_params = s->cert->dh_tmp;
 
@@ -1334,157 +1334,69 @@ ssl3_send_server_kex_dhe(SSL *s, CBB *cbb)
                            SSL_C_PKEYLENGTH(S3I(s)->hs.cipher));
 
                if (dh_params == NULL) {
-                       al = SSL_AD_HANDSHAKE_FAILURE;
                        SSLerror(s, SSL_R_MISSING_TMP_DH_KEY);
-                       goto fatal_err;
+                       ssl3_send_alert(s, SSL3_AL_FATAL,
+                           SSL_AD_HANDSHAKE_FAILURE);
+                       goto err;
                }
 
-               if (!ssl_kex_generate_dhe(dh, dh_params))
+               if (!tls_key_share_set_dh_params(S3I(s)->hs.key_share, dh_params))
                        goto err;
        }
 
-       if (!ssl_kex_params_dhe(dh, cbb))
-               goto err;
-       if (!ssl_kex_public_dhe(dh, cbb))
+       if (!tls_key_share_generate(S3I(s)->hs.key_share))
                goto err;
 
-       if (S3I(s)->tmp.dh != NULL) {
-               SSLerror(s, ERR_R_INTERNAL_ERROR);
+       if (!tls_key_share_params(S3I(s)->hs.key_share, cbb))
+               goto err;
+       if (!tls_key_share_public(S3I(s)->hs.key_share, cbb))
                goto err;
-       }
-       S3I(s)->tmp.dh = dh;
 
        return 1;
 
- fatal_err:
-       ssl3_send_alert(s, SSL3_AL_FATAL, al);
  err:
-       DH_free(dh);
-
        return 0;
 }
 
 static int
-ssl3_send_server_kex_ecdhe_ecp(SSL *s, int nid, CBB *cbb)
+ssl3_send_server_kex_ecdhe(SSL *s, CBB *cbb)
 {
-       uint16_t curve_id;
-       EC_KEY *ecdh;
-       CBB ecpoint;
-       int al;
+       CBB public;
+       int nid;
 
-       /*
-        * Only named curves are supported in ECDH ephemeral key exchanges.
-        * For supported named curves, curve_id is non-zero.
-        */
-       if ((curve_id = tls1_ec_nid2curve_id(nid)) == 0) {
+       if ((nid = tls1_get_shared_curve(s)) == NID_undef) {
                SSLerror(s, SSL_R_UNSUPPORTED_ELLIPTIC_CURVE);
+               ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
                goto err;
        }
 
-       if (S3I(s)->tmp.ecdh != NULL) {
-               SSLerror(s, ERR_R_INTERNAL_ERROR);
+       tls_key_share_free(S3I(s)->hs.key_share);
+       if ((S3I(s)->hs.key_share = tls_key_share_new_nid(nid)) == NULL)
                goto err;
-       }
 
-       if ((S3I(s)->tmp.ecdh = EC_KEY_new()) == NULL) {
-               al = SSL_AD_HANDSHAKE_FAILURE;
-               SSLerror(s, SSL_R_MISSING_TMP_ECDH_KEY);
-               goto fatal_err;
-       }
-       S3I(s)->tmp.ecdh_nid = nid;
-       ecdh = S3I(s)->tmp.ecdh;
-
-       if (!ssl_kex_generate_ecdhe_ecp(ecdh, nid))
+       if (!tls_key_share_generate(S3I(s)->hs.key_share))
                goto err;
 
        /*
-        * Encode the public key.
-        *
-        * Only named curves are supported in ECDH ephemeral key exchanges.
-        * In this case the ServerKeyExchange message has:
-        * [1 byte CurveType], [2 byte CurveName]
-        * [1 byte length of encoded point], followed by
-        * the actual encoded point itself.
+        * ECC key exchange - see RFC 8422, section 5.4.
         */
        if (!CBB_add_u8(cbb, NAMED_CURVE_TYPE))
                goto err;
-       if (!CBB_add_u16(cbb, curve_id))
+       if (!CBB_add_u16(cbb, tls_key_share_group(S3I(s)->hs.key_share)))
                goto err;
-       if (!CBB_add_u8_length_prefixed(cbb, &ecpoint))
+       if (!CBB_add_u8_length_prefixed(cbb, &public))
                goto err;
-       if (!ssl_kex_public_ecdhe_ecp(ecdh, &ecpoint))
+       if (!tls_key_share_public(S3I(s)->hs.key_share, &public))
                goto err;
        if (!CBB_flush(cbb))
                goto err;
 
        return 1;
 
- fatal_err:
-       ssl3_send_alert(s, SSL3_AL_FATAL, al);
  err:
        return 0;
 }
 
-static int
-ssl3_send_server_kex_ecdhe_ecx(SSL *s, int nid, CBB *cbb)
-{
-       uint8_t *public_key = NULL, *private_key = NULL;
-       uint16_t curve_id;
-       CBB ecpoint;
-       int ret = 0;
-
-       /* Generate an X25519 key pair. */
-       if (S3I(s)->tmp.x25519 != NULL) {
-               SSLerror(s, ERR_R_INTERNAL_ERROR);
-               goto err;
-       }
-       if ((private_key = malloc(X25519_KEY_LENGTH)) == NULL)
-               goto err;
-       if ((public_key = malloc(X25519_KEY_LENGTH)) == NULL)
-               goto err;
-       X25519_keypair(public_key, private_key);
-
-       /* Serialize public key. */
-       if ((curve_id = tls1_ec_nid2curve_id(nid)) == 0) {
-               SSLerror(s, SSL_R_UNSUPPORTED_ELLIPTIC_CURVE);
-               goto err;
-       }
-
-       if (!CBB_add_u8(cbb, NAMED_CURVE_TYPE))
-               goto err;
-       if (!CBB_add_u16(cbb, curve_id))
-               goto err;
-       if (!CBB_add_u8_length_prefixed(cbb, &ecpoint))
-               goto err;
-       if (!CBB_add_bytes(&ecpoint, public_key, X25519_KEY_LENGTH))
-               goto err;
-       if (!CBB_flush(cbb))
-               goto err;
-
-       S3I(s)->tmp.x25519 = private_key;
-       private_key = NULL;
-       ret = 1;
-
- err:
-       free(public_key);
-       freezero(private_key, X25519_KEY_LENGTH);
-
-       return ret;
-}
-
-static int
-ssl3_send_server_kex_ecdhe(SSL *s, CBB *cbb)
-{
-       int nid;
-
-       nid = tls1_get_shared_curve(s);
-
-       if (nid == NID_X25519)
-               return ssl3_send_server_kex_ecdhe_ecx(s, nid, cbb);
-
-       return ssl3_send_server_kex_ecdhe_ecp(s, nid, cbb);
-}
-
 int
 ssl3_send_server_key_exchange(SSL *s)
 {
@@ -1791,147 +1703,73 @@ ssl3_get_client_kex_rsa(SSL *s, CBS *cbs)
 static int
 ssl3_get_client_kex_dhe(SSL *s, CBS *cbs)
 {
-       DH *dh_clnt = NULL;
-       DH *dh_srvr;
-       int invalid_key;
        uint8_t *key = NULL;
        size_t key_len = 0;
+       int invalid_key;
        int ret = 0;
 
-       if ((dh_srvr = S3I(s)->tmp.dh) == NULL) {
+       if (S3I(s)->hs.key_share == NULL) {
                ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
                SSLerror(s, SSL_R_MISSING_TMP_DH_KEY);
                goto err;
        }
 
-       if ((dh_clnt = DHparams_dup(dh_srvr)) == NULL)
-               goto err;
-
-       if (!ssl_kex_peer_public_dhe(dh_clnt, cbs, &invalid_key)) {
-               ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-               SSLerror(s, SSL_R_BAD_PACKET_LENGTH);
+       if (!tls_key_share_peer_public(S3I(s)->hs.key_share, cbs,
+           &invalid_key))
                goto err;
-       }
        if (invalid_key) {
                ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
                SSLerror(s, SSL_R_BAD_DH_PUB_KEY_LENGTH);
                goto err;
        }
 
-       if (!ssl_kex_derive_dhe(dh_srvr, dh_clnt, &key, &key_len))
+       if (!tls_key_share_derive(S3I(s)->hs.key_share, &key, &key_len))
                goto err;
 
        if (!tls12_derive_master_secret(s, key, key_len))
                goto err;
 
-       DH_free(S3I(s)->tmp.dh);
-       S3I(s)->tmp.dh = NULL;
-
        ret = 1;
 
  err:
        freezero(key, key_len);
-       DH_free(dh_clnt);
 
        return ret;
 }
 
 static int
-ssl3_get_client_kex_ecdhe_ecp(SSL *s, CBS *cbs)
+ssl3_get_client_kex_ecdhe(SSL *s, CBS *cbs)
 {
        uint8_t *key = NULL;
        size_t key_len = 0;
-       EC_KEY *ecdh_peer = NULL;
-       EC_KEY *ecdh;
        CBS public;
        int ret = 0;
 
-       /*
-        * Use the ephemeral values we saved when generating the
-        * ServerKeyExchange message.
-        */
-       if ((ecdh = S3I(s)->tmp.ecdh) == NULL) {
-               SSLerror(s, ERR_R_INTERNAL_ERROR);
+       if (S3I(s)->hs.key_share == NULL) {
+               ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
+               SSLerror(s, SSL_R_MISSING_TMP_DH_KEY);
                goto err;
        }
 
-       /*
-        * Get client's public key from encoded point in the ClientKeyExchange
-        * message.
-        */
        if (!CBS_get_u8_length_prefixed(cbs, &public))
                goto err;
-       if (CBS_len(cbs) != 0)
+       if (!tls_key_share_peer_public(S3I(s)->hs.key_share, &public, NULL))
                goto err;
 
-       if ((ecdh_peer = EC_KEY_new()) == NULL)
+       if (!tls_key_share_derive(S3I(s)->hs.key_share, &key, &key_len))
                goto err;
 
-       if (!ssl_kex_peer_public_ecdhe_ecp(ecdh_peer, S3I(s)->tmp.ecdh_nid,
-           &public))
-               goto err;
-
-       /* Derive the shared secret and compute master secret. */
-       if (!ssl_kex_derive_ecdhe_ecp(ecdh, ecdh_peer, &key, &key_len))
-               goto err;
        if (!tls12_derive_master_secret(s, key, key_len))
                goto err;
 
-       EC_KEY_free(S3I(s)->tmp.ecdh);
-       S3I(s)->tmp.ecdh = NULL;
-       S3I(s)->tmp.ecdh_nid = NID_undef;
-
        ret = 1;
 
  err:
        freezero(key, key_len);
-       EC_KEY_free(ecdh_peer);
 
        return ret;
 }
 
-static int
-ssl3_get_client_kex_ecdhe_ecx(SSL *s, CBS *cbs)
-{
-       uint8_t *shared_key = NULL;
-       CBS ecpoint;
-       int ret = 0;
-
-       if (!CBS_get_u8_length_prefixed(cbs, &ecpoint))
-               goto err;
-       if (CBS_len(cbs) != 0)
-               goto err;
-       if (CBS_len(&ecpoint) != X25519_KEY_LENGTH)
-               goto err;
-
-       if ((shared_key = malloc(X25519_KEY_LENGTH)) == NULL)
-               goto err;
-       if (!X25519(shared_key, S3I(s)->tmp.x25519, CBS_data(&ecpoint)))
-               goto err;
-
-       freezero(S3I(s)->tmp.x25519, X25519_KEY_LENGTH);
-       S3I(s)->tmp.x25519 = NULL;
-
-       if (!tls12_derive_master_secret(s, shared_key, X25519_KEY_LENGTH))
-               goto err;
-
-       ret = 1;
-
- err:
-       freezero(shared_key, X25519_KEY_LENGTH);
-
-       return ret;
-}
-
-static int
-ssl3_get_client_kex_ecdhe(SSL *s, CBS *cbs)
-{
-       if (S3I(s)->tmp.x25519 != NULL)
-               return ssl3_get_client_kex_ecdhe_ecx(s, cbs);
-
-       return ssl3_get_client_kex_ecdhe_ecp(s, cbs);
-}
-
 static int
 ssl3_get_client_kex_gost(SSL *s, CBS *cbs)
 {
index 739ef06..c32ae22 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls13_server.c,v 1.90 2022/01/05 17:10:02 jsing Exp $ */
+/* $OpenBSD: tls13_server.c,v 1.91 2022/01/07 15:46:30 jsing Exp $ */
 /*
  * Copyright (c) 2019, 2020 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2020 Bob Beck <beck@openbsd.org>
@@ -360,8 +360,8 @@ tls13_server_engage_record_protection(struct tls13_ctx *ctx)
        SSL *s = ctx->ssl;
        int ret = 0;
 
-       if (!tls_key_share_derive(ctx->hs->key_share,
-           &shared_key, &shared_key_len))
+       if (!tls_key_share_derive(ctx->hs->key_share, &shared_key,
+           &shared_key_len))
                goto err;
 
        s->session->cipher = ctx->hs->cipher;
index 7e2bead..f7f9392 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_internal.h,v 1.3 2022/01/06 18:23:56 jsing Exp $ */
+/* $OpenBSD: tls_internal.h,v 1.4 2022/01/07 15:46:30 jsing Exp $ */
 /*
  * Copyright (c) 2018, 2019, 2021 Joel Sing <jsing@openbsd.org>
  *
@@ -18,6 +18,7 @@
 #ifndef HEADER_TLS_INTERNAL_H
 #define HEADER_TLS_INTERNAL_H
 
+#include <openssl/dh.h>
 #include <openssl/evp.h>
 
 #include "bytestring.h"
@@ -64,8 +65,11 @@ void tls_key_share_free(struct tls_key_share *ks);
 
 uint16_t tls_key_share_group(struct tls_key_share *ks);
 int tls_key_share_nid(struct tls_key_share *ks);
+void tls_key_share_set_key_bits(struct tls_key_share *ks, size_t key_bits);
+int tls_key_share_set_dh_params(struct tls_key_share *ks, DH *dh_params);
 int tls_key_share_peer_pkey(struct tls_key_share *ks, EVP_PKEY *pkey);
 int tls_key_share_generate(struct tls_key_share *ks);
+int tls_key_share_params(struct tls_key_share *ks, CBB *cbb);
 int tls_key_share_public(struct tls_key_share *ks, CBB *cbb);
 int tls_key_share_peer_params(struct tls_key_share *ks, CBS *cbs,
     int *invalid_params);
index 6e390f4..eb30a0e 100644 (file)
@@ -1,6 +1,6 @@
-/* $OpenBSD: tls_key_share.c,v 1.2 2022/01/06 18:23:56 jsing Exp $ */
+/* $OpenBSD: tls_key_share.c,v 1.3 2022/01/07 15:46:30 jsing Exp $ */
 /*
- * Copyright (c) 2020 Joel Sing <jsing@openbsd.org>
+ * Copyright (c) 2020, 2021 Joel Sing <jsing@openbsd.org>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -18,7 +18,9 @@
 #include <stdlib.h>
 
 #include <openssl/curve25519.h>
+#include <openssl/dh.h>
 #include <openssl/ec.h>
+#include <openssl/evp.h>
 
 #include "bytestring.h"
 #include "ssl_locl.h"
@@ -27,6 +29,7 @@
 struct tls_key_share {
        int nid;
        uint16_t group_id;
+       size_t key_bits;
 
        DH *dhe;
        DH *dhe_peer;
@@ -108,6 +111,28 @@ tls_key_share_nid(struct tls_key_share *ks)
        return ks->nid;
 }
 
+void
+tls_key_share_set_key_bits(struct tls_key_share *ks, size_t key_bits)
+{
+       ks->key_bits = key_bits;
+}
+
+int
+tls_key_share_set_dh_params(struct tls_key_share *ks, DH *dh_params)
+{
+       if (ks->nid != NID_dhKeyAgreement)
+               return 0;
+       if (ks->dhe != NULL || ks->dhe_peer != NULL)
+               return 0;
+
+       if ((ks->dhe = DHparams_dup(dh_params)) == NULL)
+               return 0;
+       if ((ks->dhe_peer = DHparams_dup(dh_params)) == NULL)
+               return 0;
+
+       return 1;
+}
+
 int
 tls_key_share_peer_pkey(struct tls_key_share *ks, EVP_PKEY *pkey)
 {
@@ -126,10 +151,28 @@ tls_key_share_peer_pkey(struct tls_key_share *ks, EVP_PKEY *pkey)
 static int
 tls_key_share_generate_dhe(struct tls_key_share *ks)
 {
-       if (ks->dhe == NULL)
+       /*
+        * If auto params are not being used then we must already have DH
+        * parameters set.
+        */
+       if (ks->key_bits == 0) {
+               if (ks->dhe == NULL)
+                       return 0;
+
+               return ssl_kex_generate_dhe(ks->dhe, ks->dhe);
+       }
+
+       if (ks->dhe != NULL || ks->dhe_peer != NULL)
+               return 0;
+
+       if ((ks->dhe = DH_new()) == NULL)
+               return 0;
+       if (!ssl_kex_generate_dhe_params_auto(ks->dhe, ks->key_bits))
+               return 0;
+       if ((ks->dhe_peer = DHparams_dup(ks->dhe)) == NULL)
                return 0;
 
-       return ssl_kex_generate_dhe(ks->dhe, ks->dhe);
+       return 1;
 }
 
 static int
@@ -199,6 +242,24 @@ tls_key_share_generate(struct tls_key_share *ks)
        return tls_key_share_generate_ecdhe_ecp(ks);
 }
 
+static int
+tls_key_share_params_dhe(struct tls_key_share *ks, CBB *cbb)
+{
+       if (ks->dhe == NULL)
+               return 0;
+
+       return ssl_kex_params_dhe(ks->dhe, cbb);
+}
+
+int
+tls_key_share_params(struct tls_key_share *ks, CBB *cbb)
+{
+       if (ks->nid == NID_dhKeyAgreement)
+               return tls_key_share_params_dhe(ks, cbb);
+
+       return 0;
+}
+
 static int
 tls_key_share_public_dhe(struct tls_key_share *ks, CBB *cbb)
 {