Improve server certificate selection for TLSv1.3.

This allows an EC certificate to be selected and used, if the client
sigalgs would allow it.

With feedback from tb@

ok inoguchi@ tb@

2 files changed, 94 insertions, 23 deletions

diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index 046c4bba528..e7e3e561543 100644
--- a/lib/libssl/ssl_locl.h
+++ b/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.276 2020/05/29 17:39:42 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.277 2020/05/29 18:00:10 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -435,6 +435,12 @@ typedef struct ssl_handshake_st {
 	uint8_t *sigalgs;
 } SSL_HANDSHAKE;
 
+typedef struct cert_pkey_st {
+	X509 *x509;
+	EVP_PKEY *privatekey;
+	STACK_OF(X509) *chain;
+} CERT_PKEY;
+
 typedef struct ssl_handshake_tls13_st {
 	uint16_t min_version;
 	uint16_t max_version;
@@ -443,6 +449,10 @@ typedef struct ssl_handshake_tls13_st {
 	int use_legacy;
 	int hrr;
 
+	/* Certificate and sigalg selected for use (static pointers). */
+	const CERT_PKEY *cpk;
+	const struct ssl_sigalg *sigalg;
+
 	/* Version proposed by peer server. */
 	uint16_t server_version;
 
@@ -985,12 +995,6 @@ typedef struct dtls1_state_internal_st {
 } DTLS1_STATE_INTERNAL;
 #define D1I(s) (s->d1->internal)
 
-typedef struct cert_pkey_st {
-	X509 *x509;
-	EVP_PKEY *privatekey;
-	STACK_OF(X509) *chain;
-} CERT_PKEY;
-
 typedef struct cert_st {
 	/* Current active set */
 	CERT_PKEY *key; /* ALWAYS points to an element of the pkeys array
diff --git a/lib/libssl/tls13_server.c b/lib/libssl/tls13_server.c
index 181ba583a06..e9fecdee267 100644
--- a/lib/libssl/tls13_server.c
+++ b/lib/libssl/tls13_server.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls13_server.c,v 1.54 2020/05/29 17:47:30 jsing Exp $ */
+/* $OpenBSD: tls13_server.c,v 1.55 2020/05/29 18:00:10 jsing Exp $ */
 /*
  * Copyright (c) 2019, 2020 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2020 Bob Beck <beck@openbsd.org>
@@ -16,6 +16,8 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include <openssl/x509v3.h>
+
 #include "ssl_locl.h"
 #include "ssl_tlsext.h"
 
@@ -448,19 +450,85 @@ tls13_server_certificate_request_send(struct tls13_ctx *ctx, CBB *cbb)
 	return 0;
 }
 
+static int
+tls13_server_check_certificate(struct tls13_ctx *ctx, CERT_PKEY *cpk,
+    int *ok, const struct ssl_sigalg **out_sigalg)
+{
+	const struct ssl_sigalg *sigalg;
+	SSL *s = ctx->ssl;
+
+	*ok = 0;
+	*out_sigalg = NULL;
+
+	if (cpk->x509 == NULL || cpk->privatekey == NULL)
+		goto done;
+
+	if (!X509_check_purpose(cpk->x509, -1, 0))
+		return 0;
+
+	/*
+	 * The digitalSignature bit MUST be set if the Key Usage extension is
+	 * present as per RFC 8446 section 4.4.2.2.
+	 */ 
+	if ((cpk->x509->ex_flags & EXFLAG_KUSAGE) &&
+	    !(cpk->x509->ex_kusage & X509v3_KU_DIGITAL_SIGNATURE))
+		goto done;
+
+	if ((sigalg = ssl_sigalg_select(s, cpk->privatekey)) == NULL)
+		goto done;
+
+	*ok = 1;
+	*out_sigalg = sigalg;
+
+ done:
+	return 1;
+}
+ 
+static int
+tls13_server_select_certificate(struct tls13_ctx *ctx, CERT_PKEY **out_cpk,
+    const struct ssl_sigalg **out_sigalg)
+{
+	SSL *s = ctx->ssl;
+	const struct ssl_sigalg *sigalg;
+	CERT_PKEY *cpk;
+	int cert_ok;
+
+	*out_cpk = NULL;
+	*out_sigalg = NULL;
+
+	cpk = &s->cert->pkeys[SSL_PKEY_ECC];
+	if (!tls13_server_check_certificate(ctx, cpk, &cert_ok, &sigalg))
+		return 0;
+	if (cert_ok)
+		goto done;
+
+	cpk = &s->cert->pkeys[SSL_PKEY_RSA];
+	if (!tls13_server_check_certificate(ctx, cpk, &cert_ok, &sigalg))
+		return 0;
+	if (cert_ok)
+		goto done;
+
+	return 0;
+
+ done:
+	*out_cpk = cpk;
+	*out_sigalg = sigalg;
+
+	return 1;
+}
+
 int
 tls13_server_certificate_send(struct tls13_ctx *ctx, CBB *cbb)
 {
 	SSL *s = ctx->ssl;
 	CBB cert_request_context, cert_list;
+	const struct ssl_sigalg *sigalg;
 	STACK_OF(X509) *chain;
 	CERT_PKEY *cpk;
 	X509 *cert;
 	int i, ret = 0;
 
-	/* XXX - Need to revisit certificate selection. */
-	cpk = &s->cert->pkeys[SSL_PKEY_RSA];
-	if (cpk->x509 == NULL) {
+	if (!tls13_server_select_certificate(ctx, &cpk, &sigalg)) {
 		/* A server must always provide a certificate. */
 		ctx->alert = TLS13_ALERT_HANDSHAKE_FAILURE;
 		tls13_set_errorx(ctx, TLS13_ERR_NO_CERTIFICATE, 0,
@@ -468,6 +536,9 @@ tls13_server_certificate_send(struct tls13_ctx *ctx, CBB *cbb)
 		goto err;
 	}
 
+	ctx->hs->cpk = cpk;
+	ctx->hs->sigalg = sigalg;
+
 	if ((chain = cpk->chain) == NULL)
 		chain = s->ctx->extra_certs;
 
@@ -502,27 +573,23 @@ tls13_server_certificate_send(struct tls13_ctx *ctx, CBB *cbb)
 int
 tls13_server_certificate_verify_send(struct tls13_ctx *ctx, CBB *cbb)
 {
-	SSL *s = ctx->ssl;
-	const struct ssl_sigalg *sigalg = NULL;
+	const struct ssl_sigalg *sigalg;
 	uint8_t *sig = NULL, *sig_content = NULL;
 	size_t sig_len, sig_content_len;
 	EVP_MD_CTX *mdctx = NULL;
 	EVP_PKEY_CTX *pctx;
 	EVP_PKEY *pkey;
-	CERT_PKEY *cpk;
+	const CERT_PKEY *cpk;
 	CBB sig_cbb;
 	int ret = 0;
 
 	memset(&sig_cbb, 0, sizeof(sig_cbb));
 
-	/* XXX - Need to revisit certificate selection. */
-	cpk = &s->cert->pkeys[SSL_PKEY_RSA];
-	pkey = cpk->privatekey;
-
-	if ((sigalg = ssl_sigalg_select(s, pkey)) == NULL) {
-		/* XXX - SSL_R_SIGNATURE_ALGORITHMS_ERROR */
+	if ((cpk = ctx->hs->cpk) == NULL)
+ 		goto err;
+	if ((sigalg = ctx->hs->sigalg) == NULL)
 		goto err;
-	}
+	pkey = cpk->privatekey;
 
 	if (!CBB_init(&sig_cbb, 0))
 		goto err;
@@ -829,9 +896,9 @@ tls13_client_certificate_verify_recv(struct tls13_ctx *ctx, CBS *cbs)
 	ret = 1;
 
  err:
-	if (!ret && ctx->alert == 0) {
+	if (!ret && ctx->alert == 0)
 		ctx->alert = TLS13_ALERT_DECODE_ERROR;
-	}
+
 	CBB_cleanup(&cbb);
 	EVP_MD_CTX_free(mdctx);
 	free(sig_content);