mirror/mbedtls
1
0
Fork 0
mirror of https://github.com/Mbed-TLS/mbedtls.git synced 2025-03-26 02:37:08 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

parse_attribute_value_hex_der_encoded: clean up length validation

Browse Source 
Separate the fits-in-buffer check (*data_length <= data_size) from the
we-think-it's-a-sensible-size check (*data_length <=
MBEDTLS_X509_MAX_DN_NAME_SIZE).

This requires using an intermediate buffer for the DER data, since its
maximum sensible size has to be larger than the maximum sensible size for
the payload, due to the overhead of the ASN.1 tag+length.

Remove test cases focusing on the DER length since the implementation no
longer has a threshold for it.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This commit is contained in:
Gilles Peskine 2023-09-21 18:13:17 +02:00
parent 26dd764dc3
commit 7f420faf03
2 changed files with 53 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

83
library/x509_create.c
View File

@ -189,11 +189,12 @@ static int parse_attribute_value_string(const char *s,
* *
* \param s A string of \p len bytes hexadecimal digits. * \param s A string of \p len bytes hexadecimal digits.
* \param len Number of bytes to read from \p s. * \param len Number of bytes to read from \p s.
* \param data Output buffer of size #MBEDTLS_X509_MAX_DN_NAME_SIZE. * \param data Output buffer of size \p data_size.
* On success, it contains the payload that's DER-encoded * On success, it contains the payload that's DER-encoded
* in the input (content without the tag and length). * in the input (content without the tag and length).
* If the DER tag is a string tag, the payload is guaranteed * If the DER tag is a string tag, the payload is guaranteed
* not to contain null bytes. * not to contain null bytes.
* \param data_size Length of the \p data buffer.
* \param data_len On success, the length of the parsed string. * \param data_len On success, the length of the parsed string.
* It is guaranteed to be less than * It is guaranteed to be less than
* #MBEDTLS_X509_MAX_DN_NAME_SIZE. * #MBEDTLS_X509_MAX_DN_NAME_SIZE.
@ -203,68 +204,88 @@ static int parse_attribute_value_string(const char *s,
* \retval #MBEDTLS_ERR_X509_INVALID_NAME if \p s does not contain * \retval #MBEDTLS_ERR_X509_INVALID_NAME if \p s does not contain
* a valid hexstring, * a valid hexstring,
* or if the decoded hexstring is not valid DER, * or if the decoded hexstring is not valid DER,
* or if the decoded hexstring does not fit in \p data, * or if the payload does not fit in \p data,
* or if the payload is more than
* #MBEDTLS_X509_MAX_DN_NAME_SIZE bytes,
* of if \p *tag is an ASN.1 string tag and the payload * of if \p *tag is an ASN.1 string tag and the payload
* contains a null byte. * contains a null byte.
* \retval #MBEDTLS_ERR_X509_ALLOC_FAILED on low memory.
*/ */
static int parse_attribute_value_hex_der_encoded(const char *s, static int parse_attribute_value_hex_der_encoded(const char *s,
size_t len, size_t len,
unsigned char *data, unsigned char *data,
size_t data_size,
size_t *data_len, size_t *data_len,
int *tag) int *tag)
{ {
/* Step 1: Decode the hex string. /* Step 1: preliminary length checks. */
* We use data as an intermediate buffer. This limits the ultimate payload
* to slightly less than MBEDTLS_X509_MAX_DN_NAME_SIZE bytes due to the
* overhead of the DER tag+length. */
/* Each byte is encoded by exactly two hexadecimal digits. */ /* Each byte is encoded by exactly two hexadecimal digits. */
if (len % 2 != 0) { if (len % 2 != 0) {
/* Odd number of hex digits */ /* Odd number of hex digits */
return MBEDTLS_ERR_X509_INVALID_NAME; return MBEDTLS_ERR_X509_INVALID_NAME;
} }
size_t const der_length = len / 2; size_t const der_length = len / 2;
/* Here we treat MBEDTLS_X509_MAX_DN_NAME_SIZE as the maximum length of if (der_length > MBEDTLS_X509_MAX_DN_NAME_SIZE + 4) {
* the DER encoding. This is convenient, but seems wrong: should it /* The payload would be more than MBEDTLS_X509_MAX_DN_NAME_SIZE
* be the length of the payload (which would require a few more bytes * (after subtracting the ASN.1 tag and length). Reject this early
* in the intermediate buffer)? In practice the hex-encoded data is * to avoid allocating a large intermediate buffer. */
* expected to be much shorter anyway. */
if (der_length > MBEDTLS_X509_MAX_DN_NAME_SIZE) {
/* Not enough room in data */
return MBEDTLS_ERR_X509_INVALID_NAME; return MBEDTLS_ERR_X509_INVALID_NAME;
} }
if (der_length < 1) {
/* Avoid empty-buffer shenanigans. A valid DER encoding is never
* empty. */
return MBEDTLS_ERR_X509_INVALID_NAME;
}
/* Step 2: Decode the hex string into an intermediate buffer. */
unsigned char *der = mbedtls_calloc(1, der_length);
if (der == NULL) {
return MBEDTLS_ERR_X509_ALLOC_FAILED;
}
/* Beyond this point, der needs to be freed on exit. */
for (size_t i = 0; i < der_length; i++) { for (size_t i = 0; i < der_length; i++) {
int c = hexpair_to_int(s + 2 * i); int c = hexpair_to_int(s + 2 * i);
if (c < 0) { if (c < 0) {
return MBEDTLS_ERR_X509_INVALID_NAME; goto error;
} }
data[i] = c; der[i] = c;
} }
/* Step 2: decode the DER. */ /* Step 3: decode the DER. */
if (der_length < 1) { /* We've checked that der_length >= 1 above. */
return MBEDTLS_ERR_X509_INVALID_NAME; *tag = der[0];
unsigned char *p = der + 1;
if (mbedtls_asn1_get_len(&p, der + der_length, data_len) != 0) {
goto error;
} }
*tag = data[0]; /* Now p points to the first byte of the payload inside der,
unsigned char *p = data + 1; * and *data_len is the length of the payload. */
if (mbedtls_asn1_get_len(&p, data + der_length, data_len) != 0) {
return MBEDTLS_ERR_X509_INVALID_NAME;
}
/* Step 3: extract the payload. */
/* Now p points to the first byte of the payload inside data.
* Shift the content of data to move the payload to the beginning. */
memmove(data, p, *data_len);
/* Step 4: payload validation */ /* Step 4: payload validation */
if (*data_len > MBEDTLS_X509_MAX_DN_NAME_SIZE) {
goto error;
}
/* Strings must not contain null bytes. */
if (MBEDTLS_ASN1_IS_STRING_TAG(*tag)) { if (MBEDTLS_ASN1_IS_STRING_TAG(*tag)) {
for (size_t i = 0; i < *data_len; i++) { for (size_t i = 0; i < *data_len; i++) {
if (data[i] == 0) { if (p[i] == 0) {
return MBEDTLS_ERR_X509_INVALID_NAME; goto error;
} }
} }
} }
/* Step 5: output the payload. */
if (*data_len > data_size) {
goto error;
}
memcpy(data, p, *data_len);
mbedtls_free(der);
return 0; return 0;
error:
mbedtls_free(der);
return MBEDTLS_ERR_X509_INVALID_NAME;
} }
int mbedtls_x509_string_to_names(mbedtls_asn1_named_data **head, const char *name) int mbedtls_x509_string_to_names(mbedtls_asn1_named_data **head, const char *name)
@ -312,7 +333,7 @@ int mbedtls_x509_string_to_names(mbedtls_asn1_named_data **head, const char *nam
* else branch), hence c - s - 1 >= 0. */ * else branch), hence c - s - 1 >= 0. */
parse_ret = parse_attribute_value_hex_der_encoded( parse_ret = parse_attribute_value_hex_der_encoded(
s + 1, c - s - 1, s + 1, c - s - 1,
data, &data_len, &tag); data, sizeof(data), &data_len, &tag);
if (parse_ret != 0) { if (parse_ret != 0) {
mbedtls_free(oid.p); mbedtls_free(oid.p);
return MBEDTLS_ERR_X509_INVALID_NAME; return MBEDTLS_ERR_X509_INVALID_NAME;

8
tests/suites/test_suite_x509write.data
View File

@ -212,14 +212,8 @@ mbedtls_x509_string_to_names:"C=NL, 2.5.4.10=#0C084F6666737061726B, OU=PolarSSL"
X509 String to Names (hexstring: trailing garbage after DER is ignored) X509 String to Names (hexstring: trailing garbage after DER is ignored)
mbedtls_x509_string_to_names:"C=NL, 2.5.4.10=#0C084F6666737061726Baa, OU=PolarSSL":"C=NL, O=Offspark, OU=PolarSSL":0:0 mbedtls_x509_string_to_names:"C=NL, 2.5.4.10=#0C084F6666737061726Baa, OU=PolarSSL":"C=NL, O=Offspark, OU=PolarSSL":0:0
X509 String to Names: long hexstring (DER=256 bytes)
mbedtls_x509_string_to_names:"C=NL, 2.5.4.10=#0C81fd41414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141, OU=PolarSSL":"C=NL, O=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA, OU=PolarSSL":0:0
X509 String to Names: long hexstring (DER=257 bytes)
mbedtls_x509_string_to_names:"C=NL, 2.5.4.10=#0C81fe4141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141, OU=PolarSSL":"C=NL, O=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA, OU=PolarSSL":MBEDTLS_ERR_X509_INVALID_NAME:0
X509 String to Names: long hexstring (payload=256 bytes) X509 String to Names: long hexstring (payload=256 bytes)
mbedtls_x509_string_to_names:"C=NL, 2.5.4.10=#0C82010041414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141, OU=PolarSSL":"C=NL, O=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA, OU=PolarSSL":MBEDTLS_ERR_X509_INVALID_NAME:0 mbedtls_x509_string_to_names:"C=NL, 2.5.4.10=#0C82010041414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141, OU=PolarSSL":"C=NL, O=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA, OU=PolarSSL":0:MAY_FAIL_DN_GETS
X509 String to Names: long hexstring (payload=257 bytes) X509 String to Names: long hexstring (payload=257 bytes)
mbedtls_x509_string_to_names:"C=NL, 2.5.4.10=#0C820101aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, OU=PolarSSL":"C=NL, O=Offspark, OU=PolarSSL":MBEDTLS_ERR_X509_INVALID_NAME:0 mbedtls_x509_string_to_names:"C=NL, 2.5.4.10=#0C820101aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, OU=PolarSSL":"C=NL, O=Offspark, OU=PolarSSL":MBEDTLS_ERR_X509_INVALID_NAME:0