Tlsfuzzer external tests #17340

beldmit · 2021-12-22T17:15:59Z

This is a WIP PR to add TLSFuzzer external testing to openssl

Checklist

documentation is added or updated
tests are added or updated

beldmit · 2021-12-23T14:21:18Z

Now I switched off the failing tests.

This PR adds some tests using the TLSFuzzer test framework. Currently it's basically an infrastructure PR, but it will be extended to run more tests.

TLSFuzzer is more flexible than s_client/s_server utilities. It is extensively used in Red Hat TLS testing, and there were (and will be) a lot of issues found by it.

Could you please give a feedback whether we are interested in this test suite?

mattcaswell · 2021-12-23T14:53:06Z

Could you please give a feedback whether we are interested in this test suite?

I think it would be good to have this.

We have typically used the external tests as regression tests and only infrequently do we update the submodules and bring the external tests up to date. The more coverage we can get for regression testing the better IMO.

beldmit · 2021-12-23T14:59:44Z

Here are the examples of using TLSFuzzer with other libraries:

gnutls: https://gitlab.com/dueno/gnutls/-/tree/master/tests/suite/tls-fuzzer
nss: https://searchfox.org/mozilla-central/source/security/nss/tests/tlsfuzzer

test/recipes/95-test_external_tlsfuzzer_data/cert.json

beldmit · 2022-01-03T10:39:01Z

check_update failure seems irrelevant. Ping @levitte (?)

beldmit · 2022-01-03T11:00:49Z

Ready for review. Ping @t8m @mattcaswell @tomato42

After this PR is merged, I will significantly extend the set of the tests.

test/recipes/95-test_external_tlsfuzzer.t

test/recipes/95-test_external_tlsfuzzer_data/cert.json.in

tomato42

For first, example tests, I'd say use of test-tls13-conversation.py and test-conversation.py would be better, as they are a). simple, and b). pass.

beldmit · 2022-01-03T16:25:54Z

Current state of the tests is agreed with @tomato42. If they pass, the PR should be ready to merge. It could (and will) be extended later with other test cases.

t8m · 2022-01-03T18:02:29Z

IMO this does not belong to 3.0 branch. As this is a completely new test case.

beldmit · 2022-01-03T18:19:59Z

@t8m I'd suggest add this to master and decide if it should be added to 3.0 separately

paulidale

The stable release update policy says that this should not go into 3.0. This would need to be raised at an OTC call and a vote taken to permit it into 3.0.

beldmit · 2022-01-04T06:55:48Z

No problem, I'm more disturbed by possible regressions during refactoring

t8m · 2022-01-04T09:06:36Z

test/recipes/95-test_external_tlsfuzzer_data/tlsfuzzer.sh

+if [ "$SRCTOP" != "$BLDTOP" ] ; then
+    echo "Out of tree builds not supported with TLSFuzzer test!"
+    exit 1
+fi


Is there any particular reason why out of tree builds are not supported with this test? It was necessary to do with the gost test because it did not support multiple paths for include directories, however here I do not see any compilation depending on the OpenSSL sources. Or am I missing something?

No, there is no real reason for that.

t8m · 2022-01-04T09:09:22Z

test/recipes/95-test_external_tlsfuzzer.t

+plan skip_all => "TLSFuzzer tests not available on Windows or VMS"
+    if $^O =~ /^(VMS|MSWin32)$/;
+plan skip_all => "TLSFuzzer tests not supported in out of tree builds"
+    if bldtop_dir() ne srctop_dir();


Perhaps the test should be also skipped if the tlsfuzzer submodule is not present. Similarly to how some other external test check presence of the external sources.

Not sure - we have submodules to ensure the presence of the sources, don't we?

Yes, but you might want to run just some of the external tests. Of course you can always selectively run the individual tests, but... But yeah the gost external test recipe does not check for the presence of the gost sources either.

openssl-machine · 2022-01-05T01:00:20Z

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

beldmit · 2022-01-05T10:26:36Z

Merged. Many thanks!

To be continued...

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #17340)

bernd-edlinger · 2022-01-07T06:54:03Z

Hi, I see test failure of dayly "coverage" test:

