Fix s2n_connection_get_client_cert_chain for TLS1.3 (#3156)

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) {

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);

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();
 }

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,18 +130,18 @@ 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;
 }
 
 
 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;
 }

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);

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);

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));

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);