Defer sigalgs selection until the certificate is known.

Previously the signature algorithm was selected when the TLS extension was
parsed (or the client received a certificate request), however the actual
certificate to be used is not known at this stage. This leads to various
problems, including the selection of a signature algorithm that cannot be
used with the certificate key size (as found by jeremy@ via ruby regress).

Instead, store the signature algorithms list and only select a signature
algorithm when we're ready to do signature generation.

Joint work with beck@.

9 files changed, 124 insertions, 48 deletions

diff --git a/src/lib/libssl/s3_lib.c b/src/lib/libssl/s3_lib.c
index de928bd70e..49f402d065 100644
--- a/src/lib/libssl/s3_lib.c
+++ b/src/lib/libssl/s3_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: s3_lib.c,v 1.184 2019/02/09 15:26:15 jsing Exp $ */
+/* $OpenBSD: s3_lib.c,v 1.185 2019/03/25 17:21:18 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1559,6 +1559,7 @@ ssl3_free(SSL *s)
         tls1_cleanup_key_block(s);
         ssl3_release_read_buffer(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);
@@ -1598,6 +1599,9 @@ ssl3_clear(SSL *s)
         S3I(s)->tmp.dh = NULL;
         EC_KEY_free(S3I(s)->tmp.ecdh);
         S3I(s)->tmp.ecdh = NULL;
+        freezero(S3I(s)->hs.sigalgs, S3I(s)->hs.sigalgs_len);
+        S3I(s)->hs.sigalgs = NULL;
+        S3I(s)->hs.sigalgs_len = 0;
 
         freezero(S3I(s)->tmp.x25519, X25519_KEY_LENGTH);
         S3I(s)->tmp.x25519 = NULL;
diff --git a/src/lib/libssl/ssl_clnt.c b/src/lib/libssl/ssl_clnt.c
index 262e09fe5e..2174e3a83d 100644
--- a/src/lib/libssl/ssl_clnt.c
+++ b/src/lib/libssl/ssl_clnt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_clnt.c,v 1.59 2019/03/25 16:35:48 jsing Exp $ */
+/* $OpenBSD: ssl_clnt.c,v 1.60 2019/03/25 17:21:18 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1512,7 +1512,7 @@ ssl3_get_server_key_exchange(SSL *s)
                         if (!CBS_get_u16(&cbs, &sigalg_value))
                                 goto truncated;
                         if ((sigalg = ssl_sigalg(sigalg_value, tls12_sigalgs,
-                                    tls12_sigalgs_len)) == NULL) {
+                            tls12_sigalgs_len)) == NULL) {
                                 SSLerror(s, SSL_R_UNKNOWN_DIGEST);
                                 al = SSL_AD_DECODE_ERROR;
                                 goto f_err;
@@ -1522,7 +1522,7 @@ ssl3_get_server_key_exchange(SSL *s)
                                 al = SSL_AD_DECODE_ERROR;
                                 goto f_err;
                         }
-                        if (!ssl_sigalg_pkey_ok(sigalg, pkey)) {
+                        if (!ssl_sigalg_pkey_ok(sigalg, pkey, 0)) {
                                 SSLerror(s, SSL_R_WRONG_SIGNATURE_TYPE);
                                 al = SSL_AD_DECODE_ERROR;
                                 goto f_err;
@@ -1671,21 +1671,19 @@ ssl3_get_certificate_request(SSL *s)
                         SSLerror(s, SSL_R_DATA_LENGTH_TOO_LONG);
                         goto err;
                 }
-
-                /* Check we have enough room for signature algorithms and
-                 * following length value.
-                 */
                 if (!CBS_get_u16_length_prefixed(&cert_request, &sigalgs)) {
                         ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
                         SSLerror(s, SSL_R_DATA_LENGTH_TOO_LONG);
                         goto err;
                 }
-                if (!tls1_process_sigalgs(s, &sigalgs, tls12_sigalgs,
-                    tls12_sigalgs_len)) {
+                if (CBS_len(&sigalgs) % 2 != 0 || CBS_len(&sigalgs) > 64) {
                         ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
                         SSLerror(s, SSL_R_SIGNATURE_ALGORITHMS_ERROR);
                         goto err;
                 }
+                if (!CBS_stow(&sigalgs, &S3I(s)->hs.sigalgs,
+                    &S3I(s)->hs.sigalgs_len))
+                        goto err;
         }
 
         /* get the CA RDNs */
