[Openvpn-devel,v8,1/1] Add unit tests for engine keys

Message ID 20200622232319.8143-2-James.Bottomley@HansenPartnership.com
State Accepted
Headers show
Series
  • Add unit tests for engine keys
Related show

Commit Message

James Bottomley June 22, 2020, 11:23 p.m.
Testing engines is problematic, so one of the prerequisites built for
the tests is a simple openssl engine that reads a non-standard PEM
guarded key.  The test is simply can we run a client/server
configuration with the usual sample key replaced by an engine key.
The trivial engine prints out some operations and we check for these
in the log to make sure the engine was used to load the key and that
it correctly got the password.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

---
v8: Fix openssl.cnf.in build rule for BSD
v7: Hard code .so for dll
v6: add absolute path instead of env variable in openssl.cnf (MacOS)
v5: do not hard code dynamic library extension into openssl.cnf (MacOS)
v4: add OPENSSL_config(NULL) so debian checks will work
v3: added this patch
---
 configure.ac                                  |   2 +
 tests/unit_tests/Makefile.am                  |   3 +
 tests/unit_tests/engine-key/Makefile.am       |  24 +++++
 .../engine-key/check_engine_keys.sh           |  30 ++++++
 tests/unit_tests/engine-key/libtestengine.c   | 101 ++++++++++++++++++
 tests/unit_tests/engine-key/openssl.cnf.in    |  12 +++
 6 files changed, 172 insertions(+)
 create mode 100644 tests/unit_tests/engine-key/Makefile.am
 create mode 100755 tests/unit_tests/engine-key/check_engine_keys.sh
 create mode 100644 tests/unit_tests/engine-key/libtestengine.c
 create mode 100644 tests/unit_tests/engine-key/openssl.cnf.in

Comments

Gert Doering June 23, 2020, 6:28 a.m. | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

Tested on :
 - MacOS Mojave with OpenSSL 1.1.1c (brew) and out-of-tree build, works.
 - Linux with mbedtls (does not try engine tests, good :-) )
 - Linux with OpenSSL 1.1.1, works
 - FreeBSD 11.3 with OpenSSL 1.0.2s -> v6 fails, v6 works \o/

Conferred with Arne, we agreed on "this is good enough, and who wants 
something more sophisticated is welcome to send more patches".

Your patch has been applied to the master branch.

Thanks :)

commit 542c69c37b347d1895dde076508d0f6554544860
Author: James Bottomley
Date:   Mon Jun 22 16:23:19 2020 -0700

     Add unit tests for engine keys

     Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20200622232319.8143-2-James.Bottomley@HansenPartnership.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20075.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Gert Doering June 23, 2020, 7:21 a.m. | #2
Hi,

On Tue, Jun 23, 2020 at 08:28:36AM +0200, Gert Doering wrote:
> Acked-by: Gert Doering <gert@greenie.muc.de>
> 
> Tested on :
>  - MacOS Mojave with OpenSSL 1.1.1c (brew) and out-of-tree build, works.
>  - Linux with mbedtls (does not try engine tests, good :-) )
>  - Linux with OpenSSL 1.1.1, works
>  - FreeBSD 11.3 with OpenSSL 1.0.2s -> v6 fails, v6 works \o/
> 
> Conferred with Arne, we agreed on "this is good enough, and who wants 
> something more sophisticated is welcome to send more patches".

Oh well.  We do need another round - Travis tells me that "make distcheck"
is failing.  Which hints at "autoconf is not told what to pack in the
tarball"

https://travis-ci.org/github/OpenVPN/openvpn/jobs/701158155
...
make[6]: Entering directory '/home/travis/build/OpenVPN/openvpn/openvpn-2.5_git/_build/sub/tests/unit_tests/engine-key'
3672make[6]: *** No rule to make target '../../../../../tests/unit_tests/engine-key/openssl.cnf.in', needed by 'openssl.cnf'.  Stop.
3673make[6]: Leaving directory '/home/travis/build/OpenVPN/openvpn/openvpn-2.5_git/_build/sub/tests/unit_tests/engine-key'
3674

(so now the source file is missing)

Please... :-9

gert
James Bottomley June 23, 2020, 1:05 p.m. | #3
On Tue, 2020-06-23 at 09:21 +0200, Gert Doering wrote:
> Hi,
> 
> On Tue, Jun 23, 2020 at 08:28:36AM +0200, Gert Doering wrote:
> > Acked-by: Gert Doering <gert@greenie.muc.de>
> > 
> > Tested on :
> >  - MacOS Mojave with OpenSSL 1.1.1c (brew) and out-of-tree build,
> > works.
> >  - Linux with mbedtls (does not try engine tests, good :-) )
> >  - Linux with OpenSSL 1.1.1, works
> >  - FreeBSD 11.3 with OpenSSL 1.0.2s -> v6 fails, v6 works \o/
> > 
> > Conferred with Arne, we agreed on "this is good enough, and who
> > wants 
> > something more sophisticated is welcome to send more patches".
> 
> Oh well.  We do need another round - Travis tells me that "make
> distcheck"
> is failing.  Which hints at "autoconf is not told what to pack in the
> tarball"
> 
> https://travis-ci.org/github/OpenVPN/openvpn/jobs/701158155
> ...
> make[6]: Entering directory
> '/home/travis/build/OpenVPN/openvpn/openvpn-
> 2.5_git/_build/sub/tests/unit_tests/engine-key'
> 3672make[6]: *** No rule to make target
> '../../../../../tests/unit_tests/engine-key/openssl.cnf.in', needed
> by 'openssl.cnf'.  Stop.
> 3673make[6]: Leaving directory
> '/home/travis/build/OpenVPN/openvpn/openvpn-
> 2.5_git/_build/sub/tests/unit_tests/engine-key'
> 3674
> 
> (so now the source file is missing)
> 
> Please... :-)

Sorry about that ... it's missing files from EXTRA_DIST ... plus I
don't usually use make dist, so I never remember to run make distcheck.
 It passes after this

---8>8>8><8<8<8---

From: James Bottomley <James.Bottomley@HansenPartnership.com>
Subject: [PATCH] Fix make distcheck for new engine key unit test

Add config precursor and script to extra dist and make sure
built and test leftover files are cleaned up afterwards.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 tests/unit_tests/engine-key/Makefile.am | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/unit_tests/engine-key/Makefile.am b/tests/unit_tests/engine-key/Makefile.am
index 95e7d868..0bfdfcd4 100644
--- a/tests/unit_tests/engine-key/Makefile.am
+++ b/tests/unit_tests/engine-key/Makefile.am
@@ -2,6 +2,9 @@ AUTOMAKE_OPTIONS = foreign
 
 check_LTLIBRARIES = libtestengine.la
 conffiles = openssl.cnf
+EXTRA_DIST = \
+	openssl.cnf.in \
+	check_engine_keys.sh
 
 TESTS_ENVIRONMENT = srcdir="$(abs_srcdir)"; \
 	builddir="$(abs_builddir)"; \
@@ -12,8 +15,11 @@ TESTS_ENVIRONMENT = srcdir="$(abs_srcdir)"; \
 TESTS = check_engine_keys.sh
 check_engine_keys.sh: $(conffiles)
 
-clean-local:
-	rm -f $(conffiles)
+CLEANFILES = \
+	client.key \
+	passwd \
+	log.txt \
+	$(conffiles)
 
 $(builddir)/openssl.cnf: $(srcdir)/openssl.cnf.in
 	sed "s|ABSBUILDDIR|$(abs_builddir)|" < $< > $@
Илья Шипицин June 23, 2020, 4:42 p.m. | #4
apparently, it fails for some build on travis
https://travis-ci.org/github/OpenVPN/openvpn/jobs/701158156

вт, 23 июн. 2020 г. в 18:07, James Bottomley <
James.Bottomley@hansenpartnership.com>:

> On Tue, 2020-06-23 at 09:21 +0200, Gert Doering wrote:
> > Hi,
> >
> > On Tue, Jun 23, 2020 at 08:28:36AM +0200, Gert Doering wrote:
> > > Acked-by: Gert Doering <gert@greenie.muc.de>
> > >
> > > Tested on :
> > >  - MacOS Mojave with OpenSSL 1.1.1c (brew) and out-of-tree build,
> > > works.
> > >  - Linux with mbedtls (does not try engine tests, good :-) )
> > >  - Linux with OpenSSL 1.1.1, works
> > >  - FreeBSD 11.3 with OpenSSL 1.0.2s -> v6 fails, v6 works \o/
> > >
> > > Conferred with Arne, we agreed on "this is good enough, and who
> > > wants
> > > something more sophisticated is welcome to send more patches".
> >
> > Oh well.  We do need another round - Travis tells me that "make
> > distcheck"
> > is failing.  Which hints at "autoconf is not told what to pack in the
> > tarball"
> >
> > https://travis-ci.org/github/OpenVPN/openvpn/jobs/701158155
> > ...
> > make[6]: Entering directory
> > '/home/travis/build/OpenVPN/openvpn/openvpn-
> > 2.5_git/_build/sub/tests/unit_tests/engine-key'
> > 3672make[6]: *** No rule to make target
> > '../../../../../tests/unit_tests/engine-key/openssl.cnf.in', needed
> > by 'openssl.cnf'.  Stop.
> > 3673make[6]: Leaving directory
> > '/home/travis/build/OpenVPN/openvpn/openvpn-
> > 2.5_git/_build/sub/tests/unit_tests/engine-key'
> > 3674
> >
> > (so now the source file is missing)
> >
> > Please... :-)
>
> Sorry about that ... it's missing files from EXTRA_DIST ... plus I
> don't usually use make dist, so I never remember to run make distcheck.
>  It passes after this
>
> ---8>8>8><8<8<8---
>
> From: James Bottomley <James.Bottomley@HansenPartnership.com>
> Subject: [PATCH] Fix make distcheck for new engine key unit test
>
> Add config precursor and script to extra dist and make sure
> built and test leftover files are cleaned up afterwards.
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>  tests/unit_tests/engine-key/Makefile.am | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/tests/unit_tests/engine-key/Makefile.am
> b/tests/unit_tests/engine-key/Makefile.am
> index 95e7d868..0bfdfcd4 100644
> --- a/tests/unit_tests/engine-key/Makefile.am
> +++ b/tests/unit_tests/engine-key/Makefile.am
> @@ -2,6 +2,9 @@ AUTOMAKE_OPTIONS = foreign
>
>  check_LTLIBRARIES = libtestengine.la
>  conffiles = openssl.cnf
> +EXTRA_DIST = \
> +       openssl.cnf.in \
> +       check_engine_keys.sh
>
>  TESTS_ENVIRONMENT = srcdir="$(abs_srcdir)"; \
>         builddir="$(abs_builddir)"; \
> @@ -12,8 +15,11 @@ TESTS_ENVIRONMENT = srcdir="$(abs_srcdir)"; \
>  TESTS = check_engine_keys.sh
>  check_engine_keys.sh: $(conffiles)
>
> -clean-local:
> -       rm -f $(conffiles)
> +CLEANFILES = \
> +       client.key \
> +       passwd \
> +       log.txt \
> +       $(conffiles)
>
>  $(builddir)/openssl.cnf: $(srcdir)/openssl.cnf.in
>         sed "s|ABSBUILDDIR|$(abs_builddir)|" < $< > $@
> --
> 2.26.2
>
>
>
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
<div dir="ltr"><div>apparently, it fails for some build on travis</div><div><a href="https://travis-ci.org/github/OpenVPN/openvpn/jobs/701158156">https://travis-ci.org/github/OpenVPN/openvpn/jobs/701158156</a></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">вт, 23 июн. 2020 г. в 18:07, James Bottomley &lt;<a href="mailto:James.Bottomley@hansenpartnership.com">James.Bottomley@hansenpartnership.com</a>&gt;:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, 2020-06-23 at 09:21 +0200, Gert Doering wrote:<br>
&gt; Hi,<br>
&gt; <br>
&gt; On Tue, Jun 23, 2020 at 08:28:36AM +0200, Gert Doering wrote:<br>
&gt; &gt; Acked-by: Gert Doering &lt;<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>&gt;<br>
&gt; &gt; <br>
&gt; &gt; Tested on :<br>
&gt; &gt;  - MacOS Mojave with OpenSSL 1.1.1c (brew) and out-of-tree build,<br>
&gt; &gt; works.<br>
&gt; &gt;  - Linux with mbedtls (does not try engine tests, good :-) )<br>
&gt; &gt;  - Linux with OpenSSL 1.1.1, works<br>
&gt; &gt;  - FreeBSD 11.3 with OpenSSL 1.0.2s -&gt; v6 fails, v6 works \o/<br>
&gt; &gt; <br>
&gt; &gt; Conferred with Arne, we agreed on &quot;this is good enough, and who<br>
&gt; &gt; wants <br>
&gt; &gt; something more sophisticated is welcome to send more patches&quot;.<br>
&gt; <br>
&gt; Oh well.  We do need another round - Travis tells me that &quot;make<br>
&gt; distcheck&quot;<br>
&gt; is failing.  Which hints at &quot;autoconf is not told what to pack in the<br>
&gt; tarball&quot;<br>
&gt; <br>
&gt; <a href="https://travis-ci.org/github/OpenVPN/openvpn/jobs/701158155" rel="noreferrer" target="_blank">https://travis-ci.org/github/OpenVPN/openvpn/jobs/701158155</a><br>
&gt; ...<br>
&gt; make[6]: Entering directory<br>
&gt; &#39;/home/travis/build/OpenVPN/openvpn/openvpn-<br>
&gt; 2.5_git/_build/sub/tests/unit_tests/engine-key&#39;<br>
&gt; 3672make[6]: *** No rule to make target<br>
&gt; &#39;../../../../../tests/unit_tests/engine-key/<a href="http://openssl.cnf.in" rel="noreferrer" target="_blank">openssl.cnf.in</a>&#39;, needed<br>
&gt; by &#39;openssl.cnf&#39;.  Stop.<br>
&gt; 3673make[6]: Leaving directory<br>
&gt; &#39;/home/travis/build/OpenVPN/openvpn/openvpn-<br>
&gt; 2.5_git/_build/sub/tests/unit_tests/engine-key&#39;<br>
&gt; 3674<br>
&gt; <br>
&gt; (so now the source file is missing)<br>
&gt; <br>
&gt; Please... :-)<br>
<br>
Sorry about that ... it&#39;s missing files from EXTRA_DIST ... plus I<br>
don&#39;t usually use make dist, so I never remember to run make distcheck.<br>
 It passes after this<br>
<br>
---8&gt;8&gt;8&gt;&lt;8&lt;8&lt;8---<br>
<br>
From: James Bottomley &lt;James.Bottomley@HansenPartnership.com&gt;<br>
Subject: [PATCH] Fix make distcheck for new engine key unit test<br>
<br>
Add config precursor and script to extra dist and make sure<br>
built and test leftover files are cleaned up afterwards.<br>
<br>
Signed-off-by: James Bottomley &lt;James.Bottomley@HansenPartnership.com&gt;<br>
---<br>
 tests/unit_tests/engine-key/Makefile.am | 10 ++++++++--<br>
 1 file changed, 8 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/tests/unit_tests/engine-key/Makefile.am b/tests/unit_tests/engine-key/Makefile.am<br>
index 95e7d868..0bfdfcd4 100644<br>
--- a/tests/unit_tests/engine-key/Makefile.am<br>
+++ b/tests/unit_tests/engine-key/Makefile.am<br>
@@ -2,6 +2,9 @@ AUTOMAKE_OPTIONS = foreign<br>
<br>
 check_LTLIBRARIES = <a href="http://libtestengine.la" rel="noreferrer" target="_blank">libtestengine.la</a><br>
 conffiles = openssl.cnf<br>
+EXTRA_DIST = \<br>
+       <a href="http://openssl.cnf.in" rel="noreferrer" target="_blank">openssl.cnf.in</a> \<br>
+       check_engine_keys.sh<br>
<br>
 TESTS_ENVIRONMENT = srcdir=&quot;$(abs_srcdir)&quot;; \<br>
        builddir=&quot;$(abs_builddir)&quot;; \<br>
@@ -12,8 +15,11 @@ TESTS_ENVIRONMENT = srcdir=&quot;$(abs_srcdir)&quot;; \<br>
 TESTS = check_engine_keys.sh<br>
 check_engine_keys.sh: $(conffiles)<br>
<br>
-clean-local:<br>
-       rm -f $(conffiles)<br>
+CLEANFILES = \<br>
+       client.key \<br>
+       passwd \<br>
+       log.txt \<br>
+       $(conffiles)<br>
<br>
 $(builddir)/openssl.cnf: $(srcdir)/<a href="http://openssl.cnf.in" rel="noreferrer" target="_blank">openssl.cnf.in</a><br>
        sed &quot;s|ABSBUILDDIR|$(abs_builddir)|&quot; &lt; $&lt; &gt; $@<br>
-- <br>
2.26.2<br>
<br>
<br>
<br>
_______________________________________________<br>
Openvpn-devel mailing list<br>
<a href="mailto:Openvpn-devel@lists.sourceforge.net" target="_blank">Openvpn-devel@lists.sourceforge.net</a><br>
<a href="https://lists.sourceforge.net/lists/listinfo/openvpn-devel" rel="noreferrer" target="_blank">https://lists.sourceforge.net/lists/listinfo/openvpn-devel</a><br>
</blockquote></div>
Илья Шипицин June 23, 2020, 4:43 p.m. | #5
as far as I understand, openssl-1.0.2 does not support engines ?

вт, 23 июн. 2020 г. в 21:42, Илья Шипицин <chipitsine@gmail.com>:

> apparently, it fails for some build on travis
> https://travis-ci.org/github/OpenVPN/openvpn/jobs/701158156
>
> вт, 23 июн. 2020 г. в 18:07, James Bottomley <
> James.Bottomley@hansenpartnership.com>:
>
>> On Tue, 2020-06-23 at 09:21 +0200, Gert Doering wrote:
>> > Hi,
>> >
>> > On Tue, Jun 23, 2020 at 08:28:36AM +0200, Gert Doering wrote:
>> > > Acked-by: Gert Doering <gert@greenie.muc.de>
>> > >
>> > > Tested on :
>> > >  - MacOS Mojave with OpenSSL 1.1.1c (brew) and out-of-tree build,
>> > > works.
>> > >  - Linux with mbedtls (does not try engine tests, good :-) )
>> > >  - Linux with OpenSSL 1.1.1, works
>> > >  - FreeBSD 11.3 with OpenSSL 1.0.2s -> v6 fails, v6 works \o/
>> > >
>> > > Conferred with Arne, we agreed on "this is good enough, and who
>> > > wants
>> > > something more sophisticated is welcome to send more patches".
>> >
>> > Oh well.  We do need another round - Travis tells me that "make
>> > distcheck"
>> > is failing.  Which hints at "autoconf is not told what to pack in the
>> > tarball"
>> >
>> > https://travis-ci.org/github/OpenVPN/openvpn/jobs/701158155
>> > ...
>> > make[6]: Entering directory
>> > '/home/travis/build/OpenVPN/openvpn/openvpn-
>> > 2.5_git/_build/sub/tests/unit_tests/engine-key'
>> > 3672make[6]: *** No rule to make target
>> > '../../../../../tests/unit_tests/engine-key/openssl.cnf.in', needed
>> > by 'openssl.cnf'.  Stop.
>> > 3673make[6]: Leaving directory
>> > '/home/travis/build/OpenVPN/openvpn/openvpn-
>> > 2.5_git/_build/sub/tests/unit_tests/engine-key'
>> > 3674
>> >
>> > (so now the source file is missing)
>> >
>> > Please... :-)
>>
>> Sorry about that ... it's missing files from EXTRA_DIST ... plus I
>> don't usually use make dist, so I never remember to run make distcheck.
>>  It passes after this
>>
>> ---8>8>8><8<8<8---
>>
>> From: James Bottomley <James.Bottomley@HansenPartnership.com>
>> Subject: [PATCH] Fix make distcheck for new engine key unit test
>>
>> Add config precursor and script to extra dist and make sure
>> built and test leftover files are cleaned up afterwards.
>>
>> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>> ---
>>  tests/unit_tests/engine-key/Makefile.am | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/unit_tests/engine-key/Makefile.am
>> b/tests/unit_tests/engine-key/Makefile.am
>> index 95e7d868..0bfdfcd4 100644
>> --- a/tests/unit_tests/engine-key/Makefile.am
>> +++ b/tests/unit_tests/engine-key/Makefile.am
>> @@ -2,6 +2,9 @@ AUTOMAKE_OPTIONS = foreign
>>
>>  check_LTLIBRARIES = libtestengine.la
>>  conffiles = openssl.cnf
>> +EXTRA_DIST = \
>> +       openssl.cnf.in \
>> +       check_engine_keys.sh
>>
>>  TESTS_ENVIRONMENT = srcdir="$(abs_srcdir)"; \
>>         builddir="$(abs_builddir)"; \
>> @@ -12,8 +15,11 @@ TESTS_ENVIRONMENT = srcdir="$(abs_srcdir)"; \
>>  TESTS = check_engine_keys.sh
>>  check_engine_keys.sh: $(conffiles)
>>
>> -clean-local:
>> -       rm -f $(conffiles)
>> +CLEANFILES = \
>> +       client.key \
>> +       passwd \
>> +       log.txt \
>> +       $(conffiles)
>>
>>  $(builddir)/openssl.cnf: $(srcdir)/openssl.cnf.in
>>         sed "s|ABSBUILDDIR|$(abs_builddir)|" < $< > $@
>> --
>> 2.26.2
>>
>>
>>
>> _______________________________________________
>> Openvpn-devel mailing list
>> Openvpn-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>>
>
<div dir="ltr">as far as I understand, openssl-1.0.2 does not support engines ?<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">вт, 23 июн. 2020 г. в 21:42, Илья Шипицин &lt;<a href="mailto:chipitsine@gmail.com">chipitsine@gmail.com</a>&gt;:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>apparently, it fails for some build on travis</div><div><a href="https://travis-ci.org/github/OpenVPN/openvpn/jobs/701158156" target="_blank">https://travis-ci.org/github/OpenVPN/openvpn/jobs/701158156</a></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">вт, 23 июн. 2020 г. в 18:07, James Bottomley &lt;<a href="mailto:James.Bottomley@hansenpartnership.com" target="_blank">James.Bottomley@hansenpartnership.com</a>&gt;:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, 2020-06-23 at 09:21 +0200, Gert Doering wrote:<br>
&gt; Hi,<br>
&gt; <br>
&gt; On Tue, Jun 23, 2020 at 08:28:36AM +0200, Gert Doering wrote:<br>
&gt; &gt; Acked-by: Gert Doering &lt;<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>&gt;<br>
&gt; &gt; <br>
&gt; &gt; Tested on :<br>
&gt; &gt;  - MacOS Mojave with OpenSSL 1.1.1c (brew) and out-of-tree build,<br>
&gt; &gt; works.<br>
&gt; &gt;  - Linux with mbedtls (does not try engine tests, good :-) )<br>
&gt; &gt;  - Linux with OpenSSL 1.1.1, works<br>
&gt; &gt;  - FreeBSD 11.3 with OpenSSL 1.0.2s -&gt; v6 fails, v6 works \o/<br>
&gt; &gt; <br>
&gt; &gt; Conferred with Arne, we agreed on &quot;this is good enough, and who<br>
&gt; &gt; wants <br>
&gt; &gt; something more sophisticated is welcome to send more patches&quot;.<br>
&gt; <br>
&gt; Oh well.  We do need another round - Travis tells me that &quot;make<br>
&gt; distcheck&quot;<br>
&gt; is failing.  Which hints at &quot;autoconf is not told what to pack in the<br>
&gt; tarball&quot;<br>
&gt; <br>
&gt; <a href="https://travis-ci.org/github/OpenVPN/openvpn/jobs/701158155" rel="noreferrer" target="_blank">https://travis-ci.org/github/OpenVPN/openvpn/jobs/701158155</a><br>
&gt; ...<br>
&gt; make[6]: Entering directory<br>
&gt; &#39;/home/travis/build/OpenVPN/openvpn/openvpn-<br>
&gt; 2.5_git/_build/sub/tests/unit_tests/engine-key&#39;<br>
&gt; 3672make[6]: *** No rule to make target<br>
&gt; &#39;../../../../../tests/unit_tests/engine-key/<a href="http://openssl.cnf.in" rel="noreferrer" target="_blank">openssl.cnf.in</a>&#39;, needed<br>
&gt; by &#39;openssl.cnf&#39;.  Stop.<br>
&gt; 3673make[6]: Leaving directory<br>
&gt; &#39;/home/travis/build/OpenVPN/openvpn/openvpn-<br>
&gt; 2.5_git/_build/sub/tests/unit_tests/engine-key&#39;<br>
&gt; 3674<br>
&gt; <br>
&gt; (so now the source file is missing)<br>
&gt; <br>
&gt; Please... :-)<br>
<br>
Sorry about that ... it&#39;s missing files from EXTRA_DIST ... plus I<br>
don&#39;t usually use make dist, so I never remember to run make distcheck.<br>
 It passes after this<br>
<br>
---8&gt;8&gt;8&gt;&lt;8&lt;8&lt;8---<br>
<br>
From: James Bottomley &lt;James.Bottomley@HansenPartnership.com&gt;<br>
Subject: [PATCH] Fix make distcheck for new engine key unit test<br>
<br>
Add config precursor and script to extra dist and make sure<br>
built and test leftover files are cleaned up afterwards.<br>
<br>
Signed-off-by: James Bottomley &lt;James.Bottomley@HansenPartnership.com&gt;<br>
---<br>
 tests/unit_tests/engine-key/Makefile.am | 10 ++++++++--<br>
 1 file changed, 8 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/tests/unit_tests/engine-key/Makefile.am b/tests/unit_tests/engine-key/Makefile.am<br>
index 95e7d868..0bfdfcd4 100644<br>
--- a/tests/unit_tests/engine-key/Makefile.am<br>
+++ b/tests/unit_tests/engine-key/Makefile.am<br>
@@ -2,6 +2,9 @@ AUTOMAKE_OPTIONS = foreign<br>
<br>
 check_LTLIBRARIES = <a href="http://libtestengine.la" rel="noreferrer" target="_blank">libtestengine.la</a><br>
 conffiles = openssl.cnf<br>
+EXTRA_DIST = \<br>
+       <a href="http://openssl.cnf.in" rel="noreferrer" target="_blank">openssl.cnf.in</a> \<br>
+       check_engine_keys.sh<br>
<br>
 TESTS_ENVIRONMENT = srcdir=&quot;$(abs_srcdir)&quot;; \<br>
        builddir=&quot;$(abs_builddir)&quot;; \<br>
@@ -12,8 +15,11 @@ TESTS_ENVIRONMENT = srcdir=&quot;$(abs_srcdir)&quot;; \<br>
 TESTS = check_engine_keys.sh<br>
 check_engine_keys.sh: $(conffiles)<br>
<br>
-clean-local:<br>
-       rm -f $(conffiles)<br>
+CLEANFILES = \<br>
+       client.key \<br>
+       passwd \<br>
+       log.txt \<br>
+       $(conffiles)<br>
<br>
 $(builddir)/openssl.cnf: $(srcdir)/<a href="http://openssl.cnf.in" rel="noreferrer" target="_blank">openssl.cnf.in</a><br>
        sed &quot;s|ABSBUILDDIR|$(abs_builddir)|&quot; &lt; $&lt; &gt; $@<br>
-- <br>
2.26.2<br>
<br>
<br>
<br>
_______________________________________________<br>
Openvpn-devel mailing list<br>
<a href="mailto:Openvpn-devel@lists.sourceforge.net" target="_blank">Openvpn-devel@lists.sourceforge.net</a><br>
<a href="https://lists.sourceforge.net/lists/listinfo/openvpn-devel" rel="noreferrer" target="_blank">https://lists.sourceforge.net/lists/listinfo/openvpn-devel</a><br>
</blockquote></div>
</blockquote></div>
Gert Doering June 23, 2020, 5:19 p.m. | #6
Hi,

On Tue, Jun 23, 2020 at 09:42:51PM +0500, ???????? ?????????????? wrote:
> apparently, it fails for some build on travis
> https://travis-ci.org/github/OpenVPN/openvpn/jobs/701158156

This is the reason why James sent this new patch - what we have in
master works but fails "make distcheck".

[..]
> > > >  - Linux with mbedtls (does not try engine tests, good :-) )
> > > >  - Linux with OpenSSL 1.1.1, works
> > > >  - FreeBSD 11.3 with OpenSSL 1.0.2s -> v6 fails, v6 works \o/

As I've tested the patch with 1.0.2s, I can confirm that it supports
engines, at least "enough for our needs".

(Haven't had time to verify that "distcheck" works now, busy with
other issues to wrangle today)

gert
James Bottomley June 23, 2020, 6:17 p.m. | #7
On Tue, 2020-06-23 at 21:43 +0500, Илья Шипицин wrote:
> as far as I understand, openssl-1.0.2 does not support engines ?

No, it does.  Engines were a pre 0.9.8 thing.  I support openssl in my
builds for the TPM engine down to 1.0.1

However, the failure:

> Key mismatch not detected
> 
> FAIL: check_engine_keys.sh
> 
> ====================================================
> 
> 1 of 1 test failed
> 
> Please report to openvpn-users@lists.sourceforge.net
> 
> ====================================================

Is because an expected message isn't found in the output.  I think it's
this:

   # 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
   grep -q 'X509_check_private_key:key values mismatch' log.txt || { echo "Key mismatch not detected"; exit 1; }

   If I could get hold of log.txt that would confirm that the test is
   outputting something slightly different from what's expected.

   I did run this test on openssl-1.0.2j (I keep a copy of
   openSUSE_Leap_42.3 around precisely for this openssl testing) but it
   ran just fine. so there's clearly something different about the 1.0.2u
   you're using (might be a locale issue?).

   James
Илья Шипицин June 23, 2020, 6:36 p.m. | #8
вт, 23 июн. 2020 г. в 23:17, James Bottomley <
James.Bottomley@hansenpartnership.com>:

> On Tue, 2020-06-23 at 21:43 +0500, Илья Шипицин wrote:
> > as far as I understand, openssl-1.0.2 does not support engines ?
>
> No, it does.  Engines were a pre 0.9.8 thing.  I support openssl in my
> builds for the TPM engine down to 1.0.1
>
> However, the failure:
>
> > Key mismatch not detected
> >
> > FAIL: check_engine_keys.sh
> >
> > ====================================================
> >
> > 1 of 1 test failed
> >
> > Please report to openvpn-users@lists.sourceforge.net
> >
> > ====================================================
>
> Is because an expected message isn't found in the output.  I think it's
> this:
>
>    # 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
>    grep -q 'X509_check_private_key:key values mismatch' log.txt || { echo
> "Key mismatch not detected"; exit 1; }
>
>    If I could get hold of log.txt that would confirm that the test is
>    outputting something slightly different from what's expected.
>
>    I did run this test on openssl-1.0.2j (I keep a copy of
>    openSUSE_Leap_42.3 around precisely for this openssl testing) but it
>    ran just fine. so there's clearly something different about the 1.0.2u
>    you're using (might be a locale issue?).
>

I'll have a look. Also, I think we should out log.txt in case of failure.


>
>    James
>
>
<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">вт, 23 июн. 2020 г. в 23:17, James Bottomley &lt;<a href="mailto:James.Bottomley@hansenpartnership.com">James.Bottomley@hansenpartnership.com</a>&gt;:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, 2020-06-23 at 21:43 +0500, Илья Шипицин wrote:<br>
&gt; as far as I understand, openssl-1.0.2 does not support engines ?<br>
<br>
No, it does.  Engines were a pre 0.9.8 thing.  I support openssl in my<br>
builds for the TPM engine down to 1.0.1<br>
<br>
However, the failure:<br>
<br>
&gt; Key mismatch not detected<br>
&gt; <br>
&gt; FAIL: check_engine_keys.sh<br>
&gt; <br>
&gt; ====================================================<br>
&gt; <br>
&gt; 1 of 1 test failed<br>
&gt; <br>
&gt; Please report to <a href="mailto:openvpn-users@lists.sourceforge.net" target="_blank">openvpn-users@lists.sourceforge.net</a><br>
&gt; <br>
&gt; ====================================================<br>
<br>
Is because an expected message isn&#39;t found in the output.  I think it&#39;s<br>
this:<br>
<br>
   # first off check we died because of a key mismatch.  If this doesn&#39;t<br>
   # pass, suspect openssl of returning different messages and update the<br>
   # test accordingly<br>
   grep -q &#39;X509_check_private_key:key values mismatch&#39; log.txt || { echo &quot;Key mismatch not detected&quot;; exit 1; }<br>
<br>
   If I could get hold of log.txt that would confirm that the test is<br>
   outputting something slightly different from what&#39;s expected.<br>
<br>
   I did run this test on openssl-1.0.2j (I keep a copy of<br>
   openSUSE_Leap_42.3 around precisely for this openssl testing) but it<br>
   ran just fine. so there&#39;s clearly something different about the 1.0.2u<br>
   you&#39;re using (might be a locale issue?).<br></blockquote><div><br></div><div>I&#39;ll have a look. Also, I think we should out <span class="gmail-im">log.txt in case of failure.<br></span></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
   James<br>