------------------------------------------------------------------
Testing OpenSSL using TLSFuzzer:
   CWD:                /home/runner/work/openssl/openssl/test-runs/test_external_tlsfuzzer
   SRCTOP:             /home/runner/work/openssl/openssl
   BLDTOP:             /home/runner/work/openssl/openssl
   OPENSSL_ROOT_DIR:   /home/runner/work/openssl/openssl
   Python:             /usr/bin/python
   TESTDATADIR:        /home/runner/work/openssl/openssl/test/recipes/95-test_external_tlsfuzzer_data
------------------------------------------------------------------
/usr/bin/python: can't open file 'tests/scripts_retention.py': [Errno 2] No such file or directory
../../util/wrap.pl sh ../../test/recipes/95-test_external_tlsfuzzer_data/tls-fuzzer-cert.sh => 2
not ok 1 - running TLSFuzzer tests
# ------------------------------------------------------------------------------
#   Failed test 'running TLSFuzzer tests'
#   at test/recipes/95-test_external_tlsfuzzer.t line 27.
# Looks like you failed 1 test of 1.95-test_external_tlsfuzzer.t ....... 
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests

what does that mean?

beldmit · 2022-01-07T07:07:00Z

I suspect lack of the submodules added for this test, speaking frankly.

t8m · 2022-01-07T08:56:52Z

I suspect lack of the submodules added for this test, speaking frankly.

@beldmit would you please fix it? I'd recommend also adding the skip of the test in the test recipe if the submodule is not present.

beldmit · 2022-01-07T13:52:55Z

I can't do it before Monday, sorry

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from openssl#17340) (cherry picked from commit cccbb4f)

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from openssl#17340) (cherry picked from commit db87f89)

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from openssl#17340) (cherry picked from commit e66c417)

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #17340) (cherry picked from commit cccbb4f) Reviewed-by: Hugo Landau <hlandau@openssl.org>

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #17340) (cherry picked from commit db87f89) Reviewed-by: Hugo Landau <hlandau@openssl.org>

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #17340) (cherry picked from commit e66c417) Reviewed-by: Hugo Landau <hlandau@openssl.org>

beldmit changed the title ~~Tlsfuzzer ext~~ [WIP] Tlsfuzzer ext Dec 22, 2021

beldmit added approval: review pending This pull request needs review by a committer branch: 3.0 Merge to openssl-3.0 branch branch: master Merge to master branch labels Dec 23, 2021

beldmit changed the title ~~[WIP] Tlsfuzzer ext~~ Tlsfuzzer external tests Dec 23, 2021

t8m reviewed Dec 28, 2021

View reviewed changes

test/recipes/95-test_external_tlsfuzzer_data/cert.json Outdated Show resolved Hide resolved

TLSfuzzer: submodules

34a9811

beldmit force-pushed the tlsfuzzer_ext branch from 93379d5 to 257f630 Compare January 3, 2022 10:35

tomato42 reviewed Jan 3, 2022

View reviewed changes

test/recipes/95-test_external_tlsfuzzer.t Show resolved Hide resolved

tomato42 reviewed Jan 3, 2022

View reviewed changes

test/recipes/95-test_external_tlsfuzzer_data/cert.json.in Outdated Show resolved Hide resolved

tomato42 suggested changes Jan 3, 2022

View reviewed changes

beldmit added 2 commits January 3, 2022 17:20

TLS Fuzzer: initial test infrastructure

0c9784a

Run TLSfuzzer tests for CI

7f23a62

beldmit force-pushed the tlsfuzzer_ext branch from 90ba386 to 7f23a62 Compare January 3, 2022 16:20

tomato42 approved these changes Jan 3, 2022

View reviewed changes

t8m added the triaged: feature The issue/pr requests/adds a feature label Jan 3, 2022

paulidale approved these changes Jan 4, 2022

View reviewed changes

paulidale added approval: done This pull request has the required number of approvals and removed branch: 3.0 Merge to openssl-3.0 branch labels Jan 4, 2022

t8m reviewed Jan 4, 2022

View reviewed changes

beldmit closed this Jan 5, 2022

openssl-machine pushed a commit that referenced this pull request Jan 5, 2022

