Compilation issues with mbedtls_x509write_crt_set_key_identifier due to conditional dependency on MBEDTLS_MD_CAN_SHA1

[PragatiGarg-eaton]

Description: Under the file modules/crypto/mbedtls/library/x509write_crt.c, the function mbedtls_x509write_crt_set_key_identifier is conditionally included based on the configuration MBEDTLS_MD_CAN_SHA1. When attempting to disable the vulnerable cipher MBEDTLS_MD_CAN_SHA1, I encountered compilation issues because I need to use the mbedtls_x509write_crt_set_key_identifier function to generate certificates.

Steps to Reproduce:
Disable the configuration MBEDTLS_MD_CAN_SHA1.
Attempt to compile the code that uses mbedtls_x509write_crt_set_key_identifier.
Expected Behavior: The code should compile successfully without requiring MBEDTLS_MD_CAN_SHA1.

Actual Behavior: Compilation fails due to the conditional dependency on MBEDTLS_MD_CAN_SHA1.

Questions:
Why is there a dependency on MBEDTLS_MD_CAN_SHA1 for the mbedtls_x509write_crt_set_key_identifier function?
How can this issue be resolved to allow the use of mbedtls_x509write_crt_set_key_identifier without enabling MBEDTLS_SHA1_C?

Additional Information:
Zephyr version: 3.6.0
MbedTLS version: 3.5.2

[prayassamriya]

Some more context:
We understand that as per APIs provided, mbedtls_x509write_crt_set_key_identifier is supposed to be called only if SHA1 is supported however what is the rationale for not extending the capability to SHA256 or other secure hashing schemes. We would like to get rid of SHA1 completely however this API blocks us from removing SHA1 from the mbedtls configuration.
Note: We know that setting key identifier is optional during the certificate generation however its still nice to have for better identity management of certificate chain.

mbedtls/programs/x509/cert_write.c

Line 886 in e57ea21

ret = mbedtls_x509write_crt_set_subject_key_identifier(&crt);

[gilles-peskine-arm]

RFC 5280 suggests SHA-1 but notes that other similar methods are ok, including using a different algorithm. Hard-coding SHA-1 made sense when this code was written, but I agree that it doesn't make sense now.

For maximum flexibility, the functions should take an extra argument to indicate which hash to use. That's an API change, which is quite a bit of work, and is disruptive in a minor release where we have to keep the old function for backward compatibility. But maybe we could live with a cheaper change in 3.6, and just add a compile-time option? But add a parameter to choose the hash algorithm in the development branch (before it's released as 4.0).

Would you be willing to submit patches if we agree on a design?