diff mbox series

[Openvpn-devel] check_engine_keys: make pass with OpenSSL 3

Message ID 20230110170257.113527-1-frank@lichtenheld.com
State Accepted
Headers
Series [Openvpn-devel] check_engine_keys: make pass with OpenSSL 3 |

Commit Message

Frank Lichtenheld Jan. 10, 2023, 5:02 p.m. UTC 
  Not enabled by default with OpenSSL 3, so we don't
see this in our builds.
While here add missing entries to .gitignore (which
is what made me look at engine-key test in the first
place).

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
---
 .gitignore                                       | 4 ++++
 tests/unit_tests/engine-key/check_engine_keys.sh | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Gert Doering Jan. 11, 2023, 7:39 a.m. UTC | #1 
Hi,

On Tue, Jan 10, 2023 at 06:02:57PM +0100, Frank Lichtenheld wrote:
> @@ -27,7 +27,7 @@ ${top_builddir}/src/openvpn/openvpn --cd ${top_srcdir}/sample --config sample-co
>  # first off check we died because of a key mismatch.  If this doesn't
>  # pass, suspect openssl of returning different messages and update the
>  # test accordingly
> -loggrep '(X509_check_private_key:key values mismatch|func\(128\):reason\(116\))' log.txt || { echo "Key mismatch not detected"; exit 1; }
> +loggrep '(x509 certificate routines:(X509_check_private_key)?:key values mismatch|func\(128\):reason\(116\))' log.txt || { echo "Key mismatch not detected"; exit 1; }

This change does not convince me - assuming normal regex here, the
original grep would have found any lines with

  X509_check_private_key:key values mismatch

in them, while the new one would find

  X509_check_private_key::key values mismatch
  X509_check_private_key:X509_check_private_key:key values mismatch

but not "the old one with just one :"

Should this be

  (x509 certificate routines:(X509_check_private_key:)?key va...

with the second ":" part of the (...:)? zero-or-once match?


(... as a side note, I hate this test... it's easily the test with the
most commits to make it behave across platforms and SSL library versions)

gert
Frank Lichtenheld Jan. 11, 2023, 12:18 p.m. UTC | #2 
On Wed, Jan 11, 2023 at 08:39:51AM +0100, Gert Doering wrote:
> Hi,
> 
> On Tue, Jan 10, 2023 at 06:02:57PM +0100, Frank Lichtenheld wrote:
> > @@ -27,7 +27,7 @@ ${top_builddir}/src/openvpn/openvpn --cd ${top_srcdir}/sample --config sample-co
> >  # first off check we died because of a key mismatch.  If this doesn't
> >  # pass, suspect openssl of returning different messages and update the
> >  # test accordingly
> > -loggrep '(X509_check_private_key:key values mismatch|func\(128\):reason\(116\))' log.txt || { echo "Key mismatch not detected"; exit 1; }
> > +loggrep '(x509 certificate routines:(X509_check_private_key)?:key values mismatch|func\(128\):reason\(116\))' log.txt || { echo "Key mismatch not detected"; exit 1; }
> 
> This change does not convince me - assuming normal regex here, the
> original grep would have found any lines with
> 
>   X509_check_private_key:key values mismatch
> 
> in them, while the new one would find
> 
>   X509_check_private_key::key values mismatch
>   X509_check_private_key:X509_check_private_key:key values mismatch
> 
> but not "the old one with just one :"
> 
> Should this be
> 
>   (x509 certificate routines:(X509_check_private_key:)?key va...
> 
> with the second ":" part of the (...:)? zero-or-once match?

So, OpenSSL 3 prints:

x509 certificate routines::key values mismatch

yes, two colons!

OpenSSL 1.1 prints:

x509 certificate routines:X509_check_private_key:key values mismatch

My patch matches for both. But you are right that I made it slightly stricter,
since it previously we only matched on

X509_check_private_key:key values mismatch

for OpenSSL 1.1. If you would prefer that we could go for

(x509 certificate routines:|X509_check_private_key):key values mismatch

But I like my version better.  If that passes buildbot, I think it
should be "good enough".

> (... as a side note, I hate this test... it's easily the test with the
> most commits to make it behave across platforms and SSL library versions)

yeah, definitely.

Regards,
Gert Doering Jan. 11, 2023, 1:45 p.m. UTC | #3 
Acked-by: Gert Doering <gert@greenie.muc.de>

Looking more closely I can see that I misread the regex, and it's all
fine indeed.  Passing the test on my test candidates (ossl 1.1.x, 
ossl 3.0.x but no engine support) and also on the GHA actions with
both ossl versions.

Your patch has been applied to the master branch.

commit af25448ee19da5d225a6a1f30f26dc3949ed8921 (master)
commit d09db604dac1e8140c0e4cad73a8b3d2e21cbebc (release/2.6)
Author: Frank Lichtenheld
Date:   Tue Jan 10 18:02:57 2023 +0100

     check_engine_keys: make pass with OpenSSL 3

     Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20230110170257.113527-1-frank@lichtenheld.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25949.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/.gitignore b/.gitignore
index 7335154f..813413fe 100644
--- a/.gitignore
+++ b/.gitignore
@@ -68,6 +68,10 @@  tests/t_client-*-20??????-??????/
 t_client.rc
 t_client_ips.rc
 tests/unit_tests/**/*_testdriver
+tests/unit_tests/engine-key/client.key
+tests/unit_tests/engine-key/log.txt
+tests/unit_tests/engine-key/openssl.cnf
+tests/unit_tests/engine-key/passwd
 
 src/openvpn/openvpn
 include/openvpn-plugin.h
diff --git a/tests/unit_tests/engine-key/check_engine_keys.sh b/tests/unit_tests/engine-key/check_engine_keys.sh
index 7e9a0e80..12dd2301 100755
--- a/tests/unit_tests/engine-key/check_engine_keys.sh
+++ b/tests/unit_tests/engine-key/check_engine_keys.sh
@@ -27,7 +27,7 @@  ${top_builddir}/src/openvpn/openvpn --cd ${top_srcdir}/sample --config sample-co
 # first off check we died because of a key mismatch.  If this doesn't
 # pass, suspect openssl of returning different messages and update the
 # test accordingly
-loggrep '(X509_check_private_key:key values mismatch|func\(128\):reason\(116\))' log.txt || { echo "Key mismatch not detected"; exit 1; }
+loggrep '(x509 certificate routines:(X509_check_private_key)?:key values mismatch|func\(128\):reason\(116\))' log.txt || { echo "Key mismatch not detected"; exit 1; }
 
 # now look for the engine prints (these are under our control)
 loggrep 'ENGINE: engine_init called' || { echo "Engine initialization not detected"; exit 1; }