From 9af324f6e69273b1f7a123b5fd900367ddee6a2d Mon Sep 17 00:00:00 2001 From: Jou Ho Date: Sat, 22 Jun 2024 00:28:59 +0000 Subject: [PATCH 1/9] catch and throw a better error if used hash is not sha1 --- error/s2n_errno.c | 1 + error/s2n_errno.h | 1 + .../ocsp/ocsp_response_unsupported_hash.der | Bin 0 -> 2261 bytes tests/testlib/s2n_testlib.h | 19 ++++---- tests/unit/s2n_x509_validator_test.c | 42 ++++++++++++++++++ tls/s2n_x509_validator.c | 16 +++++++ 6 files changed, 70 insertions(+), 9 deletions(-) create mode 100644 tests/pems/ocsp/ocsp_response_unsupported_hash.der diff --git a/error/s2n_errno.c b/error/s2n_errno.c index 49aab545c09..427661c76a0 100644 --- a/error/s2n_errno.c +++ b/error/s2n_errno.c @@ -98,6 +98,7 @@ static const char *no_such_error = "Internal s2n error"; ERR_ENTRY(S2N_ERR_NON_EMPTY_RENEGOTIATION_INFO, "renegotiation_info should be empty") \ ERR_ENTRY(S2N_ERR_RECORD_LIMIT, "TLS record limit reached") \ ERR_ENTRY(S2N_ERR_CERT_UNTRUSTED, "Certificate is untrusted") \ + ERR_ENTRY(S2N_ERR_CERT_UNSUPPORTED_HASH, "OCSP response hash used in not supported") \ ERR_ENTRY(S2N_ERR_CERT_REVOKED, "Certificate has been revoked by the CA") \ ERR_ENTRY(S2N_ERR_CERT_NOT_YET_VALID, "Certificate is not yet valid") \ ERR_ENTRY(S2N_ERR_CERT_EXPIRED, "Certificate has expired") \ diff --git a/error/s2n_errno.h b/error/s2n_errno.h index 838f7d10388..616e71b0a09 100644 --- a/error/s2n_errno.h +++ b/error/s2n_errno.h @@ -115,6 +115,7 @@ typedef enum { S2N_ERR_NON_EMPTY_RENEGOTIATION_INFO, S2N_ERR_RECORD_LIMIT, S2N_ERR_CERT_UNTRUSTED, + S2N_ERR_CERT_UNSUPPORTED_HASH, S2N_ERR_CERT_REVOKED, S2N_ERR_CERT_NOT_YET_VALID, S2N_ERR_CERT_EXPIRED, diff --git a/tests/pems/ocsp/ocsp_response_unsupported_hash.der b/tests/pems/ocsp/ocsp_response_unsupported_hash.der new file mode 100644 index 0000000000000000000000000000000000000000..685a38b6b066004a6b0dac971e556024715d1e1a GIT binary patch literal 2261 zcmcIlS5%W*8YO=cQs_mh^lCu(6N(@p#ZaUOhzOwwLLyb^5xz;BVDA9;@#A@7kmMV%#k?$s+&|PL5^yuLnzb zm=u=jK?A4tr-xD*Pzsdz_lE#5UKqF3X(4)Mbn?;!%w>9ppo2efh)FO@*yMmf*x)Qs z3IJjGjZTwEf6&oK)uI+97pF@mPOq-A6)vUCIy%{k8gan?5$^Bv{w!brf?5bE5g%tj+oX|6;}hsZnVr zL;2KgRU8>tY*{_1E_Ajjal?D-M5&j$gWL7!0@SqDUV}D5+M-lo!KTI3|2^92qV_Z= z(&5JLwna~6fBn(zi!UB`d9=`wt<(ZP551fE&vwdWC`4*-q+Walhs?E9d5mIviq2qs z$Ccz;KG;D{;uKrgV}HA?A$2W#qhx$?^3D*?fVFRVi(I@x5O-bIz}lb#A+>@3_r8zS zoldtUYXY9Qk3XwQ6)UK*W2~=LvmeHz+p{Jxio^Z)q)=G~hS%N5(x=|0^ERibna!7G zv*|xL-ry&$`#wlIkTG%_`FDG=ibr|ve`_de0(jB&yrbXrGOFptYN>Nky~Uw7h$ja2|=d|&XVd1eW)u5J_gt~ujQQrPxstYan9rbp6Ovd$fk%TB zcqH>&0V&Y)B~SnYFmo62A7KF%f3@g;RSJ|8s&^G}0a9M2^aW)Z#{qS)`^vUWk91*$HI=O zdi$DVfu@HdRFZNKk4dK!H9*f>a(-RPdBc_XMe|;ZCiT( zhxw;v=iuf-(r$j$@nL{eq3G70mcd6Ks_0if+`N_(vr#@YH6uADi#Yo}ZeD4}cv&|V z;mY;%-HMfvt+%yzo~p57mZ|l`j`m8jVIya+-zVY=lDnTzbB@8fa-TxV_W2}}{)+=gN0jh>gsd z28V@XZc=CC<;7oSgggZNJqU0YiD^@eCDlD)-HLms(2+IGI~(oL>}$(aU{P!JRYqn2 zRpnH^m^*|mW8~ica0|yJ??$T^mKkcS45xN?x4t*+!4L7Z%i=IoL0%tQ4+Oq!#{dGXUf~xWEO0?89JC01*oc z9ALQve6Vhi|JqYf)L(=7YXYa!UfC%mA@mi7VM-122^v-q`rahmr?BJ!KGuDs1NEe~ zi-vl!%>F^($6UE*Vs1$0i>hlY7K>T)Mdry@6~|8ZB0R^fK8HOL*Rn%HGI6BvqWNuGQ`*~-T+}(F1kz)d->JV+1eIu1` zVJ=G6v_Pcc)=*`Pi$*BeBxxA5xy$_4kpT-T-^@*x|Y6m&sBb?XH;&K>bFZuutYzKxPU>$#`r%R_B{ zj|_}Hda{Rb&2q%Z&g literal 0 HcmV?d00001 diff --git a/tests/testlib/s2n_testlib.h b/tests/testlib/s2n_testlib.h index e05da3eefc5..0376dee604c 100644 --- a/tests/testlib/s2n_testlib.h +++ b/tests/testlib/s2n_testlib.h @@ -163,15 +163,16 @@ S2N_RESULT s2n_connection_set_test_master_secret(struct s2n_connection *conn, co #define S2N_OCSP_SERVER_CERT_EARLY_EXPIRE "../pems/ocsp/server_cert_early_expire.pem" #define S2N_OCSP_SERVER_ECDSA_CERT "../pems/ocsp/server_ecdsa_cert.pem" -#define S2N_OCSP_SERVER_KEY "../pems/ocsp/server_key.pem" -#define S2N_OCSP_CA_CERT "../pems/ocsp/ca_cert.pem" -#define S2N_OCSP_CA_KEY "../pems/ocsp/ca_key.pem" -#define S2N_OCSP_RESPONSE_DER "../pems/ocsp/ocsp_response.der" -#define S2N_OCSP_RESPONSE_EARLY_EXPIRE_DER "../pems/ocsp/ocsp_response_early_expire.der" -#define S2N_OCSP_RESPONSE_NO_NEXT_UPDATE_DER "../pems/ocsp/ocsp_response_no_next_update.der" -#define S2N_OCSP_RESPONSE_REVOKED_DER "../pems/ocsp/ocsp_response_revoked.der" -#define S2N_OCSP_RESPONSE_WRONG_SIGNER_DER "../pems/ocsp/ocsp_response_wrong_signer.der" -#define S2N_OCSP_RESPONSE_CERT "../pems/ocsp/ocsp_cert.pem" +#define S2N_OCSP_SERVER_KEY "../pems/ocsp/server_key.pem" +#define S2N_OCSP_CA_CERT "../pems/ocsp/ca_cert.pem" +#define S2N_OCSP_CA_KEY "../pems/ocsp/ca_key.pem" +#define S2N_OCSP_RESPONSE_DER "../pems/ocsp/ocsp_response.der" +#define S2N_OCSP_RESPONSE_EARLY_EXPIRE_DER "../pems/ocsp/ocsp_response_early_expire.der" +#define S2N_OCSP_RESPONSE_NO_NEXT_UPDATE_DER "../pems/ocsp/ocsp_response_no_next_update.der" +#define S2N_OCSP_RESPONSE_REVOKED_DER "../pems/ocsp/ocsp_response_revoked.der" +#define S2N_OCSP_RESPONSE_WRONG_SIGNER_DER "../pems/ocsp/ocsp_response_wrong_signer.der" +#define S2N_OCSP_RESPONSE_UNSUPPORTED_HASH_DER "../pems/ocsp/ocsp_response_unsupported_hash.der" +#define S2N_OCSP_RESPONSE_CERT "../pems/ocsp/ocsp_cert.pem" #define S2N_ALLIGATOR_SAN_CERT "../pems/sni/alligator_cert.pem" #define S2N_ALLIGATOR_SAN_KEY "../pems/sni/alligator_key.pem" diff --git a/tests/unit/s2n_x509_validator_test.c b/tests/unit/s2n_x509_validator_test.c index 9439abd99f5..60c39710dae 100644 --- a/tests/unit/s2n_x509_validator_test.c +++ b/tests/unit/s2n_x509_validator_test.c @@ -1378,6 +1378,48 @@ int main(int argc, char **argv) s2n_x509_trust_store_wipe(&trust_store); } + /* Test valid OCSP date range and data, but certificate ID values are hased with sha256 */ + { + struct s2n_x509_trust_store trust_store; + s2n_x509_trust_store_init_empty(&trust_store); + EXPECT_SUCCESS(s2n_x509_trust_store_from_ca_file(&trust_store, S2N_OCSP_CA_CERT, NULL)); + + struct s2n_x509_validator validator; + s2n_x509_validator_init(&validator, &trust_store, 1); + + struct s2n_connection *connection = s2n_connection_new(S2N_CLIENT); + EXPECT_NOT_NULL(connection); + + struct host_verify_data verify_data = { .callback_invoked = 0, .found_name = 0, .name = NULL }; + EXPECT_SUCCESS(s2n_connection_set_verify_host_callback(connection, verify_host_accept_everything, &verify_data)); + + DEFER_CLEANUP(struct s2n_stuffer cert_chain_stuffer = { 0 }, s2n_stuffer_free); + EXPECT_OK(s2n_test_cert_chain_data_from_pem(connection, S2N_OCSP_SERVER_CERT, &cert_chain_stuffer)); + uint32_t chain_len = s2n_stuffer_data_available(&cert_chain_stuffer); + uint8_t *chain_data = s2n_stuffer_raw_read(&cert_chain_stuffer, chain_len); + EXPECT_NOT_NULL(chain_data); + + struct s2n_pkey public_key_out; + EXPECT_SUCCESS(s2n_pkey_zero_init(&public_key_out)); + s2n_pkey_type pkey_type = S2N_PKEY_TYPE_UNKNOWN; + EXPECT_OK(s2n_x509_validator_validate_cert_chain(&validator, connection, chain_data, chain_len, &pkey_type, &public_key_out)); + + EXPECT_EQUAL(1, verify_data.callback_invoked); + struct s2n_stuffer ocsp_data_stuffer = { 0 }; + EXPECT_SUCCESS(read_file(&ocsp_data_stuffer, S2N_OCSP_RESPONSE_UNSUPPORTED_HASH_DER, S2N_MAX_TEST_PEM_SIZE)); + uint32_t ocsp_data_len = s2n_stuffer_data_available(&ocsp_data_stuffer); + EXPECT_TRUE(ocsp_data_len > 0); + EXPECT_ERROR_WITH_ERRNO(s2n_x509_validator_validate_cert_stapled_ocsp_response(&validator, connection, + s2n_stuffer_raw_read(&ocsp_data_stuffer, ocsp_data_len), ocsp_data_len), + S2N_ERR_CERT_UNSUPPORTED_HASH); + + s2n_stuffer_free(&ocsp_data_stuffer); + s2n_connection_free(connection); + s2n_pkey_free(&public_key_out); + s2n_x509_validator_wipe(&validator); + s2n_x509_trust_store_wipe(&trust_store); + }; + /* Test OCSP response status is revoked */ { struct s2n_x509_trust_store trust_store; diff --git a/tls/s2n_x509_validator.c b/tls/s2n_x509_validator.c index 75b6842351e..65d07f19ead 100644 --- a/tls/s2n_x509_validator.c +++ b/tls/s2n_x509_validator.c @@ -874,6 +874,22 @@ S2N_RESULT s2n_x509_validator_validate_cert_stapled_ocsp_response(struct s2n_x50 int status = 0; int reason = 0; + /* extract hash algorithm used to hash values in cert_id struct */ + const OCSP_SINGLERESP *single; + single = OCSP_resp_get0(basic_response, 0); + RESULT_ENSURE(single != NULL, S2N_ERR_CERT_UNTRUSTED); + + OCSP_CERTID *cid; + 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); + const char *dgst_str = EVP_MD_get0_name(dgst); + RESULT_ENSURE(strcmp(dgst_str, "SHA1") == 0, S2N_ERR_CERT_UNSUPPORTED_HASH); + /* sha1 is the only supported OCSP digest */ OCSP_CERTID *cert_id = OCSP_cert_to_id(EVP_sha1(), subject, issuer); RESULT_ENSURE_REF(cert_id); From 88fc79a6df5a8a0501719956ca07c34470f28666 Mon Sep 17 00:00:00 2001 From: Jou Ho Date: Mon, 24 Jun 2024 21:25:11 +0000 Subject: [PATCH 2/9] modify error name --- error/s2n_errno.c | 2 +- error/s2n_errno.h | 2 +- tests/unit/s2n_x509_validator_test.c | 2 +- tls/s2n_x509_validator.c | 12 +++++------- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/error/s2n_errno.c b/error/s2n_errno.c index 427661c76a0..ce65a91cd3e 100644 --- a/error/s2n_errno.c +++ b/error/s2n_errno.c @@ -98,7 +98,6 @@ static const char *no_such_error = "Internal s2n error"; ERR_ENTRY(S2N_ERR_NON_EMPTY_RENEGOTIATION_INFO, "renegotiation_info should be empty") \ ERR_ENTRY(S2N_ERR_RECORD_LIMIT, "TLS record limit reached") \ ERR_ENTRY(S2N_ERR_CERT_UNTRUSTED, "Certificate is untrusted") \ - ERR_ENTRY(S2N_ERR_CERT_UNSUPPORTED_HASH, "OCSP response hash used in not supported") \ ERR_ENTRY(S2N_ERR_CERT_REVOKED, "Certificate has been revoked by the CA") \ ERR_ENTRY(S2N_ERR_CERT_NOT_YET_VALID, "Certificate is not yet valid") \ ERR_ENTRY(S2N_ERR_CERT_EXPIRED, "Certificate has expired") \ @@ -224,6 +223,7 @@ static const char *no_such_error = "Internal s2n error"; ERR_ENTRY(S2N_ERR_EXTENSION_NOT_RECEIVED, "The TLS extension was not received") \ ERR_ENTRY(S2N_ERR_INVALID_SCT_LIST, "SCT list is invalid") \ ERR_ENTRY(S2N_ERR_INVALID_OCSP_RESPONSE, "OCSP response is invalid") \ + ERR_ENTRY(S2N_ERR_UNSUPPORTED_OCSP_HASH, "Hash algorithm used to generate certificate id in OCSP response is not supported") \ ERR_ENTRY(S2N_ERR_UPDATING_EXTENSION, "Updating extension data failed") \ ERR_ENTRY(S2N_ERR_INVALID_SERIALIZED_SESSION_STATE, "Serialized session state is not in valid format") \ ERR_ENTRY(S2N_ERR_SERIALIZED_SESSION_STATE_TOO_LONG, "Serialized session state is too long") \ diff --git a/error/s2n_errno.h b/error/s2n_errno.h index 616e71b0a09..39b38f6a9bf 100644 --- a/error/s2n_errno.h +++ b/error/s2n_errno.h @@ -115,7 +115,6 @@ typedef enum { S2N_ERR_NON_EMPTY_RENEGOTIATION_INFO, S2N_ERR_RECORD_LIMIT, S2N_ERR_CERT_UNTRUSTED, - S2N_ERR_CERT_UNSUPPORTED_HASH, S2N_ERR_CERT_REVOKED, S2N_ERR_CERT_NOT_YET_VALID, S2N_ERR_CERT_EXPIRED, @@ -269,6 +268,7 @@ typedef enum { S2N_ERR_EXTENSION_NOT_RECEIVED, S2N_ERR_INVALID_SCT_LIST, S2N_ERR_INVALID_OCSP_RESPONSE, + S2N_ERR_UNSUPPORTED_OCSP_HASH, S2N_ERR_UPDATING_EXTENSION, S2N_ERR_INVALID_SERIALIZED_SESSION_STATE, S2N_ERR_SERIALIZED_SESSION_STATE_TOO_LONG, diff --git a/tests/unit/s2n_x509_validator_test.c b/tests/unit/s2n_x509_validator_test.c index 60c39710dae..f39eec74c4c 100644 --- a/tests/unit/s2n_x509_validator_test.c +++ b/tests/unit/s2n_x509_validator_test.c @@ -1411,7 +1411,7 @@ int main(int argc, char **argv) EXPECT_TRUE(ocsp_data_len > 0); EXPECT_ERROR_WITH_ERRNO(s2n_x509_validator_validate_cert_stapled_ocsp_response(&validator, connection, s2n_stuffer_raw_read(&ocsp_data_stuffer, ocsp_data_len), ocsp_data_len), - S2N_ERR_CERT_UNSUPPORTED_HASH); + S2N_ERR_UNSUPPORTED_OCSP_HASH); s2n_stuffer_free(&ocsp_data_stuffer); s2n_connection_free(connection); diff --git a/tls/s2n_x509_validator.c b/tls/s2n_x509_validator.c index 65d07f19ead..f8f4490e4b3 100644 --- a/tls/s2n_x509_validator.c +++ b/tls/s2n_x509_validator.c @@ -874,21 +874,19 @@ S2N_RESULT s2n_x509_validator_validate_cert_stapled_ocsp_response(struct s2n_x50 int status = 0; int reason = 0; - /* extract hash algorithm used to hash values in cert_id struct */ - const OCSP_SINGLERESP *single; - single = OCSP_resp_get0(basic_response, 0); + /* extract hash algorithm used to hash values in cert_id struct and ensure it's sha1 */ + const OCSP_SINGLERESP *single = OCSP_resp_get0(basic_response, 0); RESULT_ENSURE(single != NULL, S2N_ERR_CERT_UNTRUSTED); - OCSP_CERTID *cid; - cid = (OCSP_CERTID *) OCSP_SINGLERESP_get0_id(single); + 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); - const char *dgst_str = EVP_MD_get0_name(dgst); - RESULT_ENSURE(strcmp(dgst_str, "SHA1") == 0, S2N_ERR_CERT_UNSUPPORTED_HASH); + int dgst_nid = EVP_MD_type(dgst); + RESULT_ENSURE(dgst_nid == NID_sha1, S2N_ERR_UNSUPPORTED_OCSP_HASH); /* sha1 is the only supported OCSP digest */ OCSP_CERTID *cert_id = OCSP_cert_to_id(EVP_sha1(), subject, issuer); From 497c3cfab166f45543a308d4a207331d245995dc Mon Sep 17 00:00:00 2001 From: Jou Ho Date: Mon, 24 Jun 2024 22:03:30 +0000 Subject: [PATCH 3/9] drop const qualifier --- tls/s2n_x509_validator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tls/s2n_x509_validator.c b/tls/s2n_x509_validator.c index f8f4490e4b3..c47a88013b2 100644 --- a/tls/s2n_x509_validator.c +++ b/tls/s2n_x509_validator.c @@ -875,7 +875,7 @@ S2N_RESULT s2n_x509_validator_validate_cert_stapled_ocsp_response(struct s2n_x50 int reason = 0; /* extract hash algorithm used to hash values in cert_id struct and ensure it's sha1 */ - const OCSP_SINGLERESP *single = OCSP_resp_get0(basic_response, 0); + 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); From 825ad58225e3e84aca1d8cd7094a482c16cf9f23 Mon Sep 17 00:00:00 2001 From: Jou Ho Date: Mon, 1 Jul 2024 21:07:01 +0000 Subject: [PATCH 4/9] provide better documentation --- api/s2n.h | 6 ++++ error/s2n_errno.c | 1 - error/s2n_errno.h | 1 - tests/testlib/s2n_testlib.h | 19 ++++++------- tests/unit/s2n_x509_validator_test.c | 42 ---------------------------- tls/s2n_x509_validator.c | 22 +++++---------- 6 files changed, 22 insertions(+), 69 deletions(-) diff --git a/api/s2n.h b/api/s2n.h index 4ebf27c3e68..26ed4f979f7 100644 --- a/api/s2n.h +++ b/api/s2n.h @@ -999,6 +999,12 @@ S2N_API extern int s2n_config_set_verify_host_callback(struct s2n_config *config * * The default value is 1 if the underlying libCrypto implementation supports OCSP. * + * @note SHA-1 is the only supported hash algorithm for the `certID` field. If a different hash + * algorithm is used, validation will fail and manual verification will be needed. This is + * different from the hash algorithm used for the signature algorithm. See + * [RFC6960](https://datatracker.ietf.org/doc/html/rfc6960#section-4.1.1) for details about + * `certID` field. + * * @param config The configuration object being updated * @param check_ocsp The desired OCSP response check configuration * @returns S2N_SUCCESS on success. S2N_FAILURE on failure diff --git a/error/s2n_errno.c b/error/s2n_errno.c index ce65a91cd3e..49aab545c09 100644 --- a/error/s2n_errno.c +++ b/error/s2n_errno.c @@ -223,7 +223,6 @@ static const char *no_such_error = "Internal s2n error"; ERR_ENTRY(S2N_ERR_EXTENSION_NOT_RECEIVED, "The TLS extension was not received") \ ERR_ENTRY(S2N_ERR_INVALID_SCT_LIST, "SCT list is invalid") \ ERR_ENTRY(S2N_ERR_INVALID_OCSP_RESPONSE, "OCSP response is invalid") \ - ERR_ENTRY(S2N_ERR_UNSUPPORTED_OCSP_HASH, "Hash algorithm used to generate certificate id in OCSP response is not supported") \ ERR_ENTRY(S2N_ERR_UPDATING_EXTENSION, "Updating extension data failed") \ ERR_ENTRY(S2N_ERR_INVALID_SERIALIZED_SESSION_STATE, "Serialized session state is not in valid format") \ ERR_ENTRY(S2N_ERR_SERIALIZED_SESSION_STATE_TOO_LONG, "Serialized session state is too long") \ diff --git a/error/s2n_errno.h b/error/s2n_errno.h index 39b38f6a9bf..838f7d10388 100644 --- a/error/s2n_errno.h +++ b/error/s2n_errno.h @@ -268,7 +268,6 @@ typedef enum { S2N_ERR_EXTENSION_NOT_RECEIVED, S2N_ERR_INVALID_SCT_LIST, S2N_ERR_INVALID_OCSP_RESPONSE, - S2N_ERR_UNSUPPORTED_OCSP_HASH, S2N_ERR_UPDATING_EXTENSION, S2N_ERR_INVALID_SERIALIZED_SESSION_STATE, S2N_ERR_SERIALIZED_SESSION_STATE_TOO_LONG, diff --git a/tests/testlib/s2n_testlib.h b/tests/testlib/s2n_testlib.h index 0376dee604c..e05da3eefc5 100644 --- a/tests/testlib/s2n_testlib.h +++ b/tests/testlib/s2n_testlib.h @@ -163,16 +163,15 @@ S2N_RESULT s2n_connection_set_test_master_secret(struct s2n_connection *conn, co #define S2N_OCSP_SERVER_CERT_EARLY_EXPIRE "../pems/ocsp/server_cert_early_expire.pem" #define S2N_OCSP_SERVER_ECDSA_CERT "../pems/ocsp/server_ecdsa_cert.pem" -#define S2N_OCSP_SERVER_KEY "../pems/ocsp/server_key.pem" -#define S2N_OCSP_CA_CERT "../pems/ocsp/ca_cert.pem" -#define S2N_OCSP_CA_KEY "../pems/ocsp/ca_key.pem" -#define S2N_OCSP_RESPONSE_DER "../pems/ocsp/ocsp_response.der" -#define S2N_OCSP_RESPONSE_EARLY_EXPIRE_DER "../pems/ocsp/ocsp_response_early_expire.der" -#define S2N_OCSP_RESPONSE_NO_NEXT_UPDATE_DER "../pems/ocsp/ocsp_response_no_next_update.der" -#define S2N_OCSP_RESPONSE_REVOKED_DER "../pems/ocsp/ocsp_response_revoked.der" -#define S2N_OCSP_RESPONSE_WRONG_SIGNER_DER "../pems/ocsp/ocsp_response_wrong_signer.der" -#define S2N_OCSP_RESPONSE_UNSUPPORTED_HASH_DER "../pems/ocsp/ocsp_response_unsupported_hash.der" -#define S2N_OCSP_RESPONSE_CERT "../pems/ocsp/ocsp_cert.pem" +#define S2N_OCSP_SERVER_KEY "../pems/ocsp/server_key.pem" +#define S2N_OCSP_CA_CERT "../pems/ocsp/ca_cert.pem" +#define S2N_OCSP_CA_KEY "../pems/ocsp/ca_key.pem" +#define S2N_OCSP_RESPONSE_DER "../pems/ocsp/ocsp_response.der" +#define S2N_OCSP_RESPONSE_EARLY_EXPIRE_DER "../pems/ocsp/ocsp_response_early_expire.der" +#define S2N_OCSP_RESPONSE_NO_NEXT_UPDATE_DER "../pems/ocsp/ocsp_response_no_next_update.der" +#define S2N_OCSP_RESPONSE_REVOKED_DER "../pems/ocsp/ocsp_response_revoked.der" +#define S2N_OCSP_RESPONSE_WRONG_SIGNER_DER "../pems/ocsp/ocsp_response_wrong_signer.der" +#define S2N_OCSP_RESPONSE_CERT "../pems/ocsp/ocsp_cert.pem" #define S2N_ALLIGATOR_SAN_CERT "../pems/sni/alligator_cert.pem" #define S2N_ALLIGATOR_SAN_KEY "../pems/sni/alligator_key.pem" diff --git a/tests/unit/s2n_x509_validator_test.c b/tests/unit/s2n_x509_validator_test.c index f39eec74c4c..9439abd99f5 100644 --- a/tests/unit/s2n_x509_validator_test.c +++ b/tests/unit/s2n_x509_validator_test.c @@ -1378,48 +1378,6 @@ int main(int argc, char **argv) s2n_x509_trust_store_wipe(&trust_store); } - /* Test valid OCSP date range and data, but certificate ID values are hased with sha256 */ - { - struct s2n_x509_trust_store trust_store; - s2n_x509_trust_store_init_empty(&trust_store); - EXPECT_SUCCESS(s2n_x509_trust_store_from_ca_file(&trust_store, S2N_OCSP_CA_CERT, NULL)); - - struct s2n_x509_validator validator; - s2n_x509_validator_init(&validator, &trust_store, 1); - - struct s2n_connection *connection = s2n_connection_new(S2N_CLIENT); - EXPECT_NOT_NULL(connection); - - struct host_verify_data verify_data = { .callback_invoked = 0, .found_name = 0, .name = NULL }; - EXPECT_SUCCESS(s2n_connection_set_verify_host_callback(connection, verify_host_accept_everything, &verify_data)); - - DEFER_CLEANUP(struct s2n_stuffer cert_chain_stuffer = { 0 }, s2n_stuffer_free); - EXPECT_OK(s2n_test_cert_chain_data_from_pem(connection, S2N_OCSP_SERVER_CERT, &cert_chain_stuffer)); - uint32_t chain_len = s2n_stuffer_data_available(&cert_chain_stuffer); - uint8_t *chain_data = s2n_stuffer_raw_read(&cert_chain_stuffer, chain_len); - EXPECT_NOT_NULL(chain_data); - - struct s2n_pkey public_key_out; - EXPECT_SUCCESS(s2n_pkey_zero_init(&public_key_out)); - s2n_pkey_type pkey_type = S2N_PKEY_TYPE_UNKNOWN; - EXPECT_OK(s2n_x509_validator_validate_cert_chain(&validator, connection, chain_data, chain_len, &pkey_type, &public_key_out)); - - EXPECT_EQUAL(1, verify_data.callback_invoked); - struct s2n_stuffer ocsp_data_stuffer = { 0 }; - EXPECT_SUCCESS(read_file(&ocsp_data_stuffer, S2N_OCSP_RESPONSE_UNSUPPORTED_HASH_DER, S2N_MAX_TEST_PEM_SIZE)); - uint32_t ocsp_data_len = s2n_stuffer_data_available(&ocsp_data_stuffer); - EXPECT_TRUE(ocsp_data_len > 0); - EXPECT_ERROR_WITH_ERRNO(s2n_x509_validator_validate_cert_stapled_ocsp_response(&validator, connection, - s2n_stuffer_raw_read(&ocsp_data_stuffer, ocsp_data_len), ocsp_data_len), - S2N_ERR_UNSUPPORTED_OCSP_HASH); - - s2n_stuffer_free(&ocsp_data_stuffer); - s2n_connection_free(connection); - s2n_pkey_free(&public_key_out); - s2n_x509_validator_wipe(&validator); - s2n_x509_trust_store_wipe(&trust_store); - }; - /* Test OCSP response status is revoked */ { struct s2n_x509_trust_store trust_store; diff --git a/tls/s2n_x509_validator.c b/tls/s2n_x509_validator.c index c47a88013b2..737d719ca26 100644 --- a/tls/s2n_x509_validator.c +++ b/tls/s2n_x509_validator.c @@ -874,21 +874,13 @@ S2N_RESULT s2n_x509_validator_validate_cert_stapled_ocsp_response(struct s2n_x50 int status = 0; int reason = 0; - /* 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); - - /* sha1 is the only supported OCSP digest */ + /* SHA-1 is the only supported OCSP digest due to its wide compatibility and established use + * in existing systems. Supporting additional hash algorithms would require changes to error + * handling and compatibility checks, which are not currently justified by user demand. For + * verifying OCSP response with non-SHA-1 hash algorithm, users can call + * s2n_connection_get_ocsp_response() to retrieve the received OCSP stapling information for + * manual verification. + */ OCSP_CERTID *cert_id = OCSP_cert_to_id(EVP_sha1(), subject, issuer); RESULT_ENSURE_REF(cert_id); From b86f5f42414d73739738f02e0eff4d25aad25b3e Mon Sep 17 00:00:00 2001 From: Jou Ho Date: Mon, 1 Jul 2024 21:12:24 +0000 Subject: [PATCH 5/9] remove unused pem --- .../pems/ocsp/ocsp_response_unsupported_hash.der | Bin 2261 -> 0 bytes 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 tests/pems/ocsp/ocsp_response_unsupported_hash.der diff --git a/tests/pems/ocsp/ocsp_response_unsupported_hash.der b/tests/pems/ocsp/ocsp_response_unsupported_hash.der deleted file mode 100644 index 685a38b6b066004a6b0dac971e556024715d1e1a..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 2261 zcmcIlS5%W*8YO=cQs_mh^lCu(6N(@p#ZaUOhzOwwLLyb^5xz;BVDA9;@#A@7kmMV%#k?$s+&|PL5^yuLnzb zm=u=jK?A4tr-xD*Pzsdz_lE#5UKqF3X(4)Mbn?;!%w>9ppo2efh)FO@*yMmf*x)Qs z3IJjGjZTwEf6&oK)uI+97pF@mPOq-A6)vUCIy%{k8gan?5$^Bv{w!brf?5bE5g%tj+oX|6;}hsZnVr zL;2KgRU8>tY*{_1E_Ajjal?D-M5&j$gWL7!0@SqDUV}D5+M-lo!KTI3|2^92qV_Z= z(&5JLwna~6fBn(zi!UB`d9=`wt<(ZP551fE&vwdWC`4*-q+Walhs?E9d5mIviq2qs z$Ccz;KG;D{;uKrgV}HA?A$2W#qhx$?^3D*?fVFRVi(I@x5O-bIz}lb#A+>@3_r8zS zoldtUYXY9Qk3XwQ6)UK*W2~=LvmeHz+p{Jxio^Z)q)=G~hS%N5(x=|0^ERibna!7G zv*|xL-ry&$`#wlIkTG%_`FDG=ibr|ve`_de0(jB&yrbXrGOFptYN>Nky~Uw7h$ja2|=d|&XVd1eW)u5J_gt~ujQQrPxstYan9rbp6Ovd$fk%TB zcqH>&0V&Y)B~SnYFmo62A7KF%f3@g;RSJ|8s&^G}0a9M2^aW)Z#{qS)`^vUWk91*$HI=O zdi$DVfu@HdRFZNKk4dK!H9*f>a(-RPdBc_XMe|;ZCiT( zhxw;v=iuf-(r$j$@nL{eq3G70mcd6Ks_0if+`N_(vr#@YH6uADi#Yo}ZeD4}cv&|V z;mY;%-HMfvt+%yzo~p57mZ|l`j`m8jVIya+-zVY=lDnTzbB@8fa-TxV_W2}}{)+=gN0jh>gsd z28V@XZc=CC<;7oSgggZNJqU0YiD^@eCDlD)-HLms(2+IGI~(oL>}$(aU{P!JRYqn2 zRpnH^m^*|mW8~ica0|yJ??$T^mKkcS45xN?x4t*+!4L7Z%i=IoL0%tQ4+Oq!#{dGXUf~xWEO0?89JC01*oc z9ALQve6Vhi|JqYf)L(=7YXYa!UfC%mA@mi7VM-122^v-q`rahmr?BJ!KGuDs1NEe~ zi-vl!%>F^($6UE*Vs1$0i>hlY7K>T)Mdry@6~|8ZB0R^fK8HOL*Rn%HGI6BvqWNuGQ`*~-T+}(F1kz)d->JV+1eIu1` zVJ=G6v_Pcc)=*`Pi$*BeBxxA5xy$_4kpT-T-^@*x|Y6m&sBb?XH;&K>bFZuutYzKxPU>$#`r%R_B{ zj|_}Hda{Rb&2q%Z&g From 43a24a5866521db2e1e9958672f46224f8429f86 Mon Sep 17 00:00:00 2001 From: Jou Ho Date: Fri, 5 Jul 2024 19:54:38 +0000 Subject: [PATCH 6/9] address doc feedback --- api/s2n.h | 20 +++++++++++++------- tls/s2n_x509_validator.c | 10 +++------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/api/s2n.h b/api/s2n.h index 26ed4f979f7..8b4c5034c6d 100644 --- a/api/s2n.h +++ b/api/s2n.h @@ -998,12 +998,6 @@ S2N_API extern int s2n_config_set_verify_host_callback(struct s2n_config *config * be skipped. * * The default value is 1 if the underlying libCrypto implementation supports OCSP. - * - * @note SHA-1 is the only supported hash algorithm for the `certID` field. If a different hash - * algorithm is used, validation will fail and manual verification will be needed. This is - * different from the hash algorithm used for the signature algorithm. See - * [RFC6960](https://datatracker.ietf.org/doc/html/rfc6960#section-4.1.1) for details about - * `certID` field. * * @param config The configuration object being updated * @param check_ocsp The desired OCSP response check configuration @@ -1117,7 +1111,19 @@ typedef enum { /** * Sets up a connection to request the certificate status of a peer during an SSL handshake. If set * to S2N_STATUS_REQUEST_NONE, no status request is made. - * + * + * @note SHA-1 is the only supported hash algorithm for the `certID` field. This is different + * from the hash algorithm used for the OCSP signature. If a different hash algorithm is used, + * validation will fail. + * If the underlying libcrypto supports OCSP validation, the OCSP stapling information will + * be automatically validated. Users can disable this OCSP verification by calling + * `s2n_config_set_check_stapled_ocsp_response()` with "0" to turn OCSP validation off. Users may + * use `s2n_connection_get_ocsp_response()` together with OCSP validation turned off to retrieve + * the received OCSP stapling information for manual verification. This workaround is however + * unlikely to be needed in most scenarios, as SHA-1 is typically used in the `certID` field. + * For more details about the `certID` field, refer to + * [RFC6960](https://datatracker.ietf.org/doc/html/rfc6960#section-4.1.1). + * * @param config The configuration object being updated * @param type The desired request status type * @returns S2N_SUCCESS on success. S2N_FAILURE on failure diff --git a/tls/s2n_x509_validator.c b/tls/s2n_x509_validator.c index 737d719ca26..2abc39632e7 100644 --- a/tls/s2n_x509_validator.c +++ b/tls/s2n_x509_validator.c @@ -874,13 +874,9 @@ S2N_RESULT s2n_x509_validator_validate_cert_stapled_ocsp_response(struct s2n_x50 int status = 0; int reason = 0; - /* SHA-1 is the only supported OCSP digest due to its wide compatibility and established use - * in existing systems. Supporting additional hash algorithms would require changes to error - * handling and compatibility checks, which are not currently justified by user demand. For - * verifying OCSP response with non-SHA-1 hash algorithm, users can call - * s2n_connection_get_ocsp_response() to retrieve the received OCSP stapling information for - * manual verification. - */ + /* SHA-1 is the only supported hash algorithm for the CertID due to its wide compatibility and + * established use in OCSP responders. + */ OCSP_CERTID *cert_id = OCSP_cert_to_id(EVP_sha1(), subject, issuer); RESULT_ENSURE_REF(cert_id); From e3c83472f4244edfeeaf2c0db9024c4c6727f04d Mon Sep 17 00:00:00 2001 From: Jou Ho Date: Fri, 5 Jul 2024 19:56:13 +0000 Subject: [PATCH 7/9] remove extra space --- api/s2n.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/s2n.h b/api/s2n.h index 8b4c5034c6d..878af94255f 100644 --- a/api/s2n.h +++ b/api/s2n.h @@ -998,7 +998,7 @@ S2N_API extern int s2n_config_set_verify_host_callback(struct s2n_config *config * be skipped. * * The default value is 1 if the underlying libCrypto implementation supports OCSP. - * + * * @param config The configuration object being updated * @param check_ocsp The desired OCSP response check configuration * @returns S2N_SUCCESS on success. S2N_FAILURE on failure From b541628d3ac9adafa9a4dfdb339707a2cc128daf Mon Sep 17 00:00:00 2001 From: Jou Ho Date: Fri, 5 Jul 2024 19:57:00 +0000 Subject: [PATCH 8/9] remove extra space --- api/s2n.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/s2n.h b/api/s2n.h index 878af94255f..2ed3ef22a3f 100644 --- a/api/s2n.h +++ b/api/s2n.h @@ -1111,7 +1111,7 @@ typedef enum { /** * Sets up a connection to request the certificate status of a peer during an SSL handshake. If set * to S2N_STATUS_REQUEST_NONE, no status request is made. - * + * * @note SHA-1 is the only supported hash algorithm for the `certID` field. This is different * from the hash algorithm used for the OCSP signature. If a different hash algorithm is used, * validation will fail. From 26e0afa45125be5b16d3b0095522b8302b601733 Mon Sep 17 00:00:00 2001 From: Jou Ho Date: Mon, 8 Jul 2024 17:05:36 +0000 Subject: [PATCH 9/9] address doc feedback --- api/s2n.h | 17 ++++++----------- tls/s2n_x509_validator.c | 4 ++-- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/api/s2n.h b/api/s2n.h index 2ed3ef22a3f..9c2776858e2 100644 --- a/api/s2n.h +++ b/api/s2n.h @@ -1112,17 +1112,12 @@ typedef enum { * Sets up a connection to request the certificate status of a peer during an SSL handshake. If set * to S2N_STATUS_REQUEST_NONE, no status request is made. * - * @note SHA-1 is the only supported hash algorithm for the `certID` field. This is different - * from the hash algorithm used for the OCSP signature. If a different hash algorithm is used, - * validation will fail. - * If the underlying libcrypto supports OCSP validation, the OCSP stapling information will - * be automatically validated. Users can disable this OCSP verification by calling - * `s2n_config_set_check_stapled_ocsp_response()` with "0" to turn OCSP validation off. Users may - * use `s2n_connection_get_ocsp_response()` together with OCSP validation turned off to retrieve - * the received OCSP stapling information for manual verification. This workaround is however - * unlikely to be needed in most scenarios, as SHA-1 is typically used in the `certID` field. - * For more details about the `certID` field, refer to - * [RFC6960](https://datatracker.ietf.org/doc/html/rfc6960#section-4.1.1). + * @note SHA-1 is the only supported hash algorithm for the `certID` field. This is different + * from the hash algorithm used for the OCSP signature. See + * [RFC 6960](https://datatracker.ietf.org/doc/html/rfc6960#section-4.1.1) for more information. + * While unlikely to be the case, if support for a different hash algorithm is required, the + * s2n-tls validation can be disabled with `s2n_config_set_check_stapled_ocsp_response()` and the + * response can be retrieved for manual validation with `s2n_connection_get_ocsp_response()`. * * @param config The configuration object being updated * @param type The desired request status type diff --git a/tls/s2n_x509_validator.c b/tls/s2n_x509_validator.c index 2abc39632e7..4de3c03b1ff 100644 --- a/tls/s2n_x509_validator.c +++ b/tls/s2n_x509_validator.c @@ -874,8 +874,8 @@ S2N_RESULT s2n_x509_validator_validate_cert_stapled_ocsp_response(struct s2n_x50 int status = 0; int reason = 0; - /* SHA-1 is the only supported hash algorithm for the CertID due to its wide compatibility and - * established use in OCSP responders. + /* SHA-1 is the only supported hash algorithm for the CertID due to its established use in + * OCSP responders. */ OCSP_CERTID *cert_id = OCSP_cert_to_id(EVP_sha1(), subject, issuer); RESULT_ENSURE_REF(cert_id);