From dccfd3612dece912f4e0d7c59add685642154fae Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Tue, 23 Jan 2024 17:07:59 +0100 Subject: [PATCH] rsa: update return values of priv/pub parse/write functions The goal is to remove usage of PK return values in order to completely eliminate that dependency. This commit also updates pkparse and test_suite_x509parse to align with this change in return values. Signed-off-by: Valerio Setti --- library/pkparse.c | 3 +- library/rsa.c | 36 ++++++++-------------- tests/suites/test_suite_x509parse.data | 10 +++--- tests/suites/test_suite_x509parse.function | 1 + 4 files changed, 20 insertions(+), 30 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 2708c8c75..17df101f0 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -1556,8 +1556,7 @@ int mbedtls_pk_parse_public_key(mbedtls_pk_context *ctx, return ret; } mbedtls_pk_free(ctx); - if (ret != (MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_INVALID_PUBKEY, - MBEDTLS_ERR_ASN1_UNEXPECTED_TAG))) { + if (ret != MBEDTLS_ERR_ASN1_UNEXPECTED_TAG) { return ret; } #endif /* MBEDTLS_RSA_C */ diff --git a/library/rsa.c b/library/rsa.c index a18c4b1b0..4ff7afacf 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -682,7 +682,7 @@ static int asn1_get_nonzero_mpi(unsigned char **p, } if (mbedtls_mpi_cmp_int(X, 0) == 0) { - return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT; + return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; } return 0; @@ -721,17 +721,17 @@ int mbedtls_rsa_key_parse(mbedtls_rsa_context *rsa, const unsigned char *key, si */ if ((ret = mbedtls_asn1_get_tag(&p, end, &len, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE)) != 0) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); + return ret; } end = p + len; if ((ret = mbedtls_asn1_get_int(&p, end, &version)) != 0) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); + return ret; } if (version != 0) { - return MBEDTLS_ERR_PK_KEY_INVALID_VERSION; + return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; } /* Import N */ @@ -823,8 +823,7 @@ int mbedtls_rsa_key_parse(mbedtls_rsa_context *rsa, const unsigned char *key, si } if (p != end) { - ret = MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, - MBEDTLS_ERR_ASN1_LENGTH_MISMATCH); + ret = MBEDTLS_ERR_ASN1_LENGTH_MISMATCH; } cleanup: @@ -832,13 +831,6 @@ cleanup: mbedtls_mpi_free(&T); if (ret != 0) { - /* Wrap error code if it's coming from a lower level */ - if ((ret & 0xff80) == 0) { - ret = MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret); - } else { - ret = MBEDTLS_ERR_PK_KEY_INVALID_FORMAT; - } - mbedtls_rsa_free(rsa); } @@ -859,46 +851,44 @@ int mbedtls_rsa_pubkey_parse(mbedtls_rsa_context *rsa, unsigned char **p, if ((ret = mbedtls_asn1_get_tag(p, end, &len, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE)) != 0) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_INVALID_PUBKEY, ret); + return ret; } if (*p + len != end) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_INVALID_PUBKEY, - MBEDTLS_ERR_ASN1_LENGTH_MISMATCH); + return MBEDTLS_ERR_ASN1_LENGTH_MISMATCH; } /* Import N */ if ((ret = mbedtls_asn1_get_tag(p, end, &len, MBEDTLS_ASN1_INTEGER)) != 0) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_INVALID_PUBKEY, ret); + return ret; } if ((ret = mbedtls_rsa_import_raw(rsa, *p, len, NULL, 0, NULL, 0, NULL, 0, NULL, 0)) != 0) { - return MBEDTLS_ERR_PK_INVALID_PUBKEY; + return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; } *p += len; /* Import E */ if ((ret = mbedtls_asn1_get_tag(p, end, &len, MBEDTLS_ASN1_INTEGER)) != 0) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_INVALID_PUBKEY, ret); + return ret; } if ((ret = mbedtls_rsa_import_raw(rsa, NULL, 0, NULL, 0, NULL, 0, NULL, 0, *p, len)) != 0) { - return MBEDTLS_ERR_PK_INVALID_PUBKEY; + return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; } *p += len; if (mbedtls_rsa_complete(rsa) != 0 || mbedtls_rsa_check_pubkey(rsa) != 0) { - return MBEDTLS_ERR_PK_INVALID_PUBKEY; + return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; } if (*p != end) { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_INVALID_PUBKEY, - MBEDTLS_ERR_ASN1_LENGTH_MISMATCH); + return MBEDTLS_ERR_ASN1_LENGTH_MISMATCH; } return 0; diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 261c220ee..6e201259c 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1774,15 +1774,15 @@ x509parse_crt:"307d3068a0030201008204deadbeef300d06092a864886f70d01010b0500300c3 X509 CRT ASN1 (TBS, inv SubPubKeyInfo, inv internal bitstring length) depends_on:MBEDTLS_RSA_C:MBEDTLS_MD_CAN_SHA256 -x509parse_crt:"308180306ba0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a300806001304546573743015300d06092A864886F70D0101010500030400300000300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_PK_INVALID_PUBKEY + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH +x509parse_crt:"308180306ba0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a300806001304546573743015300d06092A864886F70D0101010500030400300000300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_ASN1_LENGTH_MISMATCH X509 CRT ASN1 (TBS, inv SubPubKeyInfo, inv internal bitstring tag) depends_on:MBEDTLS_RSA_C:MBEDTLS_MD_CAN_SHA256 -x509parse_crt:"308180306ba0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a300806001304546573743015300d06092A864886F70D0101010500030400310000300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_PK_INVALID_PUBKEY + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG +x509parse_crt:"308180306ba0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a300806001304546573743015300d06092A864886F70D0101010500030400310000300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_ASN1_UNEXPECTED_TAG X509 CRT ASN1 (TBS, inv SubPubKeyInfo, inv RSA modulus) depends_on:MBEDTLS_RSA_C:MBEDTLS_MD_CAN_SHA256 -x509parse_crt:"3081873072a0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374301c300d06092A864886F70D0101010500030b0030080202ffff0302ffff300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_PK_INVALID_PUBKEY + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG +x509parse_crt:"3081873072a0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374301c300d06092A864886F70D0101010500030b0030080202ffff0302ffff300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_ASN1_UNEXPECTED_TAG X509 CRT ASN1 (TBS, inv SubPubKeyInfo, total length mismatch) depends_on:MBEDTLS_RSA_C:MBEDTLS_MD_CAN_SHA256 @@ -1790,11 +1790,11 @@ x509parse_crt:"3081893074a0030201008204deadbeef300d06092a864886f70d01010b0500300 X509 CRT ASN1 (TBS, inv SubPubKeyInfo, check failed) depends_on:MBEDTLS_RSA_C:MBEDTLS_MD_CAN_SHA256 -x509parse_crt:"3081873072a0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374301c300d06092A864886F70D0101010500030b0030080202ffff0202ffff300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_PK_INVALID_PUBKEY +x509parse_crt:"3081873072a0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374301c300d06092A864886F70D0101010500030b0030080202ffff0202ffff300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_RSA_BAD_INPUT_DATA X509 CRT ASN1 (TBS, inv SubPubKeyInfo, check failed, expanded length notation) depends_on:MBEDTLS_RSA_C:MBEDTLS_MD_CAN_SHA256 -x509parse_crt:"308196308180a0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210fffffffffffffffffffffffffffffffe0202ffff300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_PK_INVALID_PUBKEY +x509parse_crt:"308196308180a0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092A864886F70D010101050003190030160210fffffffffffffffffffffffffffffffe0202ffff300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_RSA_BAD_INPUT_DATA # We expect an extension parsing error here because the IssuerID is optional. # Hence, if we find an ASN.1 tag doesn't match the IssuerID, we assume the diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index c2a2f556d..a54c165e1 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -9,6 +9,7 @@ #include "mbedtls/base64.h" #include "mbedtls/error.h" #include "mbedtls/pk.h" +#include "mbedtls/rsa.h" #include "string.h" #if MBEDTLS_X509_MAX_INTERMEDIATE_CA > 19