X.509 CRT parser returns ASN.1 error code on empty TBSCertificate #2431
Labels
Comments
hanno-becker
pushed a commit
to hanno-becker/mbedtls
that referenced
this issue
Feb 12, 2019
x509_get_version() returns an ASN.1 low level error code when the attempt to parse the version tag fails with an error different from #MBEDTLS_ERR_ASN1_UNEXPECTED_TAG. This error code should be wrapped by a high-level X.509 error code as in the rest of the module. Fixes Mbed-TLS#2431.
|
ARM Internal Ref: IOTSSL-2781 |
|
This also applies to |
|
This also applies to |
hanno-becker
pushed a commit
to hanno-becker/mbedtls
that referenced
this issue
Feb 14, 2019
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 Mbed-TLS#2431.
hanno-becker
pushed a commit
to hanno-becker/mbedtls
that referenced
this issue
Mar 1, 2019
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 Mbed-TLS#2431.
hanno-becker
pushed a commit
to hanno-becker/mbedtls
that referenced
this issue
Mar 1, 2019
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 Mbed-TLS#2431.
hanno-becker
pushed a commit
to hanno-becker/mbedtls
that referenced
this issue
Mar 5, 2019
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 Mbed-TLS#2431.
hanno-becker
pushed a commit
to hanno-becker/mbedtls
that referenced
this issue
Mar 5, 2019
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 Mbed-TLS#2431.
hanno-becker
pushed a commit
to hanno-becker/mbedtls
that referenced
this issue
Apr 25, 2019
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 Mbed-TLS#2431.
hanno-becker
pushed a commit
to hanno-becker/mbedtls
that referenced
this issue
May 2, 2019
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 Mbed-TLS#2431.
hanno-becker
pushed a commit
to hanno-becker/mbedtls
that referenced
this issue
May 13, 2019
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 Mbed-TLS#2431.
hanno-becker
pushed a commit
to hanno-becker/mbedtls
that referenced
this issue
Jun 4, 2019
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 Mbed-TLS#2431.
hanno-becker
pushed a commit
to hanno-becker/mbedtls
that referenced
this issue
Jun 4, 2019
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 Mbed-TLS#2431.
hanno-becker
pushed a commit
to hanno-becker/mbedtls
that referenced
this issue
Jun 4, 2019
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 Mbed-TLS#2431.
hanno-becker
pushed a commit
to hanno-becker/mbedtls
that referenced
this issue
Jun 4, 2019
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 Mbed-TLS#2431.
hanno-becker
pushed a commit
to hanno-becker/mbedtls
that referenced
this issue
Jun 4, 2019
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 Mbed-TLS#2431.
hanno-becker
pushed a commit
to hanno-becker/mbedtls
that referenced
this issue
Jun 4, 2019
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 Mbed-TLS#2431.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Context: The first field in a
TBSCertificateis the version which is parsed throughx509_get_version():https://github.com/ARMmbed/mbedtls/blob/f352f75f6bd5734c8f671323dd6ab32472d5da34/library/x509_crt.c#L380-L400
If this function finds an empty buffer, the call to
mbedtls_asn1_get_tag()will returnMBEDTLS_ERR_ASN1_OUT_OF_DATA, which is then returned unmodified to the caller.Issue: This is not in line with the rest of the module, which wraps low-level ASN.1 error code with high-level X.509 error codes before returning.
Task: Make sure
x509_crt_parse_der_core()always returns an X.509 error code. Add a test exercising an emptyTBSCertificatebody.The text was updated successfully, but these errors were encountered: