Skip to content

Commit

Permalink
Add length check during verify cert chain
Browse files Browse the repository at this point in the history
Resolve #2701
  • Loading branch information
rw8896 committed Jun 3, 2024
1 parent ebb408e commit bdc82b8
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 1 deletion.
18 changes: 18 additions & 0 deletions library/spdm_crypt_lib/libspdm_crypt_cert.c
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,9 @@ bool libspdm_x509_common_certificate_check(const uint8_t *cert, size_t cert_size
size_t cert_version;
size_t value;
void *context;
const uint8_t *tmp_cert;
size_t tmp_cert_size;


if (cert == NULL || cert_size == 0) {
return false;
Expand All @@ -1061,6 +1064,21 @@ bool libspdm_x509_common_certificate_check(const uint8_t *cert, size_t cert_size
end_cert_from_len = 64;
end_cert_to_len = 64;

/* 0. check certificate size */
status = libspdm_x509_get_cert_from_cert_chain(cert, cert_size, 0, &tmp_cert, &tmp_cert_size);
if (!status) {
LIBSPDM_DEBUG((LIBSPDM_DEBUG_INFO,
"!!! CommonCertificateCheck - FAIL (get certificate failed)!!!\n"));
goto cleanup;
}
if (tmp_cert_size != cert_size) {
LIBSPDM_DEBUG((LIBSPDM_DEBUG_INFO,
"!!! CommonCertificateCheck - FAIL (size %d != expectd size %d)!!!\n",
cert_size, tmp_cert_size));
status = false;
goto cleanup;
}

/* 1. version*/
cert_version = 0;
status = libspdm_x509_get_version(cert, cert_size, &cert_version);
Expand Down
7 changes: 7 additions & 0 deletions os_stub/cryptlib_mbedtls/pk/x509.c
Original file line number Diff line number Diff line change
Expand Up @@ -712,11 +712,18 @@ bool libspdm_x509_verify_cert_chain(const uint8_t *root_cert, size_t root_cert_l
&tmp_ptr, cert_chain + cert_chain_length, &asn1_len,
MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE);
if (ret != 0) {
if (current_cert < cert_chain + cert_chain_length)
verify_flag = false;
break;
}

current_cert_len = asn1_len + (tmp_ptr - current_cert);

if (current_cert + current_cert_len > cert_chain + cert_chain_length) {
verify_flag = false;
break;
}

if (libspdm_x509_verify_cert(current_cert, current_cert_len,
preceding_cert,
preceding_cert_len) == false) {
Expand Down
6 changes: 6 additions & 0 deletions os_stub/cryptlib_openssl/pk/x509.c
Original file line number Diff line number Diff line change
Expand Up @@ -2074,13 +2074,19 @@ bool libspdm_x509_verify_cert_chain(const uint8_t *root_cert, size_t root_cert_l
(int *)&asn1_tag, (int *)&obj_class,
(long)(cert_chain_length + cert_chain - tmp_ptr));
if (asn1_tag != V_ASN1_SEQUENCE || ret & OPENSSL_ASN1_ERROR_MASK) {
if (current_cert < cert_chain + cert_chain_length)
verify_flag = false;
break;
}


/* Calculate current_cert length;*/

current_cert_len = tmp_ptr - current_cert + length;
if (current_cert + current_cert_len > cert_chain + cert_chain_length) {
verify_flag = false;
break;
}


/* Verify current_cert with preceding cert;*/
Expand Down
2 changes: 1 addition & 1 deletion unit_test/test_crypt/x509_verify.c
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ bool libspdm_validate_crypt_x509(char *Path, size_t len)
status = libspdm_x509_verify_cert_chain((const uint8_t *)test_ca_cert, test_ca_cert_len,
(const uint8_t *)test_ca_cert,
test_ca_cert_len + 1);
if (!status) {
if (status) {
libspdm_my_print("[Fail]\n");
goto cleanup;
} else {
Expand Down
29 changes: 29 additions & 0 deletions unit_test/test_spdm_crypt/test_spdm_crypt.c
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,14 @@ void libspdm_test_crypt_spdm_x509_set_cert_certificate_check_ex(void **state)
false,
SPDM_CERTIFICATE_INFO_CERT_MODEL_DEVICE_CERT);
assert_true(status);

status = libspdm_x509_set_cert_certificate_check_ex(file_buffer, file_buffer_size + 1,
SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_2048,
SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA_256,
false,
SPDM_CERTIFICATE_INFO_CERT_MODEL_DEVICE_CERT);
assert_false(status);

status = libspdm_x509_set_cert_certificate_check_ex(file_buffer, file_buffer_size,
SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_2048,
SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA_256,
Expand All @@ -487,6 +495,13 @@ void libspdm_test_crypt_spdm_x509_set_cert_certificate_check_ex(void **state)
SPDM_CERTIFICATE_INFO_CERT_MODEL_DEVICE_CERT);
assert_true(status);

status = libspdm_x509_set_cert_certificate_check_ex(file_buffer, file_buffer_size + 1,
SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P256,
SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA_256,
true,
SPDM_CERTIFICATE_INFO_CERT_MODEL_DEVICE_CERT);
assert_false(status);

status = libspdm_x509_set_cert_certificate_check_ex(file_buffer, file_buffer_size,
SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P256,
SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA_256,
Expand Down Expand Up @@ -526,6 +541,13 @@ void libspdm_test_crypt_spdm_verify_cert_chain_data_ex(void **state)
SPDM_CERTIFICATE_INFO_CERT_MODEL_DEVICE_CERT);
assert_true(status);

status = libspdm_verify_cert_chain_data_ex(file_buffer, file_buffer_size + 1,
SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_2048,
SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA_256,
true,
SPDM_CERTIFICATE_INFO_CERT_MODEL_DEVICE_CERT);
assert_false(status);

status = libspdm_verify_cert_chain_data_ex(file_buffer, file_buffer_size,
SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_RSASSA_2048,
SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA_256,
Expand All @@ -545,6 +567,13 @@ void libspdm_test_crypt_spdm_verify_cert_chain_data_ex(void **state)
SPDM_CERTIFICATE_INFO_CERT_MODEL_DEVICE_CERT);
assert_true(status);

status = libspdm_verify_cert_chain_data_ex(file_buffer, file_buffer_size + 1,
SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P256,
SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA_256,
false,
SPDM_CERTIFICATE_INFO_CERT_MODEL_DEVICE_CERT);
assert_false(status);

status = libspdm_verify_cert_chain_data_ex(file_buffer, file_buffer_size,
SPDM_ALGORITHMS_BASE_ASYM_ALGO_TPM_ALG_ECDSA_ECC_NIST_P256,
SPDM_ALGORITHMS_BASE_HASH_ALGO_TPM_ALG_SHA_256,
Expand Down

0 comments on commit bdc82b8

Please sign in to comment.