- Revamped x509_verify() and the SSL f_vrfy callback implementations
diff --git a/ChangeLog b/ChangeLog
index f1fb20d..28f2847 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -43,6 +43,7 @@
* 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 094a120..c460963 100644
--- a/include/polarssl/ssl.h
+++ b/include/polarssl/ssl.h
@@ -397,7 +397,7 @@
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 @@
/**
* \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 e0a2776..32aad72 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 @@
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 @@
/**
* \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 @@
* 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 a33a566..65bd7d4 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -2959,7 +2959,7 @@
}
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 3968666..a9fd0e8 100644
--- a/library/x509parse.c
+++ b/library/x509parse.c
@@ -3004,15 +3004,19 @@
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 @@
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 @@
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 @@
}
/*
- * 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 @@
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 4c4cf3e..a6915e2 100644
--- a/programs/ssl/ssl_client2.c
+++ b/programs/ssl/ssl_client2.c
@@ -82,6 +82,45 @@
}
}
+/*
+ * 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 @@
x509_cert clicert;
rsa_context rsa;
int i;
- size_t j, n;
char *p, *q;
const int *list;
@@ -180,14 +218,6 @@
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 @@
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 73249bb..3d2fb17 100644
--- a/tests/suites/test_suite_x509parse.data
+++ b/tests/suites/test_suite_x509parse.data
@@ -228,11 +228,11 @@
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 3184daa..26f5c4c 100644
--- a/tests/suites/test_suite_x509parse.function
+++ b/tests/suites/test_suite_x509parse.function
@@ -2,22 +2,22 @@
#include <polarssl/x509.h>
#include <polarssl/pem.h>
-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;
}