phuslu / nginx-ssl-fingerprint

high performance ja3 and http2 fingerprint for nginx.
BSD 2-Clause "Simplified" License
138 stars 23 forks source link

Worker crash on large client hello #37

Closed mhofstra closed 4 months ago

mhofstra commented 8 months ago

I can consistently reproduce a worker crash with the latest version of this library, with minor tweaks the build steps in the README (nginx 1.24.0 and not using ASAN).

Here's how I'm building:

# Clone

git clone -b OpenSSL_1_1_1-stable --depth=1 https://github.com/openssl/openssl
git clone -b release-1.24.0 --depth=1 https://github.com/nginx/nginx
git clone https://github.com/phuslu/nginx-ssl-fingerprint

# Patch

patch -p1 -d openssl < nginx-ssl-fingerprint/patches/openssl.1_1_1.patch
patch -p1 -d nginx < nginx-ssl-fingerprint/patches/nginx.patch

# Configure & Build

cd nginx
./auto/configure --with-openssl=$(pwd)/../openssl --add-module=$(pwd)/../nginx-ssl-fingerprint --with-http_ssl_module --with-stream_ssl_module --with-debug --with-stream --with-cc-opt="-O2 -fno-omit-frame-pointer" --with-ld-opt="-L/usr/local/lib -Wl,-E"
make

# Test

objs/nginx -p . -c $(pwd)/../nginx-ssl-fingerprint/nginx.conf

While nginx is running, from another terminal issue the tlsfuzzer test-client-hello-max-size.py against the configured port.

This commandline works for me: PYTHONPATH=. python3 scripts/test-client-hello-max-size.py -p 8444

In the nginx output window, you will notice the following lines:

2023/10/11 11:18:09 [debug] 3755132#0: *2 ngx_ssl_client_hello_ja3_cb: alloc 65544 bytes
2023/10/11 11:18:09 [debug] 3755132#0: *2 malloc: 00005593CAC2E590:65544
2023/10/11 11:18:09 [debug] 3755132#0: *2 ngx_ssl_client_hello_ja3_cb: used 65545 bytes
: worker process: malloc.c:2379: sysmalloc: Assertion `(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1))
 == 0)' failed.
2023/10/11 11:18:09 [notice] 3755131#0: signal 17 (SIGCHLD) received from 3755132
2023/10/11 11:18:09 [alert] 3755131#0: worker process 3755132 exited on signal 6

Please let me know if you need any additional information to reproduce and fix this issue.

mhofstra commented 8 months ago

@phuslu - apologies for the direct poke, but wanted to know if you intend to address this?

phuslu commented 4 months ago

Seems cannot reproduced now.

I added pipeline for testing and fuzzing, the latest result shows that nginx could survived after python3 scripts/test-client-hello-max-size.py , see https://github.com/phuslu/nginx-ssl-fingerprint/actions/runs/7794466419/job/21255846187

phuslu commented 4 months ago

all passed tests and fuzzing in https://github.com/phuslu/nginx-ssl-fingerprint/actions/runs/7795605199 I'd like to close this issue now, please feel free re-open it if the issues still exists.