build: set ENABLE_IPC to OFF when fuzzing

fanquake commented at 11:28 am on August 21, 2025: member

A BUILD_FOR_FUZZING build will currently failure to configure, with missing capnp.

Currently a `BUILD_FOR_FUZZING` build will failure to configure, with
missing `capnp`.

af4156ab75

DrahtBot added the label Build system on Aug 21, 2025

DrahtBot commented at 11:28 am on August 21, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33235.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	Crypt-iQ, dergoegge

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

maflcko commented at 11:33 am on August 21, 2025: member

A BUILD_FOR_FUZZING build will currently failure to configure, with missing capnp.

IPC is by default on, and any build will fail with missing capnp? Whatever the user/dev is doing to fix those should be trivial to apply to a fuzz build to not have to special-case it?

fanquake commented at 11:34 am on August 21, 2025: member

Whatever the user/dev is doing to fix those should be trivial to apply to a fuzz build to not have to special-case it?

It’s already special cased though: BUILD_FOR_FUZZING=ON will disable all other targets and force BUILD_FUZZ_BINARY=ON., and that will no-longer be true if we leave IPC on.

maflcko commented at 11:49 am on August 21, 2025: member

Yes, the multiprocess binary targets are correctly disabled already on current master https://cirrus-ci.com/task/4847132122808320?logs=ci#L2057:

 0[17:44:32.757] Configure summary
 1[17:44:32.757] =================
 2[17:44:32.757] Executables:
 3[17:44:32.757]   bitcoin ............................. OFF
 4[17:44:32.757]   bitcoind ............................ OFF
 5[17:44:32.757]   bitcoin-node (multiprocess) ......... OFF
 6[17:44:32.757]   bitcoin-qt (GUI) .................... OFF
 7[17:44:32.757]   bitcoin-gui (GUI, multiprocess) ..... OFF
 8[17:44:32.757]   bitcoin-cli ......................... OFF
 9[17:44:32.757]   bitcoin-tx .......................... OFF
10[17:44:32.757]   bitcoin-util ........................ OFF
11[17:44:32.757]   bitcoin-wallet ...................... OFF
12[17:44:32.757]   bitcoin-chainstate (experimental) ... OFF
13[17:44:32.757]   libbitcoinkernel (experimental) ..... OFF
14[17:44:32.757] Optional features:
15[17:44:32.757]   wallet support ...................... ON
16[17:44:32.757]   external signer ..................... OFF
17[17:44:32.757]   ZeroMQ .............................. OFF
18[17:44:32.757]   IPC ................................. ON
19[17:44:32.757]   USDT tracing ........................ OFF
20[17:44:32.757]   QR code (GUI) ....................... OFF
21[17:44:32.757]   DBus (GUI) .......................... OFF
22[17:44:32.757] Tests:
23[17:44:32.757]   test_bitcoin ........................ OFF
24[17:44:32.757]   test_bitcoin-qt ..................... OFF
25[17:44:32.757]   bench_bitcoin ....................... OFF
26[17:44:32.757]   fuzz binary ......................... ON
27[17:44:32.757] 
28[17:44:32.757] Cross compiling ....................... FALSE
29[17:44:32.757] C++ compiler .......................... Clang 20.1.8, /usr/bin/clang++
30[17:44:32.757] CMAKE_BUILD_TYPE ...................... RelWithDebInfo
31[17:44:32.757] Preprocessor defined macros ........... FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION

fanquake commented at 11:51 am on August 21, 2025: member

Yea. So failing to configure because a dependency you don’t even need, isn’t installed, doesn’t make any sense.

ryanofsky commented at 12:25 pm on August 21, 2025: contributor

IIUC, this change which toggles off the ENABLE_IPC option automatically when BUILD_FOR_FUZZING is set is safe for now because the fuzz test binary doesn’t currently call any IPC code.

The change makes it more convenient to run the fuzz test without capnp installed since you don’t have to turn off ENABLE_IPC manually.

But I think we would want to revert this change as soon as any fuzz test is written that does call IPC code, so it does seem like there may not much long term benefit in making it, if I understand correctly. It would be good to have some fuzz tests exercising IPC code.

fanquake commented at 12:29 pm on August 21, 2025: member

It would be good to have some fuzz tests exercising IPC code.

Is anyone working on that? I guess @Sjors?

ryanofsky commented at 12:44 pm on August 21, 2025: contributor

Is anyone working on that? I guess @Sjors?

I could put together something very basic like a fuzz test calling the echoipc RPC method. But there are a number of other loose ends since #31802 which should take priority so will probably not get to this very soon. @darosior has mentioned writing fuzz tests for IPC before which could be more meaningful, like fuzzing the IPC socket.

Sjors commented at 1:17 pm on August 21, 2025: member

Made a note to add a fuzz test if @darosior or @ryanofsky don’t get around to it.

Would prefer to leave this ON unless such a PR doesn’t land before branch-off. Maybe add a v30 milestone?

maflcko commented at 1:25 pm on August 21, 2025: member

I think marcofleon may pick this up next month?

dergoegge commented at 1:56 pm on August 21, 2025: member

Just leaving a few notes on fuzzing the IPC interfaces.

There are several approaches to fuzzing the IPC interface and not all of them make sense within our repo. If we add IPC fuzz tests in the repo they should not spawn separate processes and not do actual IPC over a socket. This is because the fuzz tests we have in our repo are designed to run in a single process.

We could add tests that call the c++ handler methods for the various interface functions either directly or through the capnp wrappers (only if possible without actual ipc). This should work and depending on the approach might not even require capnp as a dependency. This approach would likely suffer from instability due to state build up, just like the RPC fuzz tests.

Another approach could be to add IPC fuzz tests to fuzzamoto, where the test would essentially simulate an actual application connecting to the IPC interfaces (i.e. doing real IPC over a socket). The snapshot fuzzing approach would also avoid the state build up problem. This is the approach I would take, if I was gonna work on it.

There is also the question of what to target and what assurances it gives us. The IPC interface is trusted, so from a security perspective it’s mostly uninteresting (e.g. a crash in this interface isn’t considered a security issue). So really, we should mostly fuzz to gain confidence in stability and correctness, for which I’d suggest:

API fuzzing: basically calling “random” sequences of the available IPC functions (similar to our RPC tests). This would mostly aim at stability, i.e. there are no random unexpected crashes caused by edge case usage of the interface. It might be possible to leverage something like https://github.com/dergoegge/libcapnp-mutator to infer a “grammar” for fuzzing the API, because our capnp files already encode all the type and structural information for how to use the interface (libcapnp-mutator would need to be built out further for this to really work, but it might serve as a starting point). This would make the maintenance of this kind of test very easy, as any time we change the capnp files, the fuzzer would automatically learn about it and know how to fuzz it.
Correctness fuzzing: limit the scope of the above API fuzzing approach to a subset of interesting functions for which we can make assertions about expected behavior.

ryanofsky commented at 2:17 pm on August 21, 2025: contributor

Thanks @dergoegge this is really helpful. It’s been hard for me to think about what would constitute a meaningful fuzz test, and it still may not be exactly clear, but this provides a great framework. I just have two question

they should not spawn separate processes and not do actual IPC over a socket

Not spawning separate processes makes sense, but I assumed it might be ok to communicate across an internal sockiet, e.g. with socketpair? That is what a lot of IPC unit tests are doing.

The IPC interface is trusted, so from a security perspective it’s mostly uninteresting (e.g. a crash in this interface isn’t considered a security issue)

This is true currently, but it would be possible to make IPC interfaces that are untrusted by checking inputs more rigorously and adding resource caps like maximum number of threads or block templates (in case of the mining interface) an IPC client could create. So I’d be curious if it would affect the recommended fuzzing strategy if we did offer untrusted interfaces or decide to make the mining interface less trusted in the future.

dergoegge commented at 2:44 pm on August 21, 2025: member

Not spawning separate processes makes sense, but I assumed it might be ok to communicate across an internal sockiet, e.g. with socketpair? That is what a lot of IPC unit tests are doing.

I’d advise against this (e.g. does this work for all OSs in CI?, might be slower) but it might also be fine.

So I’d be curious if it would affect the recommended fuzzing strategy if we did offer untrusted interfaces or decide to make the mining interface less trusted in the future.

In addition to what I suggested above, I’d fuzz any capnp code that would be exposed (e.g. deserialization) and check what kind of assurances they make w.r.t. security. https://capnproto.org/faq.html#is-capn-proto-secure looks reasonable, except for “the RPC layer is not robust against resource exhaustion attacks, possibly allowing denials of service”.

marcofleon commented at 7:42 am on August 26, 2025: contributor

we should mostly fuzz to gain confidence in stability and correctness, for which I’d suggest:

I’m happy to work on this. Still thinking about the best approach. It seems like using fuzzamoto is ultimately most effective, but it might be worth trying to get a baseline single-process fuzz test into the repo? This would probably be a more practical starting point for me too, but I’m open to suggestions.

Sjors commented at 7:56 am on August 26, 2025: member

a baseline single-process fuzz test into the repo?

I think so yes, otherwise it might not make the branch-off.

Crypt-iQ commented at 12:50 pm on August 29, 2025: contributor

tACK af4156ab75565acc5a71b68e70da5e2cf407df80

I ran into this while trying to measure fuzz coverage.

dergoegge approved

dergoegge commented at 9:13 am on September 2, 2025: member

utACK af4156ab75565acc5a71b68e70da5e2cf407df80

Seems reasonable to do for now while we don’t have ipc fuzz tests and don’t require the dependency.

build: set ENABLE_IPC to OFF when fuzzing #33235

Code Coverage & Benchmarks

Reviews