When parsing a certificate with the basic constraints extension
the max_pathlen that was read from it was incremented regardless
of its value. However, if the max_pathlen is equal to INT_MAX (which
is highly unlikely), an undefined behaviour would occur.
This commit adds a check to ensure that such value is not accepted
as valid. Relevant tests for INT_MAX and INT_MAX-1 are also introduced.
Certificates added in this commit were generated using the
test_suite_x509write, function test_x509_crt_check. Input data taken
from the "Certificate write check Server1 SHA1" test case, so the generated
files are like the "server1.crt", but with the "is_ca" field set to 1 and
max_pathlen as described by the file name.
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
Enable MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 in the full config. There's
no reason to keep it out. We weren't testing it at all on the CI.
Add a missing dependency on !MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 to
some test cases that either were testing that v3 extensions are only
accepted in v3 certificates, or where parsing returns a different
error when MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 is enabled.
Add a few positive and negative test cases with
MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 enabled.
Fix one test case with MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 where the
intended output of mbedtls_x509_crt_info had changed in
890819a597 but the test case was missed
because it was never executed.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Add dependencies on !MBEDTLS_SHA512_NO_SHA384 to X.509 and SSL unit
tests that use SHA-384 (identified based on having a description that
contains "SHA384" or "SHA-384").
This commit adds multiple test cases to the X.509 CRT parsing test suite
exercising the stack's behaviour when facing CertificatePolicy extensions
that are malformed for a variety of reasons. It follows the same scheme
as in other negative parsing tests: For each ASN.1 component, have test
cases for (a) unexpected tag, (b) missing length, (c) invalid length
encoding, (d) length out of bounds.
This commit modifies the test
X509 CRT ASN1 (TBSCertificate v3, inv CertificatePolicies, data missing)
which exercises the behaviour of the X.509 CRT parser when facing a
CertificatePolicy extension with empty data field.
The following adaptations are made:
- The subject ID and issuer ID are modified to have length 0.
The previous values `aa` and `bb` are OK, but a generic ASN.1
parser will try to interpret them as ASN.1 tags and fail. For
maintainability, it's therefore better to use something that
can be parsed as ASN.1, and an empty ID is the easiest solution
here.
- The TBS part of the certificate wasn't followed by signature
algorithm and signature fields, which makes the test incompatible
with future changes swapping to breadth-first parsing of
certificates.
This commit moves the X.509 negative parsing tests for the
CertificatePolicy extension to the place where negative
testing of other extensions happens.
Judging from its name, the purpose of the test
TBSCertificate v3, ext CertificatePolicies tag, bool len missing
in test_suite_x509parse.data is to exercise the X.509 parsing stack's
behaviour when parsing a CertificatePolicy extension which lacks the
length field of the boolean 'Criticality' value.
However, the test fails at an earlier stage due to a mismatch of inner
and outer length of the explicit ASN.1 extensions structure.
Since we already have tests exercising
- mismatch of inner and outer length in the extensions structure, namely
'X509 CRT ASN1 (TBS, inv v3Ext, inner tag invalid)'
- missing length of the 'Criticality' field in an extension, namely
'X509 CRT ASN1 (TBS, inv v3Ext, critical length missing)'
and since for both tests there's no relevance to the use of the
policy extension OID, the test
'TBSCertificate v3, ext CertificatePolicies tag, bool len missing'
can be dropped.
Tweak test data for one test case to not rely on mbedtls_asn1_get_int
lacking support for leading zeros. Instead, use a number that is
actually out of range for int.
Tweak test data for one test case to not rely on
mbedtls_asn1_get_bitstring_null rejecting bitstrings shorter than two
octets. Instead, try bit strings that are genuinely invalid, or have a
nonzero number of unused bits.
Add a test case with a correct empty signature. This is commented out
because asn1parse currently does not support this. Uncomment it when
asn1parse is updated to support this.
* origin/pr/2443: (25 commits)
Fix documentation of X.509 parsing test
Add X.509 CRT parsing test for mixed time-encodings
Improve X.509 CRT parsing test names
Add negative X.509 parsing tests for v3Ext in v1/v2 CRT
Add negative X.509 parsing tests for IssuerID/SubjectID in v1 CRT
Improve name of X.509 CRT parsing test
Always use the same X.509 alg structure inside and outside of TBS
Fix test dependencies in X.509 CRT parsing suite
Fix non-DER length encoding in two X.509 CRT parsing tests
Fix test case name formatting in X.509 parsing suite
Use ASN.1 NULL TLVs when testing invalid tags
Shorten X.509 CRT parsing test names
Extend negative testing for X.509 Signature parsing
Extend negative testing for X.509 SignatureAlgorithm parsing
Extend negative testing for X.509 v3 Extension parsing
Extend negative testing for X.509 SubjectID parsing
Extend negative testing for X.509 IssuerID parsing
Extend negative testing for X.509 SubjectPublicKeyInfo parsing
Extend negative testing for X.509 Subject parsing
Extend negative testing for X.509 Validity parsing
...
* origin/pr/2430:
Document support for MD2 and MD4 in programs/x509/cert_write
Correct name of X.509 parsing test for well-formed, ill-signed CRT
Add test cases exercising successful verification of MD2/MD4/MD5 CRT
Add test case exercising verification of valid MD2 CRT
Add MD[245] test CRTs to tree
Add instructions for MD[245] test CRTs to tests/data_files/Makefile
Add suppport for MD2 to CSR and CRT writing example programs
Remove use of MD2 in further x509parse tests
Convert further x509parse tests to use lower-case hex data
Correct placement of ChangeLog entry
Adapt ChangeLog
Use SHA-256 instead of MD2 in X.509 CRT parsing tests
Consistently use lower case hex data in X.509 parsing tests
Lengths below 128 Bytes must be encoded as a single 'XX' byte in DER,
but two tests in the X.509 CRT parsing suite used the BER but non-DER
encoding '81 XX' (the first byte 10000001 indicating that the length
is to follow (high bit) and has length 1 byte (low bit)).
Previously, a test exercising the X.509 CRT parser's behaviour
on unexpected tags would use a '00' byte in place of the tag
for the expected structure. This makes reviewing the examples
harder because the binary data isn't valid DER-encoded ASN.1.
This commit uses the ASN.1 NULL TLV '05 00' to test invalid
tags, and adapts surrounding structures' length values accordingly.
This eases reviewing because now the ASN.1 structures are still
well-formed at the place where the mismatch occurs.
Some functions within the X.509 module return an ASN.1 low level
error code where instead this error code should be wrapped by a
high-level X.509 error code as in the bulk of the module.
Specifically, the following functions are affected:
- mbedtls_x509_get_ext()
- x509_get_version()
- x509_get_uid()
This commit modifies these functions to always return an
X.509 high level error code.
Care has to be taken when adapting `mbetls_x509_get_ext()`:
Currently, the callers `mbedtls_x509_crt_ext()` treat the
return code `MBEDTLS_ERR_ASN1_UNEXPECTED_TAG` specially to
gracefully detect and continue if the extension structure is not
present. Wrapping the ASN.1 error with
`MBEDTLS_ERR_X509_INVALID_EXTENSIONS` and adapting the check
accordingly would mean that an unexpected tag somewhere
down the extension parsing would be ignored by the caller.
The way out of this is the following: Luckily, the extension
structure is always the last field in the surrounding structure,
so if there is some data remaining, it must be an Extension
structure, so we don't need to deal with a tag mismatch gracefully
in the first place.
We may therefore wrap the return code from the initial call to
`mbedtls_asn1_get_tag()` in `mbedtls_x509_get_ext()` by
`MBEDTLS_ERR_X509_INVALID_EXTENSIONS` and simply remove
the special treatment of `MBEDTLS_ERR_ASN1_UNEXPECTED_TAG`
in the callers `x509_crl_get_ext()` and `x509_crt_get_ext()`.
This renders `mbedtls_x509_get_ext()` unsuitable if it ever
happened that an Extension structure is optional and does not
occur at the end of its surrounding structure, but for CRTs
and CRLs, it's fine.
The following tests need to be adapted:
- "TBSCertificate v3, issuerID wrong tag"
The issuerID is optional, so if we look for its presence
but find a different tag, we silently continue and try
parsing the subjectID, and then the extensions. The tag '00'
used in this test doesn't match either of these, and the
previous code would hence return LENGTH_MISMATCH after
unsucessfully trying issuerID, subjectID and Extensions.
With the new code, any data remaining after issuerID and
subjectID _must_ be Extension data, so we fail with
UNEXPECTED_TAG when trying to parse the Extension data.
- "TBSCertificate v3, UIDs, invalid length"
The test hardcodes the expectation of
MBEDTLS_ERR_ASN1_INVALID_LENGTH, which needs to be
wrapped in MBEDTLS_ERR_X509_INVALID_FORMAT now.
Fixes#2431.
When parsing a substructure of an ASN.1 structure, no field within
the substructure must exceed the bounds of the substructure.
Concretely, the `end` pointer passed to the ASN.1 parsing routines
must be updated to point to the end of the substructure while parsing
the latter.
This was previously not the case for the routines
- x509_get_attr_type_and_value(),
- mbedtls_x509_get_crt_ext(),
- mbedtls_x509_get_crl_ext().
These functions kept using the end of the parent structure as the
`end` pointer and would hence allow substructure fields to cross
the substructure boundary. This could lead to successful parsing
of ill-formed X.509 CRTs.
This commit fixes this.
Care has to be taken when adapting `mbedtls_x509_get_crt_ext()`
and `mbedtls_x509_get_crl_ext()`, as the underlying function
`mbedtls_x509_get_ext()` returns `0` if no extensions are present
but doesn't set the variable which holds the bounds of the Extensions
structure in case the latter is present. This commit addresses
this by returning early from `mbedtls_x509_get_crt_ext()` and
`mbedtls_x509_get_crl_ext()` if parsing has reached the end of
the input buffer.
The following X.509 parsing tests need to be adapted:
- "TBSCertificate, issuer two inner set datas"
This test exercises the X.509 CRT parser with a Subject name
which has two empty `AttributeTypeAndValue` structures.
This is supposed to fail with `MBEDTLS_ERR_ASN1_OUT_OF_DATA`
because the parser should attempt to parse the first structure
and fail because of a lack of data. Previously, it failed to
obey the (0-length) bounds of the first AttributeTypeAndValue
structure and would try to interpret the beginning of the second
AttributeTypeAndValue structure as the first field of the first
AttributeTypeAndValue structure, returning an UNEXPECTED_TAG error.
- "TBSCertificate, issuer, no full following string"
This test exercises the parser's behaviour on an AttributeTypeAndValue
structure which contains more data than expected; it should therefore
fail with MBEDTLS_ERR_ASN1_LENGTH_MISMATCH. Because of the missing bounds
check, it previously failed with UNEXPECTED_TAG because it interpreted
the remaining byte in the first AttributeTypeAndValue structure as the
first byte in the second AttributeTypeAndValue structure.
- "SubjectAltName repeated"
This test should exercise two SubjectAltNames extensions in succession,
but a wrong length values makes the second SubjectAltNames extension appear
outside of the Extensions structure. With the new bounds in place, this
therefore fails with a LENGTH_MISMATCH error. This commit adapts the test
data to put the 2nd SubjectAltNames extension inside the Extensions
structure, too.
