ASN.1 parsing: don't change *p on an unexpected tag #314

New issue

Open

opened 2025-06-12 09:12:24 -05:00 by gilles-peskine-arm · 1 comment

gilles-peskine-arm commented

2025-06-12 09:12:24 -05:00

(Migrated from github.com)

As requested by @DemiMarie in https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/184/files#r2138296230:

If the return value is MBEDTLS_ERR_ASN1_UNEXPECTED_TAG, *p should be guaranteed to be unchanged. This is needed so that parsing of OPTIONAL and CHOICE types works. Existing functions that read a single ASN.1 tag also provide this guarantee and callers do rely on it, but this is not documented.

[I fixed the spelling of the error code.]

Note that even if the current function work this way, this is explicitly not guaranteed by the documentation, and it's untested.

In addition to MBEDTLS_ERR_ASN1_UNEXPECTED_TAG, we should also guarantee that *p is unchanged if it was already at the end of the data, to simplify the handling of omitted OPTIONAL. (Note that this is a very specific case: it's currently not true that *p is unchanged in other cases that trigger MBEDTLS_ERR_ASN1_OUT_OF_DATA, and those other cases wouldn't be particularly useful, since they're structurally invalid inputs. This case is just for when mbedtls_asn1_get_len returns MBEDTLS_ERR_ASN1_OUT_OF_DATA because (end - *p) < 1 on entry.)

The goal of this task is:

Document this property.
Add tests enforcing this guarantee.
Preferably backport to 3.6. If existing applications are relying on this undocumented property, we don't promise not to break it, but we should try to be nice.

As requested by @DemiMarie in https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/184/files#r2138296230: > If the return value is `MBEDTLS_ERR_ASN1_UNEXPECTED_TAG`, `*p` should be guaranteed to be unchanged. This is needed so that parsing of OPTIONAL and CHOICE types works. Existing functions that read a single ASN.1 tag also provide this guarantee and callers do rely on it, but this is not documented. [I fixed the spelling of the error code.] Note that even if the current function work this way, this is explicitly _not guaranteed_ by the documentation, and it's untested. In addition to `MBEDTLS_ERR_ASN1_UNEXPECTED_TAG`, we should also guarantee that `*p` is unchanged if it was already at the end of the data, to simplify the handling of omitted OPTIONAL. (Note that this is a very specific case: it's currently not true that `*p` is unchanged in other cases that trigger `MBEDTLS_ERR_ASN1_OUT_OF_DATA`, and those other cases wouldn't be particularly useful, since they're structurally invalid inputs. This case is just for when `mbedtls_asn1_get_len` returns `MBEDTLS_ERR_ASN1_OUT_OF_DATA` because `(end - *p) < 1` on entry.) The goal of this task is: * Document this property. * Add tests enforcing this guarantee. * Preferably backport to 3.6. If existing applications are relying on this undocumented property, we don't promise not to break it, but we should try to be nice.

DemiMarie commented

2025-06-12 10:02:12 -05:00

(Migrated from github.com)

There is real-world code (including TF-A I believe) that does rely on this.

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-Mbed-TLS/TF-PSA-Crypto#314

No description provided.

Rows
Columns