1743672 - Websocket fuzzers hit neqo assertion.

Status: RESOLVED FIXED

(Core :: Networking: HTTP, task, P2)

Description

[Jesse Schwartzentruber (:truber)]

Neqo fuzzing layer adds an assertion that fuzzing only occurs in debug builds. We fuzz other network targets in both fuzzing-asan-opt and fuzzing-debug configurations, as well as fuzzing-ccov-opt for coverage collection. The websocket target is now hitting the neqo assertion, which blocks coverage collection and non-debug fuzzing.

Is this assertion necessary?

Hit MOZ_CRASH(Trying to run a non-debug fuzzing build.) at /builds/worker/checkouts/gecko/third_party/rust/neqo-crypto/src/aead_fuzzing.rs:19
==189==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x7f7a161820d0 bp 0x7f79f9a3e290 sp 0x7f79f9a3e280 T4)
==189==The signal is caused by a WRITE memory access.
==189==Hint: address points to the zero page.
SCARINESS: 10 (null-deref)
    #0 0x7f7a161820d0 in MOZ_Crash /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:261:3
    #1 0x7f7a161820d0 in RustMozCrash /builds/worker/checkouts/gecko/mozglue/static/rust/wrappers.cpp:18:3
    #2 0x7f7a16182006 in mozglue_static::panic_hook::h183adc4d73b027cc /builds/worker/checkouts/gecko/mozglue/static/rust/lib.rs:91:9
    #3 0x7f7a16181385 in core::ops::function::Fn::call::h2c49d8cefb0980e2 /builds/worker/fetches/rust/library/core/src/ops/function.rs:70:5
    #4 0x7f7a17d43237 in std::panicking::rust_panic_with_hook::hd83d5a96a789e1d3 (/home/worker/firefox/gtest/libxul.so+0x1acac237)
    #5 0x7f7a1490ceb8 in std::panicking::begin_panic::_$u7b$$u7b$closure$u7d$$u7d$::h87ab5ca5f7dd332f /builds/worker/fetches/rust/library/std/src/panicking.rs:544:9
    #6 0x7f7a1490c229 in std::sys_common::backtrace::__rust_end_short_backtrace::h5a4c1481157b6ce8 /builds/worker/fetches/rust/library/std/src/sys_common/backtrace.rs:141:18
    #7 0x7f7a021ebd0d in std::panicking::begin_panic::h31b1120e59481228 /builds/worker/fetches/rust/library/std/src/panicking.rs:543:12
    #8 0x7f7a1491b14b in neqo_crypto::aead_fuzzing::Aead::new::hfda36442818ffa5a /builds/worker/checkouts/gecko/third_party/rust/neqo-crypto/src/aead_fuzzing.rs:19:9
    #9 0x7f7a1485f197 in neqo_transport::crypto::CryptoDxState::new::hee3ebd7975a6907a /builds/worker/checkouts/gecko/third_party/rust/neqo-transport/src/crypto.rs:400:19
    #10 0x7f7a1485fd35 in neqo_transport::crypto::CryptoDxState::new_initial::hd73504abb7b78e04 /builds/worker/checkouts/gecko/third_party/rust/neqo-transport/src/crypto.rs:442:9
    #11 0x7f7a14865831 in neqo_transport::crypto::CryptoStates::init::h1ebb74035ec45c68 /builds/worker/checkouts/gecko/third_party/rust/neqo-transport/src/crypto.rs:890:17
    #12 0x7f7a146ae913 in neqo_transport::connection::Connection::new_client::ha13827a72d7ee79e /builds/worker/checkouts/gecko/third_party/rust/neqo-transport/src/connection/mod.rs:312:9
    #13 0x7f7a146ae913 in neqo_http3::connection_client::Http3Client::new::hd7e309aa2cff7e61 /builds/worker/checkouts/gecko/third_party/rust/neqo-http3/src/connection_client.rs:106:13
    #14 0x7f7a14653bac in neqo_glue::NeqoHttp3Conn::new::h9c189bd71ad67287 /builds/worker/checkouts/gecko/netwerk/socket/neqo_glue/src/lib.rs:140:30
    #15 0x7f7a14653bac in neqo_http3conn_new /builds/worker/checkouts/gecko/netwerk/socket/neqo_glue/src/lib.rs:235:11
    #16 0x7f7a05f598c0 in Init /builds/worker/workspace/obj-build/dist/include/mozilla/net/NeqoHttp3Conn.h:21:12
    #17 0x7f7a05f598c0 in mozilla::net::Http3Session::Init(mozilla::net::nsHttpConnectionInfo const*, nsINetAddr*, nsINetAddr*, mozilla::net::HttpConnectionUDP*, unsigned int, nsIInterfaceRequestor*) /builds/worker/checkouts/gecko/netwerk/protocol/http/Http3Session.cpp:119:17
    #18 0x7f7a0605326b in mozilla::net::HttpConnectionUDP::Init(mozilla::net::nsHttpConnectionInfo*, nsIDNSRecord*, nsresult, nsIInterfaceRequestor*, unsigned int) /builds/worker/checkouts/gecko/netwerk/protocol/http/HttpConnectionUDP.cpp:146:23
    #19 0x7f7a05ef2ec4 in mozilla::net::DnsAndConnectSocket::TransportSetup::SetupConn(mozilla::net::nsAHttpTransaction*, mozilla::net::ConnectionEntry*, nsresult, unsigned int, mozilla::net::HttpConnectionBase**) /builds/worker/checkouts/gecko/netwerk/protocol/http/DnsAndConnectSocket.cpp:1020:19
    #20 0x7f7a05eefb2b in mozilla::net::DnsAndConnectSocket::SetupConn(bool, nsresult) /builds/worker/workspace/obj-build/dist/include/mozilla/RefPtr.h
    #21 0x7f7a05eef179 in mozilla::net::DnsAndConnectSocket::OnLookupComplete(nsICancelable*, nsIDNSRecord*, nsresult) /builds/worker/checkouts/gecko/netwerk/protocol/http/DnsAndConnectSocket.cpp:446:5
    #22 0x7f7a05ef1744 in non-virtual thunk to mozilla::net::DnsAndConnectSocket::OnLookupComplete(nsICancelable*, nsIDNSRecord*, nsresult) /builds/worker/checkouts/gecko/netwerk/protocol/http/DnsAndConnectSocket.cpp
    #23 0x7f7a056b7f46 in operator() /builds/worker/checkouts/gecko/netwerk/dns/DNSListenerProxy.cpp:29:59
    #24 0x7f7a056b7f46 in mozilla::detail::RunnableFunction<mozilla::net::DNSListenerProxy::OnLookupComplete(nsICancelable*, nsIDNSRecord*, nsresult)::$_0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:531:5
    #25 0x7f7a051fae5b in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1177:16
    #26 0x7f7a05205b1c in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:467:10
    #27 0x7f7a055cae31 in mozilla::net::nsSocketTransportService::Run() /builds/worker/checkouts/gecko/netwerk/base/nsSocketTransportService2.cpp:1190:11
    #28 0x7f7a055cca8c in non-virtual thunk to mozilla::net::nsSocketTransportService::Run() /builds/worker/checkouts/gecko/netwerk/base/nsSocketTransportService2.cpp
    #29 0x7f7a051fae5b in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1177:16
    #30 0x7f7a05205b1c in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:467:10
    #31 0x7f7a066d6dbd in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:300:20
    #32 0x7f7a0655e5d1 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:331:10
    #33 0x7f7a0655e5d1 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:324:3
    #34 0x7f7a0655e5d1 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:306:3
    #35 0x7f7a051f335f in nsThread::ThreadFunc(void*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:391:10
    #36 0x7f7a22c5809e in _pt_root /builds/worker/checkouts/gecko/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #37 0x7f7a24580608 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x9608)
    #38 0x7f7a24148292 in clone (/lib/x86_64-linux-gnu/libc.so.6+0x122292)

Comment 1

[Dragana Damjanovic [:dragana]]

neqo supports fuzzing feature only in the debug mode. The fuzzing feature disables encryption. The assertion was added on purpose to make sure no one can disable encryption.

Can we configure neqo to use fuzzing feature only in the debug build?

On the other hand, I am wondering why we are opening a HTTP/3 connection in a websocket test. That is unexpected. Can you make a http log?

Comment 2

[Valentin Gosu [:valentin] (he/him)]

(In reply to Dragana Damjanovic [:dragana] from comment #1)

On the other hand, I am wondering why we are opening a HTTP/3 connection in a websocket test. That is unexpected. Can you make a http log?

Hi Jesse, do you think you can record a log of the assertion being triggered?
Or otherwise provide instructions on how to reproduce it locally? Thanks!

Comment 3

[Jesse Schwartzentruber (:truber)]

Attachment: ws-http3.log

Here's a log produced with MOZ_LOG=timestamp,nsHttp:5 FUZZER=NetworkWebsocket ~/builds/asan/firefox crash-30f37f659e96fb8be4cb7779528e1fcd78899d54

The build is asan-fuzzing-opt from taskcluster (m-c 20211206-a88659230cfd). Any non-debug fuzzing build will work.

Comment 4

[Jesse Schwartzentruber (:truber)]

Attachment: crash-30f37f659e96fb8be4cb7779528e1fcd78899d54

libFuzzer input to reproduce assertion.

Comment 5

[Jesse Schwartzentruber (:truber)]

(In reply to Dragana Damjanovic [:dragana] from comment #1)

neqo supports fuzzing feature only in the debug mode. The fuzzing feature disables encryption. The assertion was added on purpose to make sure no one can disable encryption.

Can we configure neqo to use fuzzing feature only in the debug build?

Is disabling encryption a build-time option, or could it be selected at runtime? Would it be possible to encrypt unless fuzzing feature is built and MOZ_DISABLE_NONLOCAL_CONNECTIONS=1? Then it would be safer to enable for all fuzzing builds.

Comment 6

[Dragana Damjanovic [:dragana]]

This is triggered because an HTTP/2 server that is running on the same domain the websocket is trying to connect to is sending an AltSvc frame and advertising HTTP/3.

Can we instruct the HTTP/2 server to only advertize HTTP/3 if HTTP/3 is needed, i.e. only for HTTP/3 tests?

Disabling encryption is a build-time option.

Comment 7

[Jesse Schwartzentruber (:truber)]

(In reply to Dragana Damjanovic [:dragana] from comment #6)

Can we instruct the HTTP/2 server to only advertize HTTP/3 if HTTP/3 is needed, i.e. only for HTTP/3 tests?

In this case it's possible but in bug 1747078 probably not since we don't control the server.

To fix this client-side, I guess we'd want to check the HTTP/3 fuzzing pref before upgrading the connection to HTTP/3? We will need to disable HTTP/3 encryption for all fuzzing builds if we can't do it at runtime in order to run sanitizers on the HTTP/3 test and collect coverage.

Comment 8

[Dragana Damjanovic [:dragana]]

There are a couple of options:

1. disable HTTP/3 for all Fuzzing tests except the one testing HTTP/3
2. implement a way to switch from encrypted/unencrypted version at runtime
3. remove the assertion

I will start with option 3: I will check if MT is comfortable with removing assertion. He was asking to be added.

Comment 9

[Dragana Damjanovic [:dragana]]

There is one more option:
we do not have to build neqo with fuzzing feature in non debug build. Also only run HTTP/3 tests in debug build.

Comment 10

[Dragana Damjanovic [:dragana]]

I do not mind removing this panic code. If someone builds the library with the fuzzing feature it will not work really anywhere. Anyone can make a copy of the code and remove the assertion.

MT, you ask for the assertion. Can we remove it?

Comment 11

[Martin Thomson [:mt:]]

I'm OK with that. I expected that we wouldn't be doing non-debug fuzzing builds, but if we aren't, we aren't.

Comment 12

[Jim Blandy :jimb]

(In reply to Martin Thomson [:mt:] from comment #11)

I'm OK with that. I expected that we wouldn't be doing non-debug fuzzing builds, but if we aren't, we aren't.

Filed neqo#1353 for this.

Comment 13

[Jim Blandy :jimb]

I think this was fixed by M-C d28a96b6d40a, which brought in neqo-crypto 0.6.1, which includes 888fce89 from neqo#1353.