openvpn-devel Mailing List for OpenVPN

Hi, This addition is welcome and the code does the job it promises to do, but after reviewing the code I would like to propose a different implementation. The reasons for this are gives as inline replies below. The alternative patch proposal is attached. On Thu, Mar 3, 2016 at 9:19 AM, James Yonan <ja...@op...> wrote: > Signed-off-by: James Yonan <ja...@op...> > --- > src/openvpn/ssl_verify_polarssl.c | 166 ++++++++++++++++++++++++++++++++++++++ > src/openvpn/syshead.h | 2 +- > 2 files changed, 167 insertions(+), 1 deletion(-) > > diff --git a/src/openvpn/ssl_verify_polarssl.c b/src/openvpn/ssl_verify_polarssl.c > index 9d0d086..ab693d2 100644 > --- a/src/openvpn/ssl_verify_polarssl.c > +++ b/src/openvpn/ssl_verify_polarssl.c > @@ -198,6 +198,172 @@ x509_get_subject(x509_crt *cert, struct gc_arena *gc) > return subject; > } > > +#ifdef ENABLE_X509_TRACK > + > +/* these match NID's in OpenSSL crypto/objects/objects.h */ > +#define NID_undef 0 > +#define NID_sha1 64 > +#define NID_commonName 13 > +#define NID_countryName 14 > +#define NID_localityName 15 > +#define NID_stateOrProvinceName 16 > +#define NID_organizationName 17 > +#define NID_organizationalUnitName 18 > +#define NID_pkcs9_emailAddress 48 Hmm, I don't really like to maintain lists in the source code. If possible, I would prefer an implementation that fits the polarssl approach, rather than wrapping polarssl with openssl-like code. > diff --git a/src/openvpn/syshead.h b/src/openvpn/syshead.h > index 7e77b6c..25aa69b 100644 > --- a/src/openvpn/syshead.h > +++ b/src/openvpn/syshead.h > @@ -634,7 +634,7 @@ socket_defined (const socket_descriptor_t sd) > /* > * Enable x509-track feature? > */ > -#if defined(ENABLE_CRYPTO) && defined (ENABLE_CRYPTO_OPENSSL) > +#if defined(ENABLE_CRYPTO) && (defined(ENABLE_CRYPTO_OPENSSL) || defined(ENABLE_CRYPTO_POLARSSL)) Since both crypto backends now support --x509-track, let's just get rid of this define all together :) Finally, while reviewing I noticed that the --x509-track code in options.c lives inside #ifdef MANAGEMENT, but there seems to be no valid reason for this. Let's move it outside of this #ifdef. The attached patch takes all these remarks into account. The upsides of my alternative are less code, and no lists to maintain. The downside is less error reporting. I'm curious to hear what you think of the alternative implementation. -Steffan 

> I just thought it was interesting that somebody found > *a* solution to what seems to be a common problem > and it was worth letting ppl here know. Indeed. It remains to be seen whether this overly aggressive power saving configuration is limited to Samsung, or whether it's a Android 6.0.x thing. -- Samuli Seppänen Community Manager OpenVPN Technologies, Inc irc freenode net: mattock