Fixes after testing against OpenSSL

This adjusts the implementation to match the results of exporting key material in OpenSSL. Also adds keylog support for the exporter master secret, also matching OpenSSL.
aws · Oct 2, 2023 · 5a0559f · 5a0559f
1 parent 063066c
commit 5a0559f
Show file tree

Hide file tree

Showing 8 changed files with 29 additions and 13 deletions.
diff --git a/tests/unit/s2n_self_talk_quic_support_test.c b/tests/unit/s2n_self_talk_quic_support_test.c
@@ -18,7 +18,7 @@
 #include "tls/s2n_quic_support.h"
 
 #define S2N_MODE_COUNT        2
-#define S2N_SECRET_TYPE_COUNT 5
+#define S2N_SECRET_TYPE_COUNT 6
 
 static const uint8_t CLIENT_TRANSPORT_PARAMS[] = "client transport params";
 static const uint8_t SERVER_TRANSPORT_PARAMS[] = "server transport params";

diff --git a/tests/unit/s2n_tls13_key_schedule_test.c b/tests/unit/s2n_tls13_key_schedule_test.c
@@ -24,7 +24,7 @@
 #include "tls/s2n_tls13_secrets.c"
 
 #define NO_TRIGGER_MSG       APPLICATION_DATA
-#define TRAFFIC_SECRET_COUNT 5
+#define TRAFFIC_SECRET_COUNT 6
 
 static uint8_t empty_secret[S2N_TLS13_SECRET_MAX_LEN] = { 0 };
 

diff --git a/tests/unit/s2n_tls13_secrets_rfc8448_test.c b/tests/unit/s2n_tls13_secrets_rfc8448_test.c
@@ -537,10 +537,9 @@ int main(int argc, char **argv)
                 EXPECT_OK(s2n_connection_set_test_master_secret(conn, &master_secret));
                 EXPECT_OK(s2n_connection_set_test_transcript_hash(conn, SERVER_FINISHED, &hash));
 
-                EXPECT_OK(s2n_derive_exporter_master_secret(conn));
+                EXPECT_OK(s2n_derive_exporter_master_secret(conn, &derived_secret));
                 EXPECT_EQUAL(derived_secret.size, secret.size);
-                EXPECT_BYTEARRAY_EQUAL(conn->secrets.version.tls13.exporter_master_secret,
-                        secret.data, secret.size);
+                EXPECT_BYTEARRAY_EQUAL(derived_secret.data, secret.data, secret.size);
             }
         };
     };

diff --git a/tests/unit/s2n_tls13_secrets_test.c b/tests/unit/s2n_tls13_secrets_test.c
@@ -528,8 +528,9 @@ int main(int argc, char **argv)
                     (uint8_t *) "context",
                     sizeof("context"));
             EXPECT_SUCCESS(result);
-            S2N_BLOB_FROM_HEX(expected, "e8 39 08 03 29 9f 40 c5 51 04 46 74 ff \
-                    37 42 2f 3a 0c e5 8c 45 f3 87 99 f3 e1 29 5c ce 6f f8 ca");
+            S2N_BLOB_FROM_HEX(expected, "57 93 3f 4a 3c 4c 7b b6 1c 84 51 02 \
+                    f5 87 30 28 f2 9e e6 38 06 38 2a a3 1d 6a d8 e3 36 87 cb 84");
+            EXPECT_EQUAL(32, expected.size);
             EXPECT_BYTEARRAY_EQUAL(output, expected.data, expected.size);
         };
     };

diff --git a/tls/s2n_key_log.c b/tls/s2n_key_log.c
@@ -84,6 +84,7 @@ S2N_RESULT s2n_key_log_tls13_secret(struct s2n_connection *conn, const struct s2
     const uint8_t server_handshake_label[] = "SERVER_HANDSHAKE_TRAFFIC_SECRET ";
     const uint8_t client_traffic_label[] = "CLIENT_TRAFFIC_SECRET_0 ";
     const uint8_t server_traffic_label[] = "SERVER_TRAFFIC_SECRET_0 ";
+    const uint8_t exporter_secret_label[] = "EXPORTER_SECRET ";
 
     const uint8_t *label = NULL;
     uint8_t label_size = 0;
@@ -109,6 +110,10 @@ S2N_RESULT s2n_key_log_tls13_secret(struct s2n_connection *conn, const struct s2
             label = server_traffic_label;
             label_size = sizeof(server_traffic_label) - 1;
             break;
+        case S2N_EXPORTER_SECRET:
+            label = exporter_secret_label;
+            label_size = sizeof(exporter_secret_label) - 1;
+            break;
         default:
             /* Ignore the secret types we don't understand */
             return S2N_RESULT_OK;

diff --git a/tls/s2n_quic_support.h b/tls/s2n_quic_support.h
@@ -56,6 +56,7 @@ typedef enum {
     S2N_SERVER_HANDSHAKE_TRAFFIC_SECRET,
     S2N_CLIENT_APPLICATION_TRAFFIC_SECRET,
     S2N_SERVER_APPLICATION_TRAFFIC_SECRET,
+    S2N_EXPORTER_SECRET,
 } s2n_secret_type_t;
 
 /*

diff --git a/tls/s2n_tls13_secrets.c b/tls/s2n_tls13_secrets.c
@@ -527,7 +527,7 @@ S2N_RESULT s2n_derive_resumption_master_secret(struct s2n_connection *conn)
  *#           |                     ClientHello...server Finished)
  *#           |                     = exporter_master_secret
  */
-S2N_RESULT s2n_derive_exporter_master_secret(struct s2n_connection *conn)
+S2N_RESULT s2n_derive_exporter_master_secret(struct s2n_connection *conn, struct s2n_blob *secret)
 {
     RESULT_ENSURE_REF(conn);
     /* Secret derivation requires these fields to be non-null.  */
@@ -538,7 +538,7 @@ S2N_RESULT s2n_derive_exporter_master_secret(struct s2n_connection *conn)
             S2N_MASTER_SECRET,
             &s2n_tls13_label_exporter_master_secret,
             SERVER_FINISHED,
-            &CONN_SECRET(conn, exporter_master_secret)));
+            secret));
     return S2N_RESULT_OK;
 }
 
