Re: LibreSSL regressions
On Tue, Feb 16, 2021 at 01:16:21PM +0100, Jan Klemkow wrote: > On Tue, Feb 16, 2021 at 04:36:59AM +1100, Joel Sing wrote: > > On 21-02-15 14:49:46, Jan Klemkow wrote: > > > On Sat, Feb 13, 2021 at 03:53:48PM +0100, Theo Buehler wrote: > > > > On Sat, Feb 13, 2021 at 11:58:04AM +0100, Jan Klemkow wrote: > > > > > 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? > > > > > > > > > > [1]: https://github.com/noxxi/libressl-tests > > > > > > > > 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. > > > > > > This is an updated version of the test including comments and wishes > > > from tb@ and bluhm@. > > > > > > OK? > > > > This currently drives openssl(1) for tests, which means that it is > > testing openssl(1), libssl and libcrypto, when what you're really > > wanting to test is libcrypto's verifier. While this works, the > > problem is that a change or breakage in libssl or openssl(1) results > > in a regress failure for libcrypto. If this is to land in its > > current form it really should be in regress/usr.bin/openssl - > > alternatively, it could be reworked to explicitly test libcrypto's > > APIs and remain here. > > > > Some additional comments inline. > > So, the following diff should hit all needs. > > OK? Yes, it's ok. Thanks. I played a bit with it, and it is a bit more fragile than I would like, but we may fix that in tree or redo some of it without using openssl(1) later. The ground covered is definitely worthwhile to have in regres, so please go ahead. (jsing has no objection).
Re: LibreSSL regressions
On Tue, Feb 16, 2021 at 04:36:59AM +1100, Joel Sing wrote: > On 21-02-15 14:49:46, Jan Klemkow wrote: > > On Sat, Feb 13, 2021 at 03:53:48PM +0100, Theo Buehler wrote: > > > On Sat, Feb 13, 2021 at 11:58:04AM +0100, Jan Klemkow wrote: > > > > 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? > > > > > > > > [1]: https://github.com/noxxi/libressl-tests > > > > > > 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. > > > > This is an updated version of the test including comments and wishes > > from tb@ and bluhm@. > > > > OK? > > This currently drives openssl(1) for tests, which means that it is > testing openssl(1), libssl and libcrypto, when what you're really > wanting to test is libcrypto's verifier. While this works, the > problem is that a change or breakage in libssl or openssl(1) results > in a regress failure for libcrypto. If this is to land in its > current form it really should be in regress/usr.bin/openssl - > alternatively, it could be reworked to explicitly test libcrypto's > APIs and remain here. > > Some additional comments inline. So, the following diff should hit all needs. OK? Thanks, Jan Index: usr.bin/openssl/Makefile === RCS file: /cvs/src/regress/usr.bin/openssl/Makefile,v retrieving revision 1.6 diff -u -p -r1.6 Makefile --- usr.bin/openssl/Makefile19 Mar 2018 03:41:40 - 1.6 +++ usr.bin/openssl/Makefile15 Feb 2021 20:37:11 - @@ -1,6 +1,6 @@ # $OpenBSD: Makefile,v 1.6 2018/03/19 03:41:40 beck Exp $ -SUBDIR= options +SUBDIR= options x509 CLEANFILES+= testdsa.key testdsa.pem rsakey.pem rsacert.pem dsa512.pem CLEANFILES+= appstest_dir Index: usr.bin/openssl/x509/Makefile === RCS file: usr.bin/openssl/x509/Makefile diff -N usr.bin/openssl/x509/Makefile --- /dev/null 1 Jan 1970 00:00:00 - +++ usr.bin/openssl/x509/Makefile 16 Feb 2021 12:06:10 - @@ -0,0 +1,129 @@ +# $OpenBSD$ + +# Copyright (c) 2021 Jan Klemkow +# +# Permission to use, copy, modify, and distribute this software for any +# purpose with or without fee is hereby granted, provided that the above +# copyright notice and this permission notice appear in all copies. +# +# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES +# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR +# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES +# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN +# ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF +# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + +# This regression test is based on manual test descriptions from: +# https://github.com/noxxi/libressl-tests + +# The following port must be installed for the regression tests: +# p5-IO-Socket-SSL perl interface to SSL sockets + +PERL = perl +OPENSSL ?= openssl + +PKG_REQUIRE != pkg_info -e 'p5-IO-Socket-SSL-*' +.if empty (PKG_REQUIRE) +regress: + @echo "missing package p5-IO-Socket-SSL" + @echo SKIPPED +.endif + +REGRESS_TARGETS += test-inlabel-wildcard-cert-no-CA-client +REGRESS_TARGETS += test-inlabel-wildcard-cert-CA-client +REGRESS_TARGETS += test-common-wildcard-cert-no-CA-client +REGRESS_TARGETS += test-common-wildcard-cert-CA-client +REGRESS_TARGETS += test-verify-unusual-wildcard-cert +REGRESS_TARGETS += test-openssl-verify-common-wildcard-cert +REGRESS_TARGETS += test-chain-certificates-s_server +REGRESS_TARGETS += test-alternative-chain +REGRESS_CLEANUP = cleanup-ssl +REGRESS_SETUP_ONCE = create-libressl-test-certs + +REGRESS_EXPECTED_FAILURES += test-unusual-wildcard-cert-no-CA-client +REGRESS_EXPECTED_FAILURES += test-common-wildcard-cert-no-CA-client +REGRESS_EXPECTED_FAILURES += test-common-wildcard-ce
Re: LibreSSL regressions
> > > +# server certificate where SAN contains in-label wildcards which are > > > allowed by > > > +# RFC 6125 > > > > It is worth noting that per the RFC, a client MAY allow in-label > > wildcards (this is not a MUST or even a SHOULD). Additionally, > > various software does not allow or support this (for example, libtls > > and hence ftp(1)). The purpose of this test is not to check a hostname against the in-label wildcard SAN. It is instead to show that the whole certificate is treated as invalid simply by having an in-label wildcard SAN, no matter if this specific SAN would actually be used to verify a hostname. This is in my opinion the wrong behavior. From my understanding the verification should succeed if there is a SAN matching the hostname. Any SAN which does not match the hostname should be ignored. I'm comfortable with having a strict hostname check which disallows partial wildcards. I'm not comfortable with treating the whole certificate invalid just because it has a partial wildcard in a SAN which gets not actually used. SAN are not specific to TLS but are a general property of certificates. Which SAN are allowed in a certificate should therefore follow RFC 5280 section "4.2.1.6. Subject Alternative Name". It should not be based on which algorithm will be used for hostname validation in the context of TLS. Regards, Steffen
Re: LibreSSL regressions
On Tue, Feb 16, 2021 at 04:36:59AM +1100, Joel Sing wrote:
> On 21-02-15 14:49:46, Jan Klemkow wrote:
> > +create-libressl-test-certs: create-libressl-test-certs.pl
> > + ${PERL} ${.CURDIR}/$@.pl
>
> We can see how this goes, however we may end up wanting to generate
> the certificates and commit them rather than regenerating on each
> run. The other advantage is that p5-IO-Socket-SSL would only be
> needed to regenerate the certificates and not actually run the
> tests.
What should I do? Just commit the generated files and remove the Perl
script?
> > Index: regress/lib/libcrypto/validate/create-libressl-test-certs.pl
> > ===
> > RCS file: regress/lib/libcrypto/validate/create-libressl-test-certs.pl
> > diff -N regress/lib/libcrypto/validate/create-libressl-test-certs.pl
> > --- /dev/null 1 Jan 1970 00:00:00 -
> > +++ regress/lib/libcrypto/validate/create-libressl-test-certs.pl15 Feb
> > 2021 12:54:58 -
> > @@ -0,0 +1,111 @@
> > +#!/usr/bin/perl
> > +
> > +# Copyright (c) 2021 Steffen Ullrich
> > +# Public Domain
> > +
> > +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
>
> It is worth noting that per the RFC, a client MAY allow in-label
> wildcards (this is not a MUST or even a SHOULD). Additionally,
> various software does not allow or support this (for example, libtls
> and hence ftp(1)).
What should I do here?
Thanks,
Jan
Re: LibreSSL regressions
On Tue, Feb 16, 2021 at 04:36:59AM +1100, Joel Sing wrote:
> On 21-02-15 14:49:46, Jan Klemkow wrote:
> > On Sat, Feb 13, 2021 at 03:53:48PM +0100, Theo Buehler wrote:
> > > On Sat, Feb 13, 2021 at 11:58:04AM +0100, Jan Klemkow wrote:
> > > > 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?
> > > >
> > > > [1]: https://github.com/noxxi/libressl-tests
> > >
> > > 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.
> >
> > This is an updated version of the test including comments and wishes
> > from tb@ and bluhm@.
> >
> > OK?
>
> This currently drives openssl(1) for tests, which means that it is
> testing openssl(1), libssl and libcrypto, when what you're really
> wanting to test is libcrypto's verifier. While this works, the
> problem is that a change or breakage in libssl or openssl(1) results
> in a regress failure for libcrypto. If this is to land in its
> current form it really should be in regress/usr.bin/openssl -
> alternatively, it could be reworked to explicitly test libcrypto's
> APIs and remain here.
Except for the auto chain thing...
I'm ok with it going into regress/usr.bin/openssl/x509/ in essentially
its present form (with jsings and my nits addressed):
mv regress/lib/libcrypto/validate regress/usr.bin/openssl/x509
then change the SUBDIR line in the regress/usr.bin/openssl/Makefile:
-SUBDIR= options
+SUBDIR= options x509
> Some additional comments inline.
I also have a few more.
>
> > Index: regress/lib/libcrypto/validate/Makefile
> > ===
> > RCS file: regress/lib/libcrypto/validate/Makefile
> > diff -N regress/lib/libcrypto/validate/Makefile
> > --- /dev/null 1 Jan 1970 00:00:00 -
> > +++ regress/lib/libcrypto/validate/Makefile 15 Feb 2021 13:38:22 -
> > @@ -0,0 +1,133 @@
> > +# $OpenBSD$
> > +
> > +# Copyright (c) 2021 Jan Klemkow
> > +#
> > +# Permission to use, copy, modify, and distribute this software for any
> > +# purpose with or without fee is hereby granted, provided that the above
> > +# copyright notice and this permission notice appear in all copies.
> > +#
> > +# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> > +# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> > +# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> > +# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> > +# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> > +# ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> > +# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> > +
> > +# This regression test is based on manual test descriptions from:
> > +# https://github.com/noxxi/libressl-tests
> > +
> > +# The following port must be installed for the regression tests:
> > +# p5-IO-Socket-SSL perl interface to SSL sockets
> > +
> > +PERL = perl
> > +OPENSSL ?= openssl
> > +
> > +PERL_REQUIRE !=perl -Mstrict -Mwarnings -e ' \
> > +eval { require IO::Socket::SSL } or print $@; \
> > +'
> > +.if ! empty (PERL_REQUIRE)
> > +regress:
> > + @echo "${PERL_REQUIRE}"
> > + @echo install these perl packages for additional tests
> > + @echo SKIPPED
> > +.endif
> > +
> > +REGRESS_TARGETS += test-unusual-wildcard-cert-no-CA-client
> > +REGRESS_TARGETS += test-unusual-wildcard-cert-CA-client
>
> I'd would s/unusual-wildcard/inlabel-wildcard/g
>
> > +REGRESS_TARGETS += test-common-wildcard-cert-no-CA-client
> > +REGRESS_TARGETS += test-common wildcard-cert-CA-client
>
> There's a space between "test-common" and "wildcard-cert-CA-client"
> (presumably meant to be a hyphen) - same in two places below.
>
> > +REGRESS_TARGETS += test-verify-unusual-wildcard-cert
> > +REGRESS_TARGETS += test-openssl-verify-common-wildcard-cert
> > +REGRESS_TARGETS += test-chain-certificates-s_server
> > +REGRESS_TARGETS += test-alternative-chain
> > +REGRESS_CLEANUP = cleanup-ssl
> >
Re: LibreSSL regressions
On Sat, Feb 13, 2021 at 03:53:48PM +0100, Theo Buehler wrote:
> On Sat, Feb 13, 2021 at 11:58:04AM +0100, Jan Klemkow wrote:
> > 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?
> >
> > [1]: https://github.com/noxxi/libressl-tests
>
> 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.
This is an updated version of the test including comments and wishes
from tb@ and bluhm@.
OK?
Thanks,
Jan
Index: regress/lib/libcrypto/validate/Makefile
===
RCS file: regress/lib/libcrypto/validate/Makefile
diff -N regress/lib/libcrypto/validate/Makefile
--- /dev/null 1 Jan 1970 00:00:00 -
+++ regress/lib/libcrypto/validate/Makefile 15 Feb 2021 13:38:22 -
@@ -0,0 +1,133 @@
+# $OpenBSD$
+
+# Copyright (c) 2021 Jan Klemkow
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+# ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+# This regression test is based on manual test descriptions from:
+# https://github.com/noxxi/libressl-tests
+
+# The following port must be installed for the regression tests:
+# p5-IO-Socket-SSL perl interface to SSL sockets
+
+PERL = perl
+OPENSSL ?= openssl
+
+PERL_REQUIRE !=perl -Mstrict -Mwarnings -e ' \
+eval { require IO::Socket::SSL } or print $@; \
+'
+.if ! empty (PERL_REQUIRE)
+regress:
+ @echo "${PERL_REQUIRE}"
+ @echo install these perl packages for additional tests
+ @echo SKIPPED
+.endif
+
+REGRESS_TARGETS += test-unusual-wildcard-cert-no-CA-client
+REGRESS_TARGETS += test-unusual-wildcard-cert-CA-client
+REGRESS_TARGETS += test-common-wildcard-cert-no-CA-client
+REGRESS_TARGETS += test-common wildcard-cert-CA-client
+REGRESS_TARGETS += test-verify-unusual-wildcard-cert
+REGRESS_TARGETS += test-openssl-verify-common-wildcard-cert
+REGRESS_TARGETS += test-chain-certificates-s_server
+REGRESS_TARGETS += test-alternative-chain
+REGRESS_CLEANUP = cleanup-ssl
+REGRESS_SETUP_ONCE = create-libressl-test-certs
+
+REGRESS_EXPECTED_FAILURES += test-unusual-wildcard-cert-no-CA-client
+REGRESS_EXPECTED_FAILURES += test-common-wildcard-cert-no-CA-client
+REGRESS_EXPECTED_FAILURES += test-common wildcard-cert-CA-client
+REGRESS_EXPECTED_FAILURES += test-verify-unusual-wildcard-cert
+REGRESS_EXPECTED_FAILURES += test-alternative-chain
+
+create-libressl-test-certs: create-libressl-test-certs.pl
+ ${PERL} ${.CURDIR}/$@.pl
+
+cleanup-ssl:
+ rm *.pem *.key
+
+test-unusual-wildcard-cert-no-CA-client:
+ # unusual wildcard cert, no CA given to client
+ # start client
+ ${OPENSSL} s_server -cert server-unusual-wildcard.pem \
+ -key server-unusual-wildcard.pem & \
+ timeout=$$(($$(date +%s) + 5)); \
+ while fstat -p $$! | ! grep -q 'tcp .* \*:4433$$'; \
+ do test $$(date +%s) -lt $$timeout || exit 1; done
+ # start client
+ echo "Q" | ${OPENSSL} s_client -verify_return_error \
+ | grep "Verify return code: 21"
+
+test-unusual-wildcard-cert-CA-client:
+ # unusual wildcard cert, CA given to client
+ # start server
+ ${OPENSSL} s_server -cert server-unusual-wildcard.pem \
+ -key server-unusual-wildcard.pem & \
+ timeout=$$(($$(date +%s) + 5)); \
+ while fstat -p $$! | ! grep -q 'tcp .* \*:4433$$'; \
+ do test $$(date +%s) -lt $$timeout || exit 1; done
Re: LibreSSL regressions
On 21-02-15 14:49:46, Jan Klemkow wrote:
> On Sat, Feb 13, 2021 at 03:53:48PM +0100, Theo Buehler wrote:
> > On Sat, Feb 13, 2021 at 11:58:04AM +0100, Jan Klemkow wrote:
> > > 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?
> > >
> > > [1]: https://github.com/noxxi/libressl-tests
> >
> > 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.
>
> This is an updated version of the test including comments and wishes
> from tb@ and bluhm@.
>
> OK?
This currently drives openssl(1) for tests, which means that it is
testing openssl(1), libssl and libcrypto, when what you're really
wanting to test is libcrypto's verifier. While this works, the
problem is that a change or breakage in libssl or openssl(1) results
in a regress failure for libcrypto. If this is to land in its
current form it really should be in regress/usr.bin/openssl -
alternatively, it could be reworked to explicitly test libcrypto's
APIs and remain here.
Some additional comments inline.
> Index: regress/lib/libcrypto/validate/Makefile
> ===
> RCS file: regress/lib/libcrypto/validate/Makefile
> diff -N regress/lib/libcrypto/validate/Makefile
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ regress/lib/libcrypto/validate/Makefile 15 Feb 2021 13:38:22 -
> @@ -0,0 +1,133 @@
> +# $OpenBSD$
> +
> +# Copyright (c) 2021 Jan Klemkow
> +#
> +# Permission to use, copy, modify, and distribute this software for any
> +# purpose with or without fee is hereby granted, provided that the above
> +# copyright notice and this permission notice appear in all copies.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> +# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> +# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> +# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> +# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> +# ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> +# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> +
> +# This regression test is based on manual test descriptions from:
> +# https://github.com/noxxi/libressl-tests
> +
> +# The following port must be installed for the regression tests:
> +# p5-IO-Socket-SSL perl interface to SSL sockets
> +
> +PERL = perl
> +OPENSSL ?= openssl
> +
> +PERL_REQUIRE != perl -Mstrict -Mwarnings -e ' \
> +eval { require IO::Socket::SSL } or print $@; \
> +'
> +.if ! empty (PERL_REQUIRE)
> +regress:
> + @echo "${PERL_REQUIRE}"
> + @echo install these perl packages for additional tests
> + @echo SKIPPED
> +.endif
> +
> +REGRESS_TARGETS += test-unusual-wildcard-cert-no-CA-client
> +REGRESS_TARGETS += test-unusual-wildcard-cert-CA-client
I'd would s/unusual-wildcard/inlabel-wildcard/g
> +REGRESS_TARGETS += test-common-wildcard-cert-no-CA-client
> +REGRESS_TARGETS += test-common wildcard-cert-CA-client
There's a space between "test-common" and "wildcard-cert-CA-client"
(presumably meant to be a hyphen) - same in two places below.
> +REGRESS_TARGETS += test-verify-unusual-wildcard-cert
> +REGRESS_TARGETS += test-openssl-verify-common-wildcard-cert
> +REGRESS_TARGETS += test-chain-certificates-s_server
> +REGRESS_TARGETS += test-alternative-chain
> +REGRESS_CLEANUP =cleanup-ssl
> +REGRESS_SETUP_ONCE = create-libressl-test-certs
> +
> +REGRESS_EXPECTED_FAILURES += test-unusual-wildcard-cert-no-CA-client
> +REGRESS_EXPECTED_FAILURES += test-common-wildcard-cert-no-CA-client
> +REGRESS_EXPECTED_FAILURES += test-common wildcard-cert-CA-client
> +REGRESS_EXPECTED_FAILURES += test-verify-unusual-wildcard-cert
> +REGRESS_EXPECTED_FAILURES += test-alternative-chain
I suspect that some or all of these are expected behaviour, rather
than failures. We can review and address this once it lands though.
> +create-libressl-test-certs: create-libressl-test-certs.pl
> + ${PERL} ${.CUR
Re: LibreSSL regressions
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 - 1.42
> +++ regress/lib/libssl/Makefile 12 Feb 2021 19:42:56 -
> @@ -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 -
> +++ regress/lib/libssl/validate/Makefile 13 Feb 2021 10:50:30 -
> @@ -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
> + ec
