Skip to content

Commit

Permalink
Enable allowing unsupported critical extensions in runtime
Browse files Browse the repository at this point in the history
When compile time flag MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION
is not set, certificate parsing fails if certificate contains unsupported critical extension.

This patch allows to modify this behavior in runtime.

Signed-off-by: Lev Stipakov <[email protected]>
  • Loading branch information
lstipakov committed Aug 14, 2019
1 parent d81c11b commit 6071eea
Show file tree
Hide file tree
Showing 9 changed files with 162 additions and 15 deletions.
13 changes: 12 additions & 1 deletion include/mbedtls/oid.h
Original file line number Diff line number Diff line change
Expand Up @@ -426,14 +426,25 @@ int mbedtls_oid_get_numeric_string( char *buf, size_t size, const mbedtls_asn1_b

#if defined(MBEDTLS_X509_USE_C) || defined(MBEDTLS_X509_CREATE_C)
/**
* \brief Translate an X.509 extension OID into local values
* \brief Translate supported X.509 extension OID into local values
*
* \param oid OID to use
* \param ext_type place to store the extension type
*
* \return 0 if successful, or MBEDTLS_ERR_OID_NOT_FOUND
*/
int mbedtls_oid_get_x509_ext_type( const mbedtls_asn1_buf *oid, int *ext_type );

/**
* \brief Translate supported and unsupported X.509 extension OID into local values
*
* \param oid OID to use
* \param ext_type place to store the extension type
* \param is_supported place to store flag if extension is supported (1 - supported, 0 otherwise)
*
* \return 0 if successful, or MBEDTLS_ERR_OID_NOT_FOUND
*/
int mbedtls_oid_get_x509_ext_type_supported( const mbedtls_asn1_buf *oid, int *ext_type, int *is_supported );
#endif

/**
Expand Down
22 changes: 22 additions & 0 deletions include/mbedtls/ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -954,6 +954,10 @@ struct mbedtls_ssl_config
retransmission timeout (ms) */
#endif

uint32_t allowed_unsupported_critical_exts; /*!< Bit flags which represent runtime-enabled
unsupported critical extensions, e.g.
MBEDTLS_X509_EXT_NAME_CONSTRAINTS */

#if defined(MBEDTLS_SSL_RENEGOTIATION)
int renego_max_records; /*!< grace period for renegotiation */
unsigned char renego_period[8]; /*!< value of the record counters
Expand Down Expand Up @@ -2728,6 +2732,24 @@ void mbedtls_ssl_conf_renegotiation_period( mbedtls_ssl_config *conf,
const unsigned char period[8] );
#endif /* MBEDTLS_SSL_RENEGOTIATION */

/**
* \brief Allows unsupported critical extensions
*
* Without compile-time flag MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION
* mbedTLS fails certificate verification if certificate contains
* unsupported critical extensions.
*
* This method allows to modify behavior in runtime by providing
* bit flags which represent unsupported extensions (for example MBEDTLS_X509_EXT_NAME_CONSTRAINTS)
* which should be allowed despite missing above mentioned compile-time flag.
*
* \param conf SSL configuration
* \param exts Bit flags which represent runtime-enabled unsupported critical extensions,
* e.g. MBEDTLS_X509_EXT_NAME_CONSTRAINTS
*
*/
void mbedtls_ssl_conf_allow_unsupported_critical_exts( mbedtls_ssl_config *conf, uint32_t exts );

/**
* \brief Check if there is data already read from the
* underlying transport but not yet processed.
Expand Down
2 changes: 2 additions & 0 deletions include/mbedtls/x509_crt.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ typedef struct mbedtls_x509_crt
mbedtls_pk_type_t sig_pk; /**< Internal representation of the Public Key algorithm of the signature algorithm, e.g. MBEDTLS_PK_RSA */
void *sig_opts; /**< Signature options to be passed to mbedtls_pk_verify_ext(), e.g. for RSASSA-PSS */

uint32_t allowed_unsupported_critical_exts; /**< Optional Bit flags which represent runtime-enabled unsupported critical extensions, e.g. MBEDTLS_X509_EXT_NAME_CONSTRAINTS */

struct mbedtls_x509_crt *next; /**< Next certificate in the CA-chain. */
}
mbedtls_x509_crt;
Expand Down
81 changes: 69 additions & 12 deletions library/oid.c
Original file line number Diff line number Diff line change
Expand Up @@ -257,38 +257,95 @@ FN_OID_GET_ATTR1(mbedtls_oid_get_attr_short_name, oid_x520_attr_t, x520_attr, co
typedef struct {
mbedtls_oid_descriptor_t descriptor;
int ext_type;
int is_supported;
} oid_x509_ext_t;

static const oid_x509_ext_t oid_x509_ext[] =
{
{
{ ADD_LEN( MBEDTLS_OID_BASIC_CONSTRAINTS ), "id-ce-basicConstraints", "Basic Constraints" },
MBEDTLS_X509_EXT_BASIC_CONSTRAINTS,
{ ADD_LEN( MBEDTLS_OID_AUTHORITY_KEY_IDENTIFIER ), "id-ce-authorityKeyIdentifier", "Authority Key Identifier" },
MBEDTLS_X509_EXT_AUTHORITY_KEY_IDENTIFIER, 0,
},
{
{ ADD_LEN( MBEDTLS_OID_KEY_USAGE ), "id-ce-keyUsage", "Key Usage" },
MBEDTLS_X509_EXT_KEY_USAGE,
{ ADD_LEN( MBEDTLS_OID_SUBJECT_KEY_IDENTIFIER ), "id-ce-subjectKeyIdentifier", "Subject Key Identifier" },
MBEDTLS_X509_EXT_SUBJECT_KEY_IDENTIFIER, 0,
},
{
{ ADD_LEN( MBEDTLS_OID_EXTENDED_KEY_USAGE ), "id-ce-extKeyUsage", "Extended Key Usage" },
MBEDTLS_X509_EXT_EXTENDED_KEY_USAGE,
{ ADD_LEN( MBEDTLS_OID_KEY_USAGE ), "id-ce-keyUsage", "Key Usage" },
MBEDTLS_X509_EXT_KEY_USAGE, 1,
},
{
{ ADD_LEN( MBEDTLS_OID_SUBJECT_ALT_NAME ), "id-ce-subjectAltName", "Subject Alt Name" },
MBEDTLS_X509_EXT_SUBJECT_ALT_NAME,
{ ADD_LEN( MBEDTLS_OID_CERTIFICATE_POLICIES ), "id-ce-certificatePolicies", "Certificate Policies" },
MBEDTLS_X509_EXT_CERTIFICATE_POLICIES, 0,
},
{
{ ADD_LEN( MBEDTLS_OID_NS_CERT_TYPE ), "id-netscape-certtype", "Netscape Certificate Type" },
MBEDTLS_X509_EXT_NS_CERT_TYPE,
{ ADD_LEN( MBEDTLS_OID_POLICY_MAPPINGS ), "id-ce-policyMappings", "Policy Mapping" },
MBEDTLS_X509_EXT_POLICY_MAPPINGS, 0,
},
{
{ ADD_LEN( MBEDTLS_OID_ISSUER_ALT_NAME ), "id-ce-issuerAltName", "Issuer Alt Name" },
MBEDTLS_X509_EXT_ISSUER_ALT_NAME, 0,
},
{
{ ADD_LEN( MBEDTLS_OID_SUBJECT_DIRECTORY_ATTRS ), "id-ce-subjectDirectoryAttributes", "Subject Directory Attributes" },
MBEDTLS_X509_EXT_SUBJECT_DIRECTORY_ATTRS, 0,
},
{
{ ADD_LEN( MBEDTLS_OID_BASIC_CONSTRAINTS ), "id-ce-basicConstraints", "Basic Constraints" },
MBEDTLS_X509_EXT_BASIC_CONSTRAINTS, 1,
},
{
{ ADD_LEN( MBEDTLS_OID_NAME_CONSTRAINTS ), "id-ce-nameConstraints", "Name Constraints" },
MBEDTLS_X509_EXT_NAME_CONSTRAINTS, 0,
},
{
{ ADD_LEN( MBEDTLS_OID_POLICY_CONSTRAINTS ), "id-ce-policyConstraints", "Policy Constraints" },
MBEDTLS_X509_EXT_POLICY_CONSTRAINTS, 0,
},
{
{ ADD_LEN( MBEDTLS_OID_EXTENDED_KEY_USAGE ), "id-ce-extKeyUsage", "Extended Key Usage" },
MBEDTLS_X509_EXT_EXTENDED_KEY_USAGE, 1
},
{
{ ADD_LEN( MBEDTLS_OID_CRL_DISTRIBUTION_POINTS ), "id-ce-cRLDistributionPoints", "CRL Distribution Points" },
MBEDTLS_X509_EXT_CRL_DISTRIBUTION_POINTS, 0,
},
{
{ ADD_LEN( MBEDTLS_OID_INIHIBIT_ANYPOLICY ), "id-ce-inhibitAnyPolicy", "Inhibit Any Policy" },
MBEDTLS_X509_EXT_INIHIBIT_ANYPOLICY, 0,
},
{
{ ADD_LEN( MBEDTLS_OID_FRESHEST_CRL ), "id-ce-freshestCRL", "Freshest CRL" },
MBEDTLS_X509_EXT_FRESHEST_CRL, 0,
},
{
{ ADD_LEN( MBEDTLS_OID_SUBJECT_ALT_NAME ), "id-ce-subjectAltName", "Subject Alt Name" },
MBEDTLS_X509_EXT_SUBJECT_ALT_NAME, 1
},
{
{ ADD_LEN( MBEDTLS_OID_NS_CERT_TYPE ), "id-netscape-certtype", "Netscape Certificate Type" },
MBEDTLS_X509_EXT_NS_CERT_TYPE, 1
},
{
{ NULL, 0, NULL, NULL },
0,
0, 0
},
};

FN_OID_TYPED_FROM_ASN1(oid_x509_ext_t, x509_ext, oid_x509_ext)
FN_OID_GET_ATTR1(mbedtls_oid_get_x509_ext_type, oid_x509_ext_t, x509_ext, int, ext_type)
FN_OID_GET_ATTR2(mbedtls_oid_get_x509_ext_type_supported, oid_x509_ext_t, x509_ext, int, ext_type, int, is_supported)

int mbedtls_oid_get_x509_ext_type( const mbedtls_asn1_buf *oid, int *ext_type )
{
int ret = 0;
int is_supported = 0;

ret = mbedtls_oid_get_x509_ext_type_supported(oid, ext_type, &is_supported);
if( is_supported == 0 )
ret = MBEDTLS_ERR_OID_NOT_FOUND;

return( ret );
}

static const mbedtls_oid_descriptor_t oid_ext_key_usage[] =
{
Expand Down
8 changes: 8 additions & 0 deletions library/ssl_tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -5544,6 +5544,9 @@ static int ssl_parse_certificate_chain( mbedtls_ssl_context *ssl )

mbedtls_x509_crt_init( ssl->session_negotiate->peer_cert );

ssl->session_negotiate->peer_cert->allowed_unsupported_critical_exts =
ssl->conf->allowed_unsupported_critical_exts;

i += 3;

while( i < ssl->in_hslen )
Expand Down Expand Up @@ -7690,6 +7693,11 @@ void mbedtls_ssl_conf_renegotiation_period( mbedtls_ssl_config *conf,
}
#endif /* MBEDTLS_SSL_RENEGOTIATION */

void mbedtls_ssl_conf_allow_unsupported_critical_exts( mbedtls_ssl_config *conf, uint32_t exts )
{
conf->allowed_unsupported_critical_exts = exts;
}

#if defined(MBEDTLS_SSL_SESSION_TICKETS)
#if defined(MBEDTLS_SSL_CLI_C)
void mbedtls_ssl_conf_session_tickets( mbedtls_ssl_config *conf, int use_tickets )
Expand Down
10 changes: 8 additions & 2 deletions library/x509_crt.c
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,7 @@ static int x509_get_crt_ext( unsigned char **p,
int ret;
size_t len;
unsigned char *end_ext_data, *end_ext_octet;
int is_supported;

if( *p == end )
return( 0 );
Expand Down Expand Up @@ -752,16 +753,20 @@ static int x509_get_crt_ext( unsigned char **p,
/*
* Detect supported extensions
*/
ret = mbedtls_oid_get_x509_ext_type( &extn_oid, &ext_type );
ret = mbedtls_oid_get_x509_ext_type_supported( &extn_oid, &ext_type, &is_supported );

if( ret != 0 )
if( ( ret != 0 ) || ( is_supported == 0 ) )
{
/* No parser found, skip extension */
*p = end_ext_octet;

#if !defined(MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION)
if( is_critical )
{
/* Do not fail if extension is found, but unsupported and allowed in runtime */
if( ( ret == 0 ) && ( ext_type & crt->allowed_unsupported_critical_exts ) )
continue;

/* Data is marked as critical: fail */
return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS +
MBEDTLS_ERR_ASN1_UNEXPECTED_TAG );
Expand Down Expand Up @@ -1115,6 +1120,7 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, const unsigned char *bu

prev = crt;
mbedtls_x509_crt_init( crt->next );
crt->next->allowed_unsupported_critical_exts = crt->allowed_unsupported_critical_exts;
crt = crt->next;
}

Expand Down
20 changes: 20 additions & 0 deletions tests/data_files/test-ca-nc.crt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
-----BEGIN CERTIFICATE-----
MIIDSzCCAjOgAwIBAgIJAJx/NjT4C4viMA0GCSqGSIb3DQEBCwUAMBMxETAPBgNV
BAMMCExlZXZpQ0E0MB4XDTE4MDEyNzE1MDczMloXDTI4MDEyNTE1MDczMlowEzER
MA8GA1UEAwwITGVldmlDQTQwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIB
AQDWN79RTlyFm5o0LVMSVjc68W0+gtl95xpaaD7IS6gDYjcbGnCwSefiq7y9rtck
OM1A5Bzhj5+iWbmZStUmeJUhSGgxP/FxuUaAV0fsBGJ5jDrzmbhzDkHsNxDMB2ks
XFyy4LfODcBs9TXxY43KUKuq/0meiT3WAaZWHMYle9vkQJM2l0RyH4IXHCHiIRwd
2wntin6T9QOFJOc2ietNb7KsXVne81wb7h9BVMsjCIAsbPpHa+PZQs1xFuxmRxCs
kpavnMy+SqevHhvqtvbHppcXYtZspTnkVoXWUdx3HHXgZMQKlAWlwyx57xpZBU2g
qksO+KCLVYOQMN9usmuMOpHHAgMBAAGjgaEwgZ4wHQYDVR0eAQH/BBMwEaAPMA2C
C2V4YW1wbGUuY29tMB0GA1UdDgQWBBR3T9IilPeRAFfLO8ocg216OBo+6DBDBgNV
HSMEPDA6gBR3T9IilPeRAFfLO8ocg216OBo+6KEXpBUwEzERMA8GA1UEAwwITGVl
dmlDQTSCCQCcfzY0+AuL4jAMBgNVHRMEBTADAQH/MAsGA1UdDwQEAwIBBjANBgkq
hkiG9w0BAQsFAAOCAQEAR086ciNM3ujSQNhhguqFHYGfDRRuAgOk4l7GXIfFa9te
B2KMLSwP367QaMwFxRrOoDvixIjzbpiiKB3cv+IXqGyfsRJw47XLwGK4FtSsXjst
m2M8W5iXBQ94XoLj9OKb4ZJWKI930S/PF7uuxICtWttYSoylfyMkiR45+1SLj2eF
X4EnXK3Q0H42v8LCDFqj9iNQ2WMLwA7kFPB+oOZxkFi2G0F3VuW+JZeBPQCpYdRO
0kQQ/gIZE6KEdscKHi9y6OfGSeRlDBMADky9NiZy7I3AcspLcmMQh/191/DnooNe
OwQ6w1HweApjB46bGyILpGUi9MZhvCnoLWg+cN3/wQ==
-----END CERTIFICATE-----
6 changes: 6 additions & 0 deletions tests/suites/test_suite_x509parse.data
Original file line number Diff line number Diff line change
Expand Up @@ -1810,6 +1810,12 @@ X509 File parse (trailing spaces, OK)
depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA256_C:MBEDTLS_RSA_C
x509parse_crt_file:"data_files/server7_trailing_space.crt":0

X509 File parse (unsupported critical ext Name Constraints, fail)
x509parse_crt_file:"data_files/test-ca-nc.crt":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG

X509 File parse (allowed unsupported critical ext Name Constraints, ok)
x509parse_crt_file_allow_exts:"data_files/test-ca-nc.crt":MBEDTLS_X509_EXT_NAME_CONSTRAINTS:0

X509 Get time (UTC no issues)
depends_on:MBEDTLS_X509_USE_C
x509_get_time:MBEDTLS_ASN1_UTC_TIME:"500101000000Z":0:1950:1:1:0:0:0
Expand Down
15 changes: 15 additions & 0 deletions tests/suites/test_suite_x509parse.function
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,21 @@ exit:
}
/* END_CASE */

/* BEGIN_CASE depends_on:MBEDTLS_X509_CRT_PARSE_C:MBEDTLS_FS_IO */
void x509parse_crt_file_allow_exts( char *crt_file, int exts, int result )
{
mbedtls_x509_crt crt;

mbedtls_x509_crt_init( &crt );
crt.allowed_unsupported_critical_exts = exts;

TEST_ASSERT( mbedtls_x509_crt_parse_file( &crt, crt_file ) == result );

exit:
mbedtls_x509_crt_free( &crt );
}
/* END_CASE */

/* BEGIN_CASE depends_on:MBEDTLS_X509_CRT_PARSE_C */
void x509parse_crt( data_t * buf, char * result_str, int result )
{
Expand Down

0 comments on commit 6071eea

Please sign in to comment.