Correcting documentation issues:
- Changelog entry is Feature instead of API Change
- Correcting whitespaces around braces
- Also adding defensive mechanism to x509_get_subject_key_id
to avoid malfunction in case of trailing garbage
Signed-off-by: toth92g <toth92g@gmail.com>
diff --git a/ChangeLog.d/X509Parse_SignatureKeyId_AuthorityKeyId.txt b/ChangeLog.d/X509Parse_SignatureKeyId_AuthorityKeyId.txt
index cf4c9e9..9aa3ff9 100644
--- a/ChangeLog.d/X509Parse_SignatureKeyId_AuthorityKeyId.txt
+++ b/ChangeLog.d/X509Parse_SignatureKeyId_AuthorityKeyId.txt
@@ -1,2 +1,3 @@
-API changes
- * x509 certificate parse functionality is extended with the possibility of extracting SignatureKeyId and AuthorityKeyId fields
+Features
+ * When parsing X.509 certificates, support the extensions
+ SignatureKeyIdentifier and AuthorityKeyIdentifier.
diff --git a/library/x509_crt.c b/library/x509_crt.c
index 5395f45..5a356af 100644
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@@ -624,8 +624,8 @@
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
size_t len = 0u;
- if ( (ret = mbedtls_asn1_get_tag(p, end, &len,
- MBEDTLS_ASN1_OCTET_STRING)) != 0 )
+ if ( ( ret = mbedtls_asn1_get_tag( p, end, &len,
+ MBEDTLS_ASN1_OCTET_STRING ) ) != 0 )
{
return( ret );
}
@@ -636,6 +636,10 @@
subject_key_id->p = *p;
*p += len;
}
+
+ if( *p != end )
+ return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS +
+ MBEDTLS_ERR_ASN1_LENGTH_MISMATCH );
return( 0 );
}
@@ -655,14 +659,14 @@
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
size_t len = 0u;
- if ( (ret = mbedtls_asn1_get_tag(p, end, &len,
- MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE)) != 0 )
+ if ( ( ret = mbedtls_asn1_get_tag( p, end, &len,
+ MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 )
{
return( ret );
}
- if ( (ret = mbedtls_asn1_get_tag(p, end, &len,
- MBEDTLS_ASN1_CONTEXT_SPECIFIC)) != 0 )
+ if ( (ret = mbedtls_asn1_get_tag( p, end, &len,
+ MBEDTLS_ASN1_CONTEXT_SPECIFIC ) ) != 0 )
{
/* KeyIdentifier is an OPTIONAL field */
}
@@ -677,15 +681,15 @@
if ( *p < end )
{
- if ( (ret = mbedtls_asn1_get_tag(p, end, &len,
- MBEDTLS_ASN1_CONTEXT_SPECIFIC | MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_BOOLEAN)) != 0 )
+ if ( ( ret = mbedtls_asn1_get_tag( p, end, &len,
+ MBEDTLS_ASN1_CONTEXT_SPECIFIC | MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_BOOLEAN ) ) != 0 )
{
/* authorityCertIssuer is an OPTIONAL field */
}
else
{
- if ( (ret = mbedtls_asn1_get_tag(p, end, &len,
- MBEDTLS_ASN1_CONTEXT_SPECIFIC | MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_OCTET_STRING)) != 0 )
+ if ( ( ret = mbedtls_asn1_get_tag( p, end, &len,
+ MBEDTLS_ASN1_CONTEXT_SPECIFIC | MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_OCTET_STRING ) ) != 0 )
{
return( ret );
}
@@ -693,13 +697,13 @@
{
authority_key_id->raw.p = *p;
- if ( (ret = mbedtls_asn1_get_tag(p, end, &len,
- MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE)) != 0 )
+ if ( ( ret = mbedtls_asn1_get_tag( p, end, &len,
+ MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 )
{
return( ret );
}
- if ( (ret = mbedtls_x509_get_name(p, *p + len, &authority_key_id->authorityCertIssuer)) != 0 )
+ if ( ( ret = mbedtls_x509_get_name( p, *p + len, &authority_key_id->authorityCertIssuer ) ) != 0 )
{
return( ret );
}
@@ -711,8 +715,8 @@
if ( *p < end )
{
- if ( (ret = mbedtls_asn1_get_tag(p, end, &len,
- MBEDTLS_ASN1_CONTEXT_SPECIFIC | MBEDTLS_ASN1_INTEGER)) != 0 )
+ if ( ( ret = mbedtls_asn1_get_tag( p, end, &len,
+ MBEDTLS_ASN1_CONTEXT_SPECIFIC | MBEDTLS_ASN1_INTEGER ) ) != 0 )
{
/* authorityCertSerialNumber is an OPTIONAL field, but if there are still data it must be the serial number */
return( ret );
diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function
index 8408d88..3a56adf 100644
--- a/tests/suites/test_suite_x509parse.function
+++ b/tests/suites/test_suite_x509parse.function
@@ -1414,16 +1414,16 @@
TEST_ASSERT( mbedtls_x509_crt_parse_file( &crt, crt_path ) == retVal );
- if(retVal != 0)
+ if( retVal != 0 )
{
- while(i < subjectKeyIdLength)
+ while( i < subjectKeyIdLength )
{
result |= crt.subject_key_id.p[i] != subjectKeyId[i*2];
result |= crt.subject_key_id.p[i+1] != subjectKeyId[i*2+1];
i++;
}
- TEST_ASSERT(result == 0);
+ TEST_ASSERT( result == 0 );
}
exit:
@@ -1444,10 +1444,10 @@
TEST_ASSERT( mbedtls_x509_crt_parse_file( &crt, crt_path ) == retVal );
- if (retVal != 0)
+ if ( retVal != 0 )
{
/* KeyId test */
- while(i < keyIdLength)
+ while( i < keyIdLength )
{
result |= crt.authority_key_id.keyIdentifier.p[i] != authorityKeyId_keyId[i*2];
result |= crt.authority_key_id.keyIdentifier.p[i+1] != authorityKeyId_keyId[i*2+1];
@@ -1456,11 +1456,11 @@
/* Issuer test */
mbedtls_x509_name* issuerPtr = &crt.authority_key_id.authorityCertIssuer;
- while (issuerPtr != NULL)
+ while ( issuerPtr != NULL )
{
- for (issuerCounter = 0u; issuerCounter < issuerPtr->val.len; issuerCounter++)
+ for ( issuerCounter = 0u; issuerCounter < issuerPtr->val.len; issuerCounter++ )
{
- result |= (authorityKeyId_issuer[bufferCounter++] != issuerPtr->val.p[issuerCounter]);
+ result |= ( authorityKeyId_issuer[bufferCounter++] != issuerPtr->val.p[issuerCounter] );
}
bufferCounter++; /* Skipping the slash */
issuerPtr = issuerPtr->next;
@@ -1468,14 +1468,14 @@
/* Serial test */
i = 0;
- while(i < serialLength)
+ while( i < serialLength )
{
result |= crt.authority_key_id.authorityCertSerialNumber.p[i] != authorityKeyId_serial[i*2];
result |= crt.authority_key_id.authorityCertSerialNumber.p[i+1] != authorityKeyId_serial[i*2+1];
i++;
}
- TEST_ASSERT(result == 0);
+ TEST_ASSERT( result == 0 );
}
exit: