refactor: switch JA3 to use stuffer hex methods (#4662)

tests/unit/s2n_fingerprint_test.c

@@ -291,7 +291,6 @@ int main(int argc, char **argv)
         EXPECT_EQUAL(fingerprint->method, &ja3_fingerprint);
         EXPECT_TRUE(s2n_hash_is_ready_for_input(&fingerprint->hash));
         EXPECT_NULL(fingerprint->client_hello);
-        EXPECT_FALSE(fingerprint->legacy_hash_format);
         EXPECT_EQUAL(fingerprint->raw_size, 0);
 
         /* Free cleans up the fingerprint */
@@ -313,15 +312,13 @@ int main(int argc, char **argv)
         EXPECT_NOT_NULL(fingerprint.method);
         EXPECT_TRUE(fingerprint.hash.is_ready_for_input);
         EXPECT_NOT_NULL(fingerprint.client_hello);
-        EXPECT_TRUE(fingerprint.legacy_hash_format);
         EXPECT_NOT_EQUAL(fingerprint.raw_size, 0);
 
         /* Verify that wipe only clears the expected fields */
         EXPECT_SUCCESS(s2n_fingerprint_wipe(&fingerprint));
         EXPECT_NOT_NULL(fingerprint.method);
         EXPECT_TRUE(fingerprint.hash.is_ready_for_input);
         EXPECT_NULL(fingerprint.client_hello);
-        EXPECT_TRUE(fingerprint.legacy_hash_format);
         EXPECT_EQUAL(fingerprint.raw_size, 0);
     };
 

tls/s2n_fingerprint.c

