From fe3066ee4072e226601209f1b5fb1d343457cef8 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 3 Jan 2017 10:01:39 +0000 Subject: [PATCH] Extend PSS signature support to TLSv1.2 TLSv1.3 introduces PSS based sigalgs. Offering these in a TLSv1.3 client implies that the client is prepared to accept these sigalgs even in TLSv1.2. Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/2157) --- ssl/ssl_locl.h | 7 +++- ssl/statem/statem_clnt.c | 43 ++++++++++++------- ssl/statem/statem_lib.c | 11 ++--- ssl/statem/statem_srvr.c | 37 ++++++++++++----- ssl/t1_lib.c | 69 ++++++++++++++++++++----------- test/recipes/70-test_sslsigalgs.t | 6 +-- 6 files changed, 114 insertions(+), 59 deletions(-) diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index efb03e2129..1bff6ada41 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1703,6 +1703,11 @@ typedef enum tlsext_index_en { #define TLSEXT_SIGALG_gostr34102012_512_gostr34112012_512 0xefef #define TLSEXT_SIGALG_gostr34102001_gostr3411 0xeded +#define SIGID_IS_PSS(sigid) ((sigid) == TLSEXT_SIGALG_rsa_pss_sha256 \ + || (sigid) == TLSEXT_SIGALG_rsa_pss_sha384 \ + || (sigid) == TLSEXT_SIGALG_rsa_pss_sha512) + + /* A dummy signature value not valid for TLSv1.2 signature algs */ #define TLSEXT_signature_rsa_pss 0x0101 @@ -2147,7 +2152,7 @@ __owur int tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello, __owur int tls_use_ticket(SSL *s); __owur int tls12_get_sigandhash(SSL *s, WPACKET *pkt, const EVP_PKEY *pk, - const EVP_MD *md); + const EVP_MD *md, int *ispss); __owur const EVP_MD *tls12_get_hash(int hash_nid); void ssl_set_sig_mask(uint32_t *pmask_a, SSL *s, int op); diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 432dc915b7..5eec0d1af3 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1824,9 +1824,11 @@ static int tls_process_ske_ecdhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al) MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) { - int al = -1; + int al = -1, ispss = 0; long alg_k; EVP_PKEY *pkey = NULL; + EVP_MD_CTX *md_ctx = NULL; + EVP_PKEY_CTX *pctx = NULL; PACKET save_param_start, signature; alg_k = s->s3->tmp.new_cipher->algorithm_mkey; @@ -1865,7 +1867,6 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) PACKET params; int maxsig; const EVP_MD *md = NULL; - EVP_MD_CTX *md_ctx; /* * |pkt| now points to the beginning of the signature, so the difference @@ -1896,6 +1897,7 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) al = SSL_AD_DECODE_ERROR; goto err; } + ispss = SIGID_IS_PSS(sigalg); #ifdef SSL_DEBUG fprintf(stderr, "USING TLSv1.2 HASH %s\n", EVP_MD_name(md)); #endif @@ -1936,29 +1938,39 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) goto err; } - if (EVP_VerifyInit_ex(md_ctx, md, NULL) <= 0 - || EVP_VerifyUpdate(md_ctx, &(s->s3->client_random[0]), - SSL3_RANDOM_SIZE) <= 0 - || EVP_VerifyUpdate(md_ctx, &(s->s3->server_random[0]), - SSL3_RANDOM_SIZE) <= 0 - || EVP_VerifyUpdate(md_ctx, PACKET_data(¶ms), - PACKET_remaining(¶ms)) <= 0) { - EVP_MD_CTX_free(md_ctx); + if (EVP_DigestVerifyInit(md_ctx, &pctx, md, NULL, pkey) <= 0) { + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_EVP_LIB); + goto err; + } + if (ispss) { + if (EVP_PKEY_CTX_set_rsa_padding(pctx, RSA_PKCS1_PSS_PADDING) <= 0 + /* -1 here means set saltlen to the digest len */ + || EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, -1) <= 0) { + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_EVP_LIB); + goto err; + } + } + if (EVP_DigestVerifyUpdate(md_ctx, &(s->s3->client_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_DigestVerifyUpdate(md_ctx, &(s->s3->server_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_DigestVerifyUpdate(md_ctx, PACKET_data(¶ms), + PACKET_remaining(¶ms)) <= 0) { al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, ERR_R_EVP_LIB); goto err; } - /* TODO(size_t): Convert this call */ - if (EVP_VerifyFinal(md_ctx, PACKET_data(&signature), - (unsigned int)PACKET_remaining(&signature), - pkey) <= 0) { + if (EVP_DigestVerifyFinal(md_ctx, PACKET_data(&signature), + PACKET_remaining(&signature)) <= 0) { /* bad signature */ - EVP_MD_CTX_free(md_ctx); al = SSL_AD_DECRYPT_ERROR; SSLerr(SSL_F_TLS_PROCESS_KEY_EXCHANGE, SSL_R_BAD_SIGNATURE); goto err; } EVP_MD_CTX_free(md_ctx); + md_ctx = NULL; } else { /* aNULL, aSRP or PSK do not need public keys */ if (!(s->s3->tmp.new_cipher->algorithm_auth & (SSL_aNULL | SSL_aSRP)) @@ -1986,6 +1998,7 @@ MSG_PROCESS_RETURN tls_process_key_exchange(SSL *s, PACKET *pkt) if (al != -1) ssl3_send_alert(s, SSL3_AL_FATAL, al); ossl_statem_set_error(s); + EVP_MD_CTX_free(md_ctx); return MSG_PROCESS_ERROR; } diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index 5b8ad3a63b..03efdecdac 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -136,7 +136,7 @@ int tls_construct_cert_verify(SSL *s, WPACKET *pkt) void *hdata; unsigned char *sig = NULL; unsigned char tls13tbs[TLS13_TBS_PREAMBLE_SIZE + EVP_MAX_MD_SIZE]; - int pktype; + int pktype, ispss = 0; if (s->server) { /* Only happens in TLSv1.3 */ @@ -166,7 +166,7 @@ int tls_construct_cert_verify(SSL *s, WPACKET *pkt) goto err; } - if (SSL_USE_SIGALGS(s) && !tls12_get_sigandhash(s, pkt, pkey, md)) { + if (SSL_USE_SIGALGS(s) && !tls12_get_sigandhash(s, pkt, pkey, md, &ispss)) { SSLerr(SSL_F_TLS_CONSTRUCT_CERT_VERIFY, ERR_R_INTERNAL_ERROR); goto err; } @@ -186,7 +186,7 @@ int tls_construct_cert_verify(SSL *s, WPACKET *pkt) goto err; } - if (SSL_IS_TLS13(s) && pktype == EVP_PKEY_RSA) { + if (ispss) { if (EVP_PKEY_CTX_set_rsa_padding(pctx, RSA_PKCS1_PSS_PADDING) <= 0 /* -1 here means set saltlen to the digest len */ || EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, -1) <= 0) { @@ -243,7 +243,7 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt) unsigned char *gost_data = NULL; #endif int al = SSL_AD_INTERNAL_ERROR, ret = MSG_PROCESS_ERROR; - int type = 0, j, pktype; + int type = 0, j, pktype, ispss = 0; unsigned int len; X509 *peer; const EVP_MD *md = NULL; @@ -297,6 +297,7 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt) al = SSL_AD_DECODE_ERROR; goto f_err; } + ispss = SIGID_IS_PSS(sigalg); #ifdef SSL_DEBUG fprintf(stderr, "USING TLSv1.2 HASH %s\n", EVP_MD_name(md)); #endif @@ -358,7 +359,7 @@ MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt) } #endif - if (SSL_IS_TLS13(s) && pktype == EVP_PKEY_RSA) { + if (ispss) { if (EVP_PKEY_CTX_set_rsa_padding(pctx, RSA_PKCS1_PSS_PADDING) <= 0 /* -1 here means set saltlen to the digest len */ || EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, -1) <= 0) { diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 81a72adbe5..0573af121b 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -1956,6 +1956,7 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt) unsigned long type; const BIGNUM *r[4]; EVP_MD_CTX *md_ctx = EVP_MD_CTX_new(); + EVP_PKEY_CTX *pctx = NULL; size_t paramlen, paramoffset; if (!WPACKET_get_total_written(pkt, ¶moffset)) { @@ -2212,7 +2213,8 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt) */ if (md) { unsigned char *sigbytes1, *sigbytes2; - unsigned int siglen; + size_t siglen; + int ispss = 0; /* Get length of the parameters we have written above */ if (!WPACKET_get_length(pkt, ¶mlen)) { @@ -2222,7 +2224,7 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt) } /* send signature algorithm */ if (SSL_USE_SIGALGS(s)) { - if (!tls12_get_sigandhash(s, pkt, pkey, md)) { + if (!tls12_get_sigandhash(s, pkt, pkey, md, &ispss)) { /* Should never happen */ SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR); @@ -2240,14 +2242,29 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt) */ if (!WPACKET_sub_reserve_bytes_u16(pkt, EVP_PKEY_size(pkey), &sigbytes1) - || EVP_SignInit_ex(md_ctx, md, NULL) <= 0 - || EVP_SignUpdate(md_ctx, &(s->s3->client_random[0]), - SSL3_RANDOM_SIZE) <= 0 - || EVP_SignUpdate(md_ctx, &(s->s3->server_random[0]), - SSL3_RANDOM_SIZE) <= 0 - || EVP_SignUpdate(md_ctx, s->init_buf->data + paramoffset, - paramlen) <= 0 - || EVP_SignFinal(md_ctx, sigbytes1, &siglen, pkey) <= 0 + || EVP_DigestSignInit(md_ctx, &pctx, md, NULL, pkey) <= 0) { + SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, + ERR_R_INTERNAL_ERROR); + goto f_err; + } + if (ispss) { + if (EVP_PKEY_CTX_set_rsa_padding(pctx, + RSA_PKCS1_PSS_PADDING) <= 0 + /* -1 here means set saltlen to the digest len */ + || EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, -1) <= 0) { + SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, + ERR_R_EVP_LIB); + goto f_err; + } + } + if (EVP_DigestSignUpdate(md_ctx, &(s->s3->client_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_DigestSignUpdate(md_ctx, &(s->s3->server_random[0]), + SSL3_RANDOM_SIZE) <= 0 + || EVP_DigestSignUpdate(md_ctx, + s->init_buf->data + paramoffset, + paramlen) <= 0 + || EVP_DigestSignFinal(md_ctx, sigbytes1, &siglen) <= 0 || !WPACKET_sub_allocate_bytes_u16(pkt, siglen, &sigbytes2) || sigbytes1 != sigbytes2) { SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 4d755e4d56..6c79fe09e0 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -751,32 +751,31 @@ typedef struct sigalg_lookup_st { unsigned int sigalg; int hash; int sig; - int notls12; } SIGALG_LOOKUP; SIGALG_LOOKUP sigalg_lookup_tbl[] = { - {TLSEXT_SIGALG_ecdsa_secp256r1_sha256, NID_sha256, EVP_PKEY_EC, 0}, - {TLSEXT_SIGALG_ecdsa_secp384r1_sha384, NID_sha384, EVP_PKEY_EC, 0}, - {TLSEXT_SIGALG_ecdsa_secp521r1_sha512, NID_sha512, EVP_PKEY_EC, 0}, - {TLSEXT_SIGALG_ecdsa_sha1, NID_sha1, EVP_PKEY_EC, 0}, + {TLSEXT_SIGALG_ecdsa_secp256r1_sha256, NID_sha256, EVP_PKEY_EC}, + {TLSEXT_SIGALG_ecdsa_secp384r1_sha384, NID_sha384, EVP_PKEY_EC}, + {TLSEXT_SIGALG_ecdsa_secp521r1_sha512, NID_sha512, EVP_PKEY_EC}, + {TLSEXT_SIGALG_ecdsa_sha1, NID_sha1, EVP_PKEY_EC}, /* * PSS must appear before PKCS1 so that we prefer that when signing where * possible */ - {TLSEXT_SIGALG_rsa_pss_sha256, NID_sha256, EVP_PKEY_RSA, 1}, - {TLSEXT_SIGALG_rsa_pss_sha384, NID_sha384, EVP_PKEY_RSA, 1}, - {TLSEXT_SIGALG_rsa_pss_sha512, NID_sha512, EVP_PKEY_RSA, 1}, - {TLSEXT_SIGALG_rsa_pkcs1_sha256, NID_sha256, EVP_PKEY_RSA, 0}, - {TLSEXT_SIGALG_rsa_pkcs1_sha384, NID_sha384, EVP_PKEY_RSA, 0}, - {TLSEXT_SIGALG_rsa_pkcs1_sha512, NID_sha512, EVP_PKEY_RSA, 0}, - {TLSEXT_SIGALG_rsa_pkcs1_sha1, NID_sha1, EVP_PKEY_RSA, 0}, - {TLSEXT_SIGALG_dsa_sha256, NID_sha256, EVP_PKEY_DSA, 0}, - {TLSEXT_SIGALG_dsa_sha384, NID_sha384, EVP_PKEY_DSA, 0}, - {TLSEXT_SIGALG_dsa_sha512, NID_sha512, EVP_PKEY_DSA, 0}, - {TLSEXT_SIGALG_dsa_sha1, NID_sha1, EVP_PKEY_DSA, 0}, - {TLSEXT_SIGALG_gostr34102012_256_gostr34112012_256, NID_id_GostR3411_2012_256, NID_id_GostR3410_2012_256, 0}, - {TLSEXT_SIGALG_gostr34102012_512_gostr34112012_512, NID_id_GostR3411_2012_512, NID_id_GostR3410_2012_512, 0}, - {TLSEXT_SIGALG_gostr34102001_gostr3411, NID_id_GostR3411_94, NID_id_GostR3410_2001, 0} + {TLSEXT_SIGALG_rsa_pss_sha256, NID_sha256, EVP_PKEY_RSA}, + {TLSEXT_SIGALG_rsa_pss_sha384, NID_sha384, EVP_PKEY_RSA}, + {TLSEXT_SIGALG_rsa_pss_sha512, NID_sha512, EVP_PKEY_RSA}, + {TLSEXT_SIGALG_rsa_pkcs1_sha256, NID_sha256, EVP_PKEY_RSA}, + {TLSEXT_SIGALG_rsa_pkcs1_sha384, NID_sha384, EVP_PKEY_RSA}, + {TLSEXT_SIGALG_rsa_pkcs1_sha512, NID_sha512, EVP_PKEY_RSA}, + {TLSEXT_SIGALG_rsa_pkcs1_sha1, NID_sha1, EVP_PKEY_RSA}, + {TLSEXT_SIGALG_dsa_sha256, NID_sha256, EVP_PKEY_DSA}, + {TLSEXT_SIGALG_dsa_sha384, NID_sha384, EVP_PKEY_DSA}, + {TLSEXT_SIGALG_dsa_sha512, NID_sha512, EVP_PKEY_DSA}, + {TLSEXT_SIGALG_dsa_sha1, NID_sha1, EVP_PKEY_DSA}, + {TLSEXT_SIGALG_gostr34102012_256_gostr34112012_256, NID_id_GostR3411_2012_256, NID_id_GostR3410_2012_256}, + {TLSEXT_SIGALG_gostr34102012_512_gostr34112012_512, NID_id_GostR3411_2012_512, NID_id_GostR3410_2012_512}, + {TLSEXT_SIGALG_gostr34102001_gostr3411, NID_id_GostR3411_94, NID_id_GostR3410_2001} }; static int tls_sigalg_get_hash(unsigned int sigalg) @@ -861,7 +860,7 @@ int tls12_check_peer_sigalg(const EVP_MD **pmd, SSL *s, unsigned int sig, return 0; } #ifndef OPENSSL_NO_EC - if (EVP_PKEY_id(pkey) == EVP_PKEY_EC) { + if (pkeyid == EVP_PKEY_EC) { unsigned char curve_id[2], comp_id; /* Check compression and curve matches extensions */ if (!tls1_set_ec_id(curve_id, &comp_id, EVP_PKEY_get0_EC_KEY(pkey))) @@ -1288,9 +1287,9 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick, } int tls12_get_sigandhash(SSL *s, WPACKET *pkt, const EVP_PKEY *pk, - const EVP_MD *md) + const EVP_MD *md, int *ispss) { - int md_id, sig_id; + int md_id, sig_id, tmpispss = 0; size_t i; SIGALG_LOOKUP *curr; @@ -1303,10 +1302,27 @@ int tls12_get_sigandhash(SSL *s, WPACKET *pkt, const EVP_PKEY *pk, for (i = 0, curr = sigalg_lookup_tbl; i < OSSL_NELEM(sigalg_lookup_tbl); i++, curr++) { - if (curr->hash == md_id && curr->sig == sig_id - && (!curr->notls12 || SSL_IS_TLS13(s))) { + if (curr->hash == md_id && curr->sig == sig_id) { + if (sig_id == EVP_PKEY_RSA) { + tmpispss = SIGID_IS_PSS(curr->sigalg); + if (!SSL_IS_TLS13(s) && tmpispss) { + size_t j; + + /* + * Check peer actually sent a PSS sig id - it could have + * been a PKCS1 sig id instead. + */ + for (j = 0; j < s->cert->shared_sigalgslen; j++) + if (s->cert->shared_sigalgs[j].rsigalg == curr->sigalg) + break; + + if (j == s->cert->shared_sigalgslen) + continue; + } + } if (!WPACKET_put_bytes_u16(pkt, curr->sigalg)) return 0; + *ispss = tmpispss; return 1; } } @@ -1814,7 +1830,10 @@ int tls1_set_sigalgs(CERT *c, const int *psig_nids, size_t salglen, int client) for (j = 0, curr = sigalg_lookup_tbl; j < OSSL_NELEM(sigalg_lookup_tbl); j++, curr++) { - if (curr->hash == md_id && curr->sig == sig_id && !curr->notls12) { + /* Skip setting PSS so we get PKCS1 by default */ + if (SIGID_IS_PSS(curr->sigalg)) + continue; + if (curr->hash == md_id && curr->sig == sig_id) { *sptr++ = curr->sigalg; break; } diff --git a/test/recipes/70-test_sslsigalgs.t b/test/recipes/70-test_sslsigalgs.t index 954b9c5038..1a7aeddc13 100755 --- a/test/recipes/70-test_sslsigalgs.t +++ b/test/recipes/70-test_sslsigalgs.t @@ -146,12 +146,12 @@ SKIP: { $proxy->start(); ok(TLSProxy::Message->success, "No PSS TLSv1.2 sigalgs"); - #Test 13: Sending only TLSv1.3 PSS sig algs in TLSv1.2 should fail + #Test 13: Sending only TLSv1.3 PSS sig algs in TLSv1.2 should succeed $proxy->clear(); $testtype = PSS_ONLY_SIG_ALGS; - $proxy->clientflags("-no_tls1_3"); + $proxy->serverflags("-no_tls1_3"); $proxy->start(); - ok(TLSProxy::Message->fail, "PSS only sigalgs in TLSv1.2"); + ok(TLSProxy::Message->success, "PSS only sigalgs in TLSv1.2"); #Test 14: Sending a valid sig algs list but not including a sig type that # matches the certificate should fail in TLSv1.2 -- 2.25.1