@@ -2372,6 +2370,7 @@ err:
 static int
 ssl3_send_client_verify_sigalgs(SSL *s, CBB *cert_verify)
 {
+        const struct ssl_sigalg *sigalg;
         CBB cbb_signature;
         EVP_PKEY_CTX *pctx = NULL;
         EVP_PKEY *pkey;
@@ -2387,10 +2386,17 @@ ssl3_send_client_verify_sigalgs(SSL *s, CBB *cert_verify)
         EVP_MD_CTX_init(&mctx);
 
         pkey = s->cert->key->privatekey;
-        md = s->cert->key->sigalg->md();
+        if ((sigalg = ssl_sigalg_select(s, pkey)) == NULL) {
+                SSLerror(s, SSL_R_SIGNATURE_ALGORITHMS_ERROR);
+                goto err;
+        }
+        if ((md = sigalg->md()) == NULL) {
+                SSLerror(s, SSL_R_UNKNOWN_DIGEST);
+                goto err;
+        }
 
         if (!tls1_transcript_data(s, &hdata, &hdatalen) ||
-            !CBB_add_u16(cert_verify, s->cert->key->sigalg->value)) {
+            !CBB_add_u16(cert_verify, sigalg->value)) {
                 SSLerror(s, ERR_R_INTERNAL_ERROR);
                 goto err;
         }
@@ -2398,7 +2404,7 @@ ssl3_send_client_verify_sigalgs(SSL *s, CBB *cert_verify)
                 SSLerror(s, ERR_R_EVP_LIB);
                 goto err;
         }