@@ -658,7 +658,15 @@ S2N_RESULT s2n_tls13_secrets_update(struct s2n_connection *conn)
                     S2N_CLIENT, &CONN_SECRET(conn, client_app_secret)));
             RESULT_GUARD(s2n_tls13_derive_secret(conn, S2N_MASTER_SECRET,
                     S2N_SERVER, &CONN_SECRET(conn, server_app_secret)));
-            RESULT_GUARD(s2n_derive_exporter_master_secret(conn));
+            RESULT_GUARD(s2n_derive_exporter_master_secret(conn,
+                        &CONN_SECRET(conn, exporter_master_secret)));
+
+            if (conn->secret_cb && (s2n_connection_is_quic_enabled(conn) || s2n_in_unit_test())) {
+                RESULT_GUARD_POSIX(conn->secret_cb(conn->secret_cb_context, conn, S2N_EXPORTER_SECRET,
+                            CONN_SECRET(conn, exporter_master_secret).data, CONN_SECRET(conn, exporter_master_secret).size));
+            }
+            s2n_result_ignore(s2n_key_log_tls13_secret(conn, &CONN_SECRET(conn, exporter_master_secret), S2N_EXPORTER_SECRET));
+    return S2N_RESULT_OK;
             break;
         case CLIENT_FINISHED:
             RESULT_GUARD(s2n_calculate_transcript_digest(conn));
@@ -720,9 +728,10 @@ int s2n_connection_tls_exporter(
 
     uint8_t derived_secret_bytes[S2N_TLS13_SECRET_MAX_LEN] = { 0 };
     struct s2n_blob derived_secret = { 0 };
-    POSIX_GUARD(s2n_blob_init(&derived_secret, derived_secret_bytes, S2N_TLS13_SECRET_MAX_LEN));
+    POSIX_GUARD(s2n_blob_init(&derived_secret, derived_secret_bytes,
+                s2n_get_hash_len(CONN_HMAC_ALG(conn))));
     POSIX_GUARD_RESULT(s2n_derive_secret(hmac_alg, &CONN_SECRET(conn, exporter_master_secret),
-            &label, &CONN_HASH(conn, transcript_hash_digest), &derived_secret));
+            &label, &EMPTY_CONTEXT(hmac_alg), &derived_secret));
 
     DEFER_CLEANUP(struct s2n_hmac_state hmac_state = { 0 }, s2n_hmac_free);
     POSIX_GUARD(s2n_hmac_new(&hmac_state));
@@ -735,6 +744,7 @@ int s2n_connection_tls_exporter(
     struct s2n_blob digest = EMPTY_CONTEXT(hmac_alg);
 
     POSIX_GUARD(s2n_hash_init(&hash, hash_alg));
+    POSIX_GUARD(s2n_hash_update(&hash, context, context_length));
     POSIX_GUARD(s2n_hash_digest(&hash, digest.data, digest.size));
 
     struct s2n_blob output = { 0 };

diff --git a/tls/s2n_tls13_secrets.h b/tls/s2n_tls13_secrets.h
@@ -54,4 +54,4 @@ S2N_RESULT s2n_tls13_secrets_clean(struct s2n_connection *conn);
 
 S2N_RESULT s2n_derive_binder_key(struct s2n_psk *psk, struct s2n_blob *output);
 S2N_RESULT s2n_derive_resumption_master_secret(struct s2n_connection *conn);
-S2N_RESULT s2n_derive_exporter_master_secret(struct s2n_connection *conn);
+S2N_RESULT s2n_derive_exporter_master_secret(struct s2n_connection *conn, struct s2n_blob *output);