[openssl-commits] [openssl] master update

Matt Caswell matt at openssl.org
Wed Jul 18 09:02:36 UTC 2018


The branch master has been updated
       via  9e6a32025e6e69949ad3e53a29a0b85f61f30b85 (commit)
       via  11d2641f96ead76deb5b8fac638a3ad36a971a66 (commit)
      from  1a50eedf2a1fbb1e0e009ad616d8be678e4c6340 (commit)


- Log -----------------------------------------------------------------
commit 9e6a32025e6e69949ad3e53a29a0b85f61f30b85
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Jul 17 17:29:08 2018 +0100

    Add a test for mismatch between key OID and sig alg
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6732)

commit 11d2641f96ead76deb5b8fac638a3ad36a971a66
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Jul 17 16:31:07 2018 +0100

    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 <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6732)

-----------------------------------------------------------------------

Summary of changes:
 ssl/ssl_cert.c                    | 31 ++++++++++++++------
 ssl/ssl_locl.h                    |  1 +
 ssl/t1_lib.c                      | 10 ++++++-
 test/recipes/70-test_sslsigalgs.t | 60 ++++++++++++++++++++++++++++-----------
 util/perl/TLSProxy/Message.pm     |  9 ++++--
 5 files changed, 81 insertions(+), 30 deletions(-)

diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index b2b3427..df5cff7 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 b38052f..e7258d4 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 3c7590c..df27ba6 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) {
 
diff --git a/test/recipes/70-test_sslsigalgs.t b/test/recipes/70-test_sslsigalgs.t
index 95af8a1..f805dcf 100644
--- a/test/recipes/70-test_sslsigalgs.t
+++ b/test/recipes/70-test_sslsigalgs.t
@@ -53,12 +53,12 @@ use constant {
 
 #Test 1: Default sig algs should succeed
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
-plan tests => 21;
+plan tests => 22;
 ok(TLSProxy::Message->success, "Default sigalgs");
 my $testtype;
 
 SKIP: {
-    skip "TLSv1.3 disabled", 5 if disabled("tls1_3");
+    skip "TLSv1.3 disabled", 6 if disabled("tls1_3");
 
     $proxy->filter(\&sigalgs_filter);
 
@@ -94,12 +94,21 @@ SKIP: {
     $testtype = PSS_ONLY_SIG_ALGS;
     $proxy->start();
     ok(TLSProxy::Message->success, "PSS only sigalgs in TLSv1.3");
+
+    #Test 7: Modify the CertificateVerify sigalg from rsa_pss_rsae_sha256 to
+    #        rsa_pss_pss_sha256. This should fail because the public key OID
+    #        in the certificate is rsaEncryption and not rsassaPss
+    $proxy->filter(\&modify_cert_verify_sigalg);
+    $proxy->clear();
+    $proxy->start();
+    ok(TLSProxy::Message->fail,
+       "Mismatch between CertVerify sigalg and public key OID");
 }
 
 SKIP: {
     skip "EC or TLSv1.3 disabled", 1
         if disabled("tls1_3") || disabled("ec");
-    #Test 7: Sending a valid sig algs list but not including a sig type that
+    #Test 8: Sending a valid sig algs list but not including a sig type that
     #        matches the certificate should fail in TLSv1.3.
     $proxy->clear();
     $proxy->clientflags("-sigalgs ECDSA+SHA256");
@@ -112,7 +121,7 @@ SKIP: {
     skip "EC, TLSv1.3 or TLSv1.2 disabled", 1
         if disabled("tls1_2") || disabled("tls1_3") || disabled("ec");
 
-    #Test 8: Sending a full list of TLSv1.3 sig algs but negotiating TLSv1.2
+    #Test 9: Sending a full list of TLSv1.3 sig algs but negotiating TLSv1.2
     #        should succeed
     $proxy->clear();
     $proxy->serverflags("-no_tls1_3");
@@ -127,7 +136,7 @@ SKIP: {
 
     $proxy->filter(\&sigalgs_filter);
 
-    #Test 9: Sending no sig algs extension in TLSv1.2 should succeed
+    #Test 10: Sending no sig algs extension in TLSv1.2 should succeed
     $proxy->clear();
     $testtype = NO_SIG_ALGS_EXT;
     $proxy->clientflags("-no_tls1_3");
@@ -135,7 +144,7 @@ SKIP: {
     $proxy->start();
     ok(TLSProxy::Message->success, "No TLSv1.2 sigalgs");
 
-    #Test 10: Sending an empty sig algs extension in TLSv1.2 should fail
+    #Test 11: Sending an empty sig algs extension in TLSv1.2 should fail
     $proxy->clear();
     $testtype = EMPTY_SIG_ALGS_EXT;
     $proxy->clientflags("-no_tls1_3");
@@ -143,7 +152,7 @@ SKIP: {
     $proxy->start();
     ok(TLSProxy::Message->fail, "Empty TLSv1.2 sigalgs");
 
-    #Test 11: Sending a list with no recognised sig algs in TLSv1.2 should fail
+    #Test 12: Sending a list with no recognised sig algs in TLSv1.2 should fail
     $proxy->clear();
     $testtype = NO_KNOWN_SIG_ALGS;
     $proxy->clientflags("-no_tls1_3");
@@ -151,7 +160,7 @@ SKIP: {
     $proxy->start();
     ok(TLSProxy::Message->fail, "No known TLSv1.3 sigalgs");
 
-    #Test 12: Sending a sig algs list without pss for an RSA cert in TLSv1.2
+    #Test 13: Sending a sig algs list without pss for an RSA cert in TLSv1.2
     #         should succeed
     $proxy->clear();
     $testtype = NO_PSS_SIG_ALGS;
@@ -160,7 +169,7 @@ 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 succeed
+    #Test 14: Sending only TLSv1.3 PSS sig algs in TLSv1.2 should succeed
     $proxy->clear();
     $testtype = PSS_ONLY_SIG_ALGS;
     $proxy->serverflags("-no_tls1_3");
@@ -168,7 +177,7 @@ SKIP: {
     $proxy->start();
     ok(TLSProxy::Message->success, "PSS only sigalgs in TLSv1.2");
 
-    #Test 14: Responding with a sig alg we did not send in TLSv1.2 should fail
+    #Test 15: Responding with a sig alg we did not send in TLSv1.2 should fail
     #         We send rsa_pkcs1_sha256 and respond with rsa_pss_rsae_sha256
     #         TODO(TLS1.3): Add a similar test to the TLSv1.3 section above
     #         when we have an API capable of configuring the TLSv1.3 sig algs
@@ -179,7 +188,7 @@ SKIP: {
     $proxy->start();
     ok(TLSProxy::Message->fail, "Sigalg we did not send in TLSv1.2");
 
-    #Test 15: Sending a valid sig algs list but not including a sig type that
+    #Test 16: Sending a valid sig algs list but not including a sig type that
     #         matches the certificate should fail in TLSv1.2
     $proxy->clear();
     $proxy->clientflags("-no_tls1_3 -sigalgs ECDSA+SHA256");
@@ -189,7 +198,7 @@ SKIP: {
     ok(TLSProxy::Message->fail, "No matching TLSv1.2 sigalgs");
     $proxy->filter(\&sigalgs_filter);
 
-    #Test 16: No sig algs extension, ECDSA cert, TLSv1.2 should succeed
+    #Test 17: No sig algs extension, ECDSA cert, TLSv1.2 should succeed
     $proxy->clear();
     $testtype = NO_SIG_ALGS_EXT;
     $proxy->clientflags("-no_tls1_3");
@@ -205,7 +214,7 @@ SKIP: {
 my ($dsa_status, $sha1_status, $sha224_status);
 SKIP: {
     skip "TLSv1.3 disabled", 2 if disabled("tls1_3") || disabled("dsa");
-    #Test 17: signature_algorithms with 1.3-only ClientHello
+    #Test 18: signature_algorithms with 1.3-only ClientHello
     $testtype = PURE_SIGALGS;
     $dsa_status = $sha1_status = $sha224_status = 0;
     $proxy->clear();
@@ -215,7 +224,7 @@ SKIP: {
     ok($dsa_status && $sha1_status && $sha224_status,
        "DSA/SHA2 sigalg sent for 1.3-only ClientHello");
 
-    #Test 18: signature_algorithms with backwards compatible ClientHello
+    #Test 19: signature_algorithms with backwards compatible ClientHello
     SKIP: {
         skip "TLSv1.2 disabled", 1 if disabled("tls1_2");
         $testtype = COMPAT_SIGALGS;
@@ -230,21 +239,21 @@ SKIP: {
 
 SKIP: {
     skip "TLSv1.3 disabled", 3 if disabled("tls1_3");
-    #Test 19: Insert signature_algorithms_cert that match normal sigalgs
+    #Test 20: Insert signature_algorithms_cert that match normal sigalgs
     $testtype = SIGALGS_CERT_ALL;
     $proxy->clear();
     $proxy->filter(\&modify_sigalgs_cert_filter);
     $proxy->start();
     ok(TLSProxy::Message->success, "sigalgs_cert in TLSv1.3");
 
-    #Test 19: Insert signature_algorithms_cert that forces PKCS#1 cert
+    #Test 21: Insert signature_algorithms_cert that forces PKCS#1 cert
     $testtype = SIGALGS_CERT_PKCS;
     $proxy->clear();
     $proxy->filter(\&modify_sigalgs_cert_filter);
     $proxy->start();
     ok(TLSProxy::Message->success, "sigalgs_cert in TLSv1.3 with PKCS#1 cert");
 
-    #Test 19: Insert signature_algorithms_cert that fails
+    #Test 22: Insert signature_algorithms_cert that fails
     $testtype = SIGALGS_CERT_INVALID;
     $proxy->clear();
     $proxy->filter(\&modify_sigalgs_cert_filter);
@@ -380,3 +389,20 @@ sub modify_sigalgs_cert_filter
         }
     }
 }
+
+sub modify_cert_verify_sigalg
+{
+    my $proxy = shift;
+
+    # We're only interested in the CertificateVerify
+    if ($proxy->flight != 1) {
+        return;
+    }
+
+    foreach my $message (@{$proxy->message_list}) {
+        if ($message->mt == TLSProxy::Message::MT_CERTIFICATE_VERIFY) {
+            $message->sigalg(TLSProxy::Message::SIG_ALG_RSA_PSS_PSS_SHA256);
+            $message->repack();
+        }
+    }
+}
diff --git a/util/perl/TLSProxy/Message.pm b/util/perl/TLSProxy/Message.pm
index 4a60ba0..56570f9 100644
--- a/util/perl/TLSProxy/Message.pm
+++ b/util/perl/TLSProxy/Message.pm
@@ -103,11 +103,14 @@ use constant {
     SIG_ALG_ECDSA_SECP256R1_SHA256 => 0x0403,
     SIG_ALG_ECDSA_SECP384R1_SHA384 => 0x0503,
     SIG_ALG_ECDSA_SECP521R1_SHA512 => 0x0603,
-    SIG_ALG_RSA_PSS_SHA256 => 0x0804,
-    SIG_ALG_RSA_PSS_SHA384 => 0x0805,
-    SIG_ALG_RSA_PSS_SHA512 => 0x0806,
+    SIG_ALG_RSA_PSS_RSAE_SHA256 => 0x0804,
+    SIG_ALG_RSA_PSS_RSAE_SHA384 => 0x0805,
+    SIG_ALG_RSA_PSS_RSAE_SHA512 => 0x0806,
     SIG_ALG_ED25519 => 0x0807,
     SIG_ALG_ED448 => 0x0808,
+    SIG_ALG_RSA_PSS_PSS_SHA256 => 0x0809,
+    SIG_ALG_RSA_PSS_PSS_SHA384 => 0x080a,
+    SIG_ALG_RSA_PSS_PSS_SHA512 => 0x080b,
     SIG_ALG_RSA_PKCS1_SHA1 => 0x0201,
     SIG_ALG_ECDSA_SHA1 => 0x0203,
     SIG_ALG_DSA_SHA1 => 0x0202,


More information about the openssl-commits mailing list