-        if ((s->cert->key->sigalg->flags & SIGALG_FLAG_RSA_PSS) &&
+        if ((sigalg->flags & SIGALG_FLAG_RSA_PSS) &&
             (!EVP_PKEY_CTX_set_rsa_padding(pctx, RSA_PKCS1_PSS_PADDING) ||
             !EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, -1))) {
                 SSLerror(s, ERR_R_EVP_LIB);
diff --git a/src/lib/libssl/ssl_lib.c b/src/lib/libssl/ssl_lib.c
index 52ede46f7e..70a4c6d16f 100644
--- a/src/lib/libssl/ssl_lib.c
+++ b/src/lib/libssl/ssl_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_lib.c,v 1.202 2019/03/25 16:37:52 jsing Exp $ */
+/* $OpenBSD: ssl_lib.c,v 1.203 2019/03/25 17:21:18 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -2120,18 +2120,7 @@ ssl_get_sign_pkey(SSL *s, const SSL_CIPHER *cipher, const EVP_MD **pmd,
         }
 
         pkey = c->pkeys[idx].privatekey;
-        sigalg = c->pkeys[idx].sigalg;
-        if (!SSL_USE_SIGALGS(s)) {
-                if (pkey->type == EVP_PKEY_RSA) {
-                        sigalg = ssl_sigalg_lookup(SIGALG_RSA_PKCS1_MD5_SHA1);
-                } else if (pkey->type == EVP_PKEY_EC) {
-                        sigalg = ssl_sigalg_lookup(SIGALG_ECDSA_SHA1);
-                } else {
-                        SSLerror(s, SSL_R_UNKNOWN_PKEY_TYPE);
-                        return (NULL);
-                }
-        }
-        if (sigalg == NULL) {
+        if ((sigalg = ssl_sigalg_select(s, pkey)) == NULL) {
                 SSLerror(s, SSL_R_SIGNATURE_ALGORITHMS_ERROR);
                 return (NULL);
         }
diff --git a/src/lib/libssl/ssl_locl.h b/src/lib/libssl/ssl_locl.h
index 9d06deeb7a..44abb6d6da 100644
--- a/src/lib/libssl/ssl_locl.h
+++ b/src/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.241 2019/03/25 16:37:52 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.242 2019/03/25 17:21:18 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -432,6 +432,10 @@ typedef struct ssl_handshake_st {
 
         /* Extensions seen in this handshake. */
         uint32_t extensions_seen;
+
+        /* sigalgs offered in this handshake in wire form */
+        size_t sigalgs_len;
+        uint8_t *sigalgs;
 } SSL_HANDSHAKE;
 
 typedef struct ssl_handshake_tls13_st {
diff --git a/src/lib/libssl/ssl_sigalgs.c b/src/lib/libssl/ssl_sigalgs.c
index 3a7f6d6687..50f4802fdb 100644
--- a/src/lib/libssl/ssl_sigalgs.c
+++ b/src/lib/libssl/ssl_sigalgs.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_sigalgs.c,v 1.17 2019/03/19 16:56:04 jsing Exp $ */
+/* $OpenBSD: ssl_sigalgs.c,v 1.18 2019/03/25 17:21:18 jsing Exp $ */
 /*
  * Copyright (c) 2018-2019 Bob Beck <beck@openbsd.org>
  *
@@ -246,7 +246,8 @@ ssl_sigalgs_build(CBB *cbb, uint16_t *values, size_t len)
 }
 
 int
-ssl_sigalg_pkey_ok(const struct ssl_sigalg *sigalg, EVP_PKEY *pkey)
+ssl_sigalg_pkey_ok(const struct ssl_sigalg *sigalg, EVP_PKEY *pkey,
+    int check_curve)
 {
         if (sigalg == NULL || pkey == NULL)
                 return 0;
@@ -266,12 +267,85 @@ ssl_sigalg_pkey_ok(const struct ssl_sigalg *sigalg, EVP_PKEY *pkey)
         if (pkey->type == EVP_PKEY_EC) {
                 if (sigalg->curve_nid == 0)
                         return 0;
-                /* Curve must match for EC keys */
-                if (EC_GROUP_get_curve_name(EC_KEY_get0_group
+                /* Curve must match for EC keys. */
+                if (check_curve && EC_GROUP_get_curve_name(EC_KEY_get0_group
                     (EVP_PKEY_get0_EC_KEY(pkey))) != sigalg->curve_nid) {
-                        return 1; /* XXX www.videolan.org curve mismatch */
+                        return 0;
                 }
         }
 
         return 1;
 }
+
+const struct ssl_sigalg *
+ssl_sigalg_select(SSL *s, EVP_PKEY *pkey)
+{
+        uint16_t *tls_sigalgs = tls12_sigalgs;
+        size_t tls_sigalgs_len = tls12_sigalgs_len;
+        int check_curve = 0;
+        CBS cbs;
+
+        if (TLS1_get_version(s) >= TLS1_3_VERSION) {
+                tls_sigalgs = tls13_sigalgs;
+                tls_sigalgs_len = tls13_sigalgs_len;
+                check_curve = 1;
+        }
+
+        /* Pre TLS 1.2 defaults */
+        if (!SSL_USE_SIGALGS(s)) {
+                switch (pkey->type) {
+                case EVP_PKEY_RSA:
+                        return ssl_sigalg_lookup(SIGALG_RSA_PKCS1_MD5_SHA1);
+                case EVP_PKEY_EC:
+                        return ssl_sigalg_lookup(SIGALG_ECDSA_SHA1);
+#ifndef OPENSSL_NO_GOST
+                case EVP_PKEY_GOSTR01:
+                        return ssl_sigalg_lookup(SIGALG_GOSTR01_GOST94);
+#endif
+                }
+                SSLerror(s, SSL_R_UNKNOWN_PKEY_TYPE);
+                return (NULL);
+        }
+
+        /*
+         * RFC 5246 allows a TLS 1.2 client to send no sigalgs, in
+         * which case the server must use the the default.
+         */
+        if (TLS1_get_version(s) < TLS1_3_VERSION &&
+            S3I(s)->hs.sigalgs == NULL) {
+                switch (pkey->type) {
+                case EVP_PKEY_RSA:
+                        return ssl_sigalg_lookup(SIGALG_RSA_PKCS1_SHA1);
+                case EVP_PKEY_EC:
+                        return ssl_sigalg_lookup(SIGALG_ECDSA_SHA1);
+#ifndef OPENSSL_NO_GOST
+                case EVP_PKEY_GOSTR01:
+                        return ssl_sigalg_lookup(SIGALG_GOSTR01_GOST94);
+#endif
+                }
+                SSLerror(s, SSL_R_UNKNOWN_PKEY_TYPE);
+                return (NULL);
+        }
+
+        /*
+         * If we get here, we have client or server sent sigalgs, use one.
+         */
+        CBS_init(&cbs, S3I(s)->hs.sigalgs, S3I(s)->hs.sigalgs_len);
+        while (CBS_len(&cbs) > 0) {
+                uint16_t sig_alg;
+                const struct ssl_sigalg *sigalg;
+
+                if (!CBS_get_u16(&cbs, &sig_alg))
+                        return 0;
+
+                if ((sigalg = ssl_sigalg(sig_alg, tls_sigalgs,
+                    tls_sigalgs_len)) == NULL)
+                        continue;
+
+                if (ssl_sigalg_pkey_ok(sigalg, pkey, check_curve))
+                        return sigalg;
+        }
+
+        SSLerror(s, SSL_R_UNKNOWN_PKEY_TYPE);
+        return NULL;
+}
diff --git a/src/lib/libssl/ssl_sigalgs.h b/src/lib/libssl/ssl_sigalgs.h
index d05f66f7ba..d06731e10d 100644
--- a/src/lib/libssl/ssl_sigalgs.h
+++ b/src/lib/libssl/ssl_sigalgs.h
@@ -1,6 +1,6 @@
-/* $OpenBSD: ssl_sigalgs.h,v 1.12 2019/01/23 23:47:13 beck Exp $ */
+/* $OpenBSD: ssl_sigalgs.h,v 1.13 2019/03/25 17:21:18 jsing Exp $ */
 /*
- * Copyright (c) 2018, Bob Beck <beck@openbsd.org>
+ * Copyright (c) 2018-2019 Bob Beck <beck@openbsd.org>
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -78,7 +78,9 @@ const struct ssl_sigalg *ssl_sigalg_lookup(uint16_t sigalg);
 const struct ssl_sigalg *ssl_sigalg(uint16_t sigalg, uint16_t *values, size_t len);
 int ssl_sigalgs_build(CBB *cbb, uint16_t *values, size_t len);
 int ssl_sigalg_pkey_check(uint16_t sigalg, EVP_PKEY *pk);
-int ssl_sigalg_pkey_ok(const struct ssl_sigalg *sigalg, EVP_PKEY *pkey);
+int ssl_sigalg_pkey_ok(const struct ssl_sigalg *sigalg, EVP_PKEY *pkey,
+    int check_curve);
+const struct ssl_sigalg *ssl_sigalg_select(SSL *s, EVP_PKEY *pkey);
 
 __END_HIDDEN_DECLS
 
diff --git a/src/lib/libssl/ssl_srvr.c b/src/lib/libssl/ssl_srvr.c
index f2aafc3032..745d0228f3 100644
--- a/src/lib/libssl/ssl_srvr.c
+++ b/src/lib/libssl/ssl_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.65 2019/03/25 16:35:48 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.66 2019/03/25 17:21:18 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -2177,7 +2177,7 @@ ssl3_get_cert_verify(SSL *s)
                         al = SSL_AD_DECODE_ERROR;
                         goto f_err;
                 }
-                if (!ssl_sigalg_pkey_ok(sigalg, pkey)) {
+                if (!ssl_sigalg_pkey_ok(sigalg, pkey, 0)) {
                         SSLerror(s, SSL_R_WRONG_SIGNATURE_TYPE);
                         al = SSL_AD_DECODE_ERROR;
                         goto f_err;
diff --git a/src/lib/libssl/ssl_tlsext.c b/src/lib/libssl/ssl_tlsext.c
index de9fabd4c7..0e37cc3cc0 100644
--- a/src/lib/libssl/ssl_tlsext.c
+++ b/src/lib/libssl/ssl_tlsext.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_tlsext.c,v 1.43 2019/03/19 16:53:03 jsing Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.44 2019/03/25 17:21:18 jsing Exp $ */
 /*
  * Copyright (c) 2016, 2017, 2019 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -556,19 +556,16 @@ tlsext_sigalgs_client_build(SSL *s, CBB *cbb)
 int
 tlsext_sigalgs_server_parse(SSL *s, CBS *cbs, int *alert)
 {
-        uint16_t *tls_sigalgs = tls12_sigalgs;
-        size_t tls_sigalgs_len = tls12_sigalgs_len;
         CBS sigalgs;
 
-        if (s->version >= TLS1_3_VERSION) {
-                tls_sigalgs = tls13_sigalgs;
-                tls_sigalgs_len = tls13_sigalgs_len;
-        }
-
         if (!CBS_get_u16_length_prefixed(cbs, &sigalgs))
                 return 0;
+        if (CBS_len(&sigalgs) % 2 != 0 || CBS_len(&sigalgs) > 64)
+                return 0;
+        if (!CBS_stow(&sigalgs, &S3I(s)->hs.sigalgs, &S3I(s)->hs.sigalgs_len))
+                return 0;
 
-        return tls1_process_sigalgs(s, &sigalgs, tls_sigalgs, tls_sigalgs_len);
+        return 1;
 }
 
 int
diff --git a/src/lib/libssl/tls13_client.c b/src/lib/libssl/tls13_client.c
index 56faf3f5d4..5b4ecdb47d 100644
--- a/src/lib/libssl/tls13_client.c
+++ b/src/lib/libssl/tls13_client.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls13_client.c,v 1.13 2019/02/28 17:51:19 jsing Exp $ */
+/* $OpenBSD: tls13_client.c,v 1.14 2019/03/25 17:21:18 jsing Exp $ */
 /*
  * Copyright (c) 2018, 2019 Joel Sing <jsing@openbsd.org>
  *
@@ -527,7 +527,7 @@ tls13_server_certificate_verify_recv(struct tls13_ctx *ctx)
                 goto err;
         if ((pkey = X509_get0_pubkey(cert)) == NULL)
                 goto err;
-        if (!ssl_sigalg_pkey_ok(sigalg, pkey))
+        if (!ssl_sigalg_pkey_ok(sigalg, pkey, 1))
                 goto err;
 
         if (CBS_len(&signature) > EVP_PKEY_size(pkey))