@@ -107,12 +107,7 @@ int s2n_fingerprint_get_hash(struct s2n_fingerprint *fingerprint,
     const struct s2n_fingerprint_method *method = fingerprint->method;
     POSIX_ENSURE_REF(method);
 
-    size_t min_output_size = method->hash_str_size;
-    if (fingerprint->legacy_hash_format) {
-        min_output_size /= 2;
-    }
-
-    POSIX_ENSURE(max_output_size >= min_output_size, S2N_ERR_INSUFFICIENT_MEM_SIZE);
+    POSIX_ENSURE(max_output_size >= method->hash_str_size, S2N_ERR_INSUFFICIENT_MEM_SIZE);
     POSIX_ENSURE(output, S2N_ERR_INVALID_ARGUMENT);
     POSIX_ENSURE(output_size, S2N_ERR_INVALID_ARGUMENT);
     *output_size = 0;
@@ -122,7 +117,6 @@ int s2n_fingerprint_get_hash(struct s2n_fingerprint *fingerprint,
 
     struct s2n_fingerprint_hash hash = {
         .hash = &fingerprint->hash,
-        .legacy_hash_format = fingerprint->legacy_hash_format,
     };
     POSIX_GUARD(s2n_hash_reset(&fingerprint->hash));
 
@@ -250,12 +244,30 @@ int s2n_client_hello_get_fingerprint_hash(struct s2n_client_hello *ch, s2n_finge
         uint32_t max_output_size, uint8_t *output, uint32_t *output_size, uint32_t *str_size)
 {
     POSIX_ENSURE(type == S2N_FINGERPRINT_JA3, S2N_ERR_INVALID_ARGUMENT);
+    POSIX_ENSURE(max_output_size >= MD5_DIGEST_LENGTH, S2N_ERR_INSUFFICIENT_MEM_SIZE);
     POSIX_ENSURE(str_size, S2N_ERR_INVALID_ARGUMENT);
-    DEFER_CLEANUP(struct s2n_fingerprint fingerprint = { .legacy_hash_format = true },
-            s2n_fingerprint_free_fields);
+    POSIX_ENSURE(output_size, S2N_ERR_INVALID_ARGUMENT);
+    POSIX_ENSURE(output, S2N_ERR_INVALID_ARGUMENT);
+
+    DEFER_CLEANUP(struct s2n_fingerprint fingerprint = { 0 }, s2n_fingerprint_free_fields);
     POSIX_GUARD_RESULT(s2n_fingerprint_init(&fingerprint, type));
     POSIX_GUARD(s2n_fingerprint_set_client_hello(&fingerprint, ch));
-    POSIX_GUARD(s2n_fingerprint_get_hash(&fingerprint, max_output_size, output, output_size));
+
+    uint32_t hex_hash_size = 0;
+    uint8_t hex_hash[S2N_JA3_HASH_STR_SIZE] = { 0 };
+    POSIX_GUARD(s2n_fingerprint_get_hash(&fingerprint, sizeof(hex_hash), hex_hash, &hex_hash_size));
+
+    /* s2n_client_hello_get_fingerprint_hash expects the raw bytes of the JA3 hash,
+     * but s2n_fingerprint_get_hash returns a hex string instead.
+     * We need to translate back to the raw bytes.
+     */
+    struct s2n_stuffer bytes_out = { 0 };
+    POSIX_GUARD(s2n_blob_init(&bytes_out.blob, output, max_output_size));
+    struct s2n_blob hex_in = { 0 };
+    POSIX_GUARD(s2n_blob_init(&hex_in, hex_hash, hex_hash_size));
+    POSIX_GUARD_RESULT(s2n_stuffer_read_hex(&bytes_out, &hex_in));
+    *output_size = s2n_stuffer_data_available(&bytes_out);
+
     POSIX_GUARD(s2n_fingerprint_get_raw_size(&fingerprint, str_size));
     return S2N_SUCCESS;
 }

tls/s2n_fingerprint.h

@@ -22,25 +22,19 @@
 #include "tls/s2n_client_hello.h"
 #include "utils/s2n_result.h"
 
+#define S2N_JA3_HASH_STR_SIZE (MD5_DIGEST_LENGTH * 2)
+
 struct s2n_fingerprint {
     size_t raw_size;
     const struct s2n_fingerprint_method *method;
     struct s2n_client_hello *client_hello;
     struct s2n_hash_state hash;
-    /* Originally we represented the hash as the raw bytes of the digest.
-     * However, the JA4 hash is composed of a string prefix and two digests,
-     * so cannot reasonably be represented as raw bytes.
-     * Keep the old behavior for the old method.
-     */
-    unsigned int legacy_hash_format : 1;
 };
 
 struct s2n_fingerprint_hash {
     uint32_t bytes_digested;
     struct s2n_stuffer *buffer;
     struct s2n_hash_state *hash;
-    /* See legacy_hash_format on s2n_fingerprint */
-    unsigned int legacy_hash_format : 1;
 };
 S2N_RESULT s2n_fingerprint_hash_add_char(struct s2n_fingerprint_hash *hash, char c);
 S2N_RESULT s2n_fingerprint_hash_add_str(struct s2n_fingerprint_hash *hash, const char *str, size_t str_size);

tls/s2n_fingerprint_ja3.c

@@ -24,33 +24,19 @@
 /* UINT16_MAX == 65535 */
 #define S2N_UINT16_STR_MAX_SIZE 5
 
-#define S2N_HEX_PER_BYTE 2
-
 static S2N_RESULT s2n_fingerprint_ja3_digest(struct s2n_fingerprint_hash *hash,
         struct s2n_stuffer *out)
 {
     if (!s2n_fingerprint_hash_do_digest(hash)) {
         return S2N_RESULT_OK;
     }
 
-    if (hash->legacy_hash_format) {
-        uint8_t *digest = s2n_stuffer_raw_write(out, MD5_DIGEST_LENGTH);
-        RESULT_GUARD_PTR(digest);
-        RESULT_GUARD(s2n_fingerprint_hash_digest(hash, digest, MD5_DIGEST_LENGTH));
-        return S2N_RESULT_OK;
-    }
+    uint8_t digest_bytes[MD5_DIGEST_LENGTH] = { 0 };
+    RESULT_GUARD(s2n_fingerprint_hash_digest(hash, digest_bytes, sizeof(digest_bytes)));
 
-    uint8_t digest[MD5_DIGEST_LENGTH] = { 0 };
-    RESULT_GUARD(s2n_fingerprint_hash_digest(hash, digest, sizeof(digest)));
-
-    /* We allocate enough memory for the trailing '\0', but it is not
-     * included in the return value of snprintf. */
-    char hex[S2N_HEX_PER_BYTE + 1] = { 0 };
-    for (size_t i = 0; i < sizeof(digest); i++) {
-        int written = snprintf(hex, sizeof(hex), "%.*x", S2N_HEX_PER_BYTE, digest[i]);
-        RESULT_ENSURE_EQ(written, S2N_HEX_PER_BYTE);
-        RESULT_GUARD_POSIX(s2n_stuffer_write_str(out, hex));
-    }
+    struct s2n_blob digest = { 0 };
+    RESULT_GUARD_POSIX(s2n_blob_init(&digest, digest_bytes, sizeof(digest_bytes)));
+    RESULT_GUARD(s2n_stuffer_write_hex(out, &digest));
 
     return S2N_RESULT_OK;
 }
@@ -222,6 +208,6 @@ struct s2n_fingerprint_method ja3_fingerprint = {
      * so the weakness of MD5 shouldn't be a problem. */
     .hash = S2N_HASH_MD5,
     /* The hash string is a single MD5 digest represented as hex */
-    .hash_str_size = MD5_DIGEST_LENGTH * S2N_HEX_PER_BYTE,
+    .hash_str_size = S2N_JA3_HASH_STR_SIZE,
     .fingerprint = s2n_fingerprint_ja3,
 };