Stop keeping track of sigalgs by guessing it from digest and pkey,

just keep the sigalg around so we can remember what we actually
decided to use.
ok jsing@

lib/libssl/ssl_cert.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_cert.c,v 1.69 2018/11/08 20:55:18 jsing Exp $ */
+/* $OpenBSD: ssl_cert.c,v 1.70 2018/11/10 01:19:09 beck Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -159,14 +159,18 @@ SSL_get_ex_data_X509_STORE_CTX_idx(void)
 }
 
 static void
-ssl_cert_set_default_md(CERT *cert)
+ssl_cert_set_default_sigalgs(CERT *cert)
 {
 	/* Set digest values to defaults */
-	cert->pkeys[SSL_PKEY_RSA_SIGN].digest = EVP_sha1();
+	cert->pkeys[SSL_PKEY_RSA_SIGN].sigalg =
-	cert->pkeys[SSL_PKEY_RSA_ENC].digest = EVP_sha1();
+	    ssl_sigalg_lookup(SIGALG_RSA_PKCS1_SHA1);
-	cert->pkeys[SSL_PKEY_ECC].digest = EVP_sha1();
+	cert->pkeys[SSL_PKEY_RSA_ENC].sigalg =
+	    ssl_sigalg_lookup(SIGALG_RSA_PKCS1_SHA1);
+	cert->pkeys[SSL_PKEY_ECC].sigalg =
+	    ssl_sigalg_lookup(SIGALG_ECDSA_SHA1);
 #ifndef OPENSSL_NO_GOST
-	cert->pkeys[SSL_PKEY_GOST01].digest = EVP_gostr341194();
+	cert->pkeys[SSL_PKEY_GOST01].sigalg =
+	    ssl_sigalg_lookup(SIGALG_GOSTR01_GOST94);
 #endif
 }
 
@@ -182,7 +186,7 @@ ssl_cert_new(void)
 	}
 	ret->key = &(ret->pkeys[SSL_PKEY_RSA_ENC]);
 	ret->references = 1;
-	ssl_cert_set_default_md(ret);
+	ssl_cert_set_default_sigalgs(ret);
 	return (ret);
 }
 
@@ -280,10 +284,10 @@ ssl_cert_dup(CERT *cert)
 
 	ret->references = 1;
 	/*
-	 * Set digests to defaults. NB: we don't copy existing values
+	 * Set sigalgs to defaults. NB: we don't copy existing values
 	 * as they will be set during handshake.
 	 */
-	ssl_cert_set_default_md(ret);
+	ssl_cert_set_default_sigalgs(ret);
 
 	return (ret);
 

