From 603116c570f55f3d00f705d1862e7f685522162f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 9 Apr 2014 09:50:03 +0200 Subject: [PATCH 1/9] Add x509_crt_check_key_usage() --- include/polarssl/config.h | 14 ++++++++++++ include/polarssl/x509_crt.h | 20 +++++++++++++++++ library/x509_crt.c | 11 ++++++++++ tests/suites/test_suite_x509parse.data | 25 ++++++++++++++++++++++ tests/suites/test_suite_x509parse.function | 15 +++++++++++++ 5 files changed, 85 insertions(+) diff --git a/include/polarssl/config.h b/include/polarssl/config.h index c2c270881..2def1ee96 100644 --- a/include/polarssl/config.h +++ b/include/polarssl/config.h @@ -957,6 +957,20 @@ */ //#define POLARSSL_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION +/** + * \def POLARSSL_X509_CHECK_KEY_USAGE + * + * Enable verification of the keyUsage extension (CA and leaf certificates). + * + * Disabling this avoids problems with mis-issued and/or misused + * (intermediate) CA and leaf certificates. + * + * \warning Depending on your PKI use, disabling this can be a security risk! + * + * Comment to skip keyUsage checking for both CA and leaf certificates. + */ +#define POLARSSL_X509_CHECK_KEY_USAGE + /** * \def POLARSSL_ZLIB_SUPPORT * diff --git a/include/polarssl/x509_crt.h b/include/polarssl/x509_crt.h index e3c8b1877..e3443d016 100644 --- a/include/polarssl/x509_crt.h +++ b/include/polarssl/x509_crt.h @@ -244,6 +244,26 @@ int x509_crt_verify( x509_crt *crt, int (*f_vrfy)(void *, x509_crt *, int, int *), void *p_vrfy ); +#if defined(POLARSSL_X509_CHECK_KEY_USAGE) +/** + * \brief Check usage of certificate against keyUsage extension. + * + * \param crt Leaf certificate used. + * \param usage Intended usage(s) (eg KU_KEY_ENCIPHERMENT before using the + * certificate to perform an RSA key exchange). + * + * \return 0 is these uses of the certificate are allowed, + * POLARSSL_ERR_X509_BAD_INPUT_DATA if the keyUsage extenson + * is present but does not contain all the bits set in the + * usage argument. + * + * \note You should only call this function on leaf certificates, on + * (intermediate) CAs the keyUsage extension is automatically + * checked by \c x509_crt_verify(). + */ +int x509_crt_check_key_usage( const x509_crt *crt, int usage ); +#endif /* POLARSSL_X509_CHECK_KEY_USAGE) */ + #if defined(POLARSSL_X509_CRL_PARSE_C) /** * \brief Verify the certificate revocation status diff --git a/library/x509_crt.c b/library/x509_crt.c index d9f25edf1..6ab30e673 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1360,6 +1360,17 @@ int x509_crt_info( char *buf, size_t size, const char *prefix, return( (int) ( size - n ) ); } +#if defined(POLARSSL_X509_CHECK_KEY_USAGE) +int x509_crt_check_key_usage( const x509_crt *crt, int usage ) +{ + if( ( crt->ext_types & EXT_KEY_USAGE ) != 0 && + ( crt->key_usage & usage ) != usage ) + return( POLARSSL_ERR_X509_BAD_INPUT_DATA ); + + return( 0 ); +} +#endif + #if defined(POLARSSL_X509_CRL_PARSE_C) /* * Return 1 if the certificate is revoked, or 0 otherwise. diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index ef9a331f5..b1cf1c712 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -826,3 +826,28 @@ x509_oid_numstr:"2A864886F70D":"1.2.840.113549":15:14 X509 OID numstring #5 (arithmetic overflow) x509_oid_numstr:"2A8648F9F8F7F6F5F4F3F2F1F001":"":100:POLARSSL_ERR_OID_BUF_TOO_SMALL + +X509 crt keyUsage #1 (no extension, expected KU) +x509_check_key_usage:"data_files/server1.crt":KU_DIGITAL_SIGNATURE|KU_KEY_ENCIPHERMENT:0 + +X509 crt keyUsage #2 (no extension, suprising KU) +x509_check_key_usage:"data_files/server1.crt":KU_KEY_CERT_SIGN:0 + +X509 crt keyUsage #3 (extension present, no KU) +x509_check_key_usage:"data_files/server1.key_usage.crt":0:0 + +X509 crt keyUsage #4 (extension present, single KU present) +x509_check_key_usage:"data_files/server1.key_usage.crt":KU_DIGITAL_SIGNATURE:0 + +X509 crt keyUsage #5 (extension present, single KU absent) +x509_check_key_usage:"data_files/server1.key_usage.crt":KU_KEY_CERT_SIGN:POLARSSL_ERR_X509_BAD_INPUT_DATA + +X509 crt keyUsage #6 (extension present, combined KU present) +x509_check_key_usage:"data_files/server1.key_usage.crt":KU_DIGITAL_SIGNATURE|KU_KEY_ENCIPHERMENT:0 + +X509 crt keyUsage #7 (extension present, combined KU both absent) +x509_check_key_usage:"data_files/server1.key_usage.crt":KU_KEY_CERT_SIGN|KU_CRL_SIGN:POLARSSL_ERR_X509_BAD_INPUT_DATA + +X509 crt keyUsage #8 (extension present, combined KU one absent) +x509_check_key_usage:"data_files/server1.key_usage.crt":KU_KEY_ENCIPHERMENT|KU_KEY_AGREEMENT:POLARSSL_ERR_X509_BAD_INPUT_DATA + diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index effa4cc30..a4b2cf3a4 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -316,6 +316,21 @@ void x509_oid_numstr( char *oid_str, char *numstr, int blen, int ret ) } /* END_CASE */ +/* BEGIN_CASE depends_on:POLARSSL_X509_CRT_PARSE_C */ +void x509_check_key_usage( char *crt_file, int usage, int ret ) +{ + x509_crt crt; + + x509_crt_init( &crt ); + + TEST_ASSERT( x509_crt_parse_file( &crt, crt_file ) == 0 ); + + TEST_ASSERT( x509_crt_check_key_usage( &crt, usage ) == ret ); + + x509_crt_free( &crt ); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:POLARSSL_X509_CRT_PARSE_C:POLARSSL_SELF_TEST */ void x509_selftest() { From 7f2a07d7b2d5edbe9dfd7803103041ca30bee250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 9 Apr 2014 09:50:57 +0200 Subject: [PATCH 2/9] Check keyUsage in SSL client and server --- include/polarssl/ssl.h | 13 +++ library/ssl_srv.c | 14 +++ library/ssl_tls.c | 74 ++++++++++++++- tests/data_files/server2.ku-ds.crt | 21 +++++ tests/data_files/server2.ku-ds_ke.crt | 21 +++++ tests/data_files/server2.ku-ka.crt | 21 +++++ tests/data_files/server2.ku-ke.crt | 21 +++++ tests/data_files/server5.ku-ds.crt | 14 +++ tests/data_files/server5.ku-ka.crt | 14 +++ tests/data_files/server5.ku-ke.crt | 14 +++ tests/ssl-opt.sh | 124 +++++++++++++++++++++++++- 11 files changed, 347 insertions(+), 4 deletions(-) create mode 100644 tests/data_files/server2.ku-ds.crt create mode 100644 tests/data_files/server2.ku-ds_ke.crt create mode 100644 tests/data_files/server2.ku-ka.crt create mode 100644 tests/data_files/server2.ku-ke.crt create mode 100644 tests/data_files/server5.ku-ds.crt create mode 100644 tests/data_files/server5.ku-ka.crt create mode 100644 tests/data_files/server5.ku-ke.crt diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h index c866b6ffd..82cdb5312 100644 --- a/include/polarssl/ssl.h +++ b/include/polarssl/ssl.h @@ -1657,6 +1657,19 @@ static inline x509_crt *ssl_own_cert( ssl_context *ssl ) return( ssl->handshake->key_cert == NULL ? NULL : ssl->handshake->key_cert->cert ); } + +/* + * Check usage of a certificate wrt extensions: + * keyUsage, extendedKeyUsage (later), and nSCertType (later). + * + * Warning: cert_endpoint is the endpoint of the cert (ie, of our peer when we + * check a cert we received from them)! + * + * Return 0 if everything is OK, -1 if not. + */ +int ssl_check_cert_usage( const x509_crt *cert, + const ssl_ciphersuite_t *ciphersuite, + int cert_endpoint ); #endif /* POLARSSL_X509_CRT_PARSE_C */ /* constant-time buffer comparison */ diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 08f6eea67..8bdf237d9 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -797,6 +797,20 @@ static int ssl_pick_cert( ssl_context *ssl, if( ! pk_can_do( cur->key, pk_alg ) ) continue; + /* + * This avoids sending the client a cert it'll reject based on + * keyUsage or other extensions. + * + * It also allows the user to provision different certificates for + * different uses based on keyUsage, eg if they want to avoid signing + * and decrypting with the same RSA key. + */ + if( ssl_check_cert_usage( cur->cert, ciphersuite_info, + SSL_IS_SERVER ) != 0 ) + { + continue; + } + #if defined(POLARSSL_ECDSA_C) if( pk_alg == POLARSSL_PK_ECDSA ) { diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 38843a3cd..4c8d8f9f2 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2699,6 +2699,9 @@ int ssl_parse_certificate( ssl_context *ssl ) return( POLARSSL_ERR_SSL_CA_CHAIN_REQUIRED ); } + /* + * Main check: verify certificate + */ ret = x509_crt_verify( ssl->session_negotiate->peer_cert, ssl->ca_chain, ssl->ca_crl, ssl->peer_cn, &ssl->session_negotiate->verify_result, @@ -2708,21 +2711,35 @@ int ssl_parse_certificate( ssl_context *ssl ) { SSL_DEBUG_RET( 1, "x509_verify_cert", ret ); } + + /* + * Secondary checks: always done, but change 'ret' only if it was 0 + */ + #if defined(POLARSSL_SSL_SET_CURVES) - else { - pk_context *pk = &ssl->session_negotiate->peer_cert->pk; + const pk_context *pk = &ssl->session_negotiate->peer_cert->pk; /* If certificate uses an EC key, make sure the curve is OK */ if( pk_can_do( pk, POLARSSL_PK_ECKEY ) && ! ssl_curve_is_acceptable( ssl, pk_ec( *pk )->grp.id ) ) { SSL_DEBUG_MSG( 1, ( "bad server certificate (EC key curve)" ) ); - ret = POLARSSL_ERR_SSL_BAD_HS_CERTIFICATE; + if( ret == 0 ) + ret = POLARSSL_ERR_SSL_BAD_HS_CERTIFICATE; } } #endif + if( ssl_check_cert_usage( ssl->session_negotiate->peer_cert, + ciphersuite_info, + ! ssl->endpoint ) != 0 ) + { + SSL_DEBUG_MSG( 1, ( "bad server certificate (usage ext.)" ) ); + if( ret == 0 ) + ret = POLARSSL_ERR_SSL_BAD_HS_CERTIFICATE; + } + if( ssl->authmode != SSL_VERIFY_REQUIRED ) ret = 0; } @@ -4747,3 +4764,54 @@ int ssl_curve_is_acceptable( const ssl_context *ssl, ecp_group_id grp_id ) return( 0 ); } #endif + +int ssl_check_cert_usage( const x509_crt *cert, + const ssl_ciphersuite_t *ciphersuite, + int cert_endpoint ) +{ +#if defined(POLARSSL_X509_CHECK_KEY_USAGE) + int usage = 0; +#endif + +#if defined(POLARSSL_X509_CHECK_KEY_USAGE) + if( cert_endpoint == SSL_IS_SERVER ) + { + /* Server part of the key exchange */ + switch( ciphersuite->key_exchange ) + { + case POLARSSL_KEY_EXCHANGE_RSA: + case POLARSSL_KEY_EXCHANGE_RSA_PSK: + usage = KU_KEY_ENCIPHERMENT; + break; + + case POLARSSL_KEY_EXCHANGE_DHE_RSA: + case POLARSSL_KEY_EXCHANGE_ECDHE_RSA: + case POLARSSL_KEY_EXCHANGE_ECDHE_ECDSA: + usage = KU_DIGITAL_SIGNATURE; + break; + + case POLARSSL_KEY_EXCHANGE_ECDH_RSA: + case POLARSSL_KEY_EXCHANGE_ECDH_ECDSA: + usage = KU_KEY_AGREEMENT; + break; + + /* Don't use default: we want warnings when adding new values */ + case POLARSSL_KEY_EXCHANGE_NONE: + case POLARSSL_KEY_EXCHANGE_PSK: + case POLARSSL_KEY_EXCHANGE_DHE_PSK: + case POLARSSL_KEY_EXCHANGE_ECDHE_PSK: + usage = 0; + } + } + else + { + /* Client auth: we only implement rsa_sign and ecdsa_sign for now */ + usage = KU_DIGITAL_SIGNATURE; + } + + if( x509_crt_check_key_usage( cert, usage ) != 0 ) + return( -1 ); +#endif /* POLARSSL_X509_CHECK_KEY_USAGE */ + + return( 0 ); +} diff --git a/tests/data_files/server2.ku-ds.crt b/tests/data_files/server2.ku-ds.crt new file mode 100644 index 000000000..3bd07d0fb --- /dev/null +++ b/tests/data_files/server2.ku-ds.crt @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDijCCAnKgAwIBAgIBLDANBgkqhkiG9w0BAQUFADA7MQswCQYDVQQGEwJOTDER +MA8GA1UEChMIUG9sYXJTU0wxGTAXBgNVBAMTEFBvbGFyU1NMIFRlc3QgQ0EwHhcN +MTQwNDA5MDg0NDUxWhcNMjQwNDA2MDg0NDUxWjA0MQswCQYDVQQGEwJOTDERMA8G +A1UEChMIUG9sYXJTU0wxEjAQBgNVBAMTCWxvY2FsaG9zdDCCASIwDQYJKoZIhvcN +AQEBBQADggEPADCCAQoCggEBAMFNo93nzR3RBNdJcriZrA545Do8Ss86ExbQWuTN +owCIp+4ea5anUrSQ7y1yej4kmvy2NKwk9XfgJmSMnLAofaHa6ozmyRyWvP7BBFKz +NtSj+uGxdtiQwWG0ZlI2oiZTqqt0Xgd9GYLbKtgfoNkNHC1JZvdbJXNG6AuKT2kM +tQCQ4dqCEGZ9rlQri2V5kaHiYcPNQEkI7mgM8YuG0ka/0LiqEQMef1aoGh5EGA8P +hYvai0Re4hjGYi/HZo36Xdh98yeJKQHFkA4/J/EwyEoO79bex8cna8cFPXrEAjya +HT4P6DSYW8tzS1KW2BGiLICIaTla0w+w3lkvEcf36hIBMJcCAwEAAaOBnzCBnDAJ +BgNVHRMEAjAAMB0GA1UdDgQWBBSlBehkuNzfYA9QEk1gqGSvTYtDkzBjBgNVHSME +XDBagBS0WuSls97SUva51aaVD+s+vMf9/6E/pD0wOzELMAkGA1UEBhMCTkwxETAP +BgNVBAoTCFBvbGFyU1NMMRkwFwYDVQQDExBQb2xhclNTTCBUZXN0IENBggEAMAsG +A1UdDwQEAwIHgDANBgkqhkiG9w0BAQUFAAOCAQEAc4kubASrFXFtplkYp6FUcnUn +Pf/6laS1htI+3y+q1UHWe2PcagZtCHTCUGBSWLeUIiaIBheaIRqv+4sSFVuXB7hV +0PGXpO5btth4R8BHzGqCdObKvPujp5BDq3xgcAFicA3HUMNsJoTDv/RYXY7je1Q5 +ntVyVPeji0AWMUYQjcqHTQQPGBgdJrRTMaYglZh15IhJ16ICNd9rWIeBA0h/+r0y +QuFEBz0nfe7Dvpqct7gJCv+7/5tCujx4LT17z7oK8BZN5SePAGU2ykJsUXk8ZICT +ongaQQVQwS6/GJ6A5V8ecaUvFrTby1h9+2sOW8n2NRGiaaG5gkvxVeayemcmOQ== +-----END CERTIFICATE----- diff --git a/tests/data_files/server2.ku-ds_ke.crt b/tests/data_files/server2.ku-ds_ke.crt new file mode 100644 index 000000000..ebee7e1c3 --- /dev/null +++ b/tests/data_files/server2.ku-ds_ke.crt @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDijCCAnKgAwIBAgIBMDANBgkqhkiG9w0BAQUFADA7MQswCQYDVQQGEwJOTDER +MA8GA1UEChMIUG9sYXJTU0wxGTAXBgNVBAMTEFBvbGFyU1NMIFRlc3QgQ0EwHhcN +MTQwNDA5MTAwMjQ5WhcNMjQwNDA2MTAwMjQ5WjA0MQswCQYDVQQGEwJOTDERMA8G +A1UEChMIUG9sYXJTU0wxEjAQBgNVBAMTCWxvY2FsaG9zdDCCASIwDQYJKoZIhvcN +AQEBBQADggEPADCCAQoCggEBAMFNo93nzR3RBNdJcriZrA545Do8Ss86ExbQWuTN +owCIp+4ea5anUrSQ7y1yej4kmvy2NKwk9XfgJmSMnLAofaHa6ozmyRyWvP7BBFKz +NtSj+uGxdtiQwWG0ZlI2oiZTqqt0Xgd9GYLbKtgfoNkNHC1JZvdbJXNG6AuKT2kM +tQCQ4dqCEGZ9rlQri2V5kaHiYcPNQEkI7mgM8YuG0ka/0LiqEQMef1aoGh5EGA8P +hYvai0Re4hjGYi/HZo36Xdh98yeJKQHFkA4/J/EwyEoO79bex8cna8cFPXrEAjya +HT4P6DSYW8tzS1KW2BGiLICIaTla0w+w3lkvEcf36hIBMJcCAwEAAaOBnzCBnDAJ +BgNVHRMEAjAAMB0GA1UdDgQWBBSlBehkuNzfYA9QEk1gqGSvTYtDkzBjBgNVHSME +XDBagBS0WuSls97SUva51aaVD+s+vMf9/6E/pD0wOzELMAkGA1UEBhMCTkwxETAP +BgNVBAoTCFBvbGFyU1NMMRkwFwYDVQQDExBQb2xhclNTTCBUZXN0IENBggEAMAsG +A1UdDwQEAwIFoDANBgkqhkiG9w0BAQUFAAOCAQEAnW7+h85xBP2KJzFSpWfGirVe +ApdC9bX0Z1sVMmD486N+ty9W6BP6kJRxLDX0fOuRc3x7mCy5qZg/Yj40+yQSoA0w +bTNwJjuR8iMqWIqLw9hWR+E9T4lYLZWyGJVjlVTkO4i5wifwhoJE9Doohh/6crn5 +ImWgEkgT/wDVIHoamciO6KU36d0iAEEP2eYgxv2/sVHvjjsseTdvYh3D3VuOmQtS +uUvFxc6H5kYoq/yodJWDaOn3RS8pEpDsiW+abcWyxNTPtHFroJV7e9aaVmhlRSzw +sYDyD/ZyIlavoPSEiD3LTT/Tp6BIpz+zb4WHOHLEvUCsZputqxPVcNoEAi9xuA== +-----END CERTIFICATE----- diff --git a/tests/data_files/server2.ku-ka.crt b/tests/data_files/server2.ku-ka.crt new file mode 100644 index 000000000..90f7c4a99 --- /dev/null +++ b/tests/data_files/server2.ku-ka.crt @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDijCCAnKgAwIBAgIBKjANBgkqhkiG9w0BAQUFADA7MQswCQYDVQQGEwJOTDER +MA8GA1UEChMIUG9sYXJTU0wxGTAXBgNVBAMTEFBvbGFyU1NMIFRlc3QgQ0EwHhcN +MTQwNDA5MDg0NDIzWhcNMjQwNDA2MDg0NDIzWjA0MQswCQYDVQQGEwJOTDERMA8G +A1UEChMIUG9sYXJTU0wxEjAQBgNVBAMTCWxvY2FsaG9zdDCCASIwDQYJKoZIhvcN +AQEBBQADggEPADCCAQoCggEBAMFNo93nzR3RBNdJcriZrA545Do8Ss86ExbQWuTN +owCIp+4ea5anUrSQ7y1yej4kmvy2NKwk9XfgJmSMnLAofaHa6ozmyRyWvP7BBFKz +NtSj+uGxdtiQwWG0ZlI2oiZTqqt0Xgd9GYLbKtgfoNkNHC1JZvdbJXNG6AuKT2kM +tQCQ4dqCEGZ9rlQri2V5kaHiYcPNQEkI7mgM8YuG0ka/0LiqEQMef1aoGh5EGA8P +hYvai0Re4hjGYi/HZo36Xdh98yeJKQHFkA4/J/EwyEoO79bex8cna8cFPXrEAjya +HT4P6DSYW8tzS1KW2BGiLICIaTla0w+w3lkvEcf36hIBMJcCAwEAAaOBnzCBnDAJ +BgNVHRMEAjAAMB0GA1UdDgQWBBSlBehkuNzfYA9QEk1gqGSvTYtDkzBjBgNVHSME +XDBagBS0WuSls97SUva51aaVD+s+vMf9/6E/pD0wOzELMAkGA1UEBhMCTkwxETAP +BgNVBAoTCFBvbGFyU1NMMRkwFwYDVQQDExBQb2xhclNTTCBUZXN0IENBggEAMAsG +A1UdDwQEAwIDCDANBgkqhkiG9w0BAQUFAAOCAQEAriPloIWfu7U8d1hls97C7OBI +OiE2xFh2UmuN/9hTK2CyW6MtBf8aG3l4jQDrsutHO0gUyoR67ug4yj+s+0S/zETZ +q6mPo7cBbVwjhGciQRiYgufFpdnbXR05HDgOVPK7qqjL6UOZnbu5caIEvIJgdwXn +n8WB9x/Ii4/2S9ysmRdRhDBYekzgH3Ac2UnHJTMh1XaSL817MW6B9BDKHt4xa7pW +cplDzrFKYbmxSSxzALE4Dr+zRvmDx4bcYpBkRRfOhnnR1caQBgaZzPcX/Vu+vw8e +qs2nyBW5RBu8MBCBU1DpqOSo6jl0QTpuq3NzQZIouG9fyckqDJS5ibrxQTutPw== +-----END CERTIFICATE----- diff --git a/tests/data_files/server2.ku-ke.crt b/tests/data_files/server2.ku-ke.crt new file mode 100644 index 000000000..8daa0c13d --- /dev/null +++ b/tests/data_files/server2.ku-ke.crt @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDijCCAnKgAwIBAgIBKzANBgkqhkiG9w0BAQUFADA7MQswCQYDVQQGEwJOTDER +MA8GA1UEChMIUG9sYXJTU0wxGTAXBgNVBAMTEFBvbGFyU1NMIFRlc3QgQ0EwHhcN +MTQwNDA5MDg0NDM5WhcNMjQwNDA2MDg0NDM5WjA0MQswCQYDVQQGEwJOTDERMA8G +A1UEChMIUG9sYXJTU0wxEjAQBgNVBAMTCWxvY2FsaG9zdDCCASIwDQYJKoZIhvcN +AQEBBQADggEPADCCAQoCggEBAMFNo93nzR3RBNdJcriZrA545Do8Ss86ExbQWuTN +owCIp+4ea5anUrSQ7y1yej4kmvy2NKwk9XfgJmSMnLAofaHa6ozmyRyWvP7BBFKz +NtSj+uGxdtiQwWG0ZlI2oiZTqqt0Xgd9GYLbKtgfoNkNHC1JZvdbJXNG6AuKT2kM +tQCQ4dqCEGZ9rlQri2V5kaHiYcPNQEkI7mgM8YuG0ka/0LiqEQMef1aoGh5EGA8P +hYvai0Re4hjGYi/HZo36Xdh98yeJKQHFkA4/J/EwyEoO79bex8cna8cFPXrEAjya +HT4P6DSYW8tzS1KW2BGiLICIaTla0w+w3lkvEcf36hIBMJcCAwEAAaOBnzCBnDAJ +BgNVHRMEAjAAMB0GA1UdDgQWBBSlBehkuNzfYA9QEk1gqGSvTYtDkzBjBgNVHSME +XDBagBS0WuSls97SUva51aaVD+s+vMf9/6E/pD0wOzELMAkGA1UEBhMCTkwxETAP +BgNVBAoTCFBvbGFyU1NMMRkwFwYDVQQDExBQb2xhclNTTCBUZXN0IENBggEAMAsG +A1UdDwQEAwIFIDANBgkqhkiG9w0BAQUFAAOCAQEAqreLAIuxeLGKbhoEROYRqXxO +ndaC6uDcpxhgmEW7B2DW6ZtX8155v3ov61MuMas8fEQjD5STDP9qERxNTePnhW3m +kDZd2jUBE3ioHhTBv47i1PYU+DRe42kY6z0jUmNPK8TsTKfdbqTGXg9THe1KYB7q +hdljqGS08IgBl/q2lK2OOSycu27xhfb9Mo0BcLBab92WgyBu+cFPQsKiL4mD7QyJ ++73Ndb21EuANUjsRDQ3NPklssJcyJB2v85eekwk1acZUG21no3wdTvjxhVE/Xrdz +zUP9WkvAVfUrwGjUzG4YHE8wkHO7xKbKixNt+nQmDhe+tHVbztZjVwFJ8010gg== +-----END CERTIFICATE----- diff --git a/tests/data_files/server5.ku-ds.crt b/tests/data_files/server5.ku-ds.crt new file mode 100644 index 000000000..58dd0714b --- /dev/null +++ b/tests/data_files/server5.ku-ds.crt @@ -0,0 +1,14 @@ +-----BEGIN CERTIFICATE----- +MIICLTCCAbKgAwIBAgIBLTAKBggqhkjOPQQDAjA+MQswCQYDVQQGEwJOTDERMA8G +A1UEChMIUG9sYXJTU0wxHDAaBgNVBAMTE1BvbGFyc3NsIFRlc3QgRUMgQ0EwHhcN +MTQwNDA5MDg0ODM1WhcNMjQwNDA2MDg0ODM1WjA0MQswCQYDVQQGEwJOTDERMA8G +A1UEChMIUG9sYXJTU0wxEjAQBgNVBAMTCWxvY2FsaG9zdDBZMBMGByqGSM49AgEG +CCqGSM49AwEHA0IABDfMVtl2CR5acj7HWS3/IG7ufPkGkXTQrRS192giWWKSTuUA +2CMR/+ov0jRdXRa9iojCa3cNVc2KKg76Aci07f+jgaowgacwCQYDVR0TBAIwADAd +BgNVHQ4EFgQUUGGlj9QH2deCAQzlZX+MY0anE74wbgYDVR0jBGcwZYAUnW0gJEkB +PyvLeLUZvH4kydv7NnyhQqRAMD4xCzAJBgNVBAYTAk5MMREwDwYDVQQKEwhQb2xh +clNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQYIJAMFD4n5iQ8zoMAsG +A1UdDwQEAwIHgDAKBggqhkjOPQQDAgNpADBmAjEAzp4DkFMq7eDB0x5FeS9gYDaG +Ol8rVnWlRTLQzHZBQjKp+TcBdHZaBPoi8LyXtWA4AjEA6OWhsuTcv/qXOscQT0rL +eEh8wcCQeJK1uNd78lNvx3W0Pcxdb6cd7AhaAKgXL+r4 +-----END CERTIFICATE----- diff --git a/tests/data_files/server5.ku-ka.crt b/tests/data_files/server5.ku-ka.crt new file mode 100644 index 000000000..2447326c2 --- /dev/null +++ b/tests/data_files/server5.ku-ka.crt @@ -0,0 +1,14 @@ +-----BEGIN CERTIFICATE----- +MIICKzCCAbKgAwIBAgIBLjAKBggqhkjOPQQDAjA+MQswCQYDVQQGEwJOTDERMA8G +A1UEChMIUG9sYXJTU0wxHDAaBgNVBAMTE1BvbGFyc3NsIFRlc3QgRUMgQ0EwHhcN +MTQwNDA5MDg0ODUwWhcNMjQwNDA2MDg0ODUwWjA0MQswCQYDVQQGEwJOTDERMA8G +A1UEChMIUG9sYXJTU0wxEjAQBgNVBAMTCWxvY2FsaG9zdDBZMBMGByqGSM49AgEG +CCqGSM49AwEHA0IABDfMVtl2CR5acj7HWS3/IG7ufPkGkXTQrRS192giWWKSTuUA +2CMR/+ov0jRdXRa9iojCa3cNVc2KKg76Aci07f+jgaowgacwCQYDVR0TBAIwADAd +BgNVHQ4EFgQUUGGlj9QH2deCAQzlZX+MY0anE74wbgYDVR0jBGcwZYAUnW0gJEkB +PyvLeLUZvH4kydv7NnyhQqRAMD4xCzAJBgNVBAYTAk5MMREwDwYDVQQKEwhQb2xh +clNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQYIJAMFD4n5iQ8zoMAsG +A1UdDwQEAwIDCDAKBggqhkjOPQQDAgNnADBkAjACzKQ88/NvngMQBFc9rC484+gO +BRkXP28BqRcj8sBt3EfmEGH23BuhkZuB1OFZuMICMC4/pHgbOQtaY9WZPUROUVVZ +OuO6XsVbhiE0rb/mumqmUwuOrCtC/KFdvFZol4BNGA== +-----END CERTIFICATE----- diff --git a/tests/data_files/server5.ku-ke.crt b/tests/data_files/server5.ku-ke.crt new file mode 100644 index 000000000..41ae5ada3 --- /dev/null +++ b/tests/data_files/server5.ku-ke.crt @@ -0,0 +1,14 @@ +-----BEGIN CERTIFICATE----- +MIICKzCCAbKgAwIBAgIBLzAKBggqhkjOPQQDAjA+MQswCQYDVQQGEwJOTDERMA8G +A1UEChMIUG9sYXJTU0wxHDAaBgNVBAMTE1BvbGFyc3NsIFRlc3QgRUMgQ0EwHhcN +MTQwNDA5MDg0OTA0WhcNMjQwNDA2MDg0OTA0WjA0MQswCQYDVQQGEwJOTDERMA8G +A1UEChMIUG9sYXJTU0wxEjAQBgNVBAMTCWxvY2FsaG9zdDBZMBMGByqGSM49AgEG +CCqGSM49AwEHA0IABDfMVtl2CR5acj7HWS3/IG7ufPkGkXTQrRS192giWWKSTuUA +2CMR/+ov0jRdXRa9iojCa3cNVc2KKg76Aci07f+jgaowgacwCQYDVR0TBAIwADAd +BgNVHQ4EFgQUUGGlj9QH2deCAQzlZX+MY0anE74wbgYDVR0jBGcwZYAUnW0gJEkB +PyvLeLUZvH4kydv7NnyhQqRAMD4xCzAJBgNVBAYTAk5MMREwDwYDVQQKEwhQb2xh +clNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQYIJAMFD4n5iQ8zoMAsG +A1UdDwQEAwIFIDAKBggqhkjOPQQDAgNnADBkAjAMl0Cjv9f45bHeJTul5XpYeJeT +52ZaOLTa/uTLy948EnEIi6sj3nFb9fvsUbsOOjECMAXAMY64KOqzixefz3y3XS/d +9miyeArPOmXU2JJ3LGuNbqqj9IbABawB1OD8v8gRmg== +-----END CERTIFICATE----- diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index b35a9e4a8..ec9e5499f 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -151,8 +151,9 @@ run_test() { CLI_EXIT=$? echo "EXIT: $CLI_EXIT" >> cli_out + # psk is usefull when server only has bad certs if is_polar "$SRV_CMD"; then - "$P_CLI" request_page=SERVERQUIT tickets=0 auth_mode=none \ + "$P_CLI" request_page=SERVERQUIT tickets=0 auth_mode=none psk=abc123 \ crt_file=data_files/cli2.crt key_file=data_files/cli2.key \ >/dev/null else @@ -980,6 +981,127 @@ run_test "ALPN #6 (both, no common)" \ fi +# Tests for keyUsage in leaf certificates, part 1: +# server-side certificate/suite selection + +run_test "keyUsage srv #1 (RSA, digitalSignature -> ECDHE-RSA)" \ + "$P_SRV key_file=data_files/server2.key \ + crt_file=data_files/server2.ku-ds.crt" \ + "$P_CLI" \ + 0 \ + -c "Ciphersuite is TLS-ECDHE-RSA-WITH-" + + +run_test "keyUsage srv #2 (RSA, keyEncipherment -> RSA)" \ + "$P_SRV key_file=data_files/server2.key \ + crt_file=data_files/server2.ku-ke.crt" \ + "$P_CLI" \ + 0 \ + -c "Ciphersuite is TLS-RSA-WITH-" + +# add psk to leave an option for client to send SERVERQUIT +run_test "keyUsage srv #3 (RSA, keyAgreement -> fail)" \ + "$P_SRV psk=abc123 key_file=data_files/server2.key \ + crt_file=data_files/server2.ku-ka.crt" \ + "$P_CLI psk=badbad" \ + 1 \ + -C "Ciphersuite is " + +run_test "keyUsage srv #4 (ECDSA, digitalSignature -> ECDHE-ECDSA)" \ + "$P_SRV key_file=data_files/server5.key \ + crt_file=data_files/server5.ku-ds.crt" \ + "$P_CLI" \ + 0 \ + -c "Ciphersuite is TLS-ECDHE-ECDSA-WITH-" + + +run_test "keyUsage srv #5 (ECDSA, keyAgreement -> ECDH-)" \ + "$P_SRV key_file=data_files/server5.key \ + crt_file=data_files/server5.ku-ka.crt" \ + "$P_CLI" \ + 0 \ + -c "Ciphersuite is TLS-ECDH-" + +# add psk to leave an option for client to send SERVERQUIT +run_test "keyUsage srv #6 (ECDSA, keyEncipherment -> fail)" \ + "$P_SRV psk=abc123 key_file=data_files/server5.key \ + crt_file=data_files/server5.ku-ke.crt" \ + "$P_CLI psk=badbad" \ + 1 \ + -C "Ciphersuite is " + +# Tests for keyUsage in leaf certificates, part 2: +# client-side checks + +run_test "keyUsage cli #0 (reference, no extension)" \ + "$O_SRV -key data_files/server2.key \ + -cert data_files/server2.crt" \ + "$P_CLI debug_level=2" \ + 0 \ + -C "bad server certificate (usage ext.)" \ + -C "Processing of the Certificate handshake message failed" \ + -c "Ciphersuite is TLS-" + +run_test "keyUsage cli #1 (DigitalSignature+KeyEncipherment, RSA: OK)" \ + "$O_SRV -key data_files/server2.key \ + -cert data_files/server2.ku-ds_ke.crt" \ + "$P_CLI debug_level=2 \ + force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ + 0 \ + -C "bad server certificate (usage ext.)" \ + -C "Processing of the Certificate handshake message failed" \ + -c "Ciphersuite is TLS-" + +run_test "keyUsage cli #2 (DigitalSignature+KeyEncipherment, DHE-RSA: OK)" \ + "$O_SRV -key data_files/server2.key \ + -cert data_files/server2.ku-ds_ke.crt" \ + "$P_CLI debug_level=2 \ + force_ciphersuite=TLS-DHE-RSA-WITH-AES-128-CBC-SHA" \ + 0 \ + -C "bad server certificate (usage ext.)" \ + -C "Processing of the Certificate handshake message failed" \ + -c "Ciphersuite is TLS-" + +run_test "keyUsage cli #3 (KeyEncipherment, RSA: OK)" \ + "$O_SRV -key data_files/server2.key \ + -cert data_files/server2.ku-ke.crt" \ + "$P_CLI debug_level=2 \ + force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ + 0 \ + -C "bad server certificate (usage ext.)" \ + -C "Processing of the Certificate handshake message failed" \ + -c "Ciphersuite is TLS-" + +run_test "keyUsage cli #4 (KeyEncipherment, DHE-RSA: fail)" \ + "$O_SRV -key data_files/server2.key \ + -cert data_files/server2.ku-ke.crt" \ + "$P_CLI debug_level=2 \ + force_ciphersuite=TLS-DHE-RSA-WITH-AES-128-CBC-SHA" \ + 1 \ + -c "bad server certificate (usage ext.)" \ + -c "Processing of the Certificate handshake message failed" \ + -C "Ciphersuite is TLS-" + +run_test "keyUsage cli #5 (DigitalSignature, DHE-RSA: OK)" \ + "$O_SRV -key data_files/server2.key \ + -cert data_files/server2.ku-ds.crt" \ + "$P_CLI debug_level=2 \ + force_ciphersuite=TLS-DHE-RSA-WITH-AES-128-CBC-SHA" \ + 0 \ + -C "bad server certificate (usage ext.)" \ + -C "Processing of the Certificate handshake message failed" \ + -c "Ciphersuite is TLS-" + +run_test "keyUsage cli #5 (DigitalSignature, RSA: fail)" \ + "$O_SRV -key data_files/server2.key \ + -cert data_files/server2.ku-ds.crt" \ + "$P_CLI debug_level=2 \ + force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ + 1 \ + -c "bad server certificate (usage ext.)" \ + -c "Processing of the Certificate handshake message failed" \ + -C "Ciphersuite is TLS-" + # Final report echo "------------------------------------------------------------------------" From 3fed0b3264c96826a8298d3334fc8d60e6dcd5e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 8 Apr 2014 13:18:01 +0200 Subject: [PATCH 3/9] Factor some common code in x509_verify{,_child} --- library/x509_crt.c | 67 +++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 37 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 6ab30e673..be2e526cc 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1527,6 +1527,34 @@ static int x509_wildcard_verify( const char *cn, x509_buf *name ) return( 0 ); } +/* + * Iterate upwards in the given cert chain to find our parent. + * + * Ignore any upper cert that can't be used to sign other certificates + * (basic constraints CA=true for now, keyUsage soon). + */ +static x509_crt *x509_crt_find_parent( x509_crt *crt ) +{ + x509_crt *parent; + + for( parent = crt->next; parent != NULL; parent = parent->next ) + { + if( parent->version == 0 || + parent->ca_istrue == 0 || + crt->issuer_raw.len != parent->subject_raw.len || + memcmp( crt->issuer_raw.p, parent->subject_raw.p, + crt->issuer_raw.len ) != 0 ) + { + continue; + } + + /* If we get there, we found a suitable parent */ + break; + } + + return( parent ); +} + static int x509_crt_verify_top( x509_crt *child, x509_crt *trust_ca, x509_crl *ca_crl, int path_cnt, int *flags, @@ -1689,23 +1717,7 @@ static int x509_crt_verify_child( *flags |= x509_crt_verifycrl(child, parent, ca_crl); #endif - grandparent = parent->next; - - while( grandparent != NULL ) - { - if( grandparent->version == 0 || - grandparent->ca_istrue == 0 || - parent->issuer_raw.len != grandparent->subject_raw.len || - memcmp( parent->issuer_raw.p, grandparent->subject_raw.p, - parent->issuer_raw.len ) != 0 ) - { - grandparent = grandparent->next; - continue; - } - break; - } - - if( grandparent != NULL ) + if( ( grandparent = x509_crt_find_parent( parent) ) != NULL ) { /* * Part of the chain @@ -1800,26 +1812,7 @@ int x509_crt_verify( x509_crt *crt, } } - /* - * Iterate upwards in the given cert chain, to find our crt parent. - * Ignore any upper cert with CA != TRUE. - */ - parent = crt->next; - - while( parent != NULL && parent->version != 0 ) - { - if( parent->ca_istrue == 0 || - crt->issuer_raw.len != parent->subject_raw.len || - memcmp( crt->issuer_raw.p, parent->subject_raw.p, - crt->issuer_raw.len ) != 0 ) - { - parent = parent->next; - continue; - } - break; - } - - if( parent != NULL ) + if( ( parent = x509_crt_find_parent( crt ) ) != NULL ) { /* * Part of the chain From 99d4f1911173d75092cbde580f531639d92bfd9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 8 Apr 2014 15:10:07 +0200 Subject: [PATCH 4/9] Add keyUsage checking for CAs --- library/x509_crt.c | 24 ++++++++++++++++++++++++ tests/data_files/test-ca2.ku-crl.crt | 12 ++++++++++++ tests/data_files/test-ca2.ku-crt.crt | 12 ++++++++++++ tests/data_files/test-ca2.ku-crt_crl.crt | 12 ++++++++++++ tests/data_files/test-ca2.ku-ds.crt | 12 ++++++++++++ tests/suites/test_suite_x509parse.data | 20 ++++++++++++++++++++ 6 files changed, 92 insertions(+) create mode 100644 tests/data_files/test-ca2.ku-crl.crt create mode 100644 tests/data_files/test-ca2.ku-crt.crt create mode 100644 tests/data_files/test-ca2.ku-crt_crl.crt create mode 100644 tests/data_files/test-ca2.ku-ds.crt diff --git a/library/x509_crt.c b/library/x509_crt.c index be2e526cc..47745a759 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1424,6 +1424,17 @@ static int x509_crt_verifycrl( x509_crt *crt, x509_crt *ca, continue; } + /* + * Check if the CA is configured to sign CRLs + */ +#if defined(POLARSSL_X509_CHECK_KEY_USAGE) + if( x509_crt_check_key_usage( ca, KU_CRL_SIGN ) != 0 ) + { + flags |= BADCRL_NOT_TRUSTED; + break; + } +#endif + /* * Check if CRL is correctly signed by the trusted CA */ @@ -1548,6 +1559,11 @@ static x509_crt *x509_crt_find_parent( x509_crt *crt ) continue; } +#if defined(POLARSSL_X509_CHECK_KEY_USAGE) + if( x509_crt_check_key_usage( parent, KU_KEY_CERT_SIGN ) != 0 ) + continue; +#endif + /* If we get there, we found a suitable parent */ break; } @@ -1599,6 +1615,14 @@ static int x509_crt_verify_top( continue; } +#if defined(POLARSSL_X509_CHECK_KEY_USAGE) + if( x509_crt_check_key_usage( trust_ca, KU_KEY_CERT_SIGN ) != 0 ) + { + trust_ca = trust_ca->next; + continue; + } +#endif + /* * Reduce path_len to check against if top of the chain is * the same as the trusted CA diff --git a/tests/data_files/test-ca2.ku-crl.crt b/tests/data_files/test-ca2.ku-crl.crt new file mode 100644 index 000000000..4fb40838c --- /dev/null +++ b/tests/data_files/test-ca2.ku-crl.crt @@ -0,0 +1,12 @@ +-----BEGIN CERTIFICATE----- +MIIBzDCCAVOgAwIBAgIJAP6mZLzh0IPSMAoGCCqGSM49BAMCMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTAeFw0xNDA0MDkxMTIzMzhaFw0yNDA0MDYxMTIzMzhaMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTB2MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBu +ww5XUzM5WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiy +aY7zQa0pw7RfdadHb9UZKVVpmlM7ILRmFmAzHqMdMBswDAYDVR0TBAUwAwEB/zAL +BgNVHQ8EBAMCAQIwCgYIKoZIzj0EAwIDZwAwZAIwZOCKY0EHXYzI4cQsFnfOrxm1 +ufvNeZ4ZcSZWrkTBazW2OBCuCP9SLznec3SFOUvvAjAKe/qycfxkHivjieCEG1Kt +m2D4QKSJELUhTHr4zdkeqbzgui0y3iouaoyWsKvetNg= +-----END CERTIFICATE----- diff --git a/tests/data_files/test-ca2.ku-crt.crt b/tests/data_files/test-ca2.ku-crt.crt new file mode 100644 index 000000000..edacc64c9 --- /dev/null +++ b/tests/data_files/test-ca2.ku-crt.crt @@ -0,0 +1,12 @@ +-----BEGIN CERTIFICATE----- +MIIBzTCCAVOgAwIBAgIJAODh6PAeD9/vMAoGCCqGSM49BAMCMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTAeFw0xNDA0MDkxMTIzNTRaFw0yNDA0MDYxMTIzNTRaMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTB2MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBu +ww5XUzM5WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiy +aY7zQa0pw7RfdadHb9UZKVVpmlM7ILRmFmAzHqMdMBswDAYDVR0TBAUwAwEB/zAL +BgNVHQ8EBAMCAgQwCgYIKoZIzj0EAwIDaAAwZQIwGGlbynd1jU3WkUx6Irhk9Lob +z2B+1eIO6+eu3En8B3rh8Ipfxo0e0hpfaRFYP1MUAjEAjxxBchRWJAzZ6/47Wg/7 +UoasRINgP5B/uJhTnftS1bqyuWHastb4LW5/YLOvPbMQ +-----END CERTIFICATE----- diff --git a/tests/data_files/test-ca2.ku-crt_crl.crt b/tests/data_files/test-ca2.ku-crt_crl.crt new file mode 100644 index 000000000..ac74e402a --- /dev/null +++ b/tests/data_files/test-ca2.ku-crt_crl.crt @@ -0,0 +1,12 @@ +-----BEGIN CERTIFICATE----- +MIIBzDCCAVOgAwIBAgIJAPejOupCJS65MAoGCCqGSM49BAMCMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTAeFw0xNDA0MDkxMTIyMjVaFw0yNDA0MDYxMTIyMjVaMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTB2MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBu +ww5XUzM5WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiy +aY7zQa0pw7RfdadHb9UZKVVpmlM7ILRmFmAzHqMdMBswDAYDVR0TBAUwAwEB/zAL +BgNVHQ8EBAMCAQYwCgYIKoZIzj0EAwIDZwAwZAIwMKLVXB4YBQ0Ha4dEvFPcJtau +TS5Vd4UqG3xQ10YcJogweuqaGHSFgdnEUfoX+4p5AjApMnYXFfUjSmlyfJmTaswO +gaR5sUnnw33NA9j1ercem3asCYz6a8T0zo8/rR33XVU= +-----END CERTIFICATE----- diff --git a/tests/data_files/test-ca2.ku-ds.crt b/tests/data_files/test-ca2.ku-ds.crt new file mode 100644 index 000000000..c28e17b22 --- /dev/null +++ b/tests/data_files/test-ca2.ku-ds.crt @@ -0,0 +1,12 @@ +-----BEGIN CERTIFICATE----- +MIIBzDCCAVOgAwIBAgIJAPOkPR3wsvm5MAoGCCqGSM49BAMCMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTAeFw0xNDA0MDkxMTI0MTNaFw0yNDA0MDYxMTI0MTNaMD4xCzAJBgNVBAYT +Ak5MMREwDwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBF +QyBDQTB2MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBu +ww5XUzM5WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiy +aY7zQa0pw7RfdadHb9UZKVVpmlM7ILRmFmAzHqMdMBswDAYDVR0TBAUwAwEB/zAL +BgNVHQ8EBAMCB4AwCgYIKoZIzj0EAwIDZwAwZAIwGRCmU/rWNjW13g8ITuq3pMXb +jgwTFJHVlbMDiFJwUrRvytPV9doJOfzJ8nAQ0cZ1AjAbJ8QAV2e+DmYZpWc/p6Ug +nQdac59ev+lH+ju6wET3jNDjUthUPrdgqa54+UWQ5r4= +-----END CERTIFICATE----- diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index b1cf1c712..8b1df44a4 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -442,6 +442,26 @@ X509 Certificate verification #51 (Valid, multiple CAs, reverse order) depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECDSA_C:POLARSSL_RSA_C:POLARSSL_ECP_DP_SECP192R1_ENABLED:POLARSSL_PKCS1_V15 x509_verify:"data_files/server2.crt":"data_files/test-ca_cat21.crt":"data_files/crl.pem":"NULL":0:0:"NULL" +X509 Certificate verification #52 (CA keyUsage valid) +depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECDSA_C:POLARSSL_SHA256_C +x509_verify:"data_files/server5.crt":"data_files/test-ca2.ku-crt_crl.crt":"data_files/crl-ec-sha256.pem":"NULL":0:0:"NULL" + +X509 Certificate verification #53 (CA keyUsage missing cRLSign) +depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECDSA_C:POLARSSL_SHA256_C +x509_verify:"data_files/server5.crt":"data_files/test-ca2.ku-crt.crt":"data_files/crl-ec-sha256.pem":"NULL":POLARSSL_ERR_X509_CERT_VERIFY_FAILED:BADCRL_NOT_TRUSTED:"NULL" + +X509 Certificate verification #54 (CA keyUsage missing cRLSign, no CRL) +depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECDSA_C:POLARSSL_SHA256_C +x509_verify:"data_files/server5.crt":"data_files/test-ca2.ku-crt.crt":"data_files/crl.pem":"NULL":0:0:"NULL" + +X509 Certificate verification #55 (CA keyUsage missing keyCertSign) +depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECDSA_C:POLARSSL_SHA256_C +x509_verify:"data_files/server5.crt":"data_files/test-ca2.ku-crl.crt":"data_files/crl-ec-sha256.pem":"NULL":POLARSSL_ERR_X509_CERT_VERIFY_FAILED:BADCERT_NOT_TRUSTED:"NULL" + +X509 Certificate verification #55 (CA keyUsage plain wrong) +depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECDSA_C:POLARSSL_SHA256_C +x509_verify:"data_files/server5.crt":"data_files/test-ca2.ku-ds.crt":"data_files/crl-ec-sha256.pem":"NULL":POLARSSL_ERR_X509_CERT_VERIFY_FAILED:BADCERT_NOT_TRUSTED:"NULL" + X509 Parse Selftest depends_on:POLARSSL_MD5_C:POLARSSL_PEM_PARSE_C x509_selftest: From f93a3c433560f59885cc81800b8b0f6c7c4c69c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 9 Apr 2014 14:08:12 +0200 Subject: [PATCH 5/9] Check the CA bit on trusted CAs too --- library/x509_crt.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/x509_crt.c b/library/x509_crt.c index 47745a759..94d855211 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1607,6 +1607,7 @@ static int x509_crt_verify_top( while( trust_ca != NULL ) { if( trust_ca->version == 0 || + trust_ca->ca_istrue == 0 || child->issuer_raw.len != trust_ca->subject_raw.len || memcmp( child->issuer_raw.p, trust_ca->subject_raw.p, child->issuer_raw.len ) != 0 ) From 312010e6e9bfae05918bf24f71ee685ac53f5d39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 9 Apr 2014 14:30:11 +0200 Subject: [PATCH 6/9] Factor common parent checking code --- library/x509_crt.c | 77 ++++++++++++++++++++++------------------------ 1 file changed, 37 insertions(+), 40 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 94d855211..462cacefe 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1539,36 +1539,27 @@ static int x509_wildcard_verify( const char *cn, x509_buf *name ) } /* - * Iterate upwards in the given cert chain to find our parent. - * - * Ignore any upper cert that can't be used to sign other certificates - * (basic constraints CA=true for now, keyUsage soon). + * Check if 'parent' is a suitable parent (signing CA) for 'child'. + * Return 0 if yes, -1 if not. */ -static x509_crt *x509_crt_find_parent( x509_crt *crt ) +static int x509_crt_check_parent( const x509_crt *child, + const x509_crt *parent ) { - x509_crt *parent; - - for( parent = crt->next; parent != NULL; parent = parent->next ) + if( parent->version == 0 || + parent->ca_istrue == 0 || + child->issuer_raw.len != parent->subject_raw.len || + memcmp( child->issuer_raw.p, parent->subject_raw.p, + child->issuer_raw.len ) != 0 ) { - if( parent->version == 0 || - parent->ca_istrue == 0 || - crt->issuer_raw.len != parent->subject_raw.len || - memcmp( crt->issuer_raw.p, parent->subject_raw.p, - crt->issuer_raw.len ) != 0 ) - { - continue; - } - -#if defined(POLARSSL_X509_CHECK_KEY_USAGE) - if( x509_crt_check_key_usage( parent, KU_KEY_CERT_SIGN ) != 0 ) - continue; -#endif - - /* If we get there, we found a suitable parent */ - break; + return( -1 ); } - return( parent ); +#if defined(POLARSSL_X509_CHECK_KEY_USAGE) + if( x509_crt_check_key_usage( parent, KU_KEY_CERT_SIGN ) != 0 ) + return( -1 ); +#endif + + return( 0 ); } static int x509_crt_verify_top( @@ -1606,24 +1597,12 @@ static int x509_crt_verify_top( while( trust_ca != NULL ) { - if( trust_ca->version == 0 || - trust_ca->ca_istrue == 0 || - child->issuer_raw.len != trust_ca->subject_raw.len || - memcmp( child->issuer_raw.p, trust_ca->subject_raw.p, - child->issuer_raw.len ) != 0 ) + if( x509_crt_check_parent( child, trust_ca ) != 0 ) { trust_ca = trust_ca->next; continue; } -#if defined(POLARSSL_X509_CHECK_KEY_USAGE) - if( x509_crt_check_key_usage( trust_ca, KU_KEY_CERT_SIGN ) != 0 ) - { - trust_ca = trust_ca->next; - continue; - } -#endif - /* * Reduce path_len to check against if top of the chain is * the same as the trusted CA @@ -1742,7 +1721,17 @@ static int x509_crt_verify_child( *flags |= x509_crt_verifycrl(child, parent, ca_crl); #endif - if( ( grandparent = x509_crt_find_parent( parent) ) != NULL ) + /* Look for a grandparent upwards the chain */ + for( grandparent = parent->next; + grandparent != NULL; + grandparent = grandparent->next ) + { + if( x509_crt_check_parent( parent, grandparent ) == 0 ) + break; + } + + /* Is our parent part of the chain or at the top? */ + if( grandparent != NULL ) { /* * Part of the chain @@ -1837,7 +1826,15 @@ int x509_crt_verify( x509_crt *crt, } } - if( ( parent = x509_crt_find_parent( crt ) ) != NULL ) + /* Look for a parent upwards the chain */ + for( parent = crt->next; parent != NULL; parent = parent->next ) + { + if( x509_crt_check_parent( crt, parent ) == 0 ) + break; + } + + /* Are we part of the chain or at the top? */ + if( parent != NULL ) { /* * Part of the chain From 490047cc4497fe1c107547bb9c70c396dc783555 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 9 Apr 2014 14:30:45 +0200 Subject: [PATCH 7/9] Code cosmetics --- library/x509_crt.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 462cacefe..42fbfab20 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1595,13 +1595,10 @@ static int x509_crt_verify_top( else md( md_info, child->tbs.p, child->tbs.len, hash ); - while( trust_ca != NULL ) + for( /* trust_ca */ ; trust_ca != NULL; trust_ca = trust_ca->next ) { if( x509_crt_check_parent( child, trust_ca ) != 0 ) - { - trust_ca = trust_ca->next; continue; - } /* * Reduce path_len to check against if top of the chain is @@ -1617,7 +1614,6 @@ static int x509_crt_verify_top( if( trust_ca->max_pathlen > 0 && trust_ca->max_pathlen < check_path_cnt ) { - trust_ca = trust_ca->next; continue; } @@ -1625,7 +1621,6 @@ static int x509_crt_verify_top( pk_verify( &trust_ca->pk, child->sig_md, hash, md_info->size, child->sig.p, child->sig.len ) != 0 ) { - trust_ca = trust_ca->next; continue; } @@ -1733,16 +1728,15 @@ static int x509_crt_verify_child( /* Is our parent part of the chain or at the top? */ if( grandparent != NULL ) { - /* - * Part of the chain - */ - ret = x509_crt_verify_child( parent, grandparent, trust_ca, ca_crl, path_cnt + 1, &parent_flags, f_vrfy, p_vrfy ); + ret = x509_crt_verify_child( parent, grandparent, trust_ca, ca_crl, + path_cnt + 1, &parent_flags, f_vrfy, p_vrfy ); if( ret != 0 ) return( ret ); } else { - ret = x509_crt_verify_top( parent, trust_ca, ca_crl, path_cnt + 1, &parent_flags, f_vrfy, p_vrfy ); + ret = x509_crt_verify_top( parent, trust_ca, ca_crl, + path_cnt + 1, &parent_flags, f_vrfy, p_vrfy ); if( ret != 0 ) return( ret ); } @@ -1836,16 +1830,15 @@ int x509_crt_verify( x509_crt *crt, /* Are we part of the chain or at the top? */ if( parent != NULL ) { - /* - * Part of the chain - */ - ret = x509_crt_verify_child( crt, parent, trust_ca, ca_crl, pathlen, flags, f_vrfy, p_vrfy ); + ret = x509_crt_verify_child( crt, parent, trust_ca, ca_crl, + pathlen, flags, f_vrfy, p_vrfy ); if( ret != 0 ) return( ret ); } else { - ret = x509_crt_verify_top( crt, trust_ca, ca_crl, pathlen, flags, f_vrfy, p_vrfy ); + ret = x509_crt_verify_top( crt, trust_ca, ca_crl, + pathlen, flags, f_vrfy, p_vrfy ); if( ret != 0 ) return( ret ); } From a9db85df7345723a894ac64927c15c7073294bd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 9 Apr 2014 14:53:05 +0200 Subject: [PATCH 8/9] Add tests for keyUsage with client auth --- library/ssl_tls.c | 4 +-- tests/ssl-opt.sh | 66 +++++++++++++++++++++++++++++++++++------------ 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 4c8d8f9f2..588cbc4a2 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2724,7 +2724,7 @@ int ssl_parse_certificate( ssl_context *ssl ) if( pk_can_do( pk, POLARSSL_PK_ECKEY ) && ! ssl_curve_is_acceptable( ssl, pk_ec( *pk )->grp.id ) ) { - SSL_DEBUG_MSG( 1, ( "bad server certificate (EC key curve)" ) ); + SSL_DEBUG_MSG( 1, ( "bad certificate (EC key curve)" ) ); if( ret == 0 ) ret = POLARSSL_ERR_SSL_BAD_HS_CERTIFICATE; } @@ -2735,7 +2735,7 @@ int ssl_parse_certificate( ssl_context *ssl ) ciphersuite_info, ! ssl->endpoint ) != 0 ) { - SSL_DEBUG_MSG( 1, ( "bad server certificate (usage ext.)" ) ); + SSL_DEBUG_MSG( 1, ( "bad certificate (usage extensions)" ) ); if( ret == 0 ) ret = POLARSSL_ERR_SSL_BAD_HS_CERTIFICATE; } diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index ec9e5499f..16748b07c 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1031,16 +1031,7 @@ run_test "keyUsage srv #6 (ECDSA, keyEncipherment -> fail)" \ -C "Ciphersuite is " # Tests for keyUsage in leaf certificates, part 2: -# client-side checks - -run_test "keyUsage cli #0 (reference, no extension)" \ - "$O_SRV -key data_files/server2.key \ - -cert data_files/server2.crt" \ - "$P_CLI debug_level=2" \ - 0 \ - -C "bad server certificate (usage ext.)" \ - -C "Processing of the Certificate handshake message failed" \ - -c "Ciphersuite is TLS-" +# client-side checking of server cert run_test "keyUsage cli #1 (DigitalSignature+KeyEncipherment, RSA: OK)" \ "$O_SRV -key data_files/server2.key \ @@ -1048,7 +1039,7 @@ run_test "keyUsage cli #1 (DigitalSignature+KeyEncipherment, RSA: OK)" \ "$P_CLI debug_level=2 \ force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ 0 \ - -C "bad server certificate (usage ext.)" \ + -C "bad certificate (usage extensions)" \ -C "Processing of the Certificate handshake message failed" \ -c "Ciphersuite is TLS-" @@ -1058,7 +1049,7 @@ run_test "keyUsage cli #2 (DigitalSignature+KeyEncipherment, DHE-RSA: OK)" \ "$P_CLI debug_level=2 \ force_ciphersuite=TLS-DHE-RSA-WITH-AES-128-CBC-SHA" \ 0 \ - -C "bad server certificate (usage ext.)" \ + -C "bad certificate (usage extensions)" \ -C "Processing of the Certificate handshake message failed" \ -c "Ciphersuite is TLS-" @@ -1068,7 +1059,7 @@ run_test "keyUsage cli #3 (KeyEncipherment, RSA: OK)" \ "$P_CLI debug_level=2 \ force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ 0 \ - -C "bad server certificate (usage ext.)" \ + -C "bad certificate (usage extensions)" \ -C "Processing of the Certificate handshake message failed" \ -c "Ciphersuite is TLS-" @@ -1078,7 +1069,7 @@ run_test "keyUsage cli #4 (KeyEncipherment, DHE-RSA: fail)" \ "$P_CLI debug_level=2 \ force_ciphersuite=TLS-DHE-RSA-WITH-AES-128-CBC-SHA" \ 1 \ - -c "bad server certificate (usage ext.)" \ + -c "bad certificate (usage extensions)" \ -c "Processing of the Certificate handshake message failed" \ -C "Ciphersuite is TLS-" @@ -1088,7 +1079,7 @@ run_test "keyUsage cli #5 (DigitalSignature, DHE-RSA: OK)" \ "$P_CLI debug_level=2 \ force_ciphersuite=TLS-DHE-RSA-WITH-AES-128-CBC-SHA" \ 0 \ - -C "bad server certificate (usage ext.)" \ + -C "bad certificate (usage extensions)" \ -C "Processing of the Certificate handshake message failed" \ -c "Ciphersuite is TLS-" @@ -1098,10 +1089,53 @@ run_test "keyUsage cli #5 (DigitalSignature, RSA: fail)" \ "$P_CLI debug_level=2 \ force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ 1 \ - -c "bad server certificate (usage ext.)" \ + -c "bad certificate (usage extensions)" \ -c "Processing of the Certificate handshake message failed" \ -C "Ciphersuite is TLS-" +# Tests for keyUsage in leaf certificates, part 3: +# server-side checking of client cert + +run_test "keyUsage cli-auth #1 (RSA, DigitalSignature: OK)" \ + "$P_SRV debug_level=2 auth_mode=optional" \ + "$O_CLI -key data_files/server2.key \ + -cert data_files/server2.ku-ds.crt" \ + 0 \ + -S "bad certificate (usage extensions)" \ + -S "Processing of the Certificate handshake message failed" + +run_test "keyUsage cli-auth #2 (RSA, KeyEncipherment: fail (soft))" \ + "$P_SRV debug_level=2 auth_mode=optional" \ + "$O_CLI -key data_files/server2.key \ + -cert data_files/server2.ku-ke.crt" \ + 0 \ + -s "bad certificate (usage extensions)" \ + -S "Processing of the Certificate handshake message failed" + +run_test "keyUsage cli-auth #3 (RSA, KeyEncipherment: fail (hard))" \ + "$P_SRV debug_level=2 auth_mode=required" \ + "$O_CLI -key data_files/server2.key \ + -cert data_files/server2.ku-ke.crt" \ + 1 \ + -s "bad certificate (usage extensions)" \ + -s "Processing of the Certificate handshake message failed" + +run_test "keyUsage cli-auth #4 (ECDSA, DigitalSignature: OK)" \ + "$P_SRV debug_level=2 auth_mode=optional" \ + "$O_CLI -key data_files/server5.key \ + -cert data_files/server5.ku-ds.crt" \ + 0 \ + -S "bad certificate (usage extensions)" \ + -S "Processing of the Certificate handshake message failed" + +run_test "keyUsage cli-auth #5 (ECDSA, KeyAgreement: fail (soft))" \ + "$P_SRV debug_level=2 auth_mode=optional" \ + "$O_CLI -key data_files/server5.key \ + -cert data_files/server5.ku-ka.crt" \ + 0 \ + -s "bad certificate (usage extensions)" \ + -S "Processing of the Certificate handshake message failed" + # Final report echo "------------------------------------------------------------------------" From 02ff5ce59436d32a9bc1e86d6d2f00696707e7a4 Mon Sep 17 00:00:00 2001 From: Paul Bakker Date: Wed, 9 Apr 2014 15:53:09 +0200 Subject: [PATCH 9/9] Fixed typo --- include/polarssl/x509_crt.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/polarssl/x509_crt.h b/include/polarssl/x509_crt.h index e3443d016..93340eccd 100644 --- a/include/polarssl/x509_crt.h +++ b/include/polarssl/x509_crt.h @@ -253,7 +253,7 @@ int x509_crt_verify( x509_crt *crt, * certificate to perform an RSA key exchange). * * \return 0 is these uses of the certificate are allowed, - * POLARSSL_ERR_X509_BAD_INPUT_DATA if the keyUsage extenson + * POLARSSL_ERR_X509_BAD_INPUT_DATA if the keyUsage extension * is present but does not contain all the bits set in the * usage argument. *