diff --git a/crypto/s2n_certificate.c b/crypto/s2n_certificate.c index f4df87cb468..f1a71a0332e 100755 --- a/crypto/s2n_certificate.c +++ b/crypto/s2n_certificate.c @@ -420,6 +420,14 @@ int s2n_cert_chain_and_key_load_pem_bytes(struct s2n_cert_chain_and_key *chain_a return S2N_SUCCESS; } +S2N_CLEANUP_RESULT s2n_cert_chain_and_key_ptr_free(struct s2n_cert_chain_and_key **cert_and_key) +{ + RESULT_ENSURE_REF(cert_and_key); + RESULT_GUARD_POSIX(s2n_cert_chain_and_key_free(*cert_and_key)); + *cert_and_key = NULL; + return S2N_RESULT_OK; +} + int s2n_cert_chain_and_key_free(struct s2n_cert_chain_and_key *cert_and_key) { if (cert_and_key == NULL) { diff --git a/crypto/s2n_certificate.h b/crypto/s2n_certificate.h index a4b9c84ba98..e78601c1aed 100644 --- a/crypto/s2n_certificate.h +++ b/crypto/s2n_certificate.h @@ -67,6 +67,7 @@ int s2n_cert_chain_and_key_load_cns(struct s2n_cert_chain_and_key *chain_and_key int s2n_cert_chain_and_key_load_sans(struct s2n_cert_chain_and_key *chain_and_key, X509 *x509_cert); int s2n_cert_chain_and_key_matches_dns_name(const struct s2n_cert_chain_and_key *chain_and_key, const struct s2n_blob *dns_name); +S2N_CLEANUP_RESULT s2n_cert_chain_and_key_ptr_free(struct s2n_cert_chain_and_key **cert_and_key); int s2n_cert_set_cert_type(struct s2n_cert *cert, s2n_pkey_type pkey_type); int s2n_send_cert_chain(struct s2n_connection *conn, struct s2n_stuffer *out, struct s2n_cert_chain_and_key *chain_and_key); int s2n_send_empty_cert_chain(struct s2n_stuffer *out); diff --git a/tests/unit/s2n_certificate_test.c b/tests/unit/s2n_certificate_test.c index f59c15646a9..1cc9c7fa1a1 100644 --- a/tests/unit/s2n_certificate_test.c +++ b/tests/unit/s2n_certificate_test.c @@ -42,6 +42,11 @@ static uint8_t verify_host_fn(const char *host_name, size_t host_name_len, void return verify_data->allow; } +static uint8_t always_verify_host_fn(const char *host_name, size_t host_name_len, void *data) +{ + return true; +} + static int s2n_noop_async_pkey_fn(struct s2n_connection *conn, struct s2n_async_pkey_op *op) { return S2N_SUCCESS; @@ -100,9 +105,13 @@ int main(int argc, char **argv) const uint8_t ocsp_data[] = "ocsp data"; - struct s2n_cert_chain_and_key *chain_and_key = NULL; - EXPECT_SUCCESS( - s2n_test_cert_chain_and_key_new(&chain_and_key, S2N_DEFAULT_TEST_CERT_CHAIN, S2N_DEFAULT_TEST_PRIVATE_KEY)); + DEFER_CLEANUP(struct s2n_cert_chain_and_key *chain_and_key = NULL, s2n_cert_chain_and_key_ptr_free); + EXPECT_SUCCESS(s2n_test_cert_chain_and_key_new(&chain_and_key, + S2N_DEFAULT_TEST_CERT_CHAIN, S2N_DEFAULT_TEST_PRIVATE_KEY)); + + DEFER_CLEANUP(struct s2n_cert_chain_and_key *ecdsa_chain_and_key = NULL, s2n_cert_chain_and_key_ptr_free); + EXPECT_SUCCESS(s2n_test_cert_chain_and_key_new(&ecdsa_chain_and_key, + S2N_DEFAULT_ECDSA_TEST_CERT_CHAIN, S2N_DEFAULT_ECDSA_TEST_PRIVATE_KEY)); /* Test s2n_cert_chain_and_key_load_public_pem_bytes */ { @@ -624,6 +633,100 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_cert_chain_and_key_free(custom_cert_chain)); } - EXPECT_SUCCESS(s2n_cert_chain_and_key_free(chain_and_key)); + /* Test s2n_connection_get_client_cert_chain */ + { + DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free); + EXPECT_SUCCESS(s2n_config_set_client_auth_type(config, S2N_CERT_AUTH_REQUIRED)); + EXPECT_SUCCESS(s2n_config_set_verify_host_callback(config, always_verify_host_fn, NULL)); + EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, ecdsa_chain_and_key)); + EXPECT_SUCCESS(s2n_config_set_verification_ca_location(config, S2N_DEFAULT_ECDSA_TEST_CERT_CHAIN, NULL)); + + DEFER_CLEANUP(struct s2n_connection *tls12_client_conn = s2n_connection_new(S2N_CLIENT), s2n_connection_ptr_free); + DEFER_CLEANUP(struct s2n_connection *tls12_server_conn = s2n_connection_new(S2N_SERVER), s2n_connection_ptr_free); + DEFER_CLEANUP(struct s2n_connection *tls13_client_conn = s2n_connection_new(S2N_CLIENT), s2n_connection_ptr_free); + DEFER_CLEANUP(struct s2n_connection *tls13_server_conn = s2n_connection_new(S2N_SERVER), s2n_connection_ptr_free); + + /* Should error before the handshake is performed. + * This method is intended to be called after the handshake is complete and requires + * state set during the handshake. + */ + { + uint8_t *output = NULL; + uint32_t output_len = 0; + EXPECT_FAILURE_WITH_ERRNO(s2n_connection_get_client_cert_chain(tls12_server_conn, &output, &output_len), + S2N_ERR_NULL); + EXPECT_FAILURE_WITH_ERRNO(s2n_connection_get_client_cert_chain(tls13_server_conn, &output, &output_len), + S2N_ERR_NULL); + EXPECT_NULL(output); + EXPECT_EQUAL(output_len, 0); + } + + /* Perform TLS1.2 handshake and verify cert chain is available */ + { + EXPECT_SUCCESS(s2n_connection_set_config(tls12_client_conn, config)); + EXPECT_SUCCESS(s2n_connection_set_config(tls12_server_conn, config)); + EXPECT_SUCCESS(s2n_connection_set_cipher_preferences(tls12_client_conn, "test_all_tls12")); + EXPECT_SUCCESS(s2n_connection_set_cipher_preferences(tls12_server_conn, "test_all_tls12")); + + struct s2n_test_io_pair io_pair = { 0 }; + EXPECT_SUCCESS(s2n_io_pair_init_non_blocking(&io_pair)); + EXPECT_SUCCESS(s2n_connections_set_io_pair(tls12_client_conn, tls12_server_conn, &io_pair)); + EXPECT_SUCCESS(s2n_negotiate_test_server_and_client(tls12_server_conn, tls12_client_conn)); + + EXPECT_EQUAL(tls12_server_conn->actual_protocol_version, S2N_TLS12); + EXPECT_NOT_NULL(tls12_server_conn->handshake_params.client_cert_chain.data); + EXPECT_NOT_EQUAL(tls12_server_conn->handshake_params.client_cert_chain.size, 0); + } + + /* Perform TLS1.3 handshake and verify cert chain is NOT available. + * + * The TLS1.3 handshake is only possible if TLS1.3 is fully supported because of client auth: + * the server doesn't know whether the client will offer a RSA-PSS certificate or not. + */ + if (s2n_is_tls13_fully_supported()) { + EXPECT_SUCCESS(s2n_connection_set_config(tls13_client_conn, config)); + EXPECT_SUCCESS(s2n_connection_set_config(tls13_server_conn, config)); + EXPECT_SUCCESS(s2n_connection_set_cipher_preferences(tls13_client_conn, "default_tls13")); + EXPECT_SUCCESS(s2n_connection_set_cipher_preferences(tls13_server_conn, "default_tls13")); + + struct s2n_test_io_pair io_pair = { 0 }; + EXPECT_SUCCESS(s2n_io_pair_init_non_blocking(&io_pair)); + EXPECT_SUCCESS(s2n_connections_set_io_pair(tls13_client_conn, tls13_server_conn, &io_pair)); + EXPECT_SUCCESS(s2n_negotiate_test_server_and_client(tls13_server_conn, tls13_client_conn)); + + EXPECT_EQUAL(tls13_server_conn->actual_protocol_version, S2N_TLS13); + EXPECT_NOT_NULL(tls13_server_conn->handshake_params.client_cert_chain.data); + EXPECT_NOT_EQUAL(tls13_server_conn->handshake_params.client_cert_chain.size, 0); + } + + /* Should error if called by client */ + { + uint8_t *output = NULL; + uint32_t output_len = 0; + EXPECT_FAILURE_WITH_ERRNO(s2n_connection_get_client_cert_chain(tls12_client_conn, &output, &output_len), + S2N_ERR_NULL); + EXPECT_FAILURE_WITH_ERRNO(s2n_connection_get_client_cert_chain(tls13_client_conn, &output, &output_len), + S2N_ERR_NULL); + EXPECT_NULL(output); + EXPECT_EQUAL(output_len, 0); + } + + /* Should produce same result for TLS1.2 and TLS1.3 + * (Both connections used the same certificate chain for the handshake) + */ + if (s2n_is_tls13_fully_supported()) { + uint8_t *tls12_output = NULL; + uint32_t tls12_output_len = 0; + EXPECT_SUCCESS(s2n_connection_get_client_cert_chain(tls12_server_conn, &tls12_output, &tls12_output_len)); + + uint8_t *tls13_output = NULL; + uint32_t tls13_output_len = 0; + EXPECT_SUCCESS(s2n_connection_get_client_cert_chain(tls13_server_conn, &tls13_output, &tls13_output_len)); + + EXPECT_EQUAL(tls12_output_len, tls13_output_len); + EXPECT_BYTEARRAY_EQUAL(tls12_output, tls13_output, tls13_output_len); + } + } + END_TEST(); } diff --git a/tls/s2n_client_cert.c b/tls/s2n_client_cert.c index fffe32639ae..757b13430d8 100644 --- a/tls/s2n_client_cert.c +++ b/tls/s2n_client_cert.c @@ -27,6 +27,71 @@ #include "utils/s2n_blob.h" #include "utils/s2n_safety.h" +/* In TLS1.2, the certificate list is just an opaque vector of certificates: + * + * opaque ASN.1Cert<1..2^24-1>; + * + * struct { + * ASN.1Cert certificate_list<0..2^24-1>; + * } Certificate; + * + * This construction allowed us to store the entire certificate_list blob + * and return it from the s2n_connection_get_client_cert_chain method for + * customers to examine. + * + * However, TLS1.3 introduced per-certificate extensions: + * + * struct { + * opaque cert_data<1..2^24-1>; + * ----> Extension extensions<0..2^16-1>; <---- + * } CertificateEntry; + * + * struct { + * opaque certificate_request_context<0..2^8-1>; + * CertificateEntry certificate_list<0..2^24-1>; + * } Certificate; + * + * So in order to store / return the certificates in the same format as in TLS1.2, + * we need to first strip out the extensions. + */ +static S2N_RESULT s2n_client_cert_chain_store(struct s2n_connection *conn, struct s2n_blob *client_cert_chain) +{ + RESULT_ENSURE_REF(conn); + + /* Earlier versions are a basic copy */ + if (conn->actual_protocol_version < S2N_TLS13) { + RESULT_GUARD_POSIX(s2n_dup(client_cert_chain, &conn->handshake_params.client_cert_chain)); + return S2N_RESULT_OK; + } + + struct s2n_stuffer cert_chain_in = { 0 }; + RESULT_GUARD_POSIX(s2n_stuffer_init(&cert_chain_in, client_cert_chain)); + RESULT_GUARD_POSIX(s2n_stuffer_skip_write(&cert_chain_in, client_cert_chain->size)); + + struct s2n_stuffer cert_chain_out = { 0 }; + RESULT_GUARD_POSIX(s2n_realloc(&conn->handshake_params.client_cert_chain, client_cert_chain->size)); + RESULT_GUARD_POSIX(s2n_stuffer_init(&cert_chain_out, &conn->handshake_params.client_cert_chain)); + + uint32_t cert_size = 0; + uint16_t extensions_size = 0; + while(s2n_stuffer_data_available(&cert_chain_in)) { + RESULT_GUARD_POSIX(s2n_stuffer_read_uint24(&cert_chain_in, &cert_size)); + RESULT_GUARD_POSIX(s2n_stuffer_write_uint24(&cert_chain_out, cert_size)); + RESULT_GUARD_POSIX(s2n_stuffer_copy(&cert_chain_in, &cert_chain_out, cert_size)); + + /* The new TLS1.3 format includes extensions, which we must skip. + * Customers will not expect TLS extensions in a DER-encoded certificate. + */ + RESULT_GUARD_POSIX(s2n_stuffer_read_uint16(&cert_chain_in, &extensions_size)); + RESULT_GUARD_POSIX(s2n_stuffer_skip_read(&cert_chain_in, extensions_size)); + } + + /* We will have allocated more memory than actually necessary. + * If this becomes a problem, we should consider reallocing the correct amount of memory here. + */ + conn->handshake_params.client_cert_chain.size = s2n_stuffer_data_available(&cert_chain_out); + return S2N_RESULT_OK; +} int s2n_client_cert_recv(struct s2n_connection *conn) { @@ -65,10 +130,10 @@ int s2n_client_cert_recv(struct s2n_connection *conn) POSIX_GUARD(s2n_pkey_setup_for_type(&public_key, pkey_type)); POSIX_GUARD(s2n_pkey_check_key_exists(&public_key)); - POSIX_GUARD(s2n_dup(&client_cert_chain, &conn->handshake_params.client_cert_chain)); + POSIX_GUARD_RESULT(s2n_client_cert_chain_store(conn, &client_cert_chain)); conn->handshake_params.client_public_key = public_key; - - return 0; + + return S2N_SUCCESS; } @@ -76,7 +141,7 @@ int s2n_client_cert_send(struct s2n_connection *conn) { struct s2n_cert_chain_and_key *chain_and_key = conn->handshake_params.our_chain_and_key; - if (conn->actual_protocol_version == S2N_TLS13) { + if (conn->actual_protocol_version >= S2N_TLS13) { /* If this message is in response to a CertificateRequest, the value of * certificate_request_context in that message. * https://tools.ietf.org/html/rfc8446#section-4.4.2 @@ -95,5 +160,5 @@ int s2n_client_cert_send(struct s2n_connection *conn) } POSIX_GUARD(s2n_send_cert_chain(conn, &conn->handshake.io, chain_and_key)); - return 0; + return S2N_SUCCESS; } diff --git a/tls/s2n_config.c b/tls/s2n_config.c index 33b3a0ac431..ab648a9c5ba 100755 --- a/tls/s2n_config.c +++ b/tls/s2n_config.c @@ -334,6 +334,14 @@ int s2n_config_free_dhparams(struct s2n_config *config) return 0; } +S2N_CLEANUP_RESULT s2n_config_ptr_free(struct s2n_config **config) +{ + RESULT_ENSURE_REF(config); + RESULT_GUARD_POSIX(s2n_config_free(*config)); + *config = NULL; + return S2N_RESULT_OK; +} + int s2n_config_free(struct s2n_config *config) { s2n_config_cleanup(config); diff --git a/tls/s2n_config.h b/tls/s2n_config.h index 929d0dab7f8..e62d3209602 100644 --- a/tls/s2n_config.h +++ b/tls/s2n_config.h @@ -146,6 +146,8 @@ struct s2n_config { s2n_async_pkey_validation_mode async_pkey_validation_mode; }; +S2N_CLEANUP_RESULT s2n_config_ptr_free(struct s2n_config **config); + int s2n_config_defaults_init(void); extern struct s2n_config *s2n_fetch_default_config(void); int s2n_config_set_unsafe_for_testing(struct s2n_config *config); diff --git a/tls/s2n_connection.c b/tls/s2n_connection.c index 2fefa9338bd..0925307f28b 100755 --- a/tls/s2n_connection.c +++ b/tls/s2n_connection.c @@ -332,6 +332,14 @@ static uint8_t s2n_default_verify_host(const char *host_name, size_t len, void * return 0; } +S2N_CLEANUP_RESULT s2n_connection_ptr_free(struct s2n_connection **conn) +{ + RESULT_ENSURE_REF(conn); + RESULT_GUARD_POSIX(s2n_connection_free(*conn)); + *conn = NULL; + return S2N_RESULT_OK; +} + int s2n_connection_free(struct s2n_connection *conn) { POSIX_GUARD(s2n_connection_wipe_keys(conn)); diff --git a/tls/s2n_connection.h b/tls/s2n_connection.h index ba86728f16b..408cd4b4afa 100644 --- a/tls/s2n_connection.h +++ b/tls/s2n_connection.h @@ -381,6 +381,8 @@ struct s2n_connection { uint32_t server_keying_material_lifetime; }; +S2N_CLEANUP_RESULT s2n_connection_ptr_free(struct s2n_connection **s2n_connection); + int s2n_connection_is_managed_corked(const struct s2n_connection *s2n_connection); int s2n_connection_is_client_auth_enabled(struct s2n_connection *s2n_connection);