Add badkey-skipping to find_parent()
This is the last step towards removing the now-duplicated parent-searching
code in verify_top()
diff --git a/library/x509_crt.c b/library/x509_crt.c
index 7f181a6..10ace0e 100644
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@@ -27,6 +27,8 @@
*
* http://www.itu.int/ITU-T/studygroups/com17/languages/X.680-0207.pdf
* http://www.itu.int/ITU-T/studygroups/com17/languages/X.690-0207.pdf
+ *
+ * [SIRO] https://cabforum.org/wp-content/uploads/Chunghwatelecom201503cabforumV4.pdf
*/
#if !defined(MBEDTLS_CONFIG_FILE)
@@ -1897,9 +1899,10 @@
* Find a suitable parent for child in candidates, or return NULL.
*
* Here suitable is defined as:
- * - subject name matches child's issuer
- * - if necessary, the CA bit is set and key usage allows signing certs
- * - pathlen constraints are satisfied
+ * 1. subject name matches child's issuer
+ * 2. if necessary, the CA bit is set and key usage allows signing certs
+ * 3. for trusted roots, the signature is correct
+ * 4. pathlen constraints are satisfied
*
* Stop at the first suitable candidate, except if it's not time-valid (not
* expired nor future) *and* there is a later suitable candidate that is
@@ -1911,6 +1914,12 @@
* The reason we don't just require time-validity is that generally there is
* only one version, and if it's expired we want the flags to state that
* rather than NOT_TRUSTED, as would be the case if we required it here.
+ *
+ * The rationale for rule 3 (signature for trusted roots) is that users might
+ * have two versions of the same CA with different keys in their list, and the
+ * way we select the correct one is by checking the signature. (This is one
+ * way users might choose to handle key rollover, the other one relies on
+ * self-issued certs, see [SIRO].)
*/
static mbedtls_x509_crt *x509_crt_find_parent( mbedtls_x509_crt *child,
mbedtls_x509_crt *candidates,
@@ -1919,9 +1928,12 @@
int self_cnt )
{
mbedtls_x509_crt *parent, *badtime_parent = NULL;
+ const mbedtls_md_info_t *md_info;
+ unsigned char hash[MBEDTLS_MD_MAX_SIZE];
for( parent = candidates; parent != NULL; parent = parent->next )
{
+ /* basic parenting skills (name, CA bit, key usage) */
if( x509_crt_check_parent( child, parent, top, path_cnt == 0 ) != 0 )
continue;
@@ -1932,6 +1944,25 @@
continue;
}
+ /* Signature */
+ if( top )
+ {
+ md_info = mbedtls_md_info_from_type( child->sig_md );
+ if( mbedtls_md( md_info, child->tbs.p, child->tbs.len, hash ) != 0 )
+ {
+ /* Note: this can't happen except after an internal error */
+ continue;
+ }
+
+ if( mbedtls_pk_verify_ext( child->sig_pk, child->sig_opts, &parent->pk,
+ child->sig_md, hash, mbedtls_md_get_size( md_info ),
+ child->sig.p, child->sig.len ) != 0 )
+ {
+ continue;
+ }
+ }
+
+ /* optionnal time check */
if( mbedtls_x509_time_is_past( &parent->valid_to ) ||
mbedtls_x509_time_is_future( &parent->valid_from ) )
{
@@ -2126,7 +2157,8 @@
mbedtls_x509_crt *grandparent;
const mbedtls_md_info_t *md_info;
- /* Counting intermediate self signed certificates */
+ /* Counting intermediate self-issued (not necessarily self-signed) certs
+ * These can occur with some strategies for key rollover, see [SIRO] */
if( ( path_cnt != 0 ) && x509_name_cmp( &child->issuer, &child->subject ) == 0 )
self_cnt++;