Fix JA4 cipher counting

src/ngx_http_ssl_ja4_module.c

@@ -140,15 +140,15 @@ int ngx_ssl_ja4(ngx_connection_t *c, ngx_pool_t *pool, ngx_ssl_ja4_t *ja4)
     ja4->ciphers_sz = 0;
 
 
-    int tls_cipher_len = SSL_get0_raw_cipherlist (c->ssl->connection, NULL);
-
-    STACK_OF(SSL_CIPHER) * cp;
-    cp = SSL_get_client_ciphers (c->ssl->connection);
-    if (!cp) {
+    const unsigned char *raw_ciphers = NULL;
+    size_t tls_cipher_len = SSL_get0_raw_cipherlist(ssl, &raw_ciphers);
+    if (!raw_ciphers || tls_cipher_len < 2) {
         return NGX_DECLINED;
     }
 
-    ja4->ciphers = ngx_pnalloc (pool, sk_SSL_CIPHER_num(cp) * sizeof(char*));
+    size_t raw_cipher_count = tls_cipher_len / 2;
+
+    ja4->ciphers = ngx_pnalloc(pool, raw_cipher_count * sizeof(char *));
     if (ja4->ciphers == NULL) {
         return NGX_DECLINED;
     }
@@ -161,13 +161,12 @@ int ngx_ssl_ja4(ngx_connection_t *c, ngx_pool_t *pool, ngx_ssl_ja4_t *ja4)
     }
 
     size_t *k = &ja4->ciphers_sz;
