- Revamped x509_verify() and the SSL f_vrfy callback implementations

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

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
+ *                 If set, the verify callback is called for each
- *                 certificate in the chain. The verification function has the
+ *                 certificate in the chain. For implementation
- *                 following parameter: (void *parameter, x509_cert certificate,
+ *                 information, please see \c x509parse_verify()
- *                 int certifcate_depth, int preverify_ok). It should
- *                 return 0 on SUCCESS.
  *
  * \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 );
 
 /**

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
- *
+ *                 or another error in case of a fatal error encountered
- * \note           TODO: add two arguments, depth and crl
+ *                 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 );
 
 /**

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;

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 ret;
-    int pathlen;
+    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,
+     * Iterate upwards in the given cert chain, to find our crt parent.
-     * ignoring any upper cert with CA != TRUE.
+     * 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;
         }
-
+        break;
-        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;
     }
 
-    /*
+    if( parent != NULL )
-     * Attempt to validate topmost cert with our CA chain.
-     */
-    *flags |= BADCERT_NOT_TRUSTED;
-
-    while( trust_ca != NULL && trust_ca->version != 0 )
     {
-        if( crt->issuer_raw.len != trust_ca->subject_raw.len ||
+        /*
-            memcmp( crt->issuer_raw.p, trust_ca->subject_raw.p,
+         * Part of the chain
-                    crt->issuer_raw.len ) != 0 )
+         */
-        {
+        ret = x509parse_verify_child( crt, parent, trust_ca, ca_crl, &pathlen, flags, f_vrfy, p_vrfy );
-            trust_ca = trust_ca->next;
+        if( ret != 0 )
-            continue;
+            return( ret );
-        }
+    } 
-
+    else
-        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 )
     {
-        if( f_vrfy(p_vrfy, crt, pathlen-1, ( *flags == 0 ) ) != 0 ) 
+        ret = x509parse_verify_top( crt, trust_ca, ca_crl, &pathlen, flags, f_vrfy, p_vrfy );
-            return( POLARSSL_ERR_X509_CERT_VERIFY_FAILED );
+        if( ret != 0 )
-        else
+            return( ret );
-            *flags = 0;
     }
-    else if( *flags != 0 )
+
+    if( *flags != 0 )
         return( POLARSSL_ERR_X509_CERT_VERIFY_FAILED );
 
     return( 0 );

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

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

tests/suites/test_suite_x509parse.function

@@ -2,22 +2,22 @@ BEGIN_HEADER
 #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);
+    *flags |= BADCERT_OTHER;
-
+    
-    return 1;
+    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;
 }