On Sat, Feb 13, 2021 at 11:58:04AM +0100, Jan Klemkow wrote: > Hi, > > A coworker of mine has made tests with LibreSSL [1] and found some > regressions. I took his test descriptions and created the following > automated regression test. In the repository he described his findings > in detail. I kept the numbers of the files and subtests in the target > names for now. So, its easier to match it with his files. > > I don't know how to handle the result of "test-01-ssl". Thats why its > just a comment. Someone may have an idea to handle this properly. > > Any comments, wishes or OK's?
First of all thanks for the effort! The perl script and probably also the Makefile should have a license. Please add a check that tests whether the required perl modules are installed (p5-IO-Socket-SSL and p5-Net-SSLeay) and otherwise prints SKIPPED and their names, so I can install them if they're not present. I never remember their exact capitalization and hyphenation... Various comments inline, and a patch for openssl(1) at the end that may simplify some things. > [1]: https://github.com/noxxi/libressl-tests > > Index: regress/lib/libssl/Makefile > =================================================================== > RCS file: /cvs/src/regress/lib/libssl/Makefile,v > retrieving revision 1.42 > diff -u -p -r1.42 Makefile > --- regress/lib/libssl/Makefile 14 Oct 2020 15:53:22 -0000 1.42 > +++ regress/lib/libssl/Makefile 12 Feb 2021 19:42:56 -0000 > @@ -16,6 +16,7 @@ SUBDIR += tlsext > SUBDIR += tlslegacy > SUBDIR += key_schedule > SUBDIR += unit > +SUBDIR += validate I'm not too keen on hooking up a test that fails. It breaks my workflow. I use REGRESS_FAIL_EARLY in my environment to spot unexpected failures. Could you add the currently failing tests to REGRESS_EXPECTED_FAILURES? I also don't think it's a libssl regress test. It's either for lib/libcrypto or usr.bin/openssl, probably the latter. > # Things that take a long time should go below here. > SUBDIR += tlsfuzzer > Index: regress/lib/libssl/validate/Makefile > =================================================================== > RCS file: regress/lib/libssl/validate/Makefile > diff -N regress/lib/libssl/validate/Makefile > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ regress/lib/libssl/validate/Makefile 13 Feb 2021 10:50:30 -0000 > @@ -0,0 +1,104 @@ > +# Tests from: https://github.com/noxxi/libressl-tests > + > +PERL=perl Please use consistent spacing. PERL = perl Could you add OPENSSL ?= openssl and use ${OPENSSL} instead of plain openssl throughout the Makefile? This way we can compare with OpenSSL by doing 'make OPENSSL=eopenssl11'. > + > +REGRESS_TARGETS = test-00-01-ssl > +REGRESS_TARGETS += test-00-02-ssl > +REGRESS_TARGETS += test-00-03-ssl > +REGRESS_TARGETS += test-00-04-ssl > +REGRESS_TARGETS += test-00-05-ssl > +REGRESS_TARGETS += test-00-06-ssl > +REGRESS_TARGETS += test-01-ssl > +REGRESS_TARGETS += test-02-ssl > +REGRESS_ROOT_TARGETS = ${REGRESS_TARGETS} Please drop this line. These tests do not require root. > +REGRESS_CLEANUP = cleanup-ssl > +REGRESS_SETUP = create-libressl-test-certs I think this should be REGRESS_SETUP_ONCE. No need to regen the certs for each and every test. > + > +create-libressl-test-certs: create-libressl-test-certs.pl > + ${PERL} ${.CURDIR}/$@.pl > + > +cleanup-ssl: > + pkill openssl || true > + rm *.pem *.key > + > +test-00-01-ssl: > + # unusual wildcard cert, no CA given to client > + # cleanup > + pkill openssl || true > + sleep 2 pkill + sleep 2 is a bit icky. It's not the first time I wanted to have OpenSSL's -naccept option available for testing. See patch below. > + # start client # start server > + ${KTRACE} openssl s_server -cert server-unusual-wildcard.pem \ > + -key server-unusual-wildcard.pem -www & \ I'd suggest replacing '-www' with '-naccept 1' and... > + timeout=$$(($$(date +%s) + 5)); \ > + while fstat -p $$! | ! grep -q 'tcp .* \*:4433$$'; \ > + do test $$(date +%s) -lt $$timeout || exit 1; done > + # start client > + echo "data" | openssl s_client -verify_return_error -connect > 127.0.0.1:4433 \ ... do 'echo "Q"' here. Unless you want to use a random port (which may be more appropriate than killing all openssl processes and assuming that 4433 is free), I would just drop '-connect 127.0.0.1:4433'. > + | grep "Verify return code: 21" > + > +test-00-02-ssl: > + # unusual wildcard cert, CA given to client > + # cleanup > + pkill openssl || true > + sleep 2 > + # start server > + ${KTRACE} openssl s_server -cert server-unusual-wildcard.pem \ > + -key server-unusual-wildcard.pem -www & \ > + timeout=$$(($$(date +%s) + 5)); \ > + while fstat -p $$! | ! grep -q 'tcp .* \*:4433$$'; \ > + do test $$(date +%s) -lt $$timeout || exit 1; done > + # start client > + echo "data" | openssl s_client -connect 127.0.0.1:4433 -CAfile caR.pem \ > + | grep "Verify return code: 0" > + > +test-00-03-ssl: > + # common wildcard cert, no CA given to client > + # cleanup > + pkill openssl || true > + sleep 2 > + # start server > + ${KTRACE} openssl s_server -cert server-common-wildcard.pem \ > + -key server-common-wildcard.pem -www & \ > + timeout=$$(($$(date +%s) + 5)); \ > + while fstat -p $$! | ! grep -q 'tcp .* \*:4433$$'; \ > + do test $$(date +%s) -lt $$timeout || exit 1; done > + # start client > + echo "data" | openssl s_client -connect 127.0.0.1:4433 \ > + | grep "Verify return code: 21" > + > +test-00-04-ssl: > + # common wildcard cert, CA given to client > + # cleanup > + pkill openssl || true > + sleep 2 > + # start server > + ${KTRACE} openssl s_server -cert server-unusual-wildcard.pem \ > + -key server-unusual-wildcard.pem -www & \ > + timeout=$$(($$(date +%s) + 5)); \ > + while fstat -p $$! | ! grep -q 'tcp .* \*:4433$$'; \ > + do test $$(date +%s) -lt $$timeout || exit 1; done > + # start client > + echo "data" | openssl s_client -connect 127.0.0.1:4433 -CAfile caR.pem \ > + | grep "Verify return code: 21" > + > +test-00-05-ssl: > + # openssl verify, unusual wildcard cert > + openssl verify -CAfile caR.pem server-unusual-wildcard.pem \ > + | grep "server-unusual-wildcard.pem: OK" > + > +test-00-06-ssl: > + # openssl verify, common wildcard cert > + openssl verify -CAfile caR.pem server-common-wildcard.pem \ > + | grep "server-common-wildcard.pem: OK" > + > +test-01-ssl: > + # Not all chain certificates are sent in s_server > + # openssl s_server -cert server-subca.pem -CAfile subcaR.pem -www > + # XXX: don't know how to check the result Run the server with: openssl s_server -cert server-subca-chainS.pem -CAfile subcaR.pem Prior to the fix (tls13_server.c -r1.62), openssl s_client -CAfile caR.pem would return Verify return code: 21 (unable to verify the first certificate) after the fix it returns Verify return code: 0 (ok) > + > +test-02-ssl: > + # alternative chain not found > + openssl verify -verbose -trusted caR.pem -untrusted chainSX.pem > server-subca.pem \ > + | grep "server-subca.pem: Ok" > + > +.include <bsd.regress.mk> > Index: regress/lib/libssl/validate/create-libressl-test-certs.pl > =================================================================== > RCS file: regress/lib/libssl/validate/create-libressl-test-certs.pl > diff -N regress/lib/libssl/validate/create-libressl-test-certs.pl > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ regress/lib/libssl/validate/create-libressl-test-certs.pl 12 Feb 2021 > 16:34:20 -0000 > @@ -0,0 +1,107 @@ > +#!/usr/bin/perl > +use strict; > +use warnings; > +use IO::Socket::SSL::Utils; > + > +# primitive CA - ROOT > +my @ca = cert( > + CA => 1, > + subject => { CN => 'ROOT' } > +); > +out('caR.pem', pem(crt => $ca[0])); > +out('caR.key', pem(key => $ca[1])); > + > +# server certificate where SAN contains in-label wildcards which are allowed > by > +# RFC 6125 This was already discussed elsewhere. It's a MAY, not a MUST in the RFC, so saying that it's allowed is not quite true. The change of behavior was deliberate. > +my @leafcert = cert( > + issuer => \@ca, > + purpose => 'server', > + subject => { CN => 'server.local' }, > + subjectAltNames => [ > + [ DNS => 'bar.server.local' ], > + [ DNS => 'www*.server.local'], > + [ DNS => '*.www.server.local'], > + [ DNS => 'foo.server.local' ], > + [ DNS => 'server.local' ], > + ] > +); > +out('server-unusual-wildcard.pem', pem(@leafcert)); > + > +@leafcert = cert( > + issuer => \@ca, > + purpose => 'server', > + subject => { CN => 'server.local' }, > + subjectAltNames => [ > + [ DNS => 'bar.server.local' ], > + [ DNS => '*.www.server.local'], > + [ DNS => 'foo.server.local' ], > + [ DNS => 'server.local' ], > + ] > +); > +out('server-common-wildcard.pem', pem(@leafcert)); > + > +# alternative CA - OLD_ROOT > +my @caO = cert( > + CA => 1, > + subject => { CN => 'OLD_ROOT' } > +); > +out('caO.pem', pem(crt => $caO[0])); > +out('caO.key', pem(key => $caO[1])); > + > +# alternative ROOT CA, signed by OLD_ROOT, same key as other ROOT CA > +my @caX = cert( > + issuer => \@caO, > + CA => 1, > + subject => { CN => 'ROOT' }, > + key => $ca[1], > +); > +out('caX.pem', pem(crt => $caX[0])); > +out('caX.key', pem(key => $caX[1])); > + > +# subCA below ROOT > +my @subcaR = cert( > + issuer => \@ca, > + CA => 1, > + subject => { CN => 'SubCA.of.ROOT' } > +); > +out('subcaR.pem', pem(crt => $subcaR[0])); > +out('subcaR.key', pem(key => $subcaR[1])); > +out('chainSX.pem', pem($subcaR[0]), pem($caX[0])); > + > +@leafcert = cert( > + issuer => \@subcaR, > + purpose => 'server', > + subject => { CN => 'server.subca.local' }, > + subjectAltNames => [ > + [ DNS => 'server.subca.local' ], > + ] > +); > +out('server-subca.pem', pem(@leafcert)); > +out('server-subca-chainSX.pem', pem(@leafcert, $subcaR[0], $caX[0])); > +out('server-subca-chainS.pem', pem(@leafcert, $subcaR[0])); > + > + > +sub cert { CERT_create(not_after => 10*365*86400+time(), @_) } > +sub pem { > + my @default = qw(crt key); > + my %m = (key => \&PEM_key2string, crt => \&PEM_cert2string); > + my $result = ''; > + while (my $f = shift(@_)) { > + my $v; > + if ($f =~m{^(key|crt)$}) { > + $v = shift(@_); > + } else { > + $v = $f; > + $f = shift(@default) || 'crt'; > + } > + $f = $m{$f} || die "wrong key $f"; > + $result .= $f->($v); > + } > + return $result; > +} > + > +sub out { > + my $file = shift; > + open(my $fh,'>',"$file") or die "failed to create $file: $!"; > + print $fh @_ > +} > Index: openssl.1 =================================================================== RCS file: /cvs/src/usr.bin/openssl/openssl.1,v retrieving revision 1.128 diff -u -p -r1.128 openssl.1 --- openssl.1 30 Dec 2020 08:26:44 -0000 1.128 +++ openssl.1 13 Feb 2021 14:41:46 -0000 @@ -4599,6 +4599,7 @@ will be used. .Op Fl keymatexportlen Ar len .Op Fl msg .Op Fl mtu Ar mtu +.Op Fl naccept Ar n .Op Fl named_curve Ar arg .Op Fl nbio .Op Fl nbio_test @@ -4795,6 +4796,10 @@ Export len bytes of keying material (def Show all protocol messages with hex dump. .It Fl mtu Ar mtu Set the link layer MTU. +.It Fl naccept Ar n +Exit after +.Ar n +connections. .It Fl named_curve Ar arg Specify the elliptic curve name to use for ephemeral ECDH keys. This option is deprecated; use Index: s_apps.h =================================================================== RCS file: /cvs/src/usr.bin/openssl/s_apps.h,v retrieving revision 1.5 diff -u -p -r1.5 s_apps.h --- s_apps.h 25 Apr 2018 07:12:33 -0000 1.5 +++ s_apps.h 13 Feb 2021 13:54:02 -0000 @@ -120,7 +120,7 @@ extern int verify_return_error; int do_server(int port, int type, int *ret, int (*cb)(char *hostname, int s, unsigned char *context), - unsigned char *context); + unsigned char *context, int naccept); #ifdef HEADER_X509_H int verify_callback(int ok, X509_STORE_CTX *ctx); #endif Index: s_server.c =================================================================== RCS file: /cvs/src/usr.bin/openssl/s_server.c,v retrieving revision 1.44 diff -u -p -r1.44 s_server.c --- s_server.c 2 Oct 2020 15:43:48 -0000 1.44 +++ s_server.c 13 Feb 2021 14:37:45 -0000 @@ -267,6 +267,7 @@ static struct { uint16_t min_version; const SSL_METHOD *meth; int msg; + int naccept; char *named_curve; int nbio; int nbio_test; @@ -330,6 +331,19 @@ s_server_opt_mtu(char *arg) } static int +s_server_opt_naccept(char *arg) +{ + s_server_config.naccept = strtonum(arg, 1, INT_MAX, + &s_server_config.errstr); + if (s_server_config.errstr != NULL) { + BIO_printf(bio_err, "invalid argument %s: %s\n", + arg, s_server_config.errstr); + return (1); + } + return (0); +} + +static int s_server_protocol_version_dtls1(void) { s_server_config.meth = DTLS_server_method(); @@ -699,6 +713,13 @@ static const struct option s_server_opti }, #endif { + .name = "naccept", + .argname = "n", + .desc = "Exit after n connections", + .type = OPTION_ARG_FUNC, + .opt.argfunc = s_server_opt_naccept, + }, + { .name = "named_curve", .argname = "arg", .type = OPTION_ARG, @@ -1049,6 +1070,7 @@ s_server_main(int argc, char *argv[]) s_server_config.dcert_format = FORMAT_PEM; s_server_config.dkey_format = FORMAT_PEM; s_server_config.key_format = FORMAT_PEM; + s_server_config.naccept = -1; s_server_config.server_verify = SSL_VERIFY_NONE; s_server_config.socket_type = SOCK_STREAM; s_server_config.tlscstatp.timeout = -1; @@ -1435,10 +1457,12 @@ s_server_main(int argc, char *argv[]) (void) BIO_flush(bio_s_out); if (s_server_config.www) do_server(s_server_config.port, s_server_config.socket_type, - &accept_socket, www_body, s_server_config.context); + &accept_socket, www_body, s_server_config.context, + s_server_config.naccept); else do_server(s_server_config.port, s_server_config.socket_type, - &accept_socket, sv_body, s_server_config.context); + &accept_socket, sv_body, s_server_config.context, + s_server_config.naccept); print_stats(bio_s_out, ctx); ret = 0; end: Index: s_socket.c =================================================================== RCS file: /cvs/src/usr.bin/openssl/s_socket.c,v retrieving revision 1.11 diff -u -p -r1.11 s_socket.c --- s_socket.c 28 Jun 2019 13:35:02 -0000 1.11 +++ s_socket.c 13 Feb 2021 13:52:43 -0000 @@ -132,7 +132,7 @@ init_client(int *sock, char *host, char int do_server(int port, int type, int *ret, int (*cb) (char *hostname, int s, unsigned char *context), - unsigned char *context) + unsigned char *context, int naccept) { int sock; char *name = NULL; @@ -161,7 +161,9 @@ do_server(int port, int type, int *ret, shutdown(sock, SHUT_RDWR); close(sock); } - if (i < 0) { + if (naccept != -1) + naccept--; + if (i < 0 || naccept == 0) { shutdown(accept_socket, SHUT_RDWR); close(accept_socket); return (i);