-    for (i = 0; i < (size_t)sk_SSL_CIPHER_num(cp); i++)
+    for (i = 0; i + 1 < tls_cipher_len; i += 2)
     {
-        char hex [tls_cipher_len * sizeof (char) * 2 + 1];
-        const SSL_CIPHER *c = sk_SSL_CIPHER_value(cp, i);
-        u_int16_t id = SSL_CIPHER_get_protocol_id(c);
+        char hex[5];
+        u_int16_t id = ((u_int16_t) raw_ciphers[i] << 8) | raw_ciphers[i + 1];
 
-        ngx_sprintf ((u_char *)&hex[0], "%04xd", id);
+        ngx_sprintf((u_char *)&hex[0], "%04xd", id);
         hex [4] = '\0';
         if (ngx_ssl_ja4_is_ext_greased (hex)) {
             continue;
@@ -502,7 +501,11 @@ void ngx_ssl_ja4_fp(ngx_pool_t *pool, ngx_ssl_ja4_t *ja4, ngx_str_t *out)
     out->data[cur++] = ja4->has_sni;
 
     // 2 character count of ciphers
-    ngx_snprintf (out->data + cur, 3, "%02d", ja4->ciphers_sz);
+    size_t ciphers_sz = ja4->ciphers_sz;
+    if (ciphers_sz > 99) {
+        ciphers_sz = 99;
+    }
+    ngx_snprintf (out->data + cur, 3, "%02d", ciphers_sz);
     cur += 2;
 
     // 2 character count of extensions
@@ -648,13 +651,17 @@ void ngx_ssl_ja4_fp_string(ngx_pool_t *pool, ngx_ssl_ja4_t *ja4, ngx_str_t *out)
     out->data[cur++] = ja4->has_sni;
 
     // 2 character count of ciphers
-    if (ja4->ciphers_sz == 0)
+    size_t ciphers_sz = ja4->ciphers_sz;
+    if (ciphers_sz == 0)
     {
         ngx_snprintf(out->data + cur, 3, "00");
     }
     else
     {
-        ngx_snprintf(out->data + cur, 3, "%02zu", ja4->ciphers_sz);
+        if (ciphers_sz > 99) {
+            ciphers_sz = 99;
+        }
+        ngx_snprintf(out->data + cur, 3, "%02zu", ciphers_sz);
     }
     cur += 2;
 
@@ -799,13 +806,17 @@ void ngx_ssl_ja4one_fp(ngx_pool_t *pool, ngx_ssl_ja4_t *ja4, ngx_str_t *out)
     out->data[cur++] = ja4->has_sni;
 
     // 2 character count of ciphers
-    if (ja4->ciphers_sz == 0)
+    size_t ciphers_sz = ja4->ciphers_sz;
+    if (ciphers_sz == 0)
     {
         ngx_snprintf(out->data + cur, 3, "00");
     }
     else
     {
-        ngx_snprintf(out->data + cur, 3, "%02zu", ja4->ciphers_sz);
+        if (ciphers_sz > 99) {
+            ciphers_sz = 99;
+        }
+        ngx_snprintf(out->data + cur, 3, "%02zu", ciphers_sz);
     }
     cur += 2;
 

test/test_integration.py

@@ -1,3 +1,4 @@
+import re
 import subprocess
 import pytest
 from pathlib import Path
@@ -14,6 +15,7 @@
     ("tls12_h11",  ["--http1.1", "--tls-max", "1.2"]),
     ("no_sni_ip",  []),  # IP literal to avoid SNI
     ("ech_alps",   ["--python-test"]),  # Test ECH and ALPS extensions together
+    ("invalid_cipher_count", ["--go-invalid-cipher-test"]),  # Unknown cipher in list
 ]
 
 EXPECTED_DIR = Path(__file__).parent / "testdata"
@@ -34,6 +36,15 @@ def run_curl(name: str, args: list[str]) -> str:
         cmd = [sys.executable, str(test_script)]
         result = subprocess.run(cmd, check=True, capture_output=True, text=True)
         return result.stdout
+
+    # Go uTLS test for invalid cipher counting
+    if args and args[0] == "--go-invalid-cipher-test":
+        utls_dir = Path(__file__).parent / "utls"
+        cmd = ["go", "run", "./invalid_cipher_count.go"]
+        result = subprocess.run(
+            cmd, check=True, capture_output=True, text=True, cwd=utls_dir
+        )
+        return result.stdout
 
     # Standard curl test
     if name == "no_sni_ip":
@@ -52,6 +63,23 @@ def run_curl(name: str, args: list[str]) -> str:
 def test_integration(name, curl_args, request):
     output = run_curl(name, curl_args)
     print(f"\n=== Output for {name} ===\n{output}")
+
+    if name == "invalid_cipher_count":
+        lines = output.splitlines()
+        expected_line = next((l for l in lines if l.startswith("EXPECTED_CIPHER_COUNT=")), None)
+        assert expected_line is not None, "Missing EXPECTED_CIPHER_COUNT line"
+        expected_count = int(expected_line.split("=", 1)[1])
+
+        match = re.search(r"\bJA4:\s*([A-Za-z0-9_]+)", output)
+        assert match is not None, f"Missing JA4 line in output:\n{output}"
+        ja4 = match.group(1)
+        assert len(ja4) >= 6, f"Unexpected JA4 string: {ja4!r}"
+        actual_count = int(ja4[4:6])
+        assert actual_count == expected_count, (
+            f"Cipher count mismatch: expected {expected_count}, got {actual_count}"
+        )
+        return
+
     expected_path = EXPECTED_DIR / f"{name}.txt"
     if request.config.getoption("--record"):
         expected_path.write_text(output)

test/utls/go.mod

@@ -0,0 +1,12 @@
+module ja4-nginx-module/test/utls
+
+go 1.24
+
+require github.com/refraction-networking/utls v1.8.2
+
+require (
+	github.com/andybalholm/brotli v1.0.6 // indirect
+	github.com/klauspost/compress v1.17.4 // indirect
+	golang.org/x/crypto v0.36.0 // indirect
+	golang.org/x/sys v0.31.0 // indirect
+)

test/utls/go.sum

@@ -0,0 +1,10 @@
+github.com/andybalholm/brotli v1.0.6 h1:Yf9fFpf49Zrxb9NlQaluyE92/+X7UVHlhMNJN2sxfOI=
+github.com/andybalholm/brotli v1.0.6/go.mod h1:fO7iG3H7G2nSZ7m0zPUDn85XEX2GTukHGRSepvi9Eig=
+github.com/klauspost/compress v1.17.4 h1:Ej5ixsIri7BrIjBkRZLTo6ghwrEtHFk7ijlczPW4fZ4=
+github.com/klauspost/compress v1.17.4/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM=
+github.com/refraction-networking/utls v1.8.2 h1:j4Q1gJj0xngdeH+Ox/qND11aEfhpgoEvV+S9iJ2IdQo=
+github.com/refraction-networking/utls v1.8.2/go.mod h1:jkSOEkLqn+S/jtpEHPOsVv/4V4EVnelwbMQl4vCWXAM=
+golang.org/x/crypto v0.36.0 h1:AnAEvhDddvBdpY+uR+MyHmuZzzNqXSe/GvuDeob5L34=
+golang.org/x/crypto v0.36.0/go.mod h1:Y4J0ReaxCR1IMaabaSMugxJES1EpwhBHhv2bDHklZvc=
+golang.org/x/sys v0.31.0 h1:ioabZlmFYtWhL+TRYpcnNlLwhyxaM9kWTDEmfnprqik=
+golang.org/x/sys v0.31.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=

test/utls/invalid_cipher_count.go

@@ -0,0 +1,82 @@
+package main
+
+import (
+	"bufio"
+	"fmt"
+	"io"
+	"net"
+	"net/http"
+	"os"
+	"time"
+
+	utls "github.com/refraction-networking/utls"
+)
+
+const (
+	serverAddr = "127.0.0.1:443"
+	serverName = "localhost"
+)
+
+func main() {
+	conn, err := net.DialTimeout("tcp", serverAddr, 5*time.Second)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "ERROR: dial failed: %v\n", err)
+		os.Exit(1)
+	}
+	defer conn.Close()
+
+	config := &utls.Config{
+		ServerName:         serverName,
+		InsecureSkipVerify: true,
+	}
+
+	spec, err := utls.UTLSIdToSpec(utls.HelloChrome_120)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "ERROR: UTLSIdToSpec failed: %v\n", err)
+		os.Exit(1)
+	}
+
+	// Add an unknown (non-GREASE) cipher suite to the end of the list.
+	spec.CipherSuites = append(spec.CipherSuites, uint16(0x1234))
+
+	expectedCount := 0
+	for _, cs := range spec.CipherSuites {
+		if cs == utls.GREASE_PLACEHOLDER {
+			continue
+		}
+		expectedCount++
+	}
+
+	uconn := utls.UClient(conn, config, utls.HelloCustom)
+	if err := uconn.ApplyPreset(&spec); err != nil {
+		fmt.Fprintf(os.Stderr, "ERROR: ApplyPreset failed: %v\n", err)
+		os.Exit(1)
+	}
+	if err := uconn.Handshake(); err != nil {
+		fmt.Fprintf(os.Stderr, "ERROR: TLS handshake failed: %v\n", err)
+		os.Exit(1)
+	}
+
+	req := fmt.Sprintf("GET / HTTP/1.1\r\nHost: %s\r\nConnection: close\r\n\r\n", serverName)
+	if _, err := io.WriteString(uconn, req); err != nil {
+		fmt.Fprintf(os.Stderr, "ERROR: write request failed: %v\n", err)
+		os.Exit(1)
+	}
+
+	reader := bufio.NewReader(uconn)
+	resp, err := http.ReadResponse(reader, &http.Request{Method: "GET"})
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "ERROR: read response failed: %v\n", err)
+		os.Exit(1)
+	}
+	defer resp.Body.Close()
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "ERROR: read body failed: %v\n", err)
+		os.Exit(1)
+	}
+
+	fmt.Printf("EXPECTED_CIPHER_COUNT=%d\n", expectedCount)
+	fmt.Print(string(body))
+}