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: NRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2157)

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);
 

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(&params),
-                                PACKET_remaining(&params)) <= 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(&params),
+                                          PACKET_remaining(&params)) <= 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;
 }
 

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) {

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, &paramoffset)) {
@@ -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, &paramlen)) {
@@ -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,

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;
             }

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