TLSfuzzer: submodules

cccbb4f

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #17340)

openssl-machine pushed a commit that referenced this pull request Jan 5, 2022

TLS Fuzzer: initial test infrastructure

db87f89

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #17340)

openssl-machine pushed a commit that referenced this pull request Jan 5, 2022

Run TLSfuzzer tests for CI

e66c417

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #17340)

beldmit mentioned this pull request Jan 9, 2022

Don't run TLSFuzzer tests when it is not properly set #17448

Closed

2 tasks

t8m pushed a commit to t8m/openssl that referenced this pull request Nov 2, 2022

TLSfuzzer: submodules

5c572e0

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from openssl#17340) (cherry picked from commit cccbb4f)

t8m pushed a commit to t8m/openssl that referenced this pull request Nov 2, 2022

TLS Fuzzer: initial test infrastructure

5a27a09

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from openssl#17340) (cherry picked from commit db87f89)

t8m pushed a commit to t8m/openssl that referenced this pull request Nov 2, 2022

Run TLSfuzzer tests for CI

0bb8594

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from openssl#17340) (cherry picked from commit e66c417)

openssl-machine pushed a commit that referenced this pull request Nov 9, 2022

TLSfuzzer: submodules

6f79bda

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #17340) (cherry picked from commit cccbb4f) Reviewed-by: Hugo Landau <hlandau@openssl.org>

openssl-machine pushed a commit that referenced this pull request Nov 9, 2022

TLS Fuzzer: initial test infrastructure

cda2f1b

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #17340) (cherry picked from commit db87f89) Reviewed-by: Hugo Landau <hlandau@openssl.org>

openssl-machine pushed a commit that referenced this pull request Nov 9, 2022

Run TLSfuzzer tests for CI

8963cae

Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #17340) (cherry picked from commit e66c417) Reviewed-by: Hugo Landau <hlandau@openssl.org>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tlsfuzzer external tests #17340

Tlsfuzzer external tests #17340

beldmit commented Dec 22, 2021

beldmit commented Dec 23, 2021

mattcaswell commented Dec 23, 2021

beldmit commented Dec 23, 2021

beldmit commented Jan 3, 2022

beldmit commented Jan 3, 2022

tomato42 left a comment

beldmit commented Jan 3, 2022

t8m commented Jan 3, 2022

beldmit commented Jan 3, 2022

paulidale left a comment •

edited

beldmit commented Jan 4, 2022

t8m Jan 4, 2022

beldmit Jan 4, 2022

t8m Jan 4, 2022

beldmit Jan 4, 2022

t8m Jan 4, 2022

openssl-machine commented Jan 5, 2022

beldmit commented Jan 5, 2022

bernd-edlinger commented Jan 7, 2022

beldmit commented Jan 7, 2022

t8m commented Jan 7, 2022

beldmit commented Jan 7, 2022

Tlsfuzzer external tests #17340

Tlsfuzzer external tests #17340

Conversation

beldmit commented Dec 22, 2021

Checklist

beldmit commented Dec 23, 2021

mattcaswell commented Dec 23, 2021

beldmit commented Dec 23, 2021

beldmit commented Jan 3, 2022

beldmit commented Jan 3, 2022

tomato42 left a comment

Choose a reason for hiding this comment

beldmit commented Jan 3, 2022

t8m commented Jan 3, 2022

beldmit commented Jan 3, 2022

paulidale left a comment • edited

Choose a reason for hiding this comment

beldmit commented Jan 4, 2022

t8m Jan 4, 2022

Choose a reason for hiding this comment

beldmit Jan 4, 2022

Choose a reason for hiding this comment

t8m Jan 4, 2022

Choose a reason for hiding this comment

beldmit Jan 4, 2022

Choose a reason for hiding this comment

t8m Jan 4, 2022

Choose a reason for hiding this comment

openssl-machine commented Jan 5, 2022

beldmit commented Jan 5, 2022

bernd-edlinger commented Jan 7, 2022

beldmit commented Jan 7, 2022

t8m commented Jan 7, 2022

beldmit commented Jan 7, 2022

paulidale left a comment •

edited