aws / s2n-tls

An implementation of the TLS/SSL protocols
https://aws.github.io/s2n-tls/usage-guide/
Apache License 2.0
4.46k stars 698 forks source link

OCSP feature only supports SHA1 hashes #4595

Open jouho opened 3 weeks ago

jouho commented 3 weeks ago

Security issue notifications

If you discover a potential security issue in s2n we ask that you notify AWS Security via our vulnerability reporting page. Please do not create a public github issue.

Problem:

When debugging a RHEL9 build test failures in this PR, I noticed that we assume SHA1 is used to read and verify the response cert_id and therefore the test fails with an OCSP response generated with SHA256 hashed cert_id: https://github.com/aws/s2n-tls/blob/ff03b941d74ba6aa308646d0c2d3d95457f4cb71/tls/s2n_x509_validator.c#L877-L879

There is no explicit mention of enforcing SHA1 in OCSP RFC

Solution:

Investigate why only SHA1 is supported for OSCP digest, and consider adding support for other hash algorithms. If we decide not to implement them, document the reasons.

Requirements / Acceptance Criteria:

What must a solution address in order to solve the problem? How do we know the solution is complete?

Out of scope:

Is there anything the solution will intentionally NOT address?

jouho commented 2 weeks ago

I was not able to find relevant conversation on why we only support SHA1 for cert_id hashing algorithm. Here, I documented my investigation.

What is cert_id used for?

The cert_id field in the OCSP response file is used to uniquely identify the certificate whose status is being queried. It is used to ensure that the correct certificate is being checked for its validity status.

How it is used:

Relevant code here

Impact of compromised cert_id

Since SHA-1 is vulnerable to collision attacks, an attacker could create a different certificate that produces the same cert_id hash values. This could potentially allow the attacker to obtain a valid OCSP response for a fraudulent certificate. However, other security measures in the TLS protocol mitigate potential risks:

Recommended action

SHA-1 has been adequate for uniquely identifying certificates in most scenarios, which is likely why it remains the default hashing algorithm in OpenSSL's OCSP feature (OpenSSL Documentation). While there is an option to specify a different hashing algorithm, we currently do not recognize any significant customer demand for this.

If customer needs it, we can consider adding support for other hashing algorithms, such as SHA-256.

It will be a poor user experience if an OCSP response happens to use a hashing algorithm other than SHA-1, as it will fail with an error message "CERTIFICATE UNTRUSTED" even when the certificate is actually valid. Therefore, we should look to support known hashing algorithms for the OCSP cert_id field to prevent such issues and ensure robust and flexible security practices for our users.

jouho commented 2 weeks ago

I synced up with the team regarding this issue and our recommended actions are as follows: We will improve error handling to ensure that error messages clearly indicate when an unsupported hashing algorithm is used, for instance, by displaying HASH ALG NOT SUPPORTED. This will improve the user experience. While there is currently no significant customer demand for supporting additional hashing algorithms, we are open to considering support for algorithms like SHA-256 in the future if the need arises. Until then, we will prioritize robust error handling over immediate support for other algorithms.

Update: It turns out extracting the hash algorithm field of the certificate ID is not straightforward. In OpenSSL versions above 1.0.2 and AWS-LC, we can achieve this by:

    /* extract hash algorithm used to hash values in cert_id struct and ensure it's sha1 */
    OCSP_SINGLERESP *single = OCSP_resp_get0(basic_response, 0);
    RESULT_ENSURE(single != NULL, S2N_ERR_CERT_UNTRUSTED);

    OCSP_CERTID *cid = (OCSP_CERTID *) OCSP_SINGLERESP_get0_id(single);
    RESULT_ENSURE(cid != NULL, S2N_ERR_CERT_UNTRUSTED);

    const EVP_MD *dgst = NULL;
    ASN1_OBJECT *pmd = NULL;
    RESULT_GUARD_OSSL(OCSP_id_get0_info(NULL, &pmd, NULL, NULL, cid), S2N_ERR_CERT_UNTRUSTED);
    dgst = EVP_get_digestbyobj(pmd);
    int dgst_nid = EVP_MD_type(dgst);
    RESULT_ENSURE(dgst_nid == NID_sha1, S2N_ERR_UNSUPPORTED_OCSP_HASH);

However, in OpenSSL 1.0.2, which we use, OCSP_SINGLERESP_get0_id(single) is not supported. Therefore, we would need a feature probe for this error catching to work across all environments. To avoid introducing complexity for this niche option, we will not be implementing the error catching. Instead, we will document this finding for developers and users.