In the entries (mbedtls_x509_crl_entry values) on the list constructed
by mbedtls_x509_crl_parse_der(), set entry->raw.tag to
(SEQUENCE | CONSTRUCTED) rather than to the tag of the first ASN.1
element of the entry (which happens to be the tag of the serial
number, so INTEGER or INTEGER | CONTEXT_SPECIFIC). This is doesn't
really matter in practice (and in particular the value is never used
in Mbed TLS itself), and isn't documented, but at least it's
consistent with how mbedtls_x509_buf is normally used.
The primary importance of this change is that the old code tried to
access the tag of the first element of the entry even when the entry
happened to be empty. If the entry was empty and not followed by
anything else in the CRL, this could cause a read 1 byte after the end
of the buffer containing the CRL.
The test case "X509 CRL ASN1 (TBSCertList, single empty entry at end)"
hit the problematic buffer overflow, which is detected with ASan.
Credit to OSS-Fuzz for detecting the problem.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Address remaining PR comments for #2118
- Add ChangeLog.d/x509write_csr_heap_alloc.txt.
- Fix parameter alignment per Gille's recommendation.
- Update comments to more explicitly describe the manipulation of buf.
- Replace use of `MBEDTLS_MPI_MAX_SIZE` as `sig` buffer size for
call to `x509write_csr_der_internal()` with more intuitive
`MBEDTLS_PK_SIGNATURE_MAX_SIZE`.
- Update `mbedtls_x509write_csr_der()` to return
`MBEDTLS_ERR_X509_ALLOC_FAILED` on mbedtls_calloc error.
Signed-off-by: Simon Leet <simon.leet@microsoft.com>
- it's 2020, there shouldn't be too many systems out there where SHA-1 is the
only available hash option, so its usefulness is limited
- OTOH testing configurations without SHA-2 reveal bugs that are not easy to
fix in a fully compatible way
So overall, the benefit/cost ratio is not good enough to justify keeping SHA-1
as a fallback option here.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
No effect so far, except on dependency checking, as the feature it's meant to
disable isn't implemented yet (so the descriptions in config.h and the
ChangeLog entry are anticipation for now).
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This commit uses the previously defined macro to uniformize
bounds checks in several places. It also adds bounds checks to
the ClientHello writing function that were previously missing.
Also, the functions adding extensions to the ClientHello message
can now fail if the buffer is too small or a different error
condition occurs, and moreover they take an additional buffer
end parameter to free them from the assumption that one is
writing to the default output buffer.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Convert all text files to Unix line endings unless they're Windows
stuff.
Make sure that all text files have a trailing newline.
Remove whitespace at the end of lines.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
