On Mon, 2020-05-25 at 08:04 +0200, Gert Doering wrote:
> Hi,
> 
> I see the granularity of your patch set as "not right":
> 
> On Sun, May 24, 2020 at 01:33:22PM -0700, James Bottomley wrote:
> > 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.
> 
> This patch says "add unit tests", but it contains changes to
> configure.ac
> and OpenVPN code
> 
> >  configure.ac                                  |   5 +
> >  src/openvpn/crypto_openssl.c                  |   1 +
> 
> These two hunks should go to the first patch.

The configure.ac one adds the test Makefile ... it can't move.  The
other pieces figure out what the dynamic extension is, which is only
used by the test, so I think everything in the configure.ac change is
in the right place.

The second is a bit more problematic:  Right at the moment openvpn is
blind to local openssl configuration files.  To make the test work, it
needs to be able to add an engine, which requires a local file, so it's
only function is to enable the test, but it does add to openvpn
capabilities, I suppose.

James

> Every patch should be fully testable on its own - so if I apply only
> the first hunk, I should be able to use and test engine keys, without
> having to apply the "add a unit test" patch set.
> 
> > diff --git a/src/openvpn/crypto_openssl.c
> > b/src/openvpn/crypto_openssl.c
> > index a7569623..34637ebf 100644
> > --- a/src/openvpn/crypto_openssl.c
> > +++ b/src/openvpn/crypto_openssl.c
> > @@ -92,6 +92,7 @@ setup_engine(const char *engine)
> >  {
> >      ENGINE *e = NULL;
> >  
> > +    OPENSSL_config(NULL);
> >      ENGINE_load_builtin_engines();
> >  
> >      if (engine)
> 
> For that change, I wonder what side effects it might have on existing
> setups.  Arne, can you help?  Is this "safe"?
> 
> gert

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to