CMP Fuzzer by DDvO · Pull Request #11386 · openssl/openssl

DDvO · 2020-03-23T15:38:42Z

This solves #11301 by adding fuzzing of all public CMP and CRMF data types.

tests are added or updated

DDvO · 2020-03-23T15:45:37Z

Adding CMP (and CRMF) types to fuzz/asn1.c, as suggested by @richsalz was straightforward,
except that it turned out that for visibility reasons this can be done only for the public ones.

Is this already sufficient, or what should be done in addition?

@kroeckx mentioned adding a file that implements a FuzzerTestOneInput() function,
yet asn1.c already does so generically for the given ASN.1 types.

kroeckx · 2020-03-23T17:56:29Z

I think that adding it to asn1 isn't actually useful. The coverage will ge up, the fuzzer will slow down, but it's really just fuzzing duplicates of the same code. (I actually had modified asn1.c here like that in November.) One like cms, crl and x509 might be more useful. But I think as part of your CMP work, you added things like an HTTP client. I would like to see something that does something with HTTP. I don't know if there are other protocols involved. If there are, it would be useful to test them.

DDvO · 2020-03-24T10:06:52Z

I think that adding it to asn1 isn't actually useful. The coverage will ge up, the fuzzer will slow down, but it's really just fuzzing duplicates of the same code. (I actually had modified asn1.c here like that in November.)

So shall I remove that addition?

One like cms, crl and x509 might be more useful.

I've just added fuzz/cmp.c according to the model of fuzz/cms.c because it is most closely related, but this is suspiciously simple (like adding types to fuzz/asn1.c was).
I also found that fuzz/build.info and test/recipes/99-test_fuzz.t need to be extended accordingly.

Yet when invoking

make tests TESTS="test_fuzz" V=1

I only get

    1..1
../../util/wrap.pl ../../fuzz/cmp-test ../../fuzz/corpora/cmp => 0
    ok 1
ok 1 - Fuzzing cmp

I suspect I need to set up fuzz/corpora/cmp/ with all those hash-named files, but how?

A documentation/HOWTO for the OpenSSL fuzzing facility would be helpful here.

DDvO · 2020-03-24T10:15:14Z

But I think as part of your CMP work, you added things like an HTTP client. I would like to see something that does something with HTTP.

CMP, like CMS, is actually independent of the transfer protocol - HTTP is just the default.
So I think they are better tested independently.

HTTP is not an ASN.1-based protocol, and I have no clue how to fuzz that.
When adding the generic HTTP client support I've already added non-fuzzing tests for it:

test/http_test.c
test/recipes/80-test_http.t

I don't know if there are other protocols involved. If there are, it would be useful to test them.

CMP itself does not involve any further protocols (while its data structures make use of CRMF and X509, which are formats, not protocols, so AFAICS do not play a direct role here).

kroeckx · 2020-03-24T17:37:49Z

On Tue, Mar 24, 2020 at 03:07:08AM -0700, David von Oheimb wrote: A documentation/HOWTO for the OpenSSL fuzzing facility would be helpful here.

Have you looked at fuzz/README.md? Kurt

DDvO · 2020-03-30T05:47:27Z

Thanks @kroeckx for pointing at fuzz/README.md, which I had not noticed.
This explains how to fill fuzz/corpora/cmp/.

Yet fuzz/helper.py cmp has already been running for more than two days (on one CPU core), so far producing 13 MB data. Is there a way to see a forecast how much longer/more this is going to take?

kroeckx · 2020-03-30T06:47:50Z

Yet `fuzz/helper.py cmp` has already been running for more than two days (on one CPU core), so far producing 13 MB data. Is there a way to see a forecast how much longer/more this is going to take?

It will until run until you stop it. So just stop it, and add the corpus.

DDvO · 2020-03-30T07:40:56Z

It will until run until you stop it. So just stop it, and add the corpus.

Oh! This important detail seems not mentioned in the README.

Actually I have data from two runs: 13.2 MB from the long run I mentioned, which I just stopped,
and 3.6 MB from an earlier run which I had cancelled for other reasons.

Which one is preferred? I presume there should be a balance between completeness and data size. Here is a comparison with the data sizes of the already included fuzzers:

du fuzz/corpora/* | sort -n

960	fuzz/corpora/ct
1928	fuzz/corpora/conf
1996	fuzz/corpora/bndiv
2184	fuzz/corpora/asn1parse
2196	fuzz/corpora/bignum
3600	fuzz/corpora/cmp_old
7152	fuzz/corpora/cms
8208	fuzz/corpora/server
9128	fuzz/corpora/crl
9816	fuzz/corpora/client
10168	fuzz/corpora/x509
13284	fuzz/corpora/cmp
15836	fuzz/corpora/asn1

kroeckx · 2020-03-30T16:06:12Z

On Mon, Mar 30, 2020 at 12:41:10AM -0700, David von Oheimb wrote: > It will until run until you stop it. So just stop it, and add the corpus. Oh! This important detail seems not mentioned in the README. Actually I have data from two runs: 13.2 MB from the long run I mentioned, which I just stopped, and 3.6 MB from an earlier run which I had cancelled for other reasons. Which one is preferred?

I recommend that you try to minimize it, that is run: ./cmp x509 -merge=1 new-dir old-dir Where new-dir is an empty directory. You can put all the files you have in the old-dir. Kurt

DDvO · 2020-03-31T06:19:11Z

I recommend that you try to minimize it, that is run: ./cmp x509 -merge=1 new-dir old-dir Where new-dir is an empty directory. You can put all the files you have in the old-dir.

Good to know, will do.
Should I also run AFL mentioned in the README?

 afl-fuzz -i fuzz/corpora/$FUZZER -o fuzz/corpora/$FUZZER/out fuzz/$FUZZER

BTW, the copy of my last response was a mistake - my browser view of this page was inconsistent.

kroeckx · 2020-03-31T06:51:06Z

Should I also run AFL mentioned in the README?

No, libfuzzer alone is fine. Once it's merged, Google will run that fuzzer with multiple fuzzers including AFL. I will then download their corpus as some point and merge it.

DDvO · 2020-04-01T06:44:21Z

I recommend that you try to minimize it, that is run:
./cmp x509 -merge=1 new-dir old-dir

Where new-dir is an empty directory.
You can put all the files you have in the old-dir.

This took about a day to complete, but apparently had no effect - new-dir is still empty :-(

Why to use the x509 parameter here? Maybe this is wrong.

Should I better use, e.g., this? ./cmp new-dir -merge=1 old-dir

kroeckx · 2020-04-01T07:05:00Z

On Tue, Mar 31, 2020 at 11:44:35PM -0700, David von Oheimb wrote: > I recommend that you try to minimize it, that is run: > ./cmp x509 -merge=1 new-dir old-dir > Where new-dir is an empty directory. > You can put all the files you have in the old-dir. This took about a day to complete, but apparently had no effect - `new-dir` is still empty :-( Why to use the `x509` parameter here? Maybe this is wrong. Should I better use, e.g., this? `./cmp new-dir -merge=1 old-dir`

It clearly should just have been: ./cmp -merge=1 new-dir old-dir I assume you have an x509 dir, and it put things in the x509 dir. Kurt

DDvO · 2020-04-01T07:12:16Z

./cmp -merge=1 new-dir old-dir

I'm running this now, let's see how this goes ...

I assume you have an x509 dir, and it put things in the x509 dir.

Yes, fuzz/corpora/x509/ exists also over here,
but I had already checked: nothing changed there, neither.

DDvO · 2020-04-01T07:20:48Z

Also

./cmp -merge=1 new-dir old-dir`

did not give any output and apparently terminated immediately, without effect.

DDvO · 2020-04-01T07:21:01Z

In the meantime I've tentatively added the manually combined but non-merged/minimized data to fuzz/corpora/cmp/.

make tests TESTS="test_fuzz" V=1

now does include fuzz/corpora/cmp/ and passes.

DDvO · 2020-04-01T07:36:05Z

The cmp data added just about 2.5 seconds to the overall time (which is some 80 seconds on my machine) the fuzz tests took.
I cannot judge if this is a good sign or not.

kroeckx · 2020-04-01T07:49:01Z

I just did a quick look at your fuzzer. Maybe you can pass the msg to functions like: - OSSL_CMP_SRV_process_request() - OSSL_CMP_CTX_server_perform() That is, write some basic applications that does something with such a file. I tried to minimize the corpus, and I had no problem with it: $ ./fuzz/cmp -merge=1 fuzz/new/cmp fuzz/corpora/cmp/ INFO: Seed: 3695945473 INFO: Loaded 1 modules (183405 inline 8-bit counters): 183405 [0x1d99ea0, 0x1dc6b0d), INFO: Loaded 1 PC tables (183405 PCs): 183405 [0x1dc6b10,0x20931e0), MERGE-OUTER: 4136 files, 0 in the initial corpus MERGE-OUTER: attempt 1 INFO: Seed: 3696161283 INFO: Loaded 1 modules (183405 inline 8-bit counters): 183405 [0x1d99ea0, 0x1dc6b0d), INFO: Loaded 1 PC tables (183405 PCs): 183405 [0x1dc6b10,0x20931e0), INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 1048576 bytes MERGE-INNER: using the control file '/tmp/libFuzzerTemp.1357149.txt' MERGE-INNER: 4136 total files; 0 processed earlier; will process 4136 files now #1 pulse cov: 438 exec/s: 0 rss: 48Mb #2 pulse cov: 438 exec/s: 0 rss: 50Mb #4 pulse cov: 439 exec/s: 0 rss: 50Mb #8 pulse cov: 439 exec/s: 0 rss: 50Mb #16 pulse cov: 527 exec/s: 0 rss: 50Mb #32 pulse cov: 655 exec/s: 0 rss: 50Mb #64 pulse cov: 696 exec/s: 0 rss: 51Mb #128 pulse cov: 752 exec/s: 0 rss: 52Mb #256 pulse cov: 1462 exec/s: 0 rss: 56Mb #512 pulse cov: 1828 exec/s: 512 rss: 61Mb #1024 pulse cov: 1863 exec/s: 1024 rss: 69Mb #2048 pulse cov: 1987 exec/s: 409 rss: 87Mb #4096 pulse cov: 1987 exec/s: 341 rss: 162Mb #4136 DONE cov: 1987 exec/s: 344 rss: 166Mb MERGE-OUTER: succesfull in 1 attempt(s) MERGE-OUTER: the control file has 448086 bytes MERGE-OUTER: consumed 0Mb (53Mb rss) to parse the control file MERGE-OUTER: 1268 new files with 8138 new features added; 1987 new coverage edges I've put that at https://www.roeckx.be/cmp.tar.gz Kurt

DDvO · 2020-04-01T10:47:43Z

I tried to minimize the corpus, and I had no problem with it:
$ ./fuzz/cmp -merge=1 fuzz/new/cmp fuzz/corpora/cmp/

Argh, meanwhile I found that in my builds of those excecutables like fuzz/cmp were garbled,
which explains their weird behavior.

I suspect they were (wrongly) produced by my experiments with AFL or the like.
How can one produce them correctly, as you did?
With a normal configuration they do not show up as make targets.

DDvO · 2020-04-01T10:53:54Z

BTW, would be generally very helpful in my view if Configure (and thus also config) automatically did a make clean or the like to avoid inconsistencies with artifacts of earlier builds in the same directory.

DDvO · 2020-04-01T11:08:21Z

I've put that at https://www.roeckx.be/cmp.tar.gz Kurt

Thanks! I've replaced fuzz/corpora/cmp/* with that compressed data.
The fuzz tests also succeed with this data over here.

kroeckx · 2020-04-01T11:14:40Z

On Wed, Apr 01, 2020 at 03:47:57AM -0700, David von Oheimb wrote: > I tried to minimize the corpus, and I had no problem with it: > $ ./fuzz/cmp -merge=1 fuzz/new/cmp fuzz/corpora/cmp/ Argh, meanwhile I found that in my builds of those excecutables like `fuzz/cmp` were garbled, which explains their weird behavior. I suspect they were (wrongly) produced by my experiments with AFL or the like. How can one produce them correctly, as you did? With a normal configuration they do not show up as make targets.

I did: CC=clang-9 ./config enable-fuzz-libfuzzer --with-fuzzer-lib=/usr/lib/llvm-9/lib/clang/9.0.1/lib/linux/libclang_rt.fuzzer-x86_64.a -DPEDANTIC enable-asan enable-ubsan no-shared -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize=fuzzer-no-link enable-ec_nistp_64_gcc_128 -fno-sanitize=alignment enable-weak-ssl-ciphers enable-rc5 enable-md2 enable-ssl3 enable-ssl3-method enable-nextprotoneg --debug LDCMD=clang++-9 make -j4 You might need to change the version of clang. The "enable-fuzz-libfuzzer" will change the Makefile to generate the fuzz/cmp file. Kurt

DDvO · 2020-04-01T11:44:12Z

I see. I tried your new version of the config invocation
and found that I had to remove -Wa,--noexecstack.
My clang(++) version is 7.

Now I get on LDCMD=clang++ make fuzz/cmp:

rm -f fuzz/cmp
${LDCMD:-gcc} -pthread -m64 -fno-omit-frame-pointer -g  -Wall -O0 -g -DDEBUG_UNUSED -DPEDANTIC -pedantic -Wno-long-long -Wall -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wswitch -Wsign-compare -Wshadow -Wformat -Wtype-limits -Wundef -Werror -Wmissing-prototypes -Wstrict-prototypes -fstack-protector -fno-omit-frame-pointer -std=gnu90 -L.   \
	-o fuzz/cmp \
	fuzz/cmp-bin-cmp.o fuzz/cmp-bin-driver.o \
	-lcrypto -ldl -pthread 
/usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../x86_64-linux-gnu/crt1.o: in function `_start':
(.text+0x20): undefined reference to `main'

Yet maybe I won't strictly need that fuzz/cmp executable.

kroeckx · 2020-04-01T12:13:59Z

(.text+0x20): undefined reference to `main'

Are you sure you passed the --with-fuzzer-lib? The library provides the main function.

DDvO · 2020-04-01T13:11:53Z

Now the corpora data merging works, with very similar result as yours :-)

openssl-machine · 2020-04-18T15:00:16Z

24 hours has passed since 'approval: done' was set, but this PR has failing CI tests. Once the tests pass it will get moved to 'approval: ready to merge' automatically, alternatively please review and set the label manually.

…o/cmp/

…erialNumber

DDvO · 2020-04-18T18:00:25Z

Merged - thanks @kroeckx and @mattcaswell

Reviewed-by: Kurt Roeckx <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #11386)

…o/cmp/ Reviewed-by: Kurt Roeckx <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #11386)

Reviewed-by: Kurt Roeckx <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #11386)

…erialNumber Reviewed-by: Kurt Roeckx <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #11386)

mattcaswell · 2020-04-20T10:53:07Z

This PR seems to have broken Travis. All master builds have been red since it was merged.

DDvO · 2020-04-20T14:46:10Z

Hmm, are you sure the Travis failures are due to merging this PR?
Can you please provide more detail supporting this?

For instance, #4277 which I rebased to master today,
fails at a point unrelated to CMP and fuzzing:

 Looks like you failed 40 tests of 54.
30-test_evp_fetch_prov.t ........... 
Dubious, test returned 40 (wstat 10240, 0x2800)

according to https://travis-ci.org/github/openssl/openssl/jobs/677138716

mattcaswell · 2020-04-20T14:49:50Z

I'm looking at this build:

https://travis-ci.org/github/openssl/openssl/jobs/676658702

Which fails in cmp_vfy_test:

    # ERROR: (int) 'fixture->expected == ossl_cmp_verify_popo(fixture->msg, fixture->additional_arg)' failed @ test/cmp_vfy_test.c:101
    # [0] compared to [1]
    not ok 2 - test_verify_popo_bad

This is from the first build after commit e0331e - and is the first point that travis goes red. I think there are some subsequent other problems from later commits that are unrelated to this.

mattcaswell · 2020-04-20T14:58:54Z

I see test failures locally in cmp_vfy_test if I use -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION, as is done in the build in question. Although, strangely, the errors I see are slightly different to the ones in the failing build.

DDvO · 2020-04-20T15:10:47Z

Thx for the details; now I understand what's going on.
I need to disable at least the above test in case -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
(while I wouldn't have expected that the normal tests are run while that symbol is defined).
I'll provided a fixup PR shortly...

mattcaswell · 2020-04-20T15:15:59Z

Please run the extended tests in your fixup PR - the failing job only gets run when extended tests are enabled.

DDvO · 2020-04-20T15:20:56Z

How to run the extended tests?

kroeckx · 2020-04-20T15:22:57Z

Add a line in the commit message of the last commit that says: [extended tests]

DDvO · 2020-04-20T15:31:31Z

Done in #11585

DDvO mentioned this pull request Mar 24, 2020

CMP Fuzzer #11301

Closed

DDvO closed this Mar 30, 2020

DDvO reopened this Mar 30, 2020

DDvO force-pushed the cmp_fuzzer branch from 91fe525 to 36809c3 Compare April 1, 2020 07:03

DDvO force-pushed the cmp_fuzzer branch from 36809c3 to b868e03 Compare April 1, 2020 07:05

mattcaswell approved these changes Apr 17, 2020

View reviewed changes

mattcaswell added the approval: done This pull request has the required number of approvals label Apr 17, 2020

DDvO added 5 commits April 18, 2020 19:54

Rename CMP_PROTECTEDPART to OSSL_CMP_PROTECTEDPART for consistency

905b2cc

Add a couple of hints to fuzz/README.md

8621a08

Add CMP fuzzing to fuzz/cmp.c, including a couple of helpers in crypt…

99fb797

…o/cmp/

Add CMP error reason 'missing reference cert'

129231d

Prevent crash in X509_NAME_cmp() etc. when cert has no issuer or no s…

c6226f9

…erialNumber

DDvO force-pushed the cmp_fuzzer branch from e058b99 to c6226f9 Compare April 18, 2020 17:56

DDvO closed this Apr 18, 2020

openssl-machine pushed a commit that referenced this pull request Apr 18, 2020

Rename CMP_PROTECTEDPART to OSSL_CMP_PROTECTEDPART for consistency

642f60d

Reviewed-by: Kurt Roeckx <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #11386)

openssl-machine pushed a commit that referenced this pull request Apr 18, 2020

Add a couple of hints to fuzz/README.md

a81151b

Reviewed-by: Kurt Roeckx <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #11386)

openssl-machine pushed a commit that referenced this pull request Apr 18, 2020

Add CMP fuzzing to fuzz/cmp.c, including a couple of helpers in crypt…

e599d0a

…o/cmp/ Reviewed-by: Kurt Roeckx <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #11386)

openssl-machine pushed a commit that referenced this pull request Apr 18, 2020

Add CMP error reason 'missing reference cert'

753283c

Reviewed-by: Kurt Roeckx <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #11386)

openssl-machine pushed a commit that referenced this pull request Apr 18, 2020

Prevent crash in X509_NAME_cmp() etc. when cert has no issuer or no s…

e0331eb

…erialNumber Reviewed-by: Kurt Roeckx <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #11386)

DDvO reopened this Apr 18, 2020

DDvO closed this Apr 18, 2020

DDvO mentioned this pull request Apr 20, 2020

Fix test_cmp_vfy failures with FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION #11585

Closed

1 task

mspncp mentioned this pull request May 26, 2020

Run 'make' with 5 threads instead of an infinite amount #2723

Closed

DDvO mentioned this pull request Nov 23, 2022

OSSL_CMP_validate_msg(): make sure to reject protection type mismatch #19729

Closed

2 tasks

Uh oh!

Comments

Conversation

DDvO commented Mar 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DDvO commented Mar 23, 2020

Uh oh!

kroeckx commented Mar 23, 2020 via email

Uh oh!

DDvO commented Mar 24, 2020

Uh oh!

DDvO commented Mar 24, 2020

Uh oh!

kroeckx commented Mar 24, 2020 via email

Uh oh!

DDvO commented Mar 30, 2020

Uh oh!

kroeckx commented Mar 30, 2020 via email

Uh oh!

DDvO commented Mar 30, 2020

Uh oh!

kroeckx commented Mar 30, 2020 via email

Uh oh!

DDvO commented Mar 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kroeckx commented Mar 31, 2020 via email

Uh oh!

DDvO commented Apr 1, 2020

Uh oh!

kroeckx commented Apr 1, 2020 via email

Uh oh!

DDvO commented Apr 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DDvO commented Apr 1, 2020

Uh oh!

DDvO commented Apr 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DDvO commented Apr 1, 2020

Uh oh!

kroeckx commented Apr 1, 2020 via email

Uh oh!

DDvO commented Apr 1, 2020

Uh oh!

DDvO commented Apr 1, 2020

Uh oh!

DDvO commented Apr 1, 2020

Uh oh!

kroeckx commented Apr 1, 2020 via email

Uh oh!

DDvO commented Apr 1, 2020

Uh oh!

kroeckx commented Apr 1, 2020 via email

Uh oh!

DDvO commented Apr 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openssl-machine commented Apr 18, 2020

Uh oh!

DDvO commented Apr 18, 2020

Uh oh!

mattcaswell commented Apr 20, 2020

Uh oh!

DDvO commented Apr 20, 2020

Uh oh!

mattcaswell commented Apr 20, 2020

Uh oh!

mattcaswell commented Apr 20, 2020

Uh oh!

DDvO commented Apr 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattcaswell commented Apr 20, 2020

Uh oh!

DDvO commented Apr 20, 2020

Uh oh!

kroeckx commented Apr 20, 2020 via email

DDvO commented Mar 23, 2020 •

edited

Loading

DDvO commented Mar 31, 2020 •

edited

Loading

DDvO commented Apr 1, 2020 •

edited

Loading

DDvO commented Apr 1, 2020 •

edited

Loading

DDvO commented Apr 1, 2020 •

edited

Loading

DDvO commented Apr 20, 2020 •

edited

Loading