Repository navigation
Use NIST vector for FFDH FIPS self-test - #4261
Conversation
jyao1
added
the
4.0
| * @param[in] priv_key_size The size of private key buffer in bytes. | ||
| * | ||
| * @retval true DH private key component was set successfully. | ||
| * @retval false Invalid DH private key component. |
There was a problem hiding this comment.
As the function comment and reference links mentioned above.
Actually I have no actual idea how to use Mbed-TLS to do key validation in proper way like OpenSSL in detail.
Sample code generated from my agent.
assisted-by: GitHub Copilot:Claude OPUS 5.5
mbedtls_mpi q;
bool ok;
ctx = dh_context;
mbedtls_mpi_init(&q);
ok = mbedtls_mpi_read_binary(&ctx->MBEDTLS_PRIVATE(X), priv_key, priv_key_size) == 0 &&
mbedtls_mpi_sub_int(&q, &ctx->MBEDTLS_PRIVATE(P), 1) == 0 &&
mbedtls_mpi_shift_r(&q, 1) == 0 &&
mbedtls_mpi_cmp_int(&ctx->MBEDTLS_PRIVATE(X), 1) >= 0 &&
mbedtls_mpi_cmp_mpi(&ctx->MBEDTLS_PRIVATE(X), &q) < 0;
mbedtls_mpi_free(&q);
if (!ok) {
mbedtls_mpi_free(&ctx->MBEDTLS_PRIVATE(X));
mbedtls_mpi_init(&ctx->MBEDTLS_PRIVATE(X));
}
return ok;
FYI.
There was a problem hiding this comment.
I do not understand your comment.
What is the problem of the original code? Could you please be specific?
There was a problem hiding this comment.
I reviewed this function libspdm_dh_set_priv_key several times.
Currently libspdm_dh_set_priv_key implemented in MbedTLS does not check private key as it claimed.
I can imagine the libspdm users probably utilize this API in their Known Answer Test (KAT) with given invalid private key even worst reuse this API in production codes instead of unit tests.
I found it is not easy to implement key validation by MbedTLS like EVP_PKEY_private_check() in OpenSSL.
Quoted from https://docs.openssl.org/3.0/man3/EVP_PKEY_check/#description
Refer to SP800-56A and SP800-56B for rules relating to when these functions should be called during key establishment. It is not necessary to call these functions after locally calling an approved key generation method, but may be required for assurance purposes when receiving keys from a third party.
But incomplete implementation by MbedTLS is worst than no implementation to me.
In this PR, there are still many benefits from the FIPS self-test in different perspectives.
- Evaluate any change from relying third-party cryptographic library does not break the expected behavior.
- Ensure any change to libspdm API doest not break the expected behavior.
In summary it is good to me.
There was a problem hiding this comment.
Currently libspdm_dh_set_priv_key implemented in MbedTLS does not check private key as it claimed.
Yes, that is because mbedtls does not provide.
I do not expect this API will be used in production code except FIPS test.
Anyway, I think there is no harm to mirror what OpenSSL does for the check.
| * @param[in] priv_key_size The size of private key buffer in bytes. | ||
| * | ||
| * @retval true DH private key component was set successfully. | ||
| * @retval false Invalid DH private key component. |
There was a problem hiding this comment.
I reviewed this function libspdm_dh_set_priv_key several times.
Currently libspdm_dh_set_priv_key implemented in MbedTLS does not check private key as it claimed.
I can imagine the libspdm users probably utilize this API in their Known Answer Test (KAT) with given invalid private key even worst reuse this API in production codes instead of unit tests.
I found it is not easy to implement key validation by MbedTLS like EVP_PKEY_private_check() in OpenSSL.
Quoted from https://docs.openssl.org/3.0/man3/EVP_PKEY_check/#description
Refer to SP800-56A and SP800-56B for rules relating to when these functions should be called during key establishment. It is not necessary to call these functions after locally calling an approved key generation method, but may be required for assurance purposes when receiving keys from a third party.
But incomplete implementation by MbedTLS is worst than no implementation to me.
In this PR, there are still many benefits from the FIPS self-test in different perspectives.
- Evaluate any change from relying third-party cryptographic library does not break the expected behavior.
- Ensure any change to libspdm API doest not break the expected behavior.
In summary it is good to me.
| * (domainParameterGenerationMode = ffdhe2048, scheme = dhEphem), tcId 1. | ||
| * The ffdhe2048 domain parameters are from RFC 7919. | ||
| * https://github.com/usnistgov/ACVP-Server/tree/master/gen-val/json-files/KAS-FFC-SSC-Sp800-56Ar3 | ||
| */ |
There was a problem hiding this comment.
Note:
RELEASE/v1.1.0.41
commit hash b121fee031dc65d32c96f4109a42a10dacbe2076
Add libspdm_dh_set_priv_key() to allow a fixed-key KAT (previously a pairwise-consistency test only). Vector from NIST ACVP-Server: KAS-FFC-SSC-Sp800-56Ar3 (ffdhe2048 domain parameters, RFC 7919). Signed-off-by: Jiewen Yao <jiewen.yao@intel.com> Assisted-by: GitHub Copilot:Claude Sonnet 5 Assisted-by: Claude Code:claude-sonnet-5-5
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com> Assisted-by: GitHub Copilot:Claude Sonnet 5
Document the FFDHE known-answer test and the new libspdm_dh_set_priv_key() HAL function. Signed-off-by: Jiewen Yao <jiewen.yao@intel.com> Assisted-by: GitHub Copilot:Claude Sonnet 5
Add libspdm_dh_set_priv_key() to allow a fixed-key KAT (previously a
pairwise-consistency test only). Vector from NIST ACVP-Server:
KAS-FFC-SSC-Sp800-56Ar3 (ffdhe2048 domain parameters, RFC 7919).
ref: #4198