From 915275ba78309dd0dccbda1c0035b840050ba8a1 Mon Sep 17 00:00:00 2001 From: Paul Bakker Date: Fri, 28 Sep 2012 07:10:55 +0000 Subject: [PATCH] - Revamped x509_verify() and the SSL f_vrfy callback implementations --- ChangeLog | 1 + include/polarssl/ssl.h | 12 +- include/polarssl/x509.h | 23 +- library/ssl_tls.c | 2 +- library/x509parse.c | 273 ++++++++++++++------- programs/ssl/ssl_client2.c | 51 +++- tests/suites/test_suite_x509parse.data | 4 +- tests/suites/test_suite_x509parse.function | 12 +- 8 files changed, 264 insertions(+), 114 deletions(-) diff --git a/ChangeLog b/ChangeLog index f1fb20d4d..28f2847b9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -43,6 +43,7 @@ Changes * Revamped session resumption handling * Generalized external private key implementation handling (like PKCS#11) in SSL/TLS + * Revamped x509_verify() and the SSL f_vrfy callback implementations Bugfix * Fixed handling error in mpi_cmp_mpi() on longer B values (found by diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h index 094a120dd..c460963fe 100644 --- a/include/polarssl/ssl.h +++ b/include/polarssl/ssl.h @@ -397,7 +397,7 @@ struct _ssl_context void (*f_dbg)(void *, int, const char *); int (*f_recv)(void *, unsigned char *, size_t); int (*f_send)(void *, const unsigned char *, size_t); - int (*f_vrfy)(void *, x509_cert *, int, int); + int (*f_vrfy)(void *, x509_cert *, int, int *); int (*f_get_cache)(void *, ssl_session *); int (*f_set_cache)(void *, const ssl_session *); int (*f_sni)(void *, ssl_context *, const unsigned char *, size_t); @@ -601,18 +601,16 @@ void ssl_set_authmode( ssl_context *ssl, int authmode ); /** * \brief Set the verification callback (Optional). * - * If set, the verification callback is called once for every - * certificate in the chain. The verification function has the - * following parameter: (void *parameter, x509_cert certificate, - * int certifcate_depth, int preverify_ok). It should - * return 0 on SUCCESS. + * If set, the verify callback is called for each + * certificate in the chain. For implementation + * information, please see \c x509parse_verify() * * \param ssl SSL context * \param f_vrfy verification function * \param p_vrfy verification parameter */ void ssl_set_verify( ssl_context *ssl, - int (*f_vrfy)(void *, x509_cert *, int, int), + int (*f_vrfy)(void *, x509_cert *, int, int *), void *p_vrfy ); /** diff --git a/include/polarssl/x509.h b/include/polarssl/x509.h index e0a2776e9..32aad7274 100644 --- a/include/polarssl/x509.h +++ b/include/polarssl/x509.h @@ -77,6 +77,7 @@ #define BADCRL_EXPIRED 0x20 /**< CRL is expired. */ #define BADCERT_MISSING 0x40 /**< Certificate was missing. */ #define BADCERT_SKIP_VERIFY 0x80 /**< Certificate verification was skipped. */ +#define BADCERT_OTHER 0x0100 /**< Other reason (can be used by verify callback) */ /* \} name */ /* \} addtogroup x509_module */ @@ -310,7 +311,7 @@ typedef struct _x509_cert int ext_types; /**< Bit string containing detected and parsed extensions */ int ca_istrue; /**< Optional Basic Constraint extension value: 1 if this certificate belongs to a CA, 0 otherwise. */ - int max_pathlen; /**< Optional Basic Constraint extension value: The maximum path length to the root certificate. */ + int max_pathlen; /**< Optional Basic Constraint extension value: The maximum path length to the root certificate. Path length is 1 higher than RFC 5280 'meaning', so 1+ */ unsigned char key_usage; /**< Optional key usage extension value: See the values below */ @@ -671,6 +672,20 @@ int x509parse_time_expired( const x509_time *time ); /** * \brief Verify the certificate signature * + * The verify callback is a user-supplied callback that + * can clear / modify / add flags for a certificate. If set, + * the verification callback is called for each + * certificate in the chain (from the trust-ca down to the + * presented crt). The parameters for the callback are: + * (void *parameter, x509_cert *crt, int certificate_depth, + * int *flags). With the flags representing current flags for + * that specific certificate and the certificate depth from + * the top (Trust CA depth = 0). + * + * All flags left after returning from the callback + * are also returned to the application. The function should + * return 0 for anything but a fatal error. + * * \param crt a certificate to be verified * \param trust_ca the trusted CA chain * \param ca_crl the CRL chain for trusted CA's @@ -687,14 +702,14 @@ int x509parse_time_expired( const x509_time *time ); * BADCERT_REVOKED -- * BADCERT_CN_MISMATCH -- * BADCERT_NOT_TRUSTED - * - * \note TODO: add two arguments, depth and crl + * or another error in case of a fatal error encountered + * during the verification process. */ int x509parse_verify( x509_cert *crt, x509_cert *trust_ca, x509_crl *ca_crl, const char *cn, int *flags, - int (*f_vrfy)(void *, x509_cert *, int, int), + int (*f_vrfy)(void *, x509_cert *, int, int *), void *p_vrfy ); /** diff --git a/library/ssl_tls.c b/library/ssl_tls.c index a33a56685..65bd7d431 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2959,7 +2959,7 @@ void ssl_set_authmode( ssl_context *ssl, int authmode ) } void ssl_set_verify( ssl_context *ssl, - int (*f_vrfy)(void *, x509_cert *, int, int), + int (*f_vrfy)(void *, x509_cert *, int, int *), void *p_vrfy ) { ssl->f_vrfy = f_vrfy; diff --git a/library/x509parse.c b/library/x509parse.c index 3968666c6..a9fd0e89d 100644 --- a/library/x509parse.c +++ b/library/x509parse.c @@ -3004,15 +3004,19 @@ static int x509parse_verifycrl(x509_cert *crt, x509_cert *ca, int hash_id; unsigned char hash[64]; + if( ca == NULL ) + return( flags ); + /* * TODO: What happens if no CRL is present? * Suggestion: Revocation state should be unknown if no CRL is present. * For backwards compatibility this is not yet implemented. */ - while( ca != NULL && crl_list != NULL && crl_list->version != 0 ) + while( crl_list != NULL ) { - if( crl_list->issuer_raw.len != ca->subject_raw.len || + if( crl_list->version == 0 || + crl_list->issuer_raw.len != ca->subject_raw.len || memcmp( crl_list->issuer_raw.p, ca->subject_raw.p, crl_list->issuer_raw.len ) != 0 ) { @@ -3086,6 +3090,168 @@ int x509_wildcard_verify( const char *cn, x509_buf *name ) return( 0 ); } +static int x509parse_verify_top( + x509_cert *child, x509_cert *trust_ca, + x509_crl *ca_crl, int *path_cnt, int *flags, + int (*f_vrfy)(void *, x509_cert *, int, int *), + void *p_vrfy ) +{ + int hash_id, ret; + int ca_flags = 0; + unsigned char hash[64]; + + if( x509parse_time_expired( &child->valid_to ) ) + *flags |= BADCERT_EXPIRED; + + /* + * Child is the top of the chain. Check against the trust_ca list. + */ + *flags |= BADCERT_NOT_TRUSTED; + + while( trust_ca != NULL ) + { + if( trust_ca->version == 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 ) + { + trust_ca = trust_ca->next; + continue; + } + + if( trust_ca->max_pathlen > 0 && + trust_ca->max_pathlen < *path_cnt ) + { + trust_ca = trust_ca->next; + continue; + } + + hash_id = child->sig_alg; + + x509_hash( child->tbs.p, child->tbs.len, hash_id, hash ); + + if( rsa_pkcs1_verify( &trust_ca->rsa, RSA_PUBLIC, hash_id, + 0, hash, child->sig.p ) != 0 ) + { + trust_ca = trust_ca->next; + continue; + } + + /* + * Top of chain is signed by a trusted CA + */ + *flags &= ~BADCERT_NOT_TRUSTED; + break; + } + + if( trust_ca != NULL ) + { + /* Check trusted CA's CRL for then chain's top crt */ + *flags |= x509parse_verifycrl( child, trust_ca, ca_crl ); + + if( x509parse_time_expired( &trust_ca->valid_to ) ) + ca_flags |= BADCERT_EXPIRED; + + hash_id = trust_ca->sig_alg; + + x509_hash( trust_ca->tbs.p, trust_ca->tbs.len, hash_id, hash ); + + if( rsa_pkcs1_verify( &trust_ca->rsa, RSA_PUBLIC, hash_id, + 0, hash, trust_ca->sig.p ) != 0 ) + { + ca_flags |= BADCERT_NOT_TRUSTED; + } + + if( NULL != f_vrfy ) + { + if( ( ret = f_vrfy( p_vrfy, trust_ca, 0, &ca_flags ) ) != 0 ) + return( ret ); + } + } + + /* Call callback on top cert */ + if( NULL != f_vrfy ) + { + if( ( ret = f_vrfy(p_vrfy, child, 1, flags ) ) != 0 ) + return( ret ); + } + + *path_cnt = 2; + + *flags |= ca_flags; + + return( 0 ); +} + +static int x509parse_verify_child( + x509_cert *child, x509_cert *parent, x509_cert *trust_ca, + x509_crl *ca_crl, int *path_cnt, int *flags, + int (*f_vrfy)(void *, x509_cert *, int, int *), + void *p_vrfy ) +{ + int hash_id, ret; + int parent_flags = 0; + unsigned char hash[64]; + x509_cert *grandparent; + + if( x509parse_time_expired( &child->valid_to ) ) + *flags |= BADCERT_EXPIRED; + + hash_id = child->sig_alg; + + x509_hash( child->tbs.p, child->tbs.len, hash_id, hash ); + + if( rsa_pkcs1_verify( &parent->rsa, RSA_PUBLIC, hash_id, 0, hash, + child->sig.p ) != 0 ) + *flags |= BADCERT_NOT_TRUSTED; + + /* Check trusted CA's CRL for the given crt */ + *flags |= x509parse_verifycrl(child, parent, ca_crl); + + 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; + } + + (*path_cnt)++; + if( grandparent != NULL ) + { + /* + * Part of the chain + */ + ret = x509parse_verify_child( parent, grandparent, trust_ca, ca_crl, path_cnt, &parent_flags, f_vrfy, p_vrfy ); + if( ret != 0 ) + return( ret ); + } + else + { + ret = x509parse_verify_top( parent, trust_ca, ca_crl, path_cnt, &parent_flags, f_vrfy, p_vrfy ); + if( ret != 0 ) + return( ret ); + } + + /* child is verified to be a child of the parent, call verify callback */ + if( NULL != f_vrfy ) + if( ( ret = f_vrfy( p_vrfy, child, *path_cnt, flags ) ) != 0 ) + return( ret ); + (*path_cnt)++; + + *flags |= parent_flags; + + return( 0 ); +} + /* * Verify the certificate validity */ @@ -3093,22 +3259,18 @@ int x509parse_verify( x509_cert *crt, x509_cert *trust_ca, x509_crl *ca_crl, const char *cn, int *flags, - int (*f_vrfy)(void *, x509_cert *, int, int), + int (*f_vrfy)(void *, x509_cert *, int, int *), void *p_vrfy ) { size_t cn_len; - int hash_id; - int pathlen; + int ret; + int pathlen = 1; x509_cert *parent; x509_name *name; - unsigned char hash[64]; x509_sequence *cur = NULL; *flags = 0; - if( x509parse_time_expired( &crt->valid_to ) ) - *flags = BADCERT_EXPIRED; - if( cn != NULL ) { name = &crt->subject; @@ -3161,13 +3323,11 @@ int x509parse_verify( x509_cert *crt, } /* - * Iterate upwards in the given cert chain, - * ignoring any upper cert with CA != TRUE. + * Iterate upwards in the given cert chain, to find our crt parent. + * Ignore any upper cert with CA != TRUE. */ parent = crt->next; - pathlen = 1; - while( parent != NULL && parent->version != 0 ) { if( parent->ca_istrue == 0 || @@ -3178,83 +3338,26 @@ int x509parse_verify( x509_cert *crt, parent = parent->next; continue; } - - hash_id = crt->sig_alg; - - x509_hash( crt->tbs.p, crt->tbs.len, hash_id, hash ); - - if( rsa_pkcs1_verify( &parent->rsa, RSA_PUBLIC, hash_id, 0, hash, - crt->sig.p ) != 0 ) - *flags |= BADCERT_NOT_TRUSTED; - - /* Check trusted CA's CRL for the given crt */ - *flags |= x509parse_verifycrl(crt, parent, ca_crl); - - /* crt is verified to be a child of the parent cur, call verify callback */ - if( NULL != f_vrfy ) - { - if( f_vrfy( p_vrfy, crt, pathlen - 1, ( *flags == 0 ) ) != 0 ) - return( POLARSSL_ERR_X509_CERT_VERIFY_FAILED ); - else - *flags = 0; - } - else if( *flags != 0 ) - return( POLARSSL_ERR_X509_CERT_VERIFY_FAILED ); - - pathlen++; - - crt = parent; - parent = crt->next; + break; } - /* - * Attempt to validate topmost cert with our CA chain. - */ - *flags |= BADCERT_NOT_TRUSTED; - - while( trust_ca != NULL && trust_ca->version != 0 ) + if( parent != NULL ) { - if( crt->issuer_raw.len != trust_ca->subject_raw.len || - memcmp( crt->issuer_raw.p, trust_ca->subject_raw.p, - crt->issuer_raw.len ) != 0 ) - { - trust_ca = trust_ca->next; - continue; - } - - if( trust_ca->max_pathlen > 0 && - trust_ca->max_pathlen < pathlen ) - break; - - hash_id = crt->sig_alg; - - x509_hash( crt->tbs.p, crt->tbs.len, hash_id, hash ); - - if( rsa_pkcs1_verify( &trust_ca->rsa, RSA_PUBLIC, hash_id, - 0, hash, crt->sig.p ) == 0 ) - { - /* - * cert. is signed by a trusted CA - */ - *flags &= ~BADCERT_NOT_TRUSTED; - break; - } - - trust_ca = trust_ca->next; - } - - /* Check trusted CA's CRL for the given crt */ - *flags |= x509parse_verifycrl( crt, trust_ca, ca_crl ); - - /* Verification succeeded, call callback on top cert */ - if( NULL != f_vrfy ) + /* + * Part of the chain + */ + ret = x509parse_verify_child( crt, parent, trust_ca, ca_crl, &pathlen, flags, f_vrfy, p_vrfy ); + if( ret != 0 ) + return( ret ); + } + else { - if( f_vrfy(p_vrfy, crt, pathlen-1, ( *flags == 0 ) ) != 0 ) - return( POLARSSL_ERR_X509_CERT_VERIFY_FAILED ); - else - *flags = 0; + ret = x509parse_verify_top( crt, trust_ca, ca_crl, &pathlen, flags, f_vrfy, p_vrfy ); + if( ret != 0 ) + return( ret ); } - else if( *flags != 0 ) + + if( *flags != 0 ) return( POLARSSL_ERR_X509_CERT_VERIFY_FAILED ); return( 0 ); diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 4c4cf3e71..a6915e2ee 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -82,6 +82,45 @@ void my_debug( void *ctx, int level, const char *str ) } } +/* + * Enabled if debug_level > 1 in code below + */ +int my_verify( void *data, x509_cert *crt, int depth, int *flags ) +{ + char buf[1024]; + ((void) data); + + printf( "\nVerify requested for (Depth %d):\n", depth ); + x509parse_cert_info( buf, sizeof( buf ) - 1, "", crt ); + printf( "%s", buf ); + + if( ( (*flags) & BADCERT_EXPIRED ) != 0 ) + printf( " ! server certificate has expired\n" ); + + if( ( (*flags) & BADCERT_REVOKED ) != 0 ) + printf( " ! server certificate has been revoked\n" ); + + if( ( (*flags) & BADCERT_CN_MISMATCH ) != 0 ) + printf( " ! CN mismatch\n" ); + + if( ( (*flags) & BADCERT_NOT_TRUSTED ) != 0 ) + printf( " ! self-signed or not signed by a trusted CA\n" ); + + if( ( (*flags) & BADCRL_NOT_TRUSTED ) != 0 ) + printf( " ! CRL not trusted\n" ); + + if( ( (*flags) & BADCRL_EXPIRED ) != 0 ) + printf( " ! CRL expired\n" ); + + if( ( (*flags) & BADCERT_OTHER ) != 0 ) + printf( " ! other (unknown) flag\n" ); + + if ( ( *flags ) == 0 ) + printf( " This certificate has no flags\n" ); + + return( 0 ); +} + #if defined(POLARSSL_FS_IO) #define USAGE_IO \ " ca_file=%%s default: \"\" (pre-loaded)\n" \ @@ -135,7 +174,6 @@ int main( int argc, char *argv[] ) x509_cert clicert; rsa_context rsa; int i; - size_t j, n; char *p, *q; const int *list; @@ -180,14 +218,6 @@ int main( int argc, char *argv[] ) for( i = 1; i < argc; i++ ) { - n = strlen( argv[i] ); - - for( j = 0; j < n; j++ ) - { - if( argv[i][j] >= 'A' && argv[i][j] <= 'Z' ) - argv[i][j] |= 0x20; - } - p = argv[i]; if( ( q = strchr( p, '=' ) ) == NULL ) goto usage; @@ -371,6 +401,9 @@ int main( int argc, char *argv[] ) printf( " ok\n" ); + if( opt.debug_level > 0 ) + ssl_set_verify( &ssl, my_verify, NULL ); + ssl_set_endpoint( &ssl, SSL_IS_CLIENT ); ssl_set_authmode( &ssl, SSL_VERIFY_OPTIONAL ); diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 73249bbfd..3d2fb1747 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -228,11 +228,11 @@ x509_verify:"data_files/cert_sha512.crt":"data_files/test-ca.crt":"data_files/cr X509 Certificate verification #19 (Valid Cert, denying callback) depends_on:POLARSSL_SHA4_C:POLARSSL_PEM_C:POLARSSL_FS_IO -x509_verify:"data_files/cert_sha512.crt":"data_files/test-ca.crt":"data_files/crl.pem":NULL:POLARSSL_ERR_X509_CERT_VERIFY_FAILED:0:&verify_none +x509_verify:"data_files/cert_sha512.crt":"data_files/test-ca.crt":"data_files/crl.pem":NULL:POLARSSL_ERR_X509_CERT_VERIFY_FAILED:BADCERT_OTHER:verify_none X509 Certificate verification #20 (Not trusted Cert, allowing callback) depends_on:POLARSSL_PEM_C:POLARSSL_FS_IO -x509_verify:"data_files/server2.crt":"data_files/server1.crt":"data_files/crl_expired.pem":NULL:0:0:&verify_all +x509_verify:"data_files/server2.crt":"data_files/server1.crt":"data_files/crl_expired.pem":NULL:0:0:verify_all X509 Certificate verification #21 (domain matching wildcard certificate) depends_on:POLARSSL_PEM_C:POLARSSL_FS_IO diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index 3184daa86..26f5c4c10 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -2,22 +2,22 @@ BEGIN_HEADER #include #include -int verify_none( void *data, x509_cert *crt, int certificate_depth, int preverify_ok ) +int verify_none( void *data, x509_cert *crt, int certificate_depth, int *flags ) { ((void) data); ((void) crt); ((void) certificate_depth); - ((void) preverify_ok); - - return 1; + *flags |= BADCERT_OTHER; + + return 0; } -int verify_all( void *data, x509_cert *crt, int certificate_depth, int preverify_ok ) +int verify_all( void *data, x509_cert *crt, int certificate_depth, int *flags ) { ((void) data); ((void) crt); ((void) certificate_depth); - ((void) preverify_ok); + *flags = 0; return 0; }