lib/libssl/ssl_clnt.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_clnt.c,v 1.40 2018/11/09 17:43:31 jsing Exp $ */
+/* $OpenBSD: ssl_clnt.c,v 1.41 2018/11/10 01:19:09 beck Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1509,17 +1509,19 @@ ssl3_get_server_key_exchange(SSL *s)
 	/* if it was signed, check the signature */
 	if (pkey != NULL) {
 		if (SSL_USE_SIGALGS(s)) {
-			uint16_t sigalg;
+			const struct ssl_sigalg *sigalg;
+			uint16_t sigalg_value;
 
-			if (!CBS_get_u16(&cbs, &sigalg))
+			if (!CBS_get_u16(&cbs, &sigalg_value))
 				goto truncated;
-			if ((md = ssl_sigalg_md(sigalg, tls12_sigalgs,
+			if ((sigalg = ssl_sigalg(sigalg_value, tls12_sigalgs,
-			    tls12_sigalgs_len)) == NULL) {
+			    tls12_sigalgs_len)) == NULL ||
+			    (md = sigalg->md()) == NULL) {
 				SSLerror(s, SSL_R_UNKNOWN_DIGEST);
 				al = SSL_AD_DECODE_ERROR;
 				goto f_err;
 			}
-			if (!ssl_sigalg_pkey_check(sigalg, pkey)) {
+			if (sigalg->key_type != pkey->type) {
 				SSLerror(s, SSL_R_WRONG_SIGNATURE_TYPE);
 				al = SSL_AD_DECODE_ERROR;
 				goto f_err;
@@ -2405,13 +2407,10 @@ ssl3_send_client_verify(SSL *s)
 		 * using agreed digest and cached handshake records.
 		 */
 		if (SSL_USE_SIGALGS(s)) {
-			uint16_t sigalg;
+			md = s->cert->key->sigalg->md();
-
-			md = s->cert->key->digest;
 			if (!tls1_transcript_data(s, &hdata, &hdatalen) ||
-			    (sigalg = ssl_sigalg_value(pkey, md)) ==
+			    !CBB_add_u16(&cert_verify,
-			    SIGALG_NONE ||
+				s->cert->key->sigalg->value)) {
-			    !CBB_add_u16(&cert_verify, sigalg)) {
 				SSLerror(s, ERR_R_INTERNAL_ERROR);
 				goto err;
 			}
@@ -2457,6 +2456,7 @@ ssl3_send_client_verify(SSL *s)
 			if (!EVP_DigestInit_ex(&mctx, md, NULL) ||
 			    !EVP_DigestUpdate(&mctx, hdata, hdatalen) ||
 			    !EVP_DigestFinal(&mctx, signbuf, &u) ||
+
 			    (EVP_PKEY_CTX_set_signature_md(pctx, md) <= 0) ||
 			    (EVP_PKEY_CTX_ctrl(pctx, -1, EVP_PKEY_OP_SIGN,
 				EVP_PKEY_CTRL_GOST_SIG_FORMAT,

lib/libssl/ssl_lib.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_lib.c,v 1.191 2018/11/08 20:55:18 jsing Exp $ */
+/* $OpenBSD: ssl_lib.c,v 1.192 2018/11/10 01:19:09 beck Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -156,6 +156,7 @@
 #endif
 
 #include "bytestring.h"
+#include "ssl_sigalgs.h"
 
 const char *SSL_version_str = OPENSSL_VERSION_TEXT;
 
@@ -2173,8 +2174,11 @@ ssl_get_server_send_cert(const SSL *s)
 }
 
 EVP_PKEY *
-ssl_get_sign_pkey(SSL *s, const SSL_CIPHER *cipher, const EVP_MD **pmd)
+ssl_get_sign_pkey(SSL *s, const SSL_CIPHER *cipher, const EVP_MD **pmd,
+    const struct ssl_sigalg **sap)
 {
+	const struct ssl_sigalg *sigalg = NULL;
+	EVP_PKEY *pkey = NULL;
 	unsigned long	 alg_a;
 	CERT		*c;
 	int		 idx = -1;
@@ -2194,9 +2198,27 @@ ssl_get_sign_pkey(SSL *s, const SSL_CIPHER *cipher, const EVP_MD **pmd)
 		SSLerror(s, ERR_R_INTERNAL_ERROR);
 		return (NULL);
 	}
-	if (pmd)
+
-		*pmd = c->pkeys[idx].digest;
+	pkey = c->pkeys[idx].privatekey;
-	return (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_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) {
+		SSLerror(s, SSL_R_SIGNATURE_ALGORITHMS_ERROR);
+		return (NULL);
+	}
+	*pmd = sigalg->md();
+	*sap = sigalg;
+
+	return (pkey);
 }
 
 DH *
@@ -2810,9 +2832,9 @@ SSL_set_SSL_CTX(SSL *ssl, SSL_CTX* ctx)
 	ssl->cert = ssl_cert_dup(ctx->internal->cert);
 	if (ocert != NULL) {
 		int i;
-		/* Copy negotiated digests from original certificate. */
+		/* Copy negotiated sigalg from original certificate. */
 		for (i = 0; i < SSL_PKEY_NUM; i++)
-			ssl->cert->pkeys[i].digest = ocert->pkeys[i].digest;
+			ssl->cert->pkeys[i].sigalg = ocert->pkeys[i].sigalg;
 		ssl_cert_free(ocert);
 	}
 	CRYPTO_add(&ctx->references, 1, CRYPTO_LOCK_SSL_CTX);

lib/libssl/ssl_locl.h

@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.223 2018/11/09 00:34:55 beck Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.224 2018/11/10 01:19:09 beck Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -161,6 +161,7 @@
 #include <openssl/stack.h>
 
 #include "bytestring.h"
+#include "ssl_sigalgs.h"
 
 __BEGIN_HIDDEN_DECLS
 
@@ -930,8 +931,8 @@ typedef struct dtls1_state_internal_st {
 typedef struct cert_pkey_st {
 	X509 *x509;
 	EVP_PKEY *privatekey;
-	/* Digest to use when signing */
+	/* sigalg to use when signing */
-	const EVP_MD *digest;
+	const struct ssl_sigalg *sigalg;
 } CERT_PKEY;
 
 typedef struct cert_st {
@@ -1076,7 +1077,8 @@ int ssl_undefined_void_function(void);
 int ssl_undefined_const_function(const SSL *s);
 CERT_PKEY *ssl_get_server_send_pkey(const SSL *s);
 X509 *ssl_get_server_send_cert(const SSL *);
-EVP_PKEY *ssl_get_sign_pkey(SSL *s, const SSL_CIPHER *c, const EVP_MD **pmd);
+EVP_PKEY *ssl_get_sign_pkey(SSL *s, const SSL_CIPHER *c, const EVP_MD **pmd,
+    const struct ssl_sigalg **sap);
 DH *ssl_get_auto_dh(SSL *s);
 int ssl_cert_type(X509 *x, EVP_PKEY *pkey);
 void ssl_set_cert_masks(CERT *c, const SSL_CIPHER *cipher);

lib/libssl/ssl_sigalgs.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_sigalgs.c,v 1.3 2018/11/09 05:43:39 beck Exp $ */
+/* $OpenBSD: ssl_sigalgs.c,v 1.4 2018/11/10 01:19:09 beck Exp $ */
 /*
  * Copyright (c) 2018, Bob Beck <beck@openbsd.org>
  *
@@ -143,7 +143,7 @@ const struct ssl_sigalg sigalgs[] = {
 		.value = SIGALG_RSA_PKCS1_SHA1,
 		.key_type = EVP_PKEY_RSA,
 		.pkey_idx = SSL_PKEY_RSA_SIGN,
-		.md = EVP_sha1,
+		.md = EVP_md5_sha1,
 	},
 	{
 		.value = SIGALG_ECDSA_SHA1,
@@ -187,8 +187,8 @@ ssl_sigalg_lookup(uint16_t sigalg)
 	return NULL;
 }
 
-const EVP_MD *
+const struct ssl_sigalg *
-ssl_sigalg_md(uint16_t sigalg, uint16_t *values, size_t len)
+ssl_sigalg(uint16_t sigalg, uint16_t *values, size_t len)
 {
 	const struct ssl_sigalg *sap;
 	int i;
@@ -199,23 +199,12 @@ ssl_sigalg_md(uint16_t sigalg, uint16_t *values, size_t len)
 	}
 	if (values[i] == sigalg) {
 		if ((sap = ssl_sigalg_lookup(sigalg)) != NULL)
-			return sap->md();
+			return sap;
 	}
 
 	return NULL;
 }
 
-int
-ssl_sigalg_pkey_check(uint16_t sigalg, EVP_PKEY *pk)
-{
-	const struct ssl_sigalg *sap;
-
-	if ((sap = ssl_sigalg_lookup(sigalg)) != NULL)
-		return sap->key_type == pk->type;
-
-	return 0;
-}
-
 uint16_t
 ssl_sigalg_value(const EVP_PKEY *pk, const EVP_MD *md)
 {

lib/libssl/ssl_sigalgs.h

@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_sigalgs.h,v 1.4 2018/11/09 05:43:39 beck Exp $ */
+/* $OpenBSD: ssl_sigalgs.h,v 1.5 2018/11/10 01:19:09 beck Exp $ */
 /*
  * Copyright (c) 2018, Bob Beck <beck@openbsd.org>
  *
@@ -70,7 +70,7 @@ extern uint16_t tls12_sigalgs[];
 extern size_t tls12_sigalgs_len;
 
 const struct ssl_sigalg *ssl_sigalg_lookup(uint16_t sigalg);
-const EVP_MD * ssl_sigalg_md(uint16_t sigalg, uint16_t *values, size_t len);
+const struct ssl_sigalg *ssl_sigalg(uint16_t sigalg, uint16_t *values, size_t len);
 uint16_t ssl_sigalg_value(const EVP_PKEY *pk, const EVP_MD *md);
 int ssl_sigalgs_build(CBB *cbb, uint16_t *values, size_t len);
 int ssl_sigalg_pkey_check(uint16_t sigalg, EVP_PKEY *pk);

lib/libssl/ssl_srvr.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.54 2018/11/09 05:43:39 beck Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.55 2018/11/10 01:19:09 beck Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1483,6 +1483,7 @@ int
 ssl3_send_server_key_exchange(SSL *s)
 {
 	CBB cbb, cbb_params, cbb_signature, server_kex;
+	const struct ssl_sigalg *sigalg = NULL;
 	unsigned char *signature = NULL;
 	unsigned int signature_len;
 	unsigned char *params = NULL;
@@ -1529,28 +1530,14 @@ ssl3_send_server_key_exchange(SSL *s)
 		/* Add signature unless anonymous. */
 		if (!(S3I(s)->hs.new_cipher->algorithm_auth & SSL_aNULL)) {
 			if ((pkey = ssl_get_sign_pkey(s, S3I(s)->hs.new_cipher,
-			    &md)) == NULL) {
+			    &md, &sigalg)) == NULL) {
 				al = SSL_AD_DECODE_ERROR;
 				goto f_err;
 			}
 
-			if (pkey->type == EVP_PKEY_RSA && !SSL_USE_SIGALGS(s))
-				md = EVP_md5_sha1();
-		
-			if (md == NULL) {
-				/* Is this error check actually needed? */
-				al = SSL_AD_HANDSHAKE_FAILURE;
-				SSLerror(s, SSL_R_UNKNOWN_PKEY_TYPE);
-				goto f_err;
-			}
-
 			/* Send signature algorithm. */
 			if (SSL_USE_SIGALGS(s)) {
-				uint16_t sigalg;
+				if (!CBB_add_u16(&server_kex, sigalg->value)) {
-				if ((sigalg = ssl_sigalg_value(pkey, md)) ==
-				    SIGALG_NONE ||
-				    !CBB_add_u16(&server_kex, sigalg)) {
-					/* Should never happen */
 					al = SSL_AD_INTERNAL_ERROR;
 					SSLerror(s, ERR_R_INTERNAL_ERROR);
 					goto f_err;
@@ -2155,17 +2142,19 @@ ssl3_get_cert_verify(SSL *s)
 			goto err;
 	} else {
 		if (SSL_USE_SIGALGS(s)) {
-			uint16_t sigalg;
+			const struct ssl_sigalg *sigalg;
+			uint16_t sigalg_value;
 
-			if (!CBS_get_u16(&cbs, &sigalg))
+			if (!CBS_get_u16(&cbs, &sigalg_value))
 				goto truncated;
-			if ((md = ssl_sigalg_md(sigalg, tls12_sigalgs,
+			if ((sigalg = ssl_sigalg(sigalg_value, tls12_sigalgs,
-			    tls12_sigalgs_len)) == NULL) {
+			    tls12_sigalgs_len)) == NULL ||
+			    (md = sigalg->md()) == NULL) {
 				SSLerror(s, SSL_R_UNKNOWN_DIGEST);
 				al = SSL_AD_DECODE_ERROR;
 				goto f_err;
 			}
-			if (!ssl_sigalg_pkey_check(sigalg, pkey)) {
+			if (sigalg->key_type != pkey->type) {
 				SSLerror(s, SSL_R_WRONG_SIGNATURE_TYPE);
 				al = SSL_AD_DECODE_ERROR;
 				goto f_err;

lib/libssl/t1_lib.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_lib.c,v 1.149 2018/11/09 00:34:55 beck Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.150 2018/11/10 01:19:09 beck Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1010,26 +1010,25 @@ tls1_process_sigalgs(SSL *s, CBS *cbs)
 	if (!SSL_USE_SIGALGS(s))
 		return 1;
 
-	c->pkeys[SSL_PKEY_RSA_SIGN].digest = NULL;
+	c->pkeys[SSL_PKEY_RSA_SIGN].sigalg = NULL;
-	c->pkeys[SSL_PKEY_RSA_ENC].digest = NULL;
+	c->pkeys[SSL_PKEY_RSA_ENC].sigalg = NULL;
-	c->pkeys[SSL_PKEY_ECC].digest = NULL;
+	c->pkeys[SSL_PKEY_ECC].sigalg = NULL;
 #ifndef OPENSSL_NO_GOST
-	c->pkeys[SSL_PKEY_GOST01].digest = NULL;
+	c->pkeys[SSL_PKEY_GOST01].sigalg = NULL;
 #endif
 	while (CBS_len(cbs) > 0) {
-		const EVP_MD *md;
 		uint16_t sig_alg;
 		const struct ssl_sigalg *sigalg;
 
 		if (!CBS_get_u16(cbs, &sig_alg))
 			return 0;
 
-		if ((sigalg = ssl_sigalg_lookup(sig_alg)) != NULL &&
+		if ((sigalg = ssl_sigalg(sig_alg, tls12_sigalgs,
-		    c->pkeys[sigalg->pkey_idx].digest == NULL) {
+		    tls12_sigalgs_len)) != NULL &&
-			md = sigalg->md();
+		    c->pkeys[sigalg->pkey_idx].sigalg == NULL) {
-			c->pkeys[sigalg->pkey_idx].digest = md;
+			c->pkeys[sigalg->pkey_idx].sigalg = sigalg;
 			if (sigalg->pkey_idx == SSL_PKEY_RSA_SIGN)
-				c->pkeys[SSL_PKEY_RSA_ENC].digest = md;
+				c->pkeys[SSL_PKEY_RSA_ENC].sigalg = sigalg;
 		}
 	}
 
@@ -1037,15 +1036,20 @@ tls1_process_sigalgs(SSL *s, CBS *cbs)
 	 * Set any remaining keys to default values. NOTE: if alg is not
 	 * supported it stays as NULL.
 	 */
-	if (c->pkeys[SSL_PKEY_RSA_SIGN].digest == NULL)
+	if (c->pkeys[SSL_PKEY_RSA_SIGN].sigalg == NULL)
-		c->pkeys[SSL_PKEY_RSA_SIGN].digest = EVP_sha1();
+		c->pkeys[SSL_PKEY_RSA_SIGN].sigalg =
-	if (c->pkeys[SSL_PKEY_RSA_ENC].digest == NULL)
+		    ssl_sigalg_lookup(SIGALG_RSA_PKCS1_SHA1);
-		c->pkeys[SSL_PKEY_RSA_ENC].digest = EVP_sha1();
+	if (c->pkeys[SSL_PKEY_RSA_ENC].sigalg == NULL)
-	if (c->pkeys[SSL_PKEY_ECC].digest == NULL)
+		c->pkeys[SSL_PKEY_RSA_ENC].sigalg =
-		c->pkeys[SSL_PKEY_ECC].digest = EVP_sha1();
+		    ssl_sigalg_lookup(SIGALG_RSA_PKCS1_SHA1);
+	if (c->pkeys[SSL_PKEY_ECC].sigalg == NULL)
+		c->pkeys[SSL_PKEY_RSA_ENC].sigalg =
+		    ssl_sigalg_lookup(SIGALG_ECDSA_SHA1);
+
 #ifndef OPENSSL_NO_GOST
-	if (c->pkeys[SSL_PKEY_GOST01].digest == NULL)
+	if (c->pkeys[SSL_PKEY_GOST01].sigalg == NULL)
-		c->pkeys[SSL_PKEY_GOST01].digest = EVP_gostr341194();
+		c->pkeys[SSL_PKEY_GOST01].sigalg =
+		    ssl_sigalg_lookup(SIGALG_GOSTR01_GOST94);
 #endif
 	return 1;
 }