<br>
</blockquote></div></div>
Gert Doering June 23, 2020, 6:46 p.m. | #9
Acked-by: Gert Doering <gert@greenie.muc.de>

Looks reasonable, explanation is reasonable, and it passes "make distcheck"
(only tested on Linux, but there should not be a difference here).

Your patch has been applied to the master branch.

Thanks.

commit 21e3e9fc34128d37bd612def2acca29a5a18de77 (HEAD -> master)
Author: James Bottomley <James.Bottomley@HansenPartnership.com>
Date:   Tue Jun 23 06:05:31 2020 -0700

    Fix make distcheck for new engine key unit test
    
    Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
    Acked-by: Gert Doering <gert@greenie.muc.de>
    Message-Id: <1592917531.4768.4.camel@HansenPartnership.com>
    URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20088.html
    Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Gert Doering June 23, 2020, 6:47 p.m. | #10
Hi,

On Tue, Jun 23, 2020 at 11:36:49PM +0500, ???????? ?????????????? wrote:
> Also, I think we should out log.txt in case of failure.

Indeed, this would help.

gert
Gert Doering June 23, 2020, 7:26 p.m. | #11
Hi,

On Tue, Jun 23, 2020 at 08:47:33PM +0200, Gert Doering wrote:
> On Tue, Jun 23, 2020 at 11:36:49PM +0500, ???????? ?????????????? wrote:
> > Also, I think we should out log.txt in case of failure.
> 
> Indeed, this would help.

Yep.

The "make distcheck" travis instance is now happy, but the "1.0.2u + 
enable-small" instance is still unhappy.

James, are you triggering on specific openvpn messages?  "--enable-small"
changes these (trimming some warnings and help texts).  Can you test with
"configure --enable-small", please?

https://travis-ci.org/github/OpenVPN/openvpn/builds/701385033?utm_medium=notification&utm_source=email


gert
James Bottomley June 23, 2020, 7:32 p.m. | #12
On Tue, 2020-06-23 at 21:26 +0200, Gert Doering wrote:
> Hi,
> 
> On Tue, Jun 23, 2020 at 08:47:33PM +0200, Gert Doering wrote:
> > On Tue, Jun 23, 2020 at 11:36:49PM +0500, ???????? ??????????????
> > wrote:
> > > Also, I think we should out log.txt in case of failure.
> > 
> > Indeed, this would help.
> 
> Yep.
> 
> The "make distcheck" travis instance is now happy, but the "1.0.2u + 
> enable-small" instance is still unhappy.
> 
> James, are you triggering on specific openvpn messages?  "--enable-
> small"
> changes these (trimming some warnings and help texts).  Can you test
> with
> "configure --enable-small", please?
> 
> https://travis-ci.org/github/OpenVPN/openvpn/builds/701385033?utm_med
> ium=notification&utm_source=email

Yes, that's it.  The problem is the message output by openssl is

2020-06-23 19:28:46 OpenSSL: error:0B080074:lib(11):func(128):reason(116)

Instead of:

2020-06-23 12:30:43 OpenSSL: error:0B080074:x509 certificate routines:X509_check_private_key:key values mismatch

I think I can make the grep work with the former.

James
Gert Doering June 23, 2020, 7:37 p.m. | #13
Hi,

On Tue, Jun 23, 2020 at 12:32:42PM -0700, James Bottomley wrote:
> > James, are you triggering on specific openvpn messages?  "--enable-
> > small"
> > changes these (trimming some warnings and help texts).  Can you test
> > with
> > "configure --enable-small", please?
> > 
> > https://travis-ci.org/github/OpenVPN/openvpn/builds/701385033?utm_med
> > ium=notification&utm_source=email
> 
> Yes, that's it.  The problem is the message output by openssl is
> 
> 2020-06-23 19:28:46 OpenSSL: error:0B080074:lib(11):func(128):reason(116)
> 
> Instead of:
> 
> 2020-06-23 12:30:43 OpenSSL: error:0B080074:x509 certificate routines:X509_check_private_key:key values mismatch

Indeed :-)

> I think I can make the grep work with the former.

Please do not forget to output log.txt when it fails - it eases diagnosing
remote failures where we do not have easy access to "file system things" 
(like travis or buildbot).

gert
Илья Шипицин June 23, 2020, 8:18 p.m. | #14
ср, 24 июн. 2020 г. в 00:37, Gert Doering <gert@greenie.muc.de>:

> Hi,
>
> On Tue, Jun 23, 2020 at 12:32:42PM -0700, James Bottomley wrote:
> > > James, are you triggering on specific openvpn messages?  "--enable-
> > > small"
> > > changes these (trimming some warnings and help texts).  Can you test
> > > with
> > > "configure --enable-small", please?
> > >
> > > https://travis-ci.org/github/OpenVPN/openvpn/builds/701385033?utm_med
> > > ium=notification&utm_source=email
> >
> > Yes, that's it.  The problem is the message output by openssl is
> >
> > 2020-06-23 19:28:46 OpenSSL: error:0B080074:lib(11):func(128):reason(116)
> >
> > Instead of:
> >
> > 2020-06-23 12:30:43 OpenSSL: error:0B080074:x509 certificate
> routines:X509_check_private_key:key values mismatch
>
> Indeed :-)
>
> > I think I can make the grep work with the former.
>
> Please do not forget to output log.txt when it fails - it eases diagnosing
> remote failures where we do not have easy access to "file system things"
> (like travis or buildbot).
>

I've added output of log.txt, if you are going to modify "grep" magic, can
you adopt something like that, please ?


https://travis-ci.org/github/chipitsine/openvpn/jobs/701409750


diff --git a/tests/unit_tests/engine-key/check_engine_keys.sh
b/tests/unit_tests/engine-key/check_engine_keys.sh
index e0c9d7b0..770a0c9c 100755
--- a/tests/unit_tests/engine-key/check_engine_keys.sh
+++ b/tests/unit_tests/engine-key/check_engine_keys.sh
@@ -8,6 +8,10 @@ password='AT3S4PASSWD'
 key="${builddir}/client.key"
 pwdfile="${builddir}/passwd"

+grep_a_log () {
+  grep -q $1 $2 || { echo $3; cat $2 ; exit 1; }
+}
+
 # create an engine key for us
 sed 's/PRIVATE KEY/TEST ENGINE KEY/' <
${top_srcdir}/sample/sample-keys/client.key > ${key}
 echo "$password" > $pwdfile
@@ -21,10 +25,10 @@ ${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
-grep -q 'X509_check_private_key:key values mismatch' log.txt || { echo
"Key mismatch not detected"; exit 1; }
+grep_a_log 'X509_check_private_key:key values mismatch' log.txt 'Key
mismatch not detected'

 # now look for the engine prints (these are under our control)
-grep -q 'ENGINE: engine_init called' log.txt || { echo "Engine
initialization not detected"; exit 1; }
-grep -q 'ENGINE: engine_load_key called' log.txt || { echo "Key was not
loaded from engine"; exit 1; }
-grep -q "ENGINE: engine_load_key got password ${password}" log.txt || {
echo "Key password was not retrieved by the engine"; exit 1; }
+grep_a_log 'ENGINE: engine_init called' log.txt 'Engine initialization not
detected'
+grep_a_log 'ENGINE: engine_load_key called' log.txt 'Key was not loaded
from engine'
+grep_a_log "ENGINE: engine_load_key got password ${password}" log.txt 'Key
password was not retrieved by the engine'
 exit 0
James Bottomley June 23, 2020, 11:02 p.m. | #15
On Wed, 2020-06-24 at 01:18 +0500, Илья Шипицин wrote:
[...]
> I've added output of log.txt, if you are going to modify "grep"
> magic, can you adopt something like that, please ?

OK, I folded this into the --enable-small correction

James

---8>8>8><8<8<8---
From: James Bottomley <James.Bottomley@HansenPartnership.com>
Subject: [PATCH] engine-key tests: make check_engine_keys.sh work with
 --enable-small

--enable-small eliminates one of the openssl errors the test is
looking for, so alter the grep also to account for the message in this
version.  Additionally output log.txt on failure so any test platform
gives an easy clue about what went wrong.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 tests/unit_tests/engine-key/check_engine_keys.sh | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tests/unit_tests/engine-key/check_engine_keys.sh b/tests/unit_tests/engine-key/check_engine_keys.sh
index e0c9d7b0..7e9a0e80 100755
--- a/tests/unit_tests/engine-key/check_engine_keys.sh
+++ b/tests/unit_tests/engine-key/check_engine_keys.sh
@@ -12,6 +12,12 @@ pwdfile="${builddir}/passwd"
 sed 's/PRIVATE KEY/TEST ENGINE KEY/' < ${top_srcdir}/sample/sample-keys/client.key > ${key}
 echo "$password" > $pwdfile
 
+# our version of grep to output log.txt on failure in case it's an openssl
+# error mismatch and the grep expression needs updating
+loggrep() {
+    egrep -q "$1" log.txt || { echo '---- begin log.txt ----'; cat log.txt; echo '--- end log.txt ---'; return 1; }
+}
+
 # note here we've induced a mismatch in the client key and the server
 # cert which openvpn should report and die.  Check that it does.  Note
 # also that this mismatch depends on openssl not openvpn, so it is
@@ -21,10 +27,10 @@ ${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
-grep -q 'X509_check_private_key:key values mismatch' log.txt || { echo "Key mismatch not detected"; exit 1; }
+loggrep '(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)
-grep -q 'ENGINE: engine_init called' log.txt || { echo "Engine initialization not detected"; exit 1; }
-grep -q 'ENGINE: engine_load_key called' log.txt || { echo "Key was not loaded from engine"; exit 1; }
-grep -q "ENGINE: engine_load_key got password ${password}" log.txt || { echo "Key password was not retrieved by the engine"; exit 1; }
+loggrep 'ENGINE: engine_init called' || { echo "Engine initialization not detected"; exit 1; }
+loggrep 'ENGINE: engine_load_key called' || { echo "Key was not loaded from engine"; exit 1; }
+loggrep "ENGINE: engine_load_key got password ${password}" || { echo "Key password was not retrieved by the engine"; exit 1; }
 exit 0
Gert Doering June 24, 2020, 8:16 a.m. | #16
Acked-by: Gert Doering <gert@greenie.muc.de>

Looks reasonable and does not break anything that already worked.

I could reproduce the --enable-small breakage with FreeBSD / 1.0.2s
here (interesting enough, not with Linux and 1.1.1*), and can confirm
that the patch fixes things.

Your patch has been applied to the master branch.

commit 013498ddfe0a2b7f8986e9edac2b9f062bdd5fd7 (HEAD -> master)
Author: James Bottomley <James.Bottomley@HansenPartnership.com>
Date:   Tue Jun 23 16:02:34 2020 -0700

    engine-key tests: make check_engine_keys.sh work with --enable-small
    
    Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
    Acked-by: Gert Doering <gert@greenie.muc.de>
    Message-Id: <1592953354.2103.3.camel@HansenPartnership.com>
    URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20102.html
    Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/configure.ac b/configure.ac
index 273a8d1b..53b7a967 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1387,6 +1387,7 @@  AM_CONDITIONAL([GIT_CHECKOUT], [test "${GIT_CHECKOUT}" = "yes"])
 AM_CONDITIONAL([ENABLE_PLUGIN_AUTH_PAM], [test "${enable_plugin_auth_pam}" = "yes"])
 AM_CONDITIONAL([ENABLE_PLUGIN_DOWN_ROOT], [test "${enable_plugin_down_root}" = "yes"])
 AM_CONDITIONAL([HAVE_LD_WRAP_SUPPORT], [test "${have_ld_wrap_support}" = "yes"])
+AM_CONDITIONAL([OPENSSL_ENGINE], [test "${have_openssl_engine}" = "yes"])
 
 sampledir="\$(docdir)/sample"
 AC_SUBST([plugindir])
@@ -1448,6 +1449,7 @@  AC_CONFIG_FILES([
         tests/unit_tests/openvpn/Makefile
         tests/unit_tests/plugins/Makefile
         tests/unit_tests/plugins/auth-pam/Makefile
+	tests/unit_tests/engine-key/Makefile
 	sample/Makefile
 ])
 AC_CONFIG_FILES([tests/t_client.sh], [chmod +x tests/t_client.sh])
diff --git a/tests/unit_tests/Makefile.am b/tests/unit_tests/Makefile.am
index 33fefaac..f27cd90f 100644
--- a/tests/unit_tests/Makefile.am
+++ b/tests/unit_tests/Makefile.am
@@ -2,4 +2,7 @@  AUTOMAKE_OPTIONS = foreign
 
 if ENABLE_UNITTESTS
 SUBDIRS = example_test openvpn plugins
+if OPENSSL_ENGINE
+SUBDIRS += engine-key
+endif
 endif
diff --git a/tests/unit_tests/engine-key/Makefile.am b/tests/unit_tests/engine-key/Makefile.am
new file mode 100644
index 00000000..39bed743
--- /dev/null
+++ b/tests/unit_tests/engine-key/Makefile.am
@@ -0,0 +1,24 @@ 
+AUTOMAKE_OPTIONS = foreign
+
+check_LTLIBRARIES = libtestengine.la
+conffiles = openssl.cnf
+
+TESTS_ENVIRONMENT = srcdir="$(abs_srcdir)"; \
+	builddir="$(abs_builddir)"; \
+	top_builddir="$(top_builddir)"; \
+	top_srcdir="$(top_srcdir)"; \
+	export srcdir builddir top_builddir top_srcdir;
+
+TESTS = check_engine_keys.sh
+check_engine_keys.sh: $(conffiles)
+
+clean-local:
+	rm -f $(conffiles)
+
+$(builddir)/openssl.cnf: $(srcdir)/openssl.cnf.in
+	sed "s|ABSBUILDDIR|$(abs_builddir)|" < $< > $@
+
+libtestengine_la_SOURCES = libtestengine.c
+libtestengine_la_LDFLAGS = @TEST_LDFLAGS@ -rpath /lib -shrext .so
+libtestengine_la_CFLAGS = @TEST_CFLAGS@ -I$(openvpn_srcdir) -I$(compat_srcdir)
+
diff --git a/tests/unit_tests/engine-key/check_engine_keys.sh b/tests/unit_tests/engine-key/check_engine_keys.sh
new file mode 100755
index 00000000..e0c9d7b0
--- /dev/null
+++ b/tests/unit_tests/engine-key/check_engine_keys.sh
@@ -0,0 +1,30 @@ 
+#!/bin/sh
+
+OPENSSL_CONF="${builddir}/openssl.cnf"
+export OPENSSL_CONF
+
+password='AT3S4PASSWD'
+
+key="${builddir}/client.key"
+pwdfile="${builddir}/passwd"
+
+# create an engine key for us
+sed 's/PRIVATE KEY/TEST ENGINE KEY/' < ${top_srcdir}/sample/sample-keys/client.key > ${key}
+echo "$password" > $pwdfile
+
+# note here we've induced a mismatch in the client key and the server
+# cert which openvpn should report and die.  Check that it does.  Note
+# also that this mismatch depends on openssl not openvpn, so it is
+# somewhat fragile
+${top_builddir}/src/openvpn/openvpn --cd ${top_srcdir}/sample --config sample-config-files/loopback-server --engine testengine --key ${key} --askpass $pwdfile > log.txt 2>&1
+
+# 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
+grep -q 'X509_check_private_key:key values mismatch' log.txt || { echo "Key mismatch not detected"; exit 1; }
+
+# now look for the engine prints (these are under our control)
+grep -q 'ENGINE: engine_init called' log.txt || { echo "Engine initialization not detected"; exit 1; }
+grep -q 'ENGINE: engine_load_key called' log.txt || { echo "Key was not loaded from engine"; exit 1; }
+grep -q "ENGINE: engine_load_key got password ${password}" log.txt || { echo "Key password was not retrieved by the engine"; exit 1; }
+exit 0
diff --git a/tests/unit_tests/engine-key/libtestengine.c b/tests/unit_tests/engine-key/libtestengine.c
new file mode 100644
index 00000000..46ec1e33
--- /dev/null
+++ b/tests/unit_tests/engine-key/libtestengine.c
@@ -0,0 +1,101 @@ 
+#include <string.h>
+#include <openssl/engine.h>
+#include <openssl/evp.h>
+#include <openssl/pem.h>
+
+static char *engine_id = "testengine";
+static char *engine_name = "Engine for testing openvpn engine key support";
+
+static int is_initialized = 0;
+
+static int engine_init(ENGINE *e)
+{
+	is_initialized = 1;
+	fprintf(stderr, "ENGINE: engine_init called\n");
+	return 1;
+}
+
+static int engine_finish(ENGINE *e)
+{
+	fprintf(stderr, "ENGINE: engine_finsh called\n");
+	is_initialized = 0;
+	return 1;
+}
+
+static EVP_PKEY *engine_load_key(ENGINE *e, const char *key_id,
+				 UI_METHOD *ui_method, void *cb_data)
+{
+	BIO *b;
+	EVP_PKEY *pkey;
+	PKCS8_PRIV_KEY_INFO *p8inf;
+	UI *ui;
+	char auth[256];
+
+	fprintf(stderr, "ENGINE: engine_load_key called\n");
+
+	if (!is_initialized) {
+		fprintf(stderr, "Load Key called without correct initialization\n");
+		return NULL;
+	}
+	b = BIO_new_file(key_id, "r");
+	if (!b) {
+		fprintf(stderr, "File %s does not exist or cannot be read\n", key_id);
+		return 0;
+	}
+	/* Basically read an EVP_PKEY private key file with different
+	 * PEM guards --- we are a test engine */
+	p8inf = PEM_ASN1_read_bio((d2i_of_void *)d2i_PKCS8_PRIV_KEY_INFO,
+				 "TEST ENGINE KEY", b,
+				 NULL, NULL, NULL);
+	BIO_free(b);
+	if (!p8inf) {
+		fprintf(stderr, "Failed to read engine private key\n");
+		return NULL;
+	}
+	pkey = EVP_PKCS82PKEY(p8inf);
+
+	/* now we have a private key, pretend it had a password
+	 * this verifies the password makes it through openvpn OK */
+	ui = UI_new();
+
+	if (ui_method)
+		UI_set_method(ui, ui_method);
+
+	UI_add_user_data(ui, cb_data);
+
+	if (UI_add_input_string(ui, "enter test engine key",
+				UI_INPUT_FLAG_DEFAULT_PWD,
+				auth, 0, sizeof(auth)) == 0) {
+		fprintf(stderr, "UI_add_input_string failed\n");
+		goto out;
+	}
+
+	if (UI_process(ui)) {
+		fprintf(stderr, "UI_process failed\n");
+		goto out;
+	}
+
+	fprintf(stderr, "ENGINE: engine_load_key got password %s\n", auth);
+
+ out:
+	UI_free(ui);
+
+	return pkey;
+}
+
+
+static int engine_bind_fn(ENGINE *e, const char *id)
+{
+	if (id && strcmp(id, engine_id) != 0)
+		return 0;
+	if (!ENGINE_set_id(e, engine_id) ||
+	    !ENGINE_set_name(e, engine_name) ||
+	    !ENGINE_set_init_function(e, engine_init) ||
+	    !ENGINE_set_finish_function(e, engine_finish) ||
+	    !ENGINE_set_load_privkey_function(e, engine_load_key))
+		return 0;
+	return 1;
+}
+
+IMPLEMENT_DYNAMIC_CHECK_FN()
+IMPLEMENT_DYNAMIC_BIND_FN(engine_bind_fn)
diff --git a/tests/unit_tests/engine-key/openssl.cnf.in b/tests/unit_tests/engine-key/openssl.cnf.in
new file mode 100644
index 00000000..5eda9fa9
--- /dev/null
+++ b/tests/unit_tests/engine-key/openssl.cnf.in
@@ -0,0 +1,12 @@ 
+HOME		= .
+openssl_conf	= openssl_init
+
+[req]
+[openssl_init]
+engines		= engines_section
+
+[engines_section]
+testengine	= testengine_section
+
+[testengine_section]
+dynamic_path	= ABSBUILDDIR/.libs/libtestengine.so