Discussion: how to allow access to internal functions for fuzzing testing

[cmeister2]

Currently @pauldreik and I are discussing how best to do testing of libcurl internals with regards to focused fuzz testers. I noted on curl/curl-fuzzer#32 (comment) that it might be sensible to:

• Have a new feature flag that's difficult to accidentally turn on (such as enable-internal-function-access)
• The result of which would:
  • Install a special header file when installing (e.g. can include curl/internal_functions.h)
  • Allow third party programs to compile against those functions.

In the short term I'd expect a limited number of functions that this would be useful for; for example doh_encode.

This issue is to cover thoughts about how best to implement this.

[bagder]

We already do this to some extent with the regular configure --enable-debug is used for building and running unit tests. The unit tests can invoke private functions - as debug builds make sure to not "hide" any functions and the tests can then invoke Curl_ tests etc "normally. An example is the llist tests in unit test 1300.

Is this enough?

[cmeister2]

I think it's close, but I think the header files are needed. At the moment the "hacky" solution here in the fuzzer for the curl_fnmatch fuzzer is to manually copy the curl_fnmatch.h header file into a directory which is then set as an include directory for the fuzzer. For doh.h that starts to get tricky, because there's a lot of subsequent header includes that also need copying over. In my limited testing so far I didn't manage to get it compiling because some functions were being redefined - I assume something to do with #defines, but I haven't dug into it further.

[bagder]

I think these are two independent problems:

1. A way to build curl that allows external builds to access internal functions - I think --enable-debug is good enough for that.
2. Making it possible for external builds to include headers so that the internal functions made available in (1) can get called.

It sounds to me like (2) is the problem now? You mentioned an internal_functions.h header for this, but I don't understand how that makes anything better than rather trying to make doh.h possible to use?

[cmeister2]

I think these are two independent problems:

1. A way to build curl that allows external builds to access internal functions - I think `--enable-debug` is good enough for that.

Agreed.

2. Making it possible for external builds to include headers so that the internal functions made available in (1) can get called.

It sounds to me like (2) is the problem now? You mentioned an internal_functions.h header for this, but I don't understand how that makes anything better than rather trying to make doh.h possible to use?

I think we basically do need to make doh.h (and any other files we want to test) possible to use, yes. Preferably as standalone as possible.

Equally I personally think it'd be good if we had a configuration option to install these files alongside other curl API files so that the hacky "copy it out of the lib/ directory" approach is not necessary.

[pauldreik]

I am not sure if enable-debug adds the memory debugging machinery? That can be good to be able to disable, for instance to build a fast exploring fuzzer. Fuzz testing can be orthogonal to debugging.

I used cmake for adding the internal fuzzers, and I think it was not that complicated or destroys for regular users. I am worried that maintaining functionality for being able to install internal headers not meant to be external opens up for problems. Will it require maintenance and no one except the fuzzer writers will notice when it breaks?

[cmeister2]

I used cmake for adding the internal fuzzers, and I think it was not that complicated or destroys for regular users. I am worried that maintaining functionality for being able to install internal headers not meant to be external opens up for problems. Will it require maintenance and no one except the fuzzer writers will notice when it breaks?

Thankfully, curl already has a fuzzer regression test stage: https://github.com/curl/curl/blob/master/.travis.yml#L674. If the main build breaks this new code, the curl builds won't pass.

[bagder]

I think we basically do need to make doh.h (and any other files we want to test) possible to use, yes. Preferably as standalone as possible.

Does it really need to have it standalone? I mean we can do unit tests without it, just by finding it in the lib dir - can't the fuzz build process do it like that too?

If it really needs to be standalone we probably need to split the current doh.h in two so that the stuff that doesn't need urldata.h ends up in another file. Or similar.

[cmeister2]

I can give it a shot. Might just work. If so then we probably don't need anything else here.

[pauldreik]

I can give it a shot. Might just work. If so then we probably don't need anything else here.

Good! The netrc parser and the cookie stuff also need to be accessible. It will be interesting how this is going to look, compared to the existing "in-tree" cmake solution!