From 11d2641f96ead76deb5b8fac638a3ad36a971a66 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 17 Jul 2018 16:31:07 +0100 Subject: [PATCH] Check that the public key OID matches the sig alg Using the rsa_pss_rsae_sha256 sig alg should imply that the key OID is rsaEncryption. Similarly rsa_pss_pss_sha256 implies the key OID is rsassaPss. However we did not check this and incorrectly tolerated a key OID that did not match the sig alg sent by the peer. Fixes #6611 Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/6732) --- ssl/ssl_cert.c | 31 ++++++++++++++++++++++--------- ssl/ssl_locl.h | 1 + ssl/t1_lib.c | 10 +++++++++- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index b2b342767c..df5cff79c9 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -995,22 +995,35 @@ int ssl_ctx_security(const SSL_CTX *ctx, int op, int bits, int nid, void *other) ctx->cert->sec_ex); } -const SSL_CERT_LOOKUP *ssl_cert_lookup_by_pkey(const EVP_PKEY *pk, size_t *pidx) +int ssl_cert_lookup_by_nid(int nid, size_t *pidx) { - int nid = EVP_PKEY_id(pk); size_t i; - if (nid == NID_undef) - return NULL; - for (i = 0; i < OSSL_NELEM(ssl_cert_info); i++) { if (ssl_cert_info[i].nid == nid) { - if (pidx != NULL) - *pidx = i; - return &ssl_cert_info[i]; + *pidx = i; + return 1; } } - return NULL; + + return 0; +} + +const SSL_CERT_LOOKUP *ssl_cert_lookup_by_pkey(const EVP_PKEY *pk, size_t *pidx) +{ + int nid = EVP_PKEY_id(pk); + size_t tmpidx; + + if (nid == NID_undef) + return NULL; + + if (!ssl_cert_lookup_by_nid(nid, &tmpidx)) + return NULL; + + if (pidx != NULL) + *pidx = tmpidx; + + return &ssl_cert_info[tmpidx]; } const SSL_CERT_LOOKUP *ssl_cert_lookup_by_idx(size_t idx) diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index b38052f614..e7258d439a 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -2288,6 +2288,7 @@ __owur int ssl_security(const SSL *s, int op, int bits, int nid, void *other); __owur int ssl_ctx_security(const SSL_CTX *ctx, int op, int bits, int nid, void *other); +__owur int ssl_cert_lookup_by_nid(int nid, size_t *pidx); __owur const SSL_CERT_LOOKUP *ssl_cert_lookup_by_pkey(const EVP_PKEY *pk, size_t *pidx); __owur const SSL_CERT_LOOKUP *ssl_cert_lookup_by_idx(size_t idx); diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 3c7590c31f..df27ba6b7b 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -955,7 +955,7 @@ int tls12_check_peer_sigalg(SSL *s, uint16_t sig, EVP_PKEY *pkey) const uint16_t *sent_sigs; const EVP_MD *md = NULL; char sigalgstr[2]; - size_t sent_sigslen, i; + size_t sent_sigslen, i, cidx; int pkeyid = EVP_PKEY_id(pkey); const SIGALG_LOOKUP *lu; @@ -986,6 +986,14 @@ int tls12_check_peer_sigalg(SSL *s, uint16_t sig, EVP_PKEY *pkey) SSL_R_WRONG_SIGNATURE_TYPE); return 0; } + /* Check the sigalg is consistent with the key OID */ + if (!ssl_cert_lookup_by_nid(EVP_PKEY_id(pkey), &cidx) + || lu->sig_idx != (int)cidx) { + SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS12_CHECK_PEER_SIGALG, + SSL_R_WRONG_SIGNATURE_TYPE); + return 0; + } + #ifndef OPENSSL_NO_EC if (pkeyid == EVP_PKEY_EC) { -- 2.25.1