fix: initial config should not influence sslv2

Co-authored-by: maddeleine <[email protected]>

tests/unit/s2n_client_hello_recv_test.c

@@ -102,7 +102,7 @@ int main(int argc, char **argv)
         EXPECT_SUCCESS(s2n_stuffer_write(&server_conn->handshake.io, &client_hello));
         EXPECT_SUCCESS(s2n_client_hello_recv(server_conn));
 
-        EXPECT_EQUAL(server_conn->server_protocol_version, i == 0 ? S2N_TLS12 : S2N_TLS13);
+        EXPECT_EQUAL(server_conn->server_protocol_version, S2N_TLS12);
         EXPECT_EQUAL(server_conn->actual_protocol_version, S2N_TLS12);
         EXPECT_EQUAL(server_conn->client_protocol_version, S2N_TLS12);
         EXPECT_EQUAL(server_conn->client_hello_version, S2N_SSLv2);

tests/unit/s2n_config_test.c

@@ -16,6 +16,7 @@
 #include "tls/s2n_config.h"
 
 #include <stdlib.h>
+#include <testlib/s2n_sslv2_client_hello.h>
 
 #include "api/s2n.h"
 #include "crypto/s2n_fips.h"
@@ -27,6 +28,7 @@
 #include "tls/s2n_internal.h"
 #include "tls/s2n_record.h"
 #include "tls/s2n_security_policies.h"
+#include "tls/s2n_tls.h"
 #include "tls/s2n_tls13.h"
 #include "unstable/npn.h"
 #include "utils/s2n_map.h"
@@ -1232,5 +1234,44 @@ int main(int argc, char **argv)
         }
     }
 
+    /* Checks that servers don't use a config before the client hello callback is executed on a
+     * SSLv2-formatted client hello.
+     *
+     * Parsing SSLv2 hellos uses a different code path and need to be tested separately.
+     */
+    {
+        uint8_t sslv2_client_hello[] = {
+            SSLv2_CLIENT_HELLO_PREFIX,
+            SSLv2_CLIENT_HELLO_CIPHER_SUITES,
+            SSLv2_CLIENT_HELLO_CHALLENGE,
+        };
+
+        struct s2n_blob client_hello = {
+            .data = sslv2_client_hello,
+            .size = sizeof(sslv2_client_hello),
+            .allocated = 0,
+            .growable = 0
+        };
+
+        /* Checks that the handshake gets as far as the client hello callback with a NULL config */
+        {
+            DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER),
+                    s2n_connection_ptr_free);
+            EXPECT_NOT_NULL(server_conn);
+            server_conn->config = NULL;
+
+            /* Record version and protocol version are in the header for SSLv2 */
+            server_conn->client_hello_version = S2N_SSLv2;
+            server_conn->client_protocol_version = S2N_TLS12;
+
+            /* S2N_ERR_CONFIG_NULL_BEFORE_CH_CALLBACK is only called in one location, just before the
+             * client hello callback. Therefore, we can assert that if we hit this error, we
+             * have gotten as far as the client hello callback without dereferencing the config.
+             */
+            EXPECT_SUCCESS(s2n_stuffer_write(&server_conn->handshake.io, &client_hello));
+            EXPECT_FAILURE_WITH_ERRNO(s2n_client_hello_recv(server_conn), S2N_ERR_CONFIG_NULL_BEFORE_CH_CALLBACK);
+        }
+    }
+
     END_TEST();
 }

tls/s2n_client_hello.c

@@ -446,7 +446,7 @@ int s2n_parse_client_hello(struct s2n_connection *conn)
     }
 
     if (conn->client_hello_version == S2N_SSLv2) {
-        POSIX_GUARD(s2n_sslv2_client_hello_recv(conn));
+        POSIX_GUARD(s2n_sslv2_client_hello_parse(conn));
         return S2N_SUCCESS;
     }
 
@@ -594,8 +594,13 @@ int s2n_process_client_hello(struct s2n_connection *conn)
     POSIX_CHECKED_MEMCPY(previous_cipher_suite_iana, conn->secure->cipher_suite->iana_value, S2N_TLS_CIPHER_SUITE_LEN);
 
     /* Now choose the ciphers we have certs for. */
-    POSIX_GUARD(s2n_set_cipher_as_tls_server(conn, client_hello->cipher_suites.data,
-            client_hello->cipher_suites.size / 2));
+    if (conn->client_hello_version == S2N_SSLv2) {
+        POSIX_GUARD(s2n_set_cipher_as_sslv2_server(conn, client_hello->cipher_suites.data,
+                client_hello->cipher_suites.size / S2N_SSLv2_CIPHER_SUITE_LEN));
+    } else {
+        POSIX_GUARD(s2n_set_cipher_as_tls_server(conn, client_hello->cipher_suites.data,
+                client_hello->cipher_suites.size / 2));
+    }
 
     /* Check if this is the second client hello in a hello retry handshake */
     if (s2n_is_hello_retry_handshake(conn) && conn->handshake.message_number > 0) {
@@ -685,9 +690,7 @@ int s2n_client_hello_recv(struct s2n_connection *conn)
         }
     }
 
-    if (conn->client_hello_version != S2N_SSLv2) {
-        POSIX_GUARD(s2n_process_client_hello(conn));
-    }
+    POSIX_GUARD(s2n_process_client_hello(conn));
 
     return 0;
 }
@@ -821,7 +824,7 @@ int s2n_client_hello_send(struct s2n_connection *conn)
  * Alternatively, the TLS1.0 RFC includes a more modern description of the format:
  * https://tools.ietf.org/rfc/rfc2246 Appendix E.1
  */
-int s2n_sslv2_client_hello_recv(struct s2n_connection *conn)
+int s2n_sslv2_client_hello_parse(struct s2n_connection *conn)
 {
     struct s2n_client_hello *client_hello = &conn->client_hello;
     client_hello->sslv2 = true;
@@ -831,15 +834,6 @@ int s2n_sslv2_client_hello_recv(struct s2n_connection *conn)
     POSIX_GUARD(s2n_stuffer_skip_write(&in_stuffer, client_hello->raw_message.size));
     struct s2n_stuffer *in = &in_stuffer;
 
-    const struct s2n_security_policy *security_policy = NULL;
-    POSIX_GUARD(s2n_connection_get_security_policy(conn, &security_policy));
-
-    if (conn->client_protocol_version < security_policy->minimum_protocol_version) {
-        POSIX_GUARD(s2n_queue_reader_unsupported_protocol_version_alert(conn));
-        POSIX_BAIL(S2N_ERR_PROTOCOL_VERSION_UNSUPPORTED);
-    }
-    conn->actual_protocol_version = MIN(conn->client_protocol_version, conn->server_protocol_version);
-
     /* We start 5 bytes into the record */
     uint16_t cipher_suites_length = 0;
     POSIX_GUARD(s2n_stuffer_read_uint16(in, &cipher_suites_length));
@@ -858,14 +852,6 @@ int s2n_sslv2_client_hello_recv(struct s2n_connection *conn)
     client_hello->cipher_suites.data = s2n_stuffer_raw_read(in, cipher_suites_length);
     POSIX_ENSURE_REF(client_hello->cipher_suites.data);
 
-    /* Find potential certificate matches before we choose the cipher. */
-    POSIX_GUARD(s2n_conn_find_name_matching_certs(conn));
-
-    POSIX_GUARD(s2n_set_cipher_as_sslv2_server(conn, client_hello->cipher_suites.data,
-            client_hello->cipher_suites.size / S2N_SSLv2_CIPHER_SUITE_LEN));
-    POSIX_GUARD_RESULT(s2n_signature_algorithm_select(conn));
-    POSIX_GUARD(s2n_select_certs_for_server_auth(conn, &conn->handshake_params.our_chain_and_key));
-
     S2N_ERROR_IF(session_id_length > s2n_stuffer_data_available(in), S2N_ERR_BAD_MESSAGE);
     POSIX_GUARD(s2n_blob_init(&client_hello->session_id, s2n_stuffer_raw_read(in, session_id_length), session_id_length));
     if (session_id_length > 0 && session_id_length <= S2N_TLS_SESSION_ID_MAX_LEN) {

tls/s2n_record_read.c

@@ -85,7 +85,7 @@ int s2n_sslv2_record_header_parse(
      *
      * The protocol version read here will likely not be SSLv2, since we only
      * accept SSLv2 ClientHellos offering higher protocol versions.
-     * See s2n_sslv2_client_hello_recv.
+     * See s2n_sslv2_client_hello_parse.
      */
     uint8_t protocol_version[S2N_TLS_PROTOCOL_VERSION_LEN] = { 0 };
     POSIX_GUARD(s2n_stuffer_read_bytes(header_in, protocol_version, S2N_TLS_PROTOCOL_VERSION_LEN));

tls/s2n_tls.h

@@ -27,9 +27,10 @@ int s2n_flush(struct s2n_connection *conn, s2n_blocked_status *more);
 S2N_RESULT s2n_client_hello_request_validate(struct s2n_connection *conn);
 S2N_RESULT s2n_client_hello_request_recv(struct s2n_connection *conn);
 int s2n_client_hello_send(struct s2n_connection *conn);
+int s2n_parse_client_hello(struct s2n_connection *conn);
 int s2n_client_hello_recv(struct s2n_connection *conn);
 int s2n_establish_session(struct s2n_connection *conn);
-int s2n_sslv2_client_hello_recv(struct s2n_connection *conn);
+int s2n_sslv2_client_hello_parse(struct s2n_connection *conn);
 int s2n_server_hello_retry_send(struct s2n_connection *conn);
 int s2n_server_hello_retry_recv(struct s2n_connection *conn);
 int s2n_server_hello_write_message(struct s2n_connection *conn);