Re: [Openvpn-devel] Adding Google Analytics code to Trac?

[Emmanuel Deloget]

Hello,

On Wed, Oct 24, 2018 at 3:02 PM Jan Just Keijser  wrote:
>
> Hi,
>
> On 24/10/18 13:47, Samuli Seppänen wrote:
> > Hi,
> >
> > The OpenVPN Inc. webmaster would like to add Google Analytics to
> > community.openvpn.net, i.e. our Trac wiki/bug tracker. I said we need to
> > consult the community first because GA can be seen as a form of spying.
> > Here's our webmaster's view on this subject:
> >
> > ---
> >
> > "The goal of this would be to understand what information first-time
> > users are finding the most valuable on community.openvpn.net."
> >
> > "As this is a publicly accessible community without the requirement to
> > join to read the articles, it would be advantageous for all to
> > understand which pieces of content the public are finding the most
> > useful, what encourages them to become a part of the community, and what
> > potentially persuaded them to use start using a commercial product."
> >
> > "As openvpn.net links to community.openvpn.net and vice versa this will
> > also help us to understand the complete journey of a user and help to
> > improve the website experience, which can only be seen as a positive."
> >
> > ---
> >
> > In today's community meeting there was some concern about the spying
> > aspect of Google Analytics, but nobody was strongly opposed. This was I
> > believe, in part, because it is fairly easy to block Google Analytics if
> > one so wishes.
> >

Wouldn't it be easier to use a server-side script to do the same
thing? There are many "analytics" scripts out there that don't rely on
google or any other web service. If the webmaster is in PHP, he can
use http://www.openwebanalytics.com/ or any other subsystem.

Best regards,

-- Emmanuel Deloget


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

Re: [Openvpn-devel] [PATCH] OpenSSL: Fix compilation with deprecated APIs disabled on 1.1

[Emmanuel Deloget]

Hello Rosen,

On Wed, Jun 20, 2018 at 7:00 AM Gert Doering  wrote:
>
> Hi,
>
> On Tue, Jun 19, 2018 at 09:46:50PM -0700, Rosen Penev wrote:
> > Signed-off-by: Rosen Penev 
> > ---
> >  src/openvpn/crypto_openssl.c |  9 +
> >  src/openvpn/ssl_openssl.c| 32 +++-
> >  src/openvpn/ssl_verify_openssl.c |  1 +
> >  3 files changed, 41 insertions(+), 1 deletion(-)

Can you give a better explanation of the issue ? (I'm sorry, I try to
follow the discussions on the ML, but I'm kind of slow (and busy,
which does not help)).

Best regards,

-- Emmanuel Deloget

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v3 1/2] OpenSSL: remove some EVP_PKEY type checks

[Emmanuel Deloget]

Hello,

and sorry for the delay (things like 'real life', you know).

On Sat, Jan 20, 2018 at 3:22 PM, Selva Nair <selva.n...@gmail.com> wrote:

> Hi,
>
> On Sat, Jan 20, 2018 at 6:30 AM, Steffan Karger <stef...@karger.me> wrote:
> > Hi,
> >
> > On 17-01-18 14:10, Emmanuel Deloget wrote:
> >> Calling EVP_KEY_id() before EVP_PKEY_get0_*() is unnecessary as
> >> the same check is also performed in the later.
> >>
> >> We also make the code a bit better by not calling the various
> >> EVP_PKEY_get0_*() functions twice (this needs a bit or reordering to
> >> avoid introducing yet another #ifndef OPENSSL_NO_EC in the code).
> >>
> >> Signed-off-by: Emmanuel Deloget <log...@free.fr>
> >> ---
>
> ..
>
> > Unfortunately, OpenSSL 1.1.0 treats calling EVP_PKEY_get0_foo on a
> > non-foo key as an error.  Running 'make check' with this patch and
> > openssl 1.1.0 fails:
> >
> > Sat Jan 20 12:23:41 2018 Control Channel: TLSv1.2, cipher TLSv1.2
> > ECDHE-RSA-AES256-GCM-SHA384, 2048 bit RSA
> > Sat Jan 20 12:23:41 2018 OpenSSL: error:06078081:digital envelope
> > routines:EVP_PKEY_get0_DSA:expecting a dsa key
> > Sat Jan 20 12:23:41 2018 OpenSSL: error:0608308E:digital envelope
> > routines:EVP_PKEY_get0_EC_KEY:expecting a ec key
> > Sat Jan 20 12:23:41 2018 TLS_ERROR: BIO read tls_read_plaintext error
> >
> > So, NAK.  (And Selva gets to keep EPV_PKEY_id() ;-) )
> >
> > Sorry for not spotting this earlier, could have saved us quite a bit of
> > work...
>


​I should have spotted that before I sent the change, but I only tested it
with 1.0.2 and compile-tested it against 1.1. I'm going to refactor my test
environment in order to avoid any further error like this one.



>
> I'm surprised that my argument about
>
> if (EVP_PKEY_id(foo) == ...EC..) { do EC stuff }
> else if (EVP_PKEY_id(foo) == ..RSA..) { do RSA stuff }
>
> or switch(EVP_PKEY_id(foo))
>
> being stylistically better[tm] did not work!
>
> Still pleased by the end result. Now I get to spit out some of the
> "if (EVP_PKEY_get0_RSA(..))"  that was pushed down my throat :).
> For patches already on the ML will do so after review.
>
>
​I like the switch - I find it elegant - but it does not avoid an internal
'if', so the code would look like:

switch (EVP_PKEY_id(foo)) {
case ..RSA..: {
RSA *rsa = EVP_PKEY_get0_RSA(foo);
if (rsa) {
   openvpn_printf(...);
}
  }
  break;
case ...
}

​If this is acceptable, I can ​respin patch 2. Otherwise, feel free to
discard both patch 2 and patch 3 of the series as any change in patch 2
would not be a largen enhancement of the already existing code.



> Selva
>
>
​Best regards,

-- Emmanuel Deloget
​
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v3 2/2] OpenSSL: remove EVP_PKEY_id()

[Emmanuel Deloget]

​Of course, this one is not strictly needed. If you want to keep the
function around, feel free to ignore the patch.

Given the relative simplicity of the change, one can also consider that if
the function is needed again we could simply revert this patch.

Best regards,

-- Emmanuel Deloget​
​
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v3 2/2] OpenSSL: remove EVP_PKEY_id()

[Emmanuel Deloget]

The function is no longer used so we don't need to keep it in the
OpenSSL 1.1 compatibility layer.

Signed-off-by: Emmanuel Deloget <log...@free.fr>
---
 configure.ac |  1 -
 src/openvpn/openssl_compat.h | 14 --
 2 files changed, 15 deletions(-)

diff --git a/configure.ac b/configure.ac
index 2c1937e5..e855a241 100644
--- a/configure.ac
+++ b/configure.ac
@@ -925,7 +925,6 @@ if test "${with_crypto_library}" = "openssl"; then
X509_STORE_get0_objects \
X509_OBJECT_free \
X509_OBJECT_get_type \
-   EVP_PKEY_id \
EVP_PKEY_get0_RSA \
EVP_PKEY_get0_DSA \
EVP_PKEY_get0_EC_KEY \
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index 05ec4e95..2c8913e1 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -258,20 +258,6 @@ EVP_PKEY_get0_EC_KEY(EVP_PKEY *pkey)
 }
 #endif
 
-#if !defined(HAVE_EVP_PKEY_ID)
-/**
- * Get the PKEY type
- *
- * @param pkeyPublic key object
- * @returnThe key type
- */
-static inline int
-EVP_PKEY_id(const EVP_PKEY *pkey)
-{
-return pkey ? pkey->type : EVP_PKEY_NONE;
-}
-#endif
-
 #if !defined(HAVE_EVP_PKEY_GET0_DSA)
 /**
  * Get the DSA object of a public key
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2 2/3] OpenSSL: remove some EVP_PKEY type checks

[Emmanuel Deloget]

Hello,

On Wed, Jan 17, 2018 at 1:16 PM, Steffan Karger <stef...@karger.me> wrote:

> Hi,
>
> On 15 January 2018 at 23:33, Emmanuel Deloget <log...@free.fr> wrote:
> > For the variables outside the ifs, the next C standard should allow us to
> > write something like:
> >
> > if ((RSA *rsa = EVP_PKEY_get0_RSA(pkey)) != NULL) {
>
> Yeah, such a shame that this didn't make it into C11.  Scoping a
> variable to an if block is often useful.  If only for error checking
> like if(int err = foo()) { handle error }.  Anyway, we can't have it
> :(
>
> On 16 January 2018 at 09:26, Gert Doering <g...@greenie.muc.de> wrote:
> > On Tue, Jan 16, 2018 at 01:56:11AM +0100, Emmanuel Deloget wrote:
> >> 3) rework it as proposed in the patch
> >>
> >> RSA *rsa = NULL;
> >> if ((rsa = EVP_PKEY_get0_RSA(pkey)) != NULL) { ... }
> >
> > I do not particularily like assignments in if() clauses, especially when
> > the variable is declared right above it.  So if we go for (3), my
> > favourite would be
> >
> > RSA *rsa = EVP_PKEY_get0_RSA(pkey);
> > if (rsa != NULL) { ... }
> >
> >
> > but this is just a side remark.  The question on "1, 2 or 3" I'll defer
> > to you folks that are maintaing this code (Steffan and Selva, mostly :)
> ).
>
> This indeed looks a bit better that initializing to NULL if we're
> going to take the variables outside the scope.
>
> Seems I've been overruled by majority regarding the way to go, and
> with the suggestion from cron2 taken into account I think 3 is the way
> to go.
>
> -Steffan
>

​I'm OK with this (3) variation :)

I'm respinning the patches.

BR,

-- Emmanuel Deloget​
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2 2/3] OpenSSL: remove some EVP_PKEY type checks

[Emmanuel Deloget]

Hello Selva,

On Tue, Jan 16, 2018 at 12:10 AM, Selva Nair <selva.n...@gmail.com> wrote:

>
>
> On Mon, Jan 15, 2018 at 5:33 PM, Emmanuel Deloget <log...@free.fr> wrote:
>
>> Hello Steffan,
>>
>> On Sun, Jan 14, 2018 at 11:26 AM, Steffan Karger <stef...@karger.me>
>> wrote:
>>
>>> Hi,
>>>
>>> On 12-01-18 22:37, Emmanuel Deloget wrote:
>>> > Calling EVP_KEY_id() before EVP_PKEY_get0_*() is unnecessary as
>>> > the same check is also performed in the later.
>>> >
>>> > We also make the code a bit better by not calling the various
>>> > EVP_PKEY_get0_*() functions twice (this needs a bit or reordering to
>>> > avoid introducing yet another #ifndef OPENSSL_NO_EC in the code).
>>>
>>> Checking double is a bit silly, but we can fix that by simply removing
>>> the "EVP_PKEY_id() == ... && " occurrences. (That still allows us to
>>> remove it from the compat wrapper.)
>>>
>>> I'm not sure that moving the variables outside the if() scope actually
>>> improves the code.  At least I think the original flow is easier to
>>> read.  Mostly due to the #ifdef noise, but still.  In a path that's not
>>> performance-critical, I prefer readability over performance.
>>>
>>
>>
>> ​Well, to be honest, I was definitely not trying to make any kind of
>> performance gain :)​
>>
>> The whole idea was to make code easier to read and to remove some now
>> weird duplication of functions (the kind of duplication that will come and
>> bites you later).
>>
>> For the variables outside the ifs, the next C standard should allow us to
>> write something like:
>>
>> if ((RSA *rsa = EVP_PKEY_get0_RSA(pkey)) != NULL) {
>> ...
>>
>> That should be easier to read (but then, I'm not sure when this will be
>> considered as widespread ; I guess we're slated for 2025 or so :))
>>
>> Anyway, moving the variables within the if means we're going to call
>> EVP_PKEY_get0_RSA() twice which, I feel, is not a good thing to do. But
>> even if I'm not very comfortable with this, I can still propose a patch
>> that des it (it's lighter than the currently proposed change). You tell me
>> :)
>>
>
> Can I try one last time to convince you guys to keep EVP_PKEY_id()? If I
> was targeting only 1.1.0+ that's how most of those checks would have been
> done, isn't? Makes life easier, code simpler as I have said before..
>
> Just throwing in my 0.02c, feel free to ignore.
>
> Best,
>
> Selva
>


OpenSSL 1.1.0+ and previous versions allows you to create a typed EVP_PKEY
without any key inside. For example, you can do

  EVP_PKEY *pkey = EVP_PKEY_new();
  EVP_PKEY_set_type(pkey, EVP_PKEY_RSA);

This is plainly valid: the key is prepared for later use.

So testing for EVP_PKEY_id() is not sufficient to make sure that current
and future code won't mess with us. To be extra sure, one can do

if (EVP_PKEY_id() == EVP_PKEY_RSA && EVP_PKEY_get0_RSA(pkey)) { ... }

Which is exactly what the current code does. Yet the idea (at least mine)
is to remove that to simplify code a bit.

Now, I agree, this is quite easy to read and to understand.

We have 3 solutions:

1) keep the code as is

if (EVP_PKEY_id() == EVP_PKEY_RSA && EVP_PKEY_get0_RSA(pkey) != NULL) { ...
}

2) remove EVP_PKEY_id() but keep a test on EVP_PKEY_getO_RSA()

if (EVP_PKEY_get0_RSA(pkey) != NULL) {
RSA *rsa = EVP_PKEY_get0_RSA(pkey);
...
}

3) rework it as proposed in the patch

RSA *rsa = NULL;
if ((rsa = EVP_PKEY_get0_RSA(pkey)) != NULL) { ... }

While I prefer the latest, I can understand that it might not be the best
solution. I don't have a strong opinion on the subject (that's why it's a
separate patch ;)).

​Best regards,

-- Emmanuel Deloget​
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] PKCS#11 - a little bit of help?

[Emmanuel Deloget]

Hi James,

On Tue, Jan 16, 2018 at 12:18 AM, James Bottomley <
james.bottom...@hansenpartnership.com> wrote:

> On Tue, 2018-01-16 at 00:07 +0100, Emmanuel Deloget wrote:
> > While the number of required changes were quite small (and have no
> > impact on openvpn), this was quite a journey. I guess some of the
> > merits should go to RSA, Microsoft and Intel, for their incredible
> > effort in building comprehensive industry standards that are as
> > convoluted as they are comprehensive :) (I'm kidding ; I think I
> > begin to enjoy PKCS#11 and I definitely had some fun while playing
> > with my TPM2).
> >
> > Those who are interested can contact me. I will probably try to
> > write something about this somewhere (I don't know where yet).
>
> Just a note that there is an easier way of doing this.  The engine key
> patch:
>
> https://sourceforge.net/p/openvpn/mailman/message/36147533/
>
> Achieves the same thing somewhat more simply.  You use the tpm2 engine:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jejb/
> openssl_tpm2_engine.git/
>
> And convert an existing private key into engine format with
> create_tpm2_key and place it in /etc/openvpn/ where the private key
> file usually goes.  Then all private key signature transactions go via
> the TPM.
>

The engine is of interest (although it would require me to change all my
scripts to remove the intel TPM2 stack and replace it with IBM tools ;
there is no sane reason to keep both of them in my setup). I missed it when
I tried to find an openssl engine for TPM2. The fact that I choose a
PKCS#11 engine is now final, unfortunately, at least for now. But I'll keep
an eye on it.

For the openvpn patches, I would not go this way for multiple reasons
(number one being: I already have to many patches on many things around ;
it becomes difficult to handle after a while, so I try to be as upstream as
possible) ; but this is interesting as well.



>
> James
>
>
​Thanks for your links,

-- Emmanuel Deloget​
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] PKCS#11 - a little bit of help?

[Emmanuel Deloget]

Hello Steffan,

​​Sorry fo​r the delay - I was busy doing "things​" :)


On Sat, Jan 6, 2018 at 12:11 AM, Steffan Karger <stef...@karger.me> wrote:

> Hi Emmanuel,
>
> On 03-01-18 18:13, Emmanuel Deloget wrote:
> > Hello Steffan,
> >
> > On Mon, Jan 1, 2018 at 4:36 PM, Steffan Karger <stef...@karger.me
> > <mailto:stef...@karger.me>> wrote:
> >
> > Hi,
> >
> > On 01-01-18 14:57, Emmanuel Deloget wrote:
> > > I'm trying to get openvpn read my certificates from a TPM2 using a
> > > specially crafted PKCS#11 provider (the existing tpm2-pk11 is quite
> > > limited for now but I might be able to extend it).
> > >
> > > However, the PKCS#11 API is not something I'm comfortable with,
> and I'd
> > > like to know if there is some document (design or anything,
> really) that
> > > could help me to understand what openvpn wants exactly in order
> for me
> > > to provide the missing bits. I've read the documents at [1] but
> found
> > > nothing here of interest (for me).
> > >
> > > So, does someone have any pointer?
> >
> > You're right that OpenVPN's pkcs11 options lack some high-level
> > documentation.  Maybe I can shed some light on the basics (probably
> > taking a step further back than you need).
> >
> > First, you need some shared object (.so, .dll) that provides the
> > interface specified by the PKCS11 standard.  OpenVPN will load that
> > module, and call its functions to provide private key operations.
> That
> > shared object is usually provided by your smartcard vendor.  That
> shared
> > object takes care of communication with the underlying key store
> (smart
> > card, tpm, hsm, ...).  The openvpn manpage calls this a pkcs11
> > 'provider'.
> >
> > $ openvpn --show-pkcs11-ids
> > /usr/lib/x86_64-linux-gnu/pkcs11/gnome-keyring-pkcs11.so
> >
> > The following objects are available for use.
> > Each object shown below may be used as parameter to
> > --pkcs11-id option please remember to use single quote mark.
> >
> >
> >
> > ​I seen that while testing. ​
> >
> >
> >
> >
> > Certificate
> >DN: C=KG, ST=NA, O=OpenVPN-TEST, CN=Test-Client,
> > emailAddress=me@myhost.mydomain
> >Serial: 02
> >Serialized id:
> > Gnome\x20Keyring/1\x2E0/1\x3AUSER\x3ADEFAULT/Gnome2\
> x20Key\x20Storage/1A9D824585217F1BD54603E83F042F570A2EE9F2
> >
> > (I have the openvpn client testkey in my local gnome keyring, to
> easily
> > test pkcs11 without a hardware token.)
> >
> > From that list, you pick the object you want to use, and specify the
> > provider and id in your config file.  In this case:
> >
> > pkcs11-providers
> > "/usr/lib/x86_64-linux-gnu/pkcs11/gnome-keyring-pkcs11.so"
> > pkcs11-id
> > "Gnome\\x20Keyring/1\\x2E0/1\\x3AUSER\\x3ADEFAULT/Gnome2\\
> x20Key\\x20Storage/1B3B2BEBF36576443D3A903AFA8997221B93FCC1"
> >
> > You specify this *instead* of the regular 'key' and 'cert' options.
> > Note that you'll need toe escape backslashes.
> >
> >
> > ​Good to know :)​
> >
> >
> >
> > You should now be able to use pkcs11 for the private key operations.
> >
> > Since each keystore behaves differently, I did not specify how to get
> > the private key in there.  But I guess your TPM vendor providers
> tooling
> > for that.
> >
> >
>

​​



> >
> > In my understanding, and within a context of dual authentication (server
> > identifies client, client identifies server), I should see the following
> > set of operation:
> >
> > * fetch the certificate
> > * initiate communication and so some RSA magic
> > * everything's ok? continue
> >
> > Of course, I might be wrong. What I see is (with some printf within the
> > PKCS#11 provider, you can ignore them but they give you an idea of what
> > happens)
> >
> >
> ​ ​
>
> >
> >
> > Unfortunately, there is no RSA-related log, so I cannot check what
> > openvpn is doing with the certificate it found - so at this point, I'm a
> > bit confused.
>
> It does not do any RSA operations, because it does not connect:
>
> Wed Jan  3 17:53:39 2018 us=959952 Attempting to establish TCP
> connection with [AF_INET6]2a01:240:1906:1:0:1850:6118:407

Re: [Openvpn-devel] [PATCH v2 2/3] OpenSSL: remove some EVP_PKEY type checks

[Emmanuel Deloget]

Hello Steffan,

On Sun, Jan 14, 2018 at 11:26 AM, Steffan Karger <stef...@karger.me> wrote:

> Hi,
>
> On 12-01-18 22:37, Emmanuel Deloget wrote:
> > Calling EVP_KEY_id() before EVP_PKEY_get0_*() is unnecessary as
> > the same check is also performed in the later.
> >
> > We also make the code a bit better by not calling the various
> > EVP_PKEY_get0_*() functions twice (this needs a bit or reordering to
> > avoid introducing yet another #ifndef OPENSSL_NO_EC in the code).
>
> Checking double is a bit silly, but we can fix that by simply removing
> the "EVP_PKEY_id() == ... && " occurrences. (That still allows us to
> remove it from the compat wrapper.)
>
> I'm not sure that moving the variables outside the if() scope actually
> improves the code.  At least I think the original flow is easier to
> read.  Mostly due to the #ifdef noise, but still.  In a path that's not
> performance-critical, I prefer readability over performance.
>


​Well, to be honest, I was definitely not trying to make any kind of
performance gain :)​

The whole idea was to make code easier to read and to remove some now weird
duplication of functions (the kind of duplication that will come and bites
you later).

For the variables outside the ifs, the next C standard should allow us to
write something like:

if ((RSA *rsa = EVP_PKEY_get0_RSA(pkey)) != NULL) {
...

That should be easier to read (but then, I'm not sure when this will be
considered as widespread ; I guess we're slated for 2025 or so :))

Anyway, moving the variables within the if means we're going to call
EVP_PKEY_get0_RSA() twice which, I feel, is not a good thing to do. But
even if I'm not very comfortable with this, I can still propose a patch
that des it (it's lighter than the currently proposed change). You tell me
:)



>
> > Signed-off-by: Emmanuel Deloget <log...@free.fr>
> >
> > diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> > index 711bba11..7943fb2c 100644
> > --- a/src/openvpn/ssl_openssl.c
> > +++ b/src/openvpn/ssl_openssl.c
> > @@ -1699,22 +1699,13 @@ print_details(struct key_state_ssl *ks_ssl,
> const char *prefix)
> >  EVP_PKEY *pkey = X509_get_pubkey(cert);
> >  if (pkey != NULL)
> >  {
> > -if ((EVP_PKEY_id(pkey) == EVP_PKEY_RSA) &&
> (EVP_PKEY_get0_RSA(pkey) != NULL))
> > -{
> > -RSA *rsa = EVP_PKEY_get0_RSA(pkey);
> > -openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA",
> > - RSA_bits(rsa));
> > -}
> > -else if ((EVP_PKEY_id(pkey) == EVP_PKEY_DSA) &&
> (EVP_PKEY_get0_DSA(pkey) != NULL))
> > -{
> > -DSA *dsa = EVP_PKEY_get0_DSA(pkey);
> > -openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
> > - DSA_bits(dsa));
> > -}
> > +RSA *rsa = NULL;
> > +DSA *dsa = NULL;
> >  #ifndef OPENSSL_NO_EC
> > -else if ((EVP_PKEY_id(pkey) == EVP_PKEY_EC) &&
> (EVP_PKEY_get0_EC_KEY(pkey) != NULL))
> > +EC_KEY *ec = NULL;
> > +
> > +if ((ec = EVP_PKEY_get0_EC_KEY(pkey)) != NULL)
> >  {
> > -EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
> >  const EC_GROUP *group = EC_KEY_get0_group(ec);
> >  const char* curve;
> >
> > @@ -1726,9 +1717,19 @@ print_details(struct key_state_ssl *ks_ssl, const
> char *prefix)
> >
> >  openvpn_snprintf(s2, sizeof(s2), ", %d bit EC, curve:
> %s",
> >   EC_GROUP_order_bits(group), curve);
> > -
> > -}
> > +} else
> >  #endif
> > +if ((rsa = EVP_PKEY_get0_RSA(pkey)) != NULL)
> > +{
> > +openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA",
> > + RSA_bits(rsa));
> > +}
> > +else if ((dsa = EVP_PKEY_get0_DSA(pkey)) != NULL)
> > +{
> > +openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
> > + DSA_bits(dsa));
> > +}
> > +
> >  EVP_PKEY_free(pkey);
> >  }
> >  X509_free(cert);
> >
>
> -Steffan
>


​Best regards,

-- Emmanuel Deloget​
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Use RSA_meth_free instead of free

[Emmanuel Deloget]

Hello,

On Sun, Jan 14, 2018 at 12:00 AM, <selva.n...@gmail.com> wrote:

> From: Selva Nair <selva.n...@gmail.com>
>
> - RSA_meth_new allocates memory for the name string
>   and must be released using RSA_meth_free
>
> Signed-off-by: Selva Nair <selva.n...@gmail.com>
> ---
>  src/openvpn/ssl_openssl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> index 34c31b9..d6d9acf 100644
> --- a/src/openvpn/ssl_openssl.c
> +++ b/src/openvpn/ssl_openssl.c
> @@ -1144,7 +1144,7 @@ err:
>  {
>  if (rsa_meth)
>  {
> -free(rsa_meth);
> +RSA_meth_free(rsa_meth);
>  }
>  }
>  crypto_msg(M_FATAL, "Cannot enable SSL external private key
> capability");
> --
> 2.1.4
>
>
​Good catch.

For what it's worth:

Acked-by: Emmanuel Deloget <log...@free.fr>​

​BR,

-- Emmanuel Deloget​
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 0/3] Fix EVP_PKEY key types handling

[Emmanuel Deloget]

Hello,

The whole series is also viewable on github at

https://github.com/emmanuel-deloget/openvpn/commits/fix-evp-pkey

Best regards,

​-- Emmanuel Deloget​
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2 2/3] OpenSSL: remove some EVP_PKEY type checks

[Emmanuel Deloget]

Calling EVP_KEY_id() before EVP_PKEY_get0_*() is unnecessary as
the same check is also performed in the later.

We also make the code a bit better by not calling the various
EVP_PKEY_get0_*() functions twice (this needs a bit or reordering to
avoid introducing yet another #ifndef OPENSSL_NO_EC in the code).

Signed-off-by: Emmanuel Deloget <log...@free.fr>

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 711bba11..7943fb2c 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1699,22 +1699,13 @@ print_details(struct key_state_ssl *ks_ssl, const char 
*prefix)
 EVP_PKEY *pkey = X509_get_pubkey(cert);
 if (pkey != NULL)
 {
-if ((EVP_PKEY_id(pkey) == EVP_PKEY_RSA) && 
(EVP_PKEY_get0_RSA(pkey) != NULL))
-{
-RSA *rsa = EVP_PKEY_get0_RSA(pkey);
-openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA",
- RSA_bits(rsa));
-}
-else if ((EVP_PKEY_id(pkey) == EVP_PKEY_DSA) && 
(EVP_PKEY_get0_DSA(pkey) != NULL))
-{
-DSA *dsa = EVP_PKEY_get0_DSA(pkey);
-openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
- DSA_bits(dsa));
-}
+RSA *rsa = NULL;
+DSA *dsa = NULL;
 #ifndef OPENSSL_NO_EC
-else if ((EVP_PKEY_id(pkey) == EVP_PKEY_EC) && 
(EVP_PKEY_get0_EC_KEY(pkey) != NULL))
+EC_KEY *ec = NULL;
+
+if ((ec = EVP_PKEY_get0_EC_KEY(pkey)) != NULL)
 {
-EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
 const EC_GROUP *group = EC_KEY_get0_group(ec);
 const char* curve;
 
@@ -1726,9 +1717,19 @@ print_details(struct key_state_ssl *ks_ssl, const char 
*prefix)
 
 openvpn_snprintf(s2, sizeof(s2), ", %d bit EC, curve: %s",
  EC_GROUP_order_bits(group), curve);
-
-}
+} else
 #endif
+if ((rsa = EVP_PKEY_get0_RSA(pkey)) != NULL)
+{
+openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA",
+ RSA_bits(rsa));
+}
+else if ((dsa = EVP_PKEY_get0_DSA(pkey)) != NULL)
+{
+openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
+ DSA_bits(dsa));
+}
+
 EVP_PKEY_free(pkey);
 }
 X509_free(cert);
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 2/3] OpenSSL: remove some EVP_PKEY type checks

[Emmanuel Deloget]

Hello Selva,

On Fri, Jan 12, 2018 at 6:09 PM, Selva Nair <selva.n...@gmail.com> wrote:

> Hi,
>
> I will defer to crypto experts for a proper review, but a quick remark
>
> On Fri, Jan 12, 2018 at 11:48 AM, Emmanuel Deloget <log...@free.fr> wrote:
> > Calling EVP_KEY_id() before EVP_PKEY_get0_*() is unnecessary as
> > the same check is also performed in the later.
> >
> >
> ...
>
> > +RSA *rsa = NULL;
> > +DSA *dsa = NULL;
> >  #ifndef OPENSSL_NO_EC
> > -else if ((EVP_PKEY_id(pkey) == EVP_PKEY_EC) &&
> (EVP_PKEY_get0_EC_KEY(pkey) != NULL))
> > +EC *ec = NULL;
>
> That looks wrong: do you mean EC_KEY instead of EC?
>


​I definitely mean EC_KEY instead of EC. ​

My bad. I'm sending a new version of this patch.


>
> > +
> > +if ((ec = EVP_PKEY_get0_EC_KEY(pkey)) != NULL)
>
> Selva
>

​BR,

-- Emmanuel Deloget​
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 1/3] OpenSSL: check EVP_PKEY key types before returning the pkey

[Emmanuel Deloget]

The internal EVP_PKEY::pkey member is an union thus we need to check for
the real key type before we can return the corresponding RSA, DSA or EC
public key.

Reported-by: Selva Nair <selva.n...@gmail.com>
Signed-off-by: Emmanuel Deloget <log...@free.fr>

diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index 70b19aea..8b29cdaf 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -240,7 +240,7 @@ X509_OBJECT_get_type(const X509_OBJECT *obj)
 static inline RSA *
 EVP_PKEY_get0_RSA(EVP_PKEY *pkey)
 {
-return pkey ? pkey->pkey.rsa : NULL;
+return (pkey && pkey->type == EVP_PKEY_RSA) ? pkey->pkey.rsa : NULL;
 }
 #endif
 
@@ -254,7 +254,7 @@ EVP_PKEY_get0_RSA(EVP_PKEY *pkey)
 static inline EC_KEY *
 EVP_PKEY_get0_EC_KEY(EVP_PKEY *pkey)
 {
-return pkey ? pkey->pkey.ec : NULL;
+return (pkey && pkey->type == EVP_PKEY_EC) ? pkey->pkey.ec : NULL;
 }
 #endif
 
@@ -282,7 +282,7 @@ EVP_PKEY_id(const EVP_PKEY *pkey)
 static inline DSA *
 EVP_PKEY_get0_DSA(EVP_PKEY *pkey)
 {
-return pkey ? pkey->pkey.dsa : NULL;
+return (pkey && pkey->type == EVP_PKEY_DSA) ? pkey->pkey.dsa : NULL;
 }
 #endif
 
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 0/3] Fix EVP_PKEY key types handling

[Emmanuel Deloget]

Hello, 

The dubious commiter of the OpenSSL 1.1 changes got it wrong again. 
Not sure if I can trust this guy. Not to mention that he pretends to 
be /me/... :)

Anyway, I fixed some of his mistakes again.

For reference, this fixes a bug reported by Selva (hence the Reported-By 
tag on the first patch) where openvpn crashes when it's feeded with an 
ECC key (same bug shall arise when using a DSA key).

The first patch in the series is necessary (it's the one that fixes 
the bugs). Patch 2 remove code that is no longer necessary. Patch 3
then remove an entire function which is no longer used (although I'd 
understand if one wants to keep this function around).

Hopefully, this is the last time I have to correct a bug by the previous 
commiter (I will not name him. That would sound too weird). Next time, 
he'll have to do it by himself :)

Best regards, 

-- Emmanuel Deloget

Emmanuel Deloget (3):
  OpenSSL: check EVP_PKEY key types before returning the pkey
  OpenSSL: remove some EVP_PKEY type checks
  OpenSSL: remove EVP_PKEY_id()

 configure.ac |  1 -
 src/openvpn/openssl_compat.h | 20 +++-
 src/openvpn/ssl_openssl.c| 33 +
 3 files changed, 20 insertions(+), 34 deletions(-)

-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 2/3] OpenSSL: remove some EVP_PKEY type checks

[Emmanuel Deloget]

Calling EVP_KEY_id() before EVP_PKEY_get0_*() is unnecessary as
the same check is also performed in the later.

We also make the code a bit better by not calling the various
EVP_PKEY_get0_*() functions twice (this needs a bit or reordering to
avoid introducing yet another #ifndef OPENSSL_NO_EC in the code).

Signed-off-by: Emmanuel Deloget <log...@free.fr>

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 711bba11..9f74acaa 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1699,22 +1699,13 @@ print_details(struct key_state_ssl *ks_ssl, const char 
*prefix)
 EVP_PKEY *pkey = X509_get_pubkey(cert);
 if (pkey != NULL)
 {
-if ((EVP_PKEY_id(pkey) == EVP_PKEY_RSA) && 
(EVP_PKEY_get0_RSA(pkey) != NULL))
-{
-RSA *rsa = EVP_PKEY_get0_RSA(pkey);
-openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA",
- RSA_bits(rsa));
-}
-else if ((EVP_PKEY_id(pkey) == EVP_PKEY_DSA) && 
(EVP_PKEY_get0_DSA(pkey) != NULL))
-{
-DSA *dsa = EVP_PKEY_get0_DSA(pkey);
-openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
- DSA_bits(dsa));
-}
+RSA *rsa = NULL;
+DSA *dsa = NULL;
 #ifndef OPENSSL_NO_EC
-else if ((EVP_PKEY_id(pkey) == EVP_PKEY_EC) && 
(EVP_PKEY_get0_EC_KEY(pkey) != NULL))
+EC *ec = NULL;
+
+if ((ec = EVP_PKEY_get0_EC_KEY(pkey)) != NULL)
 {
-EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
 const EC_GROUP *group = EC_KEY_get0_group(ec);
 const char* curve;
 
@@ -1726,9 +1717,19 @@ print_details(struct key_state_ssl *ks_ssl, const char 
*prefix)
 
 openvpn_snprintf(s2, sizeof(s2), ", %d bit EC, curve: %s",
  EC_GROUP_order_bits(group), curve);
-
-}
+} else
 #endif
+if ((rsa = EVP_PKEY_get0_RSA(pkey)) != NULL)
+{
+openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA",
+ RSA_bits(rsa));
+}
+else if ((dsa = EVP_PKEY_get0_DSA(pkey)) != NULL)
+{
+openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
+ DSA_bits(dsa));
+}
+
 EVP_PKEY_free(pkey);
 }
 X509_free(cert);
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 3/3] OpenSSL: remove EVP_PKEY_id()

[Emmanuel Deloget]

The function is no longer used so we don't need to keep it in the
OpenSSL 1.1 compatibility layer.

Signed-off-by: Emmanuel Deloget <log...@free.fr>

diff --git a/configure.ac b/configure.ac
index b4fd1b3f..716b45dc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -925,7 +925,6 @@ if test "${with_crypto_library}" = "openssl"; then
X509_STORE_get0_objects \
X509_OBJECT_free \
X509_OBJECT_get_type \
-   EVP_PKEY_id \
EVP_PKEY_get0_RSA \
EVP_PKEY_get0_DSA \
EVP_PKEY_get0_EC_KEY \
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index 8b29cdaf..2c4a08c1 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -258,20 +258,6 @@ EVP_PKEY_get0_EC_KEY(EVP_PKEY *pkey)
 }
 #endif
 
-#if !defined(HAVE_EVP_PKEY_ID)
-/**
- * Get the PKEY type
- *
- * @param pkeyPublic key object
- * @returnThe key type
- */
-static inline int
-EVP_PKEY_id(const EVP_PKEY *pkey)
-{
-return pkey ? pkey->type : EVP_PKEY_NONE;
-}
-#endif
-
 #if !defined(HAVE_EVP_PKEY_GET0_DSA)
 /**
  * Get the DSA object of a public key
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] PKCS#11 - a little bit of help?

[Emmanuel Deloget]

Hello Steffan,

On Mon, Jan 1, 2018 at 4:36 PM, Steffan Karger <stef...@karger.me> wrote:

> Hi,
>
> On 01-01-18 14:57, Emmanuel Deloget wrote:
> > I'm trying to get openvpn read my certificates from a TPM2 using a
> > specially crafted PKCS#11 provider (the existing tpm2-pk11 is quite
> > limited for now but I might be able to extend it).
> >
> > However, the PKCS#11 API is not something I'm comfortable with, and I'd
> > like to know if there is some document (design or anything, really) that
> > could help me to understand what openvpn wants exactly in order for me
> > to provide the missing bits. I've read the documents at [1] but found
> > nothing here of interest (for me).
> >
> > So, does someone have any pointer?
>
> You're right that OpenVPN's pkcs11 options lack some high-level
> documentation.  Maybe I can shed some light on the basics (probably
> taking a step further back than you need).
>
> First, you need some shared object (.so, .dll) that provides the
> interface specified by the PKCS11 standard.  OpenVPN will load that
> module, and call its functions to provide private key operations.  That
> shared object is usually provided by your smartcard vendor.  That shared
> object takes care of communication with the underlying key store (smart
> card, tpm, hsm, ...).  The openvpn manpage calls this a pkcs11 'provider'.
>
> $ openvpn --show-pkcs11-ids
> /usr/lib/x86_64-linux-gnu/pkcs11/gnome-keyring-pkcs11.so
>
> The following objects are available for use.
> Each object shown below may be used as parameter to
> --pkcs11-id option please remember to use single quote mark.
>


​I seen that while testing. ​



>
> Certificate
>DN: C=KG, ST=NA, O=OpenVPN-TEST, CN=Test-Client,
> emailAddress=me@myhost.mydomain
>Serial: 02
>Serialized id:
> Gnome\x20Keyring/1\x2E0/1\x3AUSER\x3ADEFAULT/Gnome2\x20Key\x
> 20Storage/1A9D824585217F1BD54603E83F042F570A2EE9F2
>
> (I have the openvpn client testkey in my local gnome keyring, to easily
> test pkcs11 without a hardware token.)
>
> From that list, you pick the object you want to use, and specify the
> provider and id in your config file.  In this case:
>
> pkcs11-providers "/usr/lib/x86_64-linux-gnu/pkc
> s11/gnome-keyring-pkcs11.so"
> pkcs11-id
> "Gnome\\x20Keyring/1\\x2E0/1\\x3AUSER\\x3ADEFAULT/Gnome2\\x2
> 0Key\\x20Storage/1B3B2BEBF36576443D3A903AFA8997221B93FCC1"
>
> You specify this *instead* of the regular 'key' and 'cert' options.
> Note that you'll need toe escape backslashes.
>
>
​Good to know :)​



> You should now be able to use pkcs11 for the private key operations.
>
> Since each keystore behaves differently, I did not specify how to get
> the private key in there.  But I guess your TPM vendor providers tooling
> for that.
>

Well, TPM2 is a keystore, so that should work :)

I am able to store the certificate in the TPM2 nvram (as a DER file) and I
have a corresponding RSA private key (also in the TPM2, in the persistent
object area).

The PKCS11 provider I'm using (tpm2-pk11) has been modified to get the
certificate from the TPM2, but I don't know how to tell openvpn to use the
corresponding key on the TPM2 (and to use the TPM2_RSA_encrypt/RSA_decrypt
functions).

I'm not sure what's missing here (probably something on my side, either
some config or some code here and there). Here are the PKCS#11 ids and my
current configuration:

root@host ~# openvpn --show-pkcs11-ids

The following objects are available for use.
Each object shown below may be used as parameter to
--pkcs11-id option please remember to use single quote mark.

Certificate
   DN: C=FR, O=ME, OU=Unit, CN=005043361C24
   Serial:
 CFC3AC7FC37F777CF509506BC8EA0A94E9FBDC121686FEBEACEF99AC474ED21D
   Serialized id:  ME/TPM2//nvram/0188

Certificate
   DN: C=US, O=ME, OU=Unit, CN=006060584526
   Serial: 266B18A3534E3179
   Serialized id:  ME/TPM2//nvram/0184






client
# -- yeah, this is not the real server :)
remote openvpn.example.com
port 443
dev tun
persist-key
persist-tun
proto tcp6-client
tun-mtu 1500
auth SHA256
connect-retry-max 3
remap-usr1 SIGTERM

remote-cert-tls server
tls-client
ca /etc/PKI/CA.pem
tls-auth /etc/PKI/TA.ta 1
cipher AES-256-CBC
script-security 2
pkcs11-providers "/usr/lib/libtpm2-pk11.so"

# -- in DER format

pkcs11-id "ME/TPM2//nvram/0188"

#  previous key configuration --
# -- same as ME/TPM2//nvram/0188
# cert /etc/PKI/client.pem
# -- also in the TPM2, but how to tell that to openvpn?
# key /etc/PKI/client.key

up /etc/openvpn/start.sh
down /etc/openvpn/stop.sh

comp-lzo
verb 11

float



In my unders

Re: [Openvpn-devel] openvpn segfaults on --management-external-key with ECC certificate

[Emmanuel Deloget]

Hello,


On Wed, Jan 3, 2018 at 9:34 AM, Arne Schwabe <a...@rfc2549.org> wrote:

> Am 03.01.18 um 09:19 schrieb Steffan Karger:
> > On 03-01-18 03:22, Selva Nair wrote:
> >> This is with openssl 1.0.1 and that could be the problem -- it may not
> >> have EVP_PKEY_get0_RSA() in which case the compatibility interface in
> >> use is probably not smart enough...
>

​You are fully right. ​​It seems I failed to see that, and I don't remember
why. I guess I'm getting old... :)



> >
> > Exactly this is the case I think.  The following should solve the issue:
> >
> > --- a/src/openvpn/openssl_compat.h
> > +++ b/src/openvpn/openssl_compat.h
> > @@ -240,7 +240,7 @@ X509_OBJECT_get_type(const X509_OBJECT *obj)
> >  static inline RSA *
> >  EVP_PKEY_get0_RSA(EVP_PKEY *pkey)
> >  {
> > -return pkey ? pkey->pkey.rsa : NULL;
> > +return (pkey && pkey->type == EVP_PKEY_RSA) ? pkey->pkey.rsa : NULL;
> >  }
> >  #endif
> >
> > (No time to properly test and send a patch now, will look into it more
> > later if nobody else does.)
>
> You are right. This is also what OpenSSL 1.1.0  does:
>
> RSA *
> ​​
> EVP_PKEY_get0_RSA(EVP_PKEY *pkey)
> {
> if (pkey->type != EVP_PKEY_RSA) {
> EVPerr(EVP_F_EVP_PKEY_GET0_RSA, EVP_R_EXPECTING_AN_RSA_KEY);
> return NULL;
> }
> return pkey->pkey.rsa;
> }
>
>
Normally, the code that calls EVP_PKEY_get0_*() is protected by a test
EVP_PKEY_id() == EVP_PKEY_*. The only case where it's not is
in tls_ctx_use_external_private_key() where it executes the following code:

pub_rsa = EVP_PKEY_get0_RSA(pkey);

/* Certificate might not be RSA but DSA or EC */
if (!pub_rsa)
{
crypto_msg(M_WARN, "management-external-key requires a RSA
certificate");
goto err;
}

But then, since pkey->pkey is an union, it will return a non-null pointer
when called on a DSA or EC key. I guess I wrongly assumed it was a struct
(BTW, the code it replaced seemed to also assume pkey->pkey was a struct;
that does not excuse my own failure).

So a better (as in: more complete) fix would be to also correct
EVP_PKEY_get0_DSA() and EVP_PKEY_get0_EC_KEY() and to remove unnecessary
calls to EVP_PKEY_id() when it makes sense. I made this while writing this
email and pushed the corresponding branch here:

https://github.com/emmanuel-deloget/openvpn/tree/fix-evp-pkey

I will send the patches on the ML as soon as I can.


Arne



​Best regards,

-- Emmanuel Deloget​
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] PKCS#11 - a little bit of help?

[Emmanuel Deloget]

Hello everybody,

I'm trying to get openvpn read my certificates from a TPM2 using a
specially crafted PKCS#11 provider (the existing tpm2-pk11 is quite limited
for now but I might be able to extend it).

However, the PKCS#11 API is not something I'm comfortable with, and I'd
like to know if there is some document (design or anything, really) that
could help me to understand what openvpn wants exactly in order for me to
provide the missing bits. I've read the documents at [1] but found nothing
here of interest (for me).

So, does someone have any pointer?

Best regards,

-- Emmanuel Deloget

[1] https://openvpn.net/index.php/open-source/documentation.html
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Windows installer with updated pkcs11-helper (1.22) available for testing

[Emmanuel Deloget]

Hi David, other,

It seems that my "I'm using my phone, sorry for top posting" went far
beyond that because I also forgot to add the list as a recipient.

So, here is my message (minus the top-posting) sent to David only (sorry David)

On Tue, Jul 25, 2017 at 9:21 PM, David Woodhouse <dw...@infradead.org> wrote:
>
> On Tue, 2017-07-25 at 19:53 +0300, Samuli Seppänen wrote:
> >
> > I released the new Windows installer but without this patch. That said,
> > the patch/PR you linked to makes sense. Does the patch have an active
> > maintainer?
>
> That would be me, I suppose. Until/unless the upstream maintainer
> applies the patch, I or someone else will continue to maintain the
> patch in the Fedora package at least.
>
> > If yes, then I propose is that we
> >
> > - Fork OpenSC/pkcs11-helper to OpenVPN/pkcs11-helper
> > - Apply the patch to our fork
> > - Produce and publish our own pkcs11-helper tarballs
> > - Point openvpn-build to our tarballs
> > - Periodically rebase our fork with upstream
> >
> > Thoughts?
>
>
> Hi,
>
> Sorry for the top posting, I'm writing that from my phone.
>
> To add to the NAK, such a move would make integration to various embedded
> distribution more difficult, as maintainers may have to deal with 2 versions 
> of the
> same lib (with possibly different behavior needed by different binaries).
>
> Do I would advise against such a solution.
>
> Best regards,
>
> -- Emmanuel Deloget

To which David replied (hope you don't mind if I quote you):

> There is no API/ABI change. One accepts cert identifier strings confirming
> to RFC7512, the other (without the patch) doesn't.
>
> So aside from the RFC-compliance they are interchangeable.

(I think I'm already using this patch, so thanks you David for your work).

A single patch would not a a problem for distro maintainers, but
subsequent/future changes in the forked repository might introduce
other, less compatible changes in the library, leading to two versions
of the same library, with maybe the same ABI and the same name -- and
that worries me a bit (not that much, but I also maintain a fork of
lede for my own purpose, and I'm not sure I'd like to deal with such
complexity :)). Aviding that would require a library name change and
I'm not sure it's a good thing at all.

And you also have to take into account weird people like me who also
add their own changes into various libraries or binaries to suit their
needs (the whole "hey, it's free software, I can change it" thing).
Pointing them to a fork is not going to work well IMHO. It would be
better if the maintainer finally decide to either implement the
required change or merge your patch to the library.

Best regards,

-- Emmanuel Deloget

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 1/2] OpenSSL: remove EVP_CIPHER_CTX_new() from the compat layer

[Emmanuel Deloget]

Hi,

For the two patches in this series, I failed to correctly write
"writer", which I spelled "write", sorry for that. And sorry to said
writer for the gratuitous attack (but then, I'm me, and I'm pretty
sure I'll be able to handle that). And I forgot to mention that the
EVP_CIPHER_CTX_free() function is broken (good thing it has never been
used) since it leaks memory.

For the record, I found this (kinda stupid?) mistake while I was
writing yet another shim to openssl 1.1. I started a discussion on the
openssh mailing list and it seems that supporting a shim in openssh is
the least prefered idea for a few people, right after "not supporting
openssl 1.1 at all". The idea would then to have an independant shim
that would cover the needs for various openssl-based projects. I'm
doing my tests on openvpn since it's the easy way (for me).

So: sorry for the annoying noise :)

Best regards,

-- Emmanuel Deloget

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 2/2] OpenSSL: remove EVP_CIPHER_CTX_free() from the compat layer

[Emmanuel Deloget]

For unknown reason, the write of the compat layer seemed to think that
this function was only present in OpenSSL 1.1. This is not the case at
all, since it has been introduced in OpenSSL before version 0.9.8.

Thus, there is no need to add this function to the compat layer, and it
can be safely removed.

Signed-off-by: Emmanuel Deloget <log...@free.fr>
---
 configure.ac |  1 -
 src/openvpn/openssl_compat.h | 13 -
 2 files changed, 14 deletions(-)

diff --git a/configure.ac b/configure.ac
index cb121795..60bb4658 100644
--- a/configure.ac
+++ b/configure.ac
@@ -919,7 +919,6 @@ if test "${enable_crypto}" = "yes" -a 
"${with_crypto_library}" = "openssl"; then
 
AC_CHECK_FUNCS(
[ \
-   EVP_CIPHER_CTX_free \
HMAC_CTX_new \
HMAC_CTX_free \
HMAC_CTX_reset \
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index cd25bd37..36f68b01 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -88,19 +88,6 @@ EVP_MD_CTX_new(void)
 }
 #endif
 
-#if !defined(HAVE_EVP_CIPHER_CTX_FREE)
-/**
- * Free an existing cipher context
- *
- * @param ctx The cipher context
- */
-static inline void
-EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *c)
-{
-   free(c);
-}
-#endif
-
 #if !defined(HAVE_HMAC_CTX_RESET)
 /**
  * Reset a HMAC context
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 1/2] OpenSSL: remove EVP_CIPHER_CTX_new() from the compat layer

[Emmanuel Deloget]

For unknown reason, the write of the compat layer seemed to think that
this function was only present in OpenSSL 1.1. This is not the case at
all, since it has been introduced in OpenSSL before version 0.9.8.

Thus, there is no need to add this function to the compat layer, and it
can be safely removed.

Signed-off-by: Emmanuel Deloget <log...@free.fr>
---
 configure.ac |  1 -
 src/openvpn/openssl_compat.h | 15 ---
 2 files changed, 16 deletions(-)

diff --git a/configure.ac b/configure.ac
index 22f91cb6..cb121795 100644
--- a/configure.ac
+++ b/configure.ac
@@ -919,7 +919,6 @@ if test "${enable_crypto}" = "yes" -a 
"${with_crypto_library}" = "openssl"; then
 
AC_CHECK_FUNCS(
[ \
-   EVP_CIPHER_CTX_new \
EVP_CIPHER_CTX_free \
HMAC_CTX_new \
HMAC_CTX_free \
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index 617410e0..cd25bd37 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -101,21 +101,6 @@ EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *c)
 }
 #endif
 
-#if !defined(HAVE_EVP_CIPHER_CTX_NEW)
-/**
- * Allocate a new cipher context object
- *
- * @returnA zero'ed cipher context object
- */
-static inline EVP_CIPHER_CTX *
-EVP_CIPHER_CTX_new(void)
-{
-EVP_CIPHER_CTX *ctx = NULL;
-ALLOC_OBJ_CLEAR(ctx, EVP_CIPHER_CTX);
-return ctx;
-}
-#endif
-
 #if !defined(HAVE_HMAC_CTX_RESET)
 /**
  * Reset a HMAC context
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2] Move adjust_power_of_2() to integer.h

[Emmanuel Deloget]

On Thu, Jun 22, 2017 at 6:08 PM, Antonio Quartulli <a...@unstable.cc> wrote:
>
> On Thu, Jun 22, 2017 at 05:33:44PM +0200, Emmanuel Deloget wrote:
> > Hi Antonio, Steffan,
> >
> > On Thu, Jun 22, 2017 at 3:31 PM, Antonio Quartulli <a...@unstable.cc> wrote:
> >
> > > Thanks for sending v2 Steffan,
> > >
> > > On Wed, Jun 21, 2017 at 11:10:43PM +0200, Steffan Karger wrote:
> > > > From: Steffan Karger <steffan.kar...@fox-it.com>
> > > >
> > > > misc.c is a mess of incoherent functions, and is therefore included by
> > > > virtually all our source files.  That makes testing harder than it 
> > > > should
> > > > be.  As a first step of cleaning up misc.c, move adjust_power_of_2() to
> > > > integer.h, which is a more suitable place for a function like this.
> > > >
> > > > This allows us to remove the duplicate implementation from test_argv.c.
> > > >
> > > > Signed-off-by: Steffan Karger <steffan.kar...@fox-it.com>
> > >
> > >
> > > This change is quite simple and passes basic tests.
> > > Thanks also for adding explicit #include statements.
> > >
> > > Acked-by: Antonio Quartulli <anto...@openvpn.net>
> > >
> > >
> > Since the code is moved, would it be interesting to use a less greedy
> > algorithm to do the same thing? I know the function is not used very often,
> > so it does not look like it's necessary (it's a suggestion, not a
> > commentary on your patch Steffan :))
>
> My personal opinion: *never* mix code refactoring with functional changes 
> (like
> what you are suggesting).
>
> Therefore we'd still need another patch.

Fair enough :)

> >
> > 
> >
> > Again, the function is not used extensively, so you can ignore this
> > suggestion as it will not speed up openvpn at all :)
>
> yeah...not sure we need to make some code a bit "obscure" if there is really 
> no
> need.
>
> Still, I'd suggest to prepare a second patch and propose it over the list.
>

I'll wait until I see Steffan's patch on master . But as you said,
/coda obscura est/ so the commit message will make that very clear.

>
> Cheers,
> --
> Antonio Quartulli

BR,

-- Emmanuel Deloget

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenVPN 2.4.3 released (with security fixes)

[Emmanuel Deloget]

Hi David,

On Wed, Jun 21, 2017 at 11:06 PM, David Sommerseth <
open...@sf.lists.topphemmelig.net> wrote:

​​


> But for reasons unknown to me, those tarballs got re-created somewhere
> later in the release chain.  The contents of all tarballs are
> essentially the same, but due to the "nice" artefact that the tar format
> is non-deterministic on the output, even though the input is the same,
> that begins to prepare the stage for this chaos.  Especially when what
> is being uploaded is partly from the initial run and then some files
> from a different run
> ​.
>

​It might be possible to pay with several tar options, including:

--sort=name : sort added files by name, and not by the order specified by
the OS
--mtime=DATE-OR-FILE : set mtime of added file to a known value (either the
mtime of a file or an arbitrary date/time string).  ​

​These two options should help​

​Both options are being used by the LEDE project​ which claim support of
reproducible builds for a limited list of targets (tar is used when
building packages [1]).

​[1]
https://git.lede-project.org/?p=source.git;a=blob;f=scripts/ipkg-build#l142​



> --
> kind regards,
>
> David Sommerseth
> OpenVPN Technologies, Inc
>
>
​BR,

-- Emmanuel Deloget​
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH applied] Re: OpenSSL: don't use direct access to the internal of HMAC_CTX

[Emmanuel Deloget]

Hi Gert,

On Sun, Jun 18, 2017 at 3:21 PM, Gert Doering <g...@greenie.muc.de> wrote:

> Your patch has been applied to the master and release/2.4 branch.
>
> I have not changed anything wrt _init()/reset() to not delay getting
> this in-tree before the upcoming v2.4.3 release - but if you agree to
> Steffan's comment on that, please send a followup patch which can then
> go in later.
>

​I'm going to do that ASAP. ​


>
> (Thanks for all your work!)
>


​It's me who want to thank you all: you did a marvellous job at pointing my
mistakes :) ​

​So thanks to you, Steffan, David, and all the other who helped me. ​



>
> commit aba98e9050eb54d72d921e70bcd422cb892b9c6c (master)
> commit 2bf4aee4b043151bd2abe7101421fd74763f1230 (release/2.4)
> Author: Emmanuel Deloget
> Date:   Mon Jun 12 15:43:29 2017 +0200
>
>  OpenSSL: don't use direct access to the internal of HMAC_CTX
>
>  Signed-off-by: Emmanuel Deloget <log...@free.fr>
>  Acked-by: Steffan Karger <steffan.kar...@fox-it.com>
>  Message-Id: <20170612134330.20971-8-log...@free.fr>
>  URL: https://www.mail-archive.com/openvpn-devel@lists.
> sourceforge.net/msg14797.html
>  Signed-off-by: Gert Doeri
> ​​
> ng <g...@greenie.muc.de>
>
>
> --
> kind regards,
>
> Gert Doering
>
>
​BR,

-- Emmanuel Deloget​
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 7/8] OpenSSL: don't use direct access to the internal of HMAC_CTX

[Emmanuel Deloget]

Hello Steffan,

On Sun, Jun 18, 2017 at 1:38 PM, Steffan Karger <stef...@karger.me> wrote:

> Hi,
>
> On 12-06-17 15:43, log...@free.fr wrote:
> > +#if !defined(HAVE_HMAC_CTX_INIT)
> > +/**
> > + * Init a HMAC context
> > + *
> > + * @param ctx The HMAC context
> > + *
> > + * Contrary to many functions in this file, HMAC_CTX_init() is not
> > + * an OpenSSL 1.1 function: it comes from previous versions and was
> > + * removed in v1.1. As a consequence, there is no distincting in
> > + * v1.1 between a cleanup, and init and a reset. Yet, previous OpenSSL
> > + * version need this distinction.
> > + *
> > + * In order to respect previous OpenSSL versions, we implement init
> > + * as reset for OpenSSL 1.1+.
> > + */
> > +static inline void
> > +HMAC_CTX_init(HMAC_CTX *ctx)
> > +{
> > +HMAC_CTX_reset(ctx);
> > +}
> > +#endif
>
> Hm, shouldn't we do this the other way around then?  Implement a
> HMAC_CTX_reset() here that calls HMAC_CTX_init(), and use _reset() in
> our hmac_ctx_init() function?
>
>
I think I did it that way to enhance code reading but it's true that it
introduces a pre-1.1 artefact into an interface which is based upon OpenSSL
1.1.

I'm sending a patch to fix that.


> ​>  >
>
> Otherwise this looks good, and passes my tests.  I would prefer to
> change the _init()/reset() thing before applying (if you agree that this
> is better), but if this is the only thing left to get 1.1 support into
> our next release that shouldn't block applying the patch.
>
> So, basically, ACK :)
>
> -Steffan
>

​BR,

-- Emmanuel Deloget​
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v7 0/8] OpenSSL: support for version 1.1

[Emmanuel Deloget]

Hi Jeremie,

On Thu, Jun 15, 2017 at 2:04 PM, Jeremie Courreges-Anglas <j...@wxcvbn.org>
wrote:

>
> Hi,
>
> fwiw, this builds fine against LibreSSL, which seems to provide some but
> not all of the functions for which you wrote fallback implementations.
> LibreSSL as shipped in OpenBSD-current, ie the development version.
>
> make check passes, openvpn seems to behave correctly in client mode.
>


​Thanks ! (and it's good to know :)​)

Frankly, testing against LibreSSL didn't even cross my mind :) I guess I
(wrongly) assumed that LibreSSL and OpenSSL were too different by now --
it's good to see than some of the API is still the same (for any definition
of good ; I guess the folk at LibreSSL find this a bit discouraging...).

Best regards,

-- Emmanuel Deloget
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenSSL 1.1 patch set - status?

[Emmanuel Deloget]

Hi Gert,

On Fri, May 19, 2017 at 1:41 PM, Gert Doering <g...@greenie.muc.de> wrote:

> Hi,
>
> On Fri, May 19, 2017 at 12:37:17PM +0200, Emmanuel Deloget wrote:
> > > I'm wondering where this got stuck - are you waiting for us to move
> > > forward (like, missing review of parts of the patch set), or are we
> > > waiting for you, and you've been busy?
> >
> > Problem is that I'm working in a more-than-full-time manner on
> > way-too-many-other subjects :)
>
> Thanks for quickly sending us the current patch set.  Now it's on us again.
>

​If there is anything bad, please tell me - I should be able to do another
round if needed :)​



>
> (And yes, we fully understand the "too many projects, too many complaining
> customers, angry wife as well, and too little time" thing :) )
>
>

​Yeah. It seems that "somebody" limited the day length to a small value
that fit on a 5 bit word. I'm wondering if this "person" was told about
uint32_t...​



>
> Over to Steffan now - I'm so happy that I do not have to understand
> OpenSSL code... ;-)
>

​Good thing is : there is nothing complex in the changes I made. I'm pretty
sure you'll be able to understand both the changes and what the original
code does :)​



>
> gert
>


​BR,

-- Emmanuel Deloget​
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 6/7] OpenSSL: don't use direct access to the internal of EVP_CIPHER_CTX

[Emmanuel Deloget]

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including EVP_CIPHER_CTX. We have to use the defined
functions to do so.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <log...@free.fr>
---
 configure.ac |  2 ++
 src/openvpn/crypto.c |  4 ++--
 src/openvpn/crypto_backend.h | 14 ++
 src/openvpn/crypto_mbedtls.c | 13 +
 src/openvpn/crypto_openssl.c | 15 +--
 src/openvpn/openssl_compat.h | 28 
 6 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index 9c7074d1..8a9a3ff3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -920,6 +920,8 @@ if test "${enable_crypto}" = "yes" -a 
"${with_crypto_library}" = "openssl"; then
 
AC_CHECK_FUNCS(
[ \
+   EVP_CIPHER_CTX_new \
+   EVP_CIPHER_CTX_free \
EVP_MD_CTX_new \
EVP_MD_CTX_free \
EVP_MD_CTX_reset \
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 50e6a734..893879cf 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -830,7 +830,7 @@ init_key_ctx(struct key_ctx *ctx, struct key *key,
 if (kt->cipher && kt->cipher_length > 0)
 {
 
-ALLOC_OBJ(ctx->cipher, cipher_ctx_t);
+ctx->cipher = cipher_ctx_new();
 cipher_ctx_init(ctx->cipher, key->cipher, kt->cipher_length,
 kt->cipher, enc);
 
@@ -879,7 +879,7 @@ free_key_ctx(struct key_ctx *ctx)
 if (ctx->cipher)
 {
 cipher_ctx_cleanup(ctx->cipher);
-free(ctx->cipher);
+cipher_ctx_free(ctx->cipher);
 ctx->cipher = NULL;
 }
 if (ctx->hmac)
diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h
index 8f03e2ba..3a911a47 100644
--- a/src/openvpn/crypto_backend.h
+++ b/src/openvpn/crypto_backend.h
@@ -301,6 +301,20 @@ bool cipher_kt_mode_aead(const cipher_kt_t *cipher);
  */
 
 /**
+ * Allocate a new cipher context
+ *
+ * @return  a new cipher context
+ */
+cipher_ctx_t *cipher_ctx_new(void);
+
+/**
+ * Free a cipher context
+ *
+ * @param ctx   Cipher context.
+ */
+void cipher_ctx_free(cipher_ctx_t *ctx);
+
+/**
  * Initialise a cipher context, based on the given key and key type.
  *
  * @param ctx   Cipher context. May not be NULL
diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c
index d6741523..4d38aadc 100644
--- a/src/openvpn/crypto_mbedtls.c
+++ b/src/openvpn/crypto_mbedtls.c
@@ -509,6 +509,19 @@ cipher_kt_mode_aead(const cipher_kt_t *cipher)
  *
  */
 
+mbedtls_cipher_context_t *
+cipher_ctx_new(void)
+{
+mbedtls_cipher_context_t *ctx;
+ALLOC_OBJ(ctx, mbedtls_cipher_context_t);
+return ctx;
+}
+
+void
+cipher_ctx_free(mbedtls_cipher_context_t *ctx)
+{
+free(ctx);
+}
 
 void
 cipher_ctx_init(mbedtls_cipher_context_t *ctx, uint8_t *key, int key_len,
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index fd599f40..0644f1c3 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -651,6 +651,19 @@ cipher_kt_mode_aead(const cipher_kt_t *cipher)
  *
  */
 
+cipher_ctx_t *
+cipher_ctx_new(void)
+{
+EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new();
+check_malloc_return(ctx);
+return ctx;
+}
+
+void
+cipher_ctx_free(EVP_CIPHER_CTX *ctx)
+{
+EVP_CIPHER_CTX_free(ctx);
+}
 
 void
 cipher_ctx_init(EVP_CIPHER_CTX *ctx, uint8_t *key, int key_len,
@@ -658,8 +671,6 @@ cipher_ctx_init(EVP_CIPHER_CTX *ctx, uint8_t *key, int 
key_len,
 {
 ASSERT(NULL != kt && NULL != ctx);
 
-CLEAR(*ctx);
-
 EVP_CIPHER_CTX_init(ctx);
 if (!EVP_CipherInit(ctx, kt, NULL, NULL, enc))
 {
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index 8305ec5b..d1be9d78 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -96,6 +96,34 @@ EVP_MD_CTX_new(void)
 }
 #endif
 
+#if !defined(HAVE_EVP_CIPHER_CTX_FREE)
+/**
+ * Free an existing cipher context
+ *
+ * @param ctx The cipher context
+ */
+static inline void
+EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *c)
+{
+   free(c);
+}
+#endif
+
+#if !defined(HAVE_EVP_CIPHER_CTX_NEW)
+/**
+ * Allocate a new cipher context object
+ *
+ * @returnA zero'ed cipher context object
+ */
+static inline EVP_CIPHER_CTX *
+EVP_CIPHER_CTX_new(void)
+{
+EVP_CIPHER_CTX *ctx = NULL;
+ALLOC_OBJ_CLEAR(ctx, EVP_CIPHER_CTX);
+return ctx;
+}
+#endif
+
 #if !defined(HAVE_SSL_CTX_GET_DEFAULT_PASSWD_CB_USERDATA)
 /**
  * Fetch the default password callback user data from the SSL context
-- 
2.11.0


---

[Openvpn-devel] [PATCH 3/7] OpenSSL: don't use direct access to the internal of RSA

[Emmanuel Deloget]

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including RSA. We have to use the defined
functions to do so.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <log...@free.fr>
---
 configure.ac |  3 ++
 src/openvpn/openssl_compat.h | 84 
 src/openvpn/ssl_openssl.c| 24 -
 3 files changed, 103 insertions(+), 8 deletions(-)

diff --git a/configure.ac b/configure.ac
index a92e8142..e4c053c8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -929,6 +929,9 @@ if test "${enable_crypto}" = "yes" -a 
"${with_crypto_library}" = "openssl"; then
EVP_PKEY_id \
EVP_PKEY_get0_RSA \
EVP_PKEY_get0_DSA \
+   RSA_set_flags \
+   RSA_get0_key \
+   RSA_set0_key \
RSA_meth_new \
RSA_meth_free \
RSA_meth_set_pub_enc \
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index 0d82cf25..29cd13a4 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -176,6 +176,90 @@ EVP_PKEY_get0_DSA(EVP_PKEY *pkey)
 }
 #endif
 
+#if !defined(HAVE_RSA_SET_FLAGS)
+/**
+ * Set the RSA flags
+ *
+ * @param rsa The RSA object
+ * @param flags   New flags value
+ */
+static inline void
+RSA_set_flags(RSA *rsa, int flags)
+{
+if (rsa)
+{
+rsa->flags = flags;
+}
+}
+#endif
+
+#if !defined(HAVE_RSA_GET0_KEY)
+/**
+ * Get the RSA parameters
+ *
+ * @param rsa The RSA object
+ * @param n   The @c n parameter
+ * @param e   The @c e parameter
+ * @param d   The @c d parameter
+ */
+static inline void
+RSA_get0_key(const RSA *rsa, const BIGNUM **n,
+ const BIGNUM **e, const BIGNUM **d)
+{
+if (n != NULL)
+{
+*n = rsa ? rsa->n : NULL;
+}
+if (e != NULL)
+{
+*e = rsa ? rsa->e : NULL;
+}
+if (d != NULL)
+{
+*d = rsa ? rsa->d : NULL;
+}
+}
+#endif
+
+#if !defined(HAVE_RSA_SET0_KEY)
+/**
+ * Set the RSA parameters
+ *
+ * @param rsa The RSA object
+ * @param n   The @c n parameter
+ * @param e   The @c e parameter
+ * @param d   The @c d parameter
+ * @return1 on success, 0 on error
+ */
+static inline int
+RSA_set0_key(RSA *rsa, BIGNUM *n, BIGNUM *e, BIGNUM *d)
+{
+if ((rsa->n == NULL && n == NULL)
+|| (rsa->e == NULL && e == NULL))
+{
+return 0;
+}
+
+if (n != NULL)
+{
+BN_free(rsa->n);
+rsa->n = n;
+}
+if (e != NULL)
+{
+BN_free(rsa->e);
+rsa->e = e;
+}
+if (d != NULL)
+{
+BN_free(rsa->d);
+rsa->d = d;
+}
+
+return 1;
+}
+#endif
+
 #if !defined(HAVE_RSA_METH_NEW)
 /**
  * Allocate a new RSA method object
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 1c73641c..48479c0d 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -975,8 +975,8 @@ rsa_priv_dec(int flen, const unsigned char *from, unsigned 
char *to, RSA *rsa, i
 static int
 rsa_finish(RSA *rsa)
 {
-RSA_meth_free(rsa->meth);
-rsa->meth = NULL;
+const RSA_METHOD *meth = RSA_get_method(rsa);
+RSA_meth_free((RSA_METHOD *)meth);
 return 1;
 }
 
@@ -1075,8 +1075,11 @@ tls_ctx_use_external_private_key(struct tls_root_ctx 
*ctx,
 pub_rsa = EVP_PKEY_get0_RSA(pkey);
 
 /* initialize RSA object */
-rsa->n = BN_dup(pub_rsa->n);
-rsa->flags |= RSA_FLAG_EXT_PKEY;
+const BIGNUM *n = NULL;
+const BIGNUM *e = NULL;
+RSA_get0_key(pub_rsa, , , NULL);
+RSA_set0_key(rsa, BN_dup(n), BN_dup(e), NULL);
+RSA_set_flags(rsa, RSA_flags(rsa) | RSA_FLAG_EXT_PKEY);
 if (!RSA_set_method(rsa, rsa_meth))
 {
 goto err;
@@ -1677,11 +1680,16 @@ print_details(struct key_state_ssl *ks_ssl, const char 
*prefix)
 EVP_PKEY *pkey = X509_get_pubkey(cert);
 if (pkey != NULL)
 {
-if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA && EVP_PKEY_get0_RSA(pkey) 
!= NULL
-&& pkey->pkey.rsa->n != NULL)
+if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA && EVP_PKEY_get0_RSA(pkey) 
!= NULL)
 {
-openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA",
- BN_num_bits(pkey->pkey.rsa->n));
+RSA *rsa = EVP_PKEY_get0_RSA(pkey);
+const BIGNUM *n = NULL;
+RSA_get0_key(rsa, , NULL, NULL);
+

[Openvpn-devel] [PATCH 1/7] OpenSSL: don't use direct access to the internal of X509

[Emmanuel Deloget]

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including X509. We have to use the defined
functions to do so.

In x509_verify_ns_cert_type() in particular, this means that we
cannot directly check for the extended flags to find whether the
certificate should be used as a client or as a server certificate.
We need to leverage the X509_check_purpose() API yet this API is
far stricter than the currently implemented check. So far, I have
not been able to find a situation where this stricter test fails
(although I must admit that I haven't tested that very well).

We double-check the certificate purpose using "direct access" to the
internal of the certificate object (of course, this is not a real
direct access, but we still fetch ASN1 strings within the X509 object
and we check the internal value of these strings). This allow us to
warn the user if there is a discrepancy between the X509_check_purpose()
return value and our internal, less strict check.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <log...@free.fr>
---
 configure.ac |  1 +
 src/openvpn/openssl_compat.h | 15 ++
 src/openvpn/ssl_openssl.c|  3 +-
 src/openvpn/ssl_verify_openssl.c | 64 ++--
 4 files changed, 73 insertions(+), 10 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7d3fce5b..9d5e340b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -922,6 +922,7 @@ if test "${enable_crypto}" = "yes" -a 
"${with_crypto_library}" = "openssl"; then
[ \
SSL_CTX_get_default_passwd_cb \
SSL_CTX_get_default_passwd_cb_userdata \
+   X509_get0_pubkey \
X509_STORE_get0_objects \
X509_OBJECT_free \
X509_OBJECT_get_type \
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index 92f014d5..29a7588c 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -74,6 +74,21 @@ SSL_CTX_get_default_passwd_cb(SSL_CTX *ctx)
 }
 #endif
 
+#if !defined(HAVE_X509_GET0_PUBKEY)
+/**
+ * Get the public key from a X509 certificate
+ *
+ * @param x  X509 certificate
+ * @return   The certificate public key
+ */
+static inline EVP_PKEY *
+X509_get0_pubkey(const X509 *x)
+{
+return (x && x->cert_info && x->cert_info->key) ?
+   x->cert_info->key->pkey : NULL;
+}
+#endif
+
 #if !defined(HAVE_X509_STORE_GET0_OBJECTS)
 /**
  * Fetch the X509 object stack from the X509 store
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 645ccf51..a082c3cd 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1070,7 +1070,8 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
 }
 
 /* get the public key */
-ASSERT(cert->cert_info->key->pkey); /* NULL before 
SSL_CTX_use_certificate() is called */
+EVP_PKEY *pkey = X509_get0_pubkey(cert);
+ASSERT(pkey); /* NULL before SSL_CTX_use_certificate() is called */
 pub_rsa = cert->cert_info->key->pkey->pkey.rsa;
 
 /* initialize RSA object */
diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c
index 9b1533bc..4785f314 100644
--- a/src/openvpn/ssl_verify_openssl.c
+++ b/src/openvpn/ssl_verify_openssl.c
@@ -294,18 +294,20 @@ backend_x509_get_serial_hex(openvpn_x509_cert_t *cert, 
struct gc_arena *gc)
 struct buffer
 x509_get_sha1_fingerprint(X509 *cert, struct gc_arena *gc)
 {
-struct buffer hash = alloc_buf_gc(sizeof(cert->sha1_hash), gc);
-memcpy(BPTR(), cert->sha1_hash, sizeof(cert->sha1_hash));
-ASSERT(buf_inc_len(, sizeof(cert->sha1_hash)));
+const EVP_MD *sha1 = EVP_sha1();
+struct buffer hash = alloc_buf_gc(EVP_MD_size(sha1), gc);
+X509_digest(cert, EVP_sha1(), BPTR(), NULL);
+ASSERT(buf_inc_len(, EVP_MD_size(sha1)));
 return hash;
 }
 
 struct buffer
 x509_get_sha256_fingerprint(X509 *cert, struct gc_arena *gc)
 {
-struct buffer hash = alloc_buf_gc((EVP_sha256())->md_size, gc);
+const EVP_MD *sha256 = EVP_sha256();
+struct buffer hash = alloc_buf_gc(EVP_MD_size(sha256), gc);
 X509_digest(cert, EVP_sha256(), BPTR(), NULL);
-ASSERT(buf_inc_len(, (EVP_sha256())->md_size));
+ASSERT(buf_inc_len(, EVP_MD_size(sha256)));
 return hash;
 }
 
@@ -578,13 +580,57 @@ x509_verify_ns_cert_type(const openvpn_x509_cert_t 
*peer_cert, const int usage)
 }
 if (usage == NS_CERT_CHECK_CLIENT)
 {
-return ((peer_cert->ex_flags & EXFLAG_NSCERT)
-&& (peer_cert->ex_nscert & NS_SSL_CLIENT)) ? SUCCESS : FAILURE;
+/*
+ * Unfortunately, X509_check_purpose() 

[Openvpn-devel] [PATCH 7/7] OpenSSL: don't use direct access to the internal of HMAC_CTX

[Emmanuel Deloget]

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including HMAC_CTX. We have to use the defined
functions to do so.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <log...@free.fr>
---
 configure.ac |  4 +++
 src/openvpn/crypto.c |  4 +--
 src/openvpn/crypto_backend.h | 14 ++
 src/openvpn/crypto_mbedtls.c | 15 ++
 src/openvpn/crypto_openssl.c | 17 ++--
 src/openvpn/ntlm.c   | 12 
 src/openvpn/openssl_compat.h | 65 
 src/openvpn/ssl.c| 38 ++
 8 files changed, 140 insertions(+), 29 deletions(-)

diff --git a/configure.ac b/configure.ac
index 8a9a3ff3..7875c4fb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -922,6 +922,10 @@ if test "${enable_crypto}" = "yes" -a 
"${with_crypto_library}" = "openssl"; then
[ \
EVP_CIPHER_CTX_new \
EVP_CIPHER_CTX_free \
+   HMAC_CTX_new \
+   HMAC_CTX_free \
+   HMAC_CTX_reset \
+   HMAC_CTX_init \
EVP_MD_CTX_new \
EVP_MD_CTX_free \
EVP_MD_CTX_reset \
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 893879cf..a80c3f7f 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -854,7 +854,7 @@ init_key_ctx(struct key_ctx *ctx, struct key *key,
 }
 if (kt->digest && kt->hmac_length > 0)
 {
-ALLOC_OBJ(ctx->hmac, hmac_ctx_t);
+ctx->hmac = hmac_ctx_new();
 hmac_ctx_init(ctx->hmac, key->hmac, kt->hmac_length, kt->digest);
 
 msg(D_HANDSHAKE,
@@ -885,7 +885,7 @@ free_key_ctx(struct key_ctx *ctx)
 if (ctx->hmac)
 {
 hmac_ctx_cleanup(ctx->hmac);
-free(ctx->hmac);
+hmac_ctx_free(ctx->hmac);
 ctx->hmac = NULL;
 }
 ctx->implicit_iv_len = 0;
diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h
index 3a911a47..3dc75ab9 100644
--- a/src/openvpn/crypto_backend.h
+++ b/src/openvpn/crypto_backend.h
@@ -584,6 +584,20 @@ void md_ctx_final(md_ctx_t *ctx, uint8_t *dst);
  */
 
 /*
+ * Create a new HMAC context
+ *
+ * @return  A new HMAC context
+ */
+hmac_ctx_t *hmac_ctx_new(void);
+
+/*
+ * Free an existing HMAC context
+ *
+ * @param  ctx   HMAC context to free
+ */
+void hmac_ctx_free(hmac_ctx_t *ctx);
+
+/*
  * Initialises the given HMAC context, using the given digest
  * and key.
  *
diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c
index 4d38aadc..f0698f61 100644
--- a/src/openvpn/crypto_mbedtls.c
+++ b/src/openvpn/crypto_mbedtls.c
@@ -841,6 +841,21 @@ md_ctx_final(mbedtls_md_context_t *ctx, uint8_t *dst)
 /*
  * TODO: re-enable dmsg for crypto debug
  */
+
+mbedtls_md_context_t *
+hmac_ctx_new(void)
+{
+mbedtls_md_context_t *ctx;
+ALLOC_OBJ(ctx, mbedtls_md_context_t);
+return ctx;
+}
+
+void
+hmac_ctx_free(mbedtls_md_context_t *ctx)
+{
+free(ctx);
+}
+
 void
 hmac_ctx_init(mbedtls_md_context_t *ctx, const uint8_t *key, int key_len,
   const mbedtls_md_info_t *kt)
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 0644f1c3..b64f7f04 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -911,6 +911,19 @@ md_ctx_final(EVP_MD_CTX *ctx, uint8_t *dst)
  *
  */
 
+HMAC_CTX *
+hmac_ctx_new(void)
+{
+HMAC_CTX *ctx = HMAC_CTX_new();
+check_malloc_return(ctx);
+return ctx;
+}
+
+void
+hmac_ctx_free(HMAC_CTX *ctx)
+{
+HMAC_CTX_free(ctx);
+}
 
 void
 hmac_ctx_init(HMAC_CTX *ctx, const uint8_t *key, int key_len,
@@ -918,8 +931,6 @@ hmac_ctx_init(HMAC_CTX *ctx, const uint8_t *key, int 
key_len,
 {
 ASSERT(NULL != kt && NULL != ctx);
 
-CLEAR(*ctx);
-
 HMAC_CTX_init(ctx);
 HMAC_Init_ex(ctx, key, key_len, kt, NULL);
 
@@ -930,7 +941,7 @@ hmac_ctx_init(HMAC_CTX *ctx, const uint8_t *key, int 
key_len,
 void
 hmac_ctx_cleanup(HMAC_CTX *ctx)
 {
-HMAC_CTX_cleanup(ctx);
+HMAC_CTX_reset(ctx);
 }
 
 int
diff --git a/src/openvpn/ntlm.c b/src/openvpn/ntlm.c
index 0c436812..4a4e8b9b 100644
--- a/src/openvpn/ntlm.c
+++ b/src/openvpn/ntlm.c
@@ -86,13 +86,13 @@ static void
 gen_hmac_md5(const char *data, int data_len, const char *key, int key_len,char 
*result)
 {
 const md_kt_t *md5_kt = md_kt_get("MD5");
-hmac_ctx_t hmac_ctx;
-CLEAR(hmac_ctx);
+hmac_ctx_t *hmac_ctx = hmac_ctx_new();
 
-hmac_ctx_init(_ctx, key, key_len, md5_kt);
-hmac_ctx_update(_ctx, (const unsigned char *)data, data_len);
-hmac_ctx_final(_ctx, (unsigned char *)result);
-hmac_ctx_cleanup(_ctx);
+hmac_ctx_init(hmac_c

[Openvpn-devel] [PATCH 5/7] OpenSSL: don't use direct access to the internal of EVP_MD_CTX

[Emmanuel Deloget]

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including EVP_MD_CTX. We have to use the defined
functions to do so.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <log...@free.fr>
---
 configure.ac |  3 ++
 src/openvpn/crypto_backend.h | 14 
 src/openvpn/crypto_mbedtls.c | 12 +++
 src/openvpn/crypto_openssl.c | 18 --
 src/openvpn/httpdigest.c | 78 +++-
 src/openvpn/misc.c   | 14 
 src/openvpn/openssl_compat.h | 50 
 src/openvpn/openvpn.h|  2 +-
 src/openvpn/push.c   | 11 ---
 9 files changed, 150 insertions(+), 52 deletions(-)

diff --git a/configure.ac b/configure.ac
index d2dc1ffd..9c7074d1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -920,6 +920,9 @@ if test "${enable_crypto}" = "yes" -a 
"${with_crypto_library}" = "openssl"; then
 
AC_CHECK_FUNCS(
[ \
+   EVP_MD_CTX_new \
+   EVP_MD_CTX_free \
+   EVP_MD_CTX_reset \
SSL_CTX_get_default_passwd_cb \
SSL_CTX_get_default_passwd_cb_userdata \
X509_get0_pubkey \
diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h
index 9b113d7b..8f03e2ba 100644
--- a/src/openvpn/crypto_backend.h
+++ b/src/openvpn/crypto_backend.h
@@ -508,6 +508,20 @@ int md_kt_size(const md_kt_t *kt);
 int md_full(const md_kt_t *kt, const uint8_t *src, int src_len, uint8_t *dst);
 
 /*
+ * Allocate a new message digest context
+ *
+ * @return  a new zeroed MD context
+ */
+md_ctx_t *md_ctx_new(void);
+
+/*
+ * Free an existing, non-null message digest context
+ *
+ * @param ctx   Message digest context
+ */
+void md_ctx_free(md_ctx_t *ctx);
+
+/*
  * Initialises the given message digest context.
  *
  * @param ctx   Message digest context
diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c
index 942684ce..d6741523 100644
--- a/src/openvpn/crypto_mbedtls.c
+++ b/src/openvpn/crypto_mbedtls.c
@@ -766,6 +766,18 @@ md_full(const md_kt_t *kt, const uint8_t *src, int 
src_len, uint8_t *dst)
 return 0 == mbedtls_md(kt, src, src_len, dst);
 }
 
+mbedtls_md_context_t *
+md_ctx_new(void)
+{
+mbedtls_md_context_t *ctx;
+ALLOC_OBJ_CLEAR(ctx, mbedtls_md_context_t);
+return ctx;
+}
+
+void md_ctx_free(mbedtls_md_context_t *ctx)
+{
+free(ctx);
+}
 
 void
 md_ctx_init(mbedtls_md_context_t *ctx, const mbedtls_md_info_t *kt)
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 881a2d13..fd599f40 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -42,6 +42,7 @@
 #include "integer.h"
 #include "crypto.h"
 #include "crypto_backend.h"
+#include "openssl_compat.h"
 
 #include 
 #include 
@@ -844,13 +845,24 @@ md_full(const EVP_MD *kt, const uint8_t *src, int 
src_len, uint8_t *dst)
 return EVP_Digest(src, src_len, dst, _md_len, kt, NULL);
 }
 
+EVP_MD_CTX *
+md_ctx_new(void)
+{
+EVP_MD_CTX *ctx = EVP_MD_CTX_new();
+check_malloc_return(ctx);
+return ctx;
+}
+
+void md_ctx_free(EVP_MD_CTX *ctx)
+{
+EVP_MD_CTX_free(ctx);
+}
+
 void
 md_ctx_init(EVP_MD_CTX *ctx, const EVP_MD *kt)
 {
 ASSERT(NULL != ctx && NULL != kt);
 
-CLEAR(*ctx);
-
 EVP_MD_CTX_init(ctx);
 EVP_DigestInit(ctx, kt);
 }
@@ -858,7 +870,7 @@ md_ctx_init(EVP_MD_CTX *ctx, const EVP_MD *kt)
 void
 md_ctx_cleanup(EVP_MD_CTX *ctx)
 {
-EVP_MD_CTX_cleanup(ctx);
+EVP_MD_CTX_reset(ctx);
 }
 
 int
diff --git a/src/openvpn/httpdigest.c b/src/openvpn/httpdigest.c
index ae4a638f..2a66d9b8 100644
--- a/src/openvpn/httpdigest.c
+++ b/src/openvpn/httpdigest.c
@@ -81,27 +81,28 @@ DigestCalcHA1(
 )
 {
 HASH HA1;
-md_ctx_t md5_ctx;
+md_ctx_t *md5_ctx = md_ctx_new();
 const md_kt_t *md5_kt = md_kt_get("MD5");
 
-md_ctx_init(_ctx, md5_kt);
-md_ctx_update(_ctx, (const uint8_t *) pszUserName, 
strlen(pszUserName));
-md_ctx_update(_ctx, (const uint8_t *) ":", 1);
-md_ctx_update(_ctx, (const uint8_t *) pszRealm, strlen(pszRealm));
-md_ctx_update(_ctx, (const uint8_t *) ":", 1);
-md_ctx_update(_ctx, (const uint8_t *) pszPassword, 
strlen(pszPassword));
-md_ctx_final(_ctx, HA1);
+md_ctx_init(md5_ctx, md5_kt);
+md_ctx_update(md5_ctx, (const uint8_t *) pszUserName, strlen(pszUserName));
+md_ctx_update(md5_ctx, (const uint8_t *) ":", 1);
+md_ctx_update(md5_ctx, (const uint8_t *) pszRealm, strlen(pszRealm));
+md_ctx_update(md5_ctx, (const uint8_t *) ":", 1);
+md_ctx_update(md5_ctx, (const uint8_t *) pszPassword, strlen(pszPassword));
+md_ctx_fin

[Openvpn-devel] [PATCH 2/7] OpenSSL: don't use direct access to the internal of EVP_PKEY

[Emmanuel Deloget]

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including EVP_PKEY. We have to use the defined
functions to do so.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <log...@free.fr>
---
 configure.ac |  3 +++
 src/openvpn/openssl_compat.h | 42 ++
 src/openvpn/ssl_openssl.c|  6 +++---
 3 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 9d5e340b..a92e8142 100644
--- a/configure.ac
+++ b/configure.ac
@@ -926,6 +926,9 @@ if test "${enable_crypto}" = "yes" -a 
"${with_crypto_library}" = "openssl"; then
X509_STORE_get0_objects \
X509_OBJECT_free \
X509_OBJECT_get_type \
+   EVP_PKEY_id \
+   EVP_PKEY_get0_RSA \
+   EVP_PKEY_get0_DSA \
RSA_meth_new \
RSA_meth_free \
RSA_meth_set_pub_enc \
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index 29a7588c..0d82cf25 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -134,6 +134,48 @@ X509_OBJECT_get_type(const X509_OBJECT *obj)
 }
 #endif
 
+#if !defined(HAVE_EVP_PKEY_GET0_RSA)
+/**
+ * Get the RSA object of a public key
+ *
+ * @param pkeyPublic key object
+ * @returnThe underlying RSA object
+ */
+static inline RSA *
+EVP_PKEY_get0_RSA(EVP_PKEY *pkey)
+{
+return pkey ? pkey->pkey.rsa : NULL;
+}
+#endif
+
+#if !defined(HAVE_EVP_PKEY_ID)
+/**
+ * Get the PKEY type
+ *
+ * @param pkeyPublic key object
+ * @returnThe key type
+ */
+static inline int
+EVP_PKEY_id(const EVP_PKEY *pkey)
+{
+return pkey ? pkey->type : EVP_PKEY_NONE;
+}
+#endif
+
+#if !defined(HAVE_EVP_PKEY_GET0_DSA)
+/**
+ * Get the DSA object of a public key
+ *
+ * @param pkeyPublic key object
+ * @returnThe underlying DSA object
+ */
+static inline DSA *
+EVP_PKEY_get0_DSA(EVP_PKEY *pkey)
+{
+return pkey ? pkey->pkey.dsa : NULL;
+}
+#endif
+
 #if !defined(HAVE_RSA_METH_NEW)
 /**
  * Allocate a new RSA method object
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index a082c3cd..1c73641c 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1072,7 +1072,7 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
 /* get the public key */
 EVP_PKEY *pkey = X509_get0_pubkey(cert);
 ASSERT(pkey); /* NULL before SSL_CTX_use_certificate() is called */
-pub_rsa = cert->cert_info->key->pkey->pkey.rsa;
+pub_rsa = EVP_PKEY_get0_RSA(pkey);
 
 /* initialize RSA object */
 rsa->n = BN_dup(pub_rsa->n);
@@ -1677,13 +1677,13 @@ print_details(struct key_state_ssl *ks_ssl, const char 
*prefix)
 EVP_PKEY *pkey = X509_get_pubkey(cert);
 if (pkey != NULL)
 {
-if (pkey->type == EVP_PKEY_RSA && pkey->pkey.rsa != NULL
+if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA && EVP_PKEY_get0_RSA(pkey) 
!= NULL
 && pkey->pkey.rsa->n != NULL)
 {
 openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA",
  BN_num_bits(pkey->pkey.rsa->n));
 }
-else if (pkey->type == EVP_PKEY_DSA && pkey->pkey.dsa != NULL
+else if (EVP_PKEY_id(pkey) == EVP_PKEY_DSA && 
EVP_PKEY_get0_DSA(pkey) != NULL
  && pkey->pkey.dsa->p != NULL)
 {
 openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 4/7] OpenSSL: don't use direct access to the internal of DSA

[Emmanuel Deloget]

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including DSA. We have to use the defined
functions to do so.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <log...@free.fr>
---
 configure.ac |  1 +
 src/openvpn/openssl_compat.h | 28 
 src/openvpn/ssl_openssl.c| 13 +
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index e4c053c8..d2dc1ffd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -932,6 +932,7 @@ if test "${enable_crypto}" = "yes" -a 
"${with_crypto_library}" = "openssl"; then
RSA_set_flags \
RSA_get0_key \
RSA_set0_key \
+   DSA_get0_pqg \
RSA_meth_new \
RSA_meth_free \
RSA_meth_set_pub_enc \
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index 29cd13a4..fdfc4a27 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -260,6 +260,34 @@ RSA_set0_key(RSA *rsa, BIGNUM *n, BIGNUM *e, BIGNUM *d)
 }
 #endif
 
+#if !defined(HAVE_DSA_GET0_PQG)
+/**
+ * Get the DSA parameters
+ *
+ * @param dsa The DSA object
+ * @param p   The @c p parameter
+ * @param q   The @c q parameter
+ * @param g   The @c g parameter
+ */
+static inline void
+DSA_get0_pqg(const DSA *dsa, const BIGNUM **p,
+ const BIGNUM **q, const BIGNUM **g)
+{
+if (p != NULL)
+{
+*p = dsa ? dsa->p : NULL;
+}
+if (q != NULL)
+{
+*q = dsa ? dsa->q : NULL;
+}
+if (g != NULL)
+{
+*g = dsa ? dsa->g : NULL;
+}
+}
+#endif
+
 #if !defined(HAVE_RSA_METH_NEW)
 /**
  * Allocate a new RSA method object
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 48479c0d..242ab397 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1691,11 +1691,16 @@ print_details(struct key_state_ssl *ks_ssl, const char 
*prefix)
  BN_num_bits(n));
 }
 }
-else if (EVP_PKEY_id(pkey) == EVP_PKEY_DSA && 
EVP_PKEY_get0_DSA(pkey) != NULL
- && pkey->pkey.dsa->p != NULL)
+else if (EVP_PKEY_id(pkey) == EVP_PKEY_DSA && 
EVP_PKEY_get0_DSA(pkey) != NULL)
 {
-openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
- BN_num_bits(pkey->pkey.dsa->p));
+DSA *dsa = EVP_PKEY_get0_DSA(pkey);
+const BIGNUM *p = NULL;
+DSA_get0_pqg(dsa, , NULL, NULL);
+if (p != NULL)
+{
+openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
+ BN_num_bits(p));
+}
 }
 EVP_PKEY_free(pkey);
 }
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenSSL 1.1 patch set - status?

[Emmanuel Deloget]

Hi Gert,


On Thu, May 18, 2017 at 10:49 PM, Gert Doering <g...@greenie.muc.de> wrote:
>
> Hi Emmanuel,
>
> On Mon, Mar 27, 2017 at 05:49:48PM +0200, Emmanuel Deloget wrote:
> > I'll post my new patches as soon as I get over every issues
> > that have been talked on the ML (is that even a valid
> > sentence?)
>
> I'm wondering where this got stuck - are you waiting for us to move
> forward (like, missing review of parts of the patch set), or are we
> waiting for you, and you've been busy?

Problem is that I'm working in a more-than-full-time manner on
way-too-many-other subjects :)

> We didn't really follow up on this from our end since the CVEs and
> 2.4.2 got in the way - but I think now would be a good time to move
> ahead with this...

I have a git tree out there that I have not fully tested yet. It
compiles OK with OpenSSL 0.9.8, 1.0.0, 1.0.1, 1.0.2 and 1.1.0 but I
haven't checked the behavior.

The main difference with the previous version of the patch is the way
the certificate purpose is checked.

A) we do a fairly full check of the purpose using
X509_check_purpose(). This check is harder that the previous version

B) if that fails, we check for the certificate purpose using a lighter
method which is strictly equivalent to what was done before (it uses
X509_get_ext_d2i() to fetch the certificate type from within the
certificate).

The branch is available for viewing on github at
https://github.com/emmanuel-deloget/openvpn/tree/openssl-1.1-v6.

The followup emails contains the 7 patches which are needed to finish the work.

BR,

-- Emmanuel Deloget

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [RFC PATCH v1 05/15] OpenSSL: don't use direct access to the internal of X509

[Emmanuel Deloget]

Hi,

I'm not sure why but it seems this mail (that I send yesterday) never found
its way to the ML. So I re-send it.

Sorry for the inconvenience.

BR,

-- Emmanuel Deloget

On Mon, Mar 27, 2017 at 5:49 PM, Emmanuel Deloget <log...@free.fr> wrote:

> Hi everyone,
>
> I got some time to try to fix all that stuff.
>
> First,
>
> On Sat, Mar 4, 2017 at 11:38 PM, David Sommerseth <openvpn@sf.lists.
> topphemmelig.net> wrote:
>
>> On 04/03/17 16:13, Steffan Karger wrote:
>> > As a last resort, we could consider keeping the old code inside #if
>> > OSSL_VER < 1.1.0 in release/2.4, but that might just create more
>> > confusion...
>>
>
>
> ​I think I found a solution -- see later in the email.​
>
> I had a closer look to the corresponding code to see what are the
> differences between the old check and the new one -- and indeed,
> the discrepancies might only happen if the tested certificate is
> kind-of-weird. For exemple, a client certificate:
>
> * has the client bit set
> * has no key, or has a key but the key usage is neither for
>   signature nor for key agreement
> * is not used to auth a client
>
> ​If a user creates a Frankenstein certificate that look like this,
> it might be a good idea to warn him that such certificate is very
> likely to be refused in the near future (one can have a similar
> reasonning about server certificates).
>
>
>
>>
>> Just a very quick thought here ... I do dislike different behaviours
>> depending on which OpenSSL version being used.  But given that this
>> feature is already deprecated and even removed in OpenSSL-1.1, I think
>> that gives us some options.
>>
>> I agree with Gert that breaking --ns-cert-type isn't good at all.
>> However, consider when people upgrade from OpenSSL v1.0 to v1.1 - that
>> is most commonly when there is major distro update.  It is not something
>> which happens "mid-term", as OpenSSL is quite commonly used by lots of
>> base system packages these days.
>>
>> Regardless of OpenSSL version, I agree to loudly deprecate
>> --ns-cert-type, through documentation, --help and log files.
>>
>> But I think we need to carry the existing behaviour for --ns-cert-type
>> when built against OpenSSL v1.0.  And we can solve through some
>> compatibility wrapper when built against OpenSSL v1.1 - with even louder
>> warnings in the logs that this may break apart.
>>
>
>
> ​Fortunately, I think I found a solution that would help
> when using OpenSSL 1.1. Idealy, this should be a call
> to X509_check_purpose(.., X509_PURPOSE_SSL_*_WEAK, ...)​
> but I don't think the OpenSSL developers will accept such
> a change :)
>
> It might be possible to use the X509_get_ext_d2i() call
> like this:
>
> void *ns = X509_get_ext_d2i(x, NID_netscape_cert_type,
> NULL, NULL);
>
> ​In this case, the ​obtained ns object contains the data
> we're interested in:
>
> * if it exists, then EXFLAG_NSCERT is set
> * if not null, ns is of type ASN1_BIT_STRING, and this
>   type is not opaque (yep, sir).
>
> ​So the code would look like:
>
> ​ASN1_BIT_STRING *ns;
> result_t result = FAILURE;
> ns = X509_get_ext_d2i(x, NID_netscape_cert_type, NULL, NULL);
> if (ns)
> {
> if (ns->length > 0)
> {
> int flags = ns->data[0];
> if (flags & NS_SSL_CLIENT)
> {
> result = SUCCESS;
> }
> }
> ASN1_BIT_STRING_free(ns);
> }
>
>
> ​For the record, it's the code ​that is used within function
> x509v3_cache_extensions() which builds the X509 flags we
> are using so I'm failry confident it's the right thing to
> do (but it's a bit convoluted and I don't like it much).
>
> ​Good news: the same code should work with nearly all the
> previous versions of OpenSSL.
>
>
>
>> ​
>>
>>
>> --
>> kind regards,
>>
>> David Sommerseth
>> OpenVPN Technologies, Inc
>>
>
>
> I'll post my new patches as soon as I get over every issues
> that have been talked on the ML (is that even a valid
> sentence?)
>
> ​Best regards,
>
> -- Emmanuel Deloget​
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] building HEAD + openssl 1.1 api fails @ "crypto.c:823:32: error: invalid application of ???sizeof??? to incomplete type ???cipher_ctx_t"

[Emmanuel Deloget]

Hi,


On Mon, Mar 27, 2017 at 8:35 PM, Gert Doering <g...@greenie.muc.de> wrote:

> Hi,
>
> On Mon, Mar 27, 2017 at 11:25:50AM -0700, PGNet Dev wrote:
> > noting
> >
> >   openvpn fails to build with openssl 1.1
> >   https://community.openvpn.net/openvpn/ticket/759
>
> Guess why that ticket is still open?
>
> >   [Openvpn-devel] [RFC PATCH v1 00/15] Add support for OpenSSL 1.1.x
> >   https://www.mail-archive.com/openvpn-devel@lists.
> sourceforge.net/msg14075.html
>
> ... because that is a patch *series*, and only half of it has been
> merged yet.
>
> So it is perfectly known that it will not build with 1.1 yet.
>


​I should be able to push a new version of the remaining patches in the
foreseeable future (let's say today or tomorrow, because I will be
unavailable at the end of this week).

I found a solution to overcome the big X509_check_purpose() issue, so now
I'm able to propose a solution that does not change the behavior of OpenVPN.

BR,

-- Emmanuel Deloget
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [RFC PATCH v1 05/15] OpenSSL: don't use direct access to the internal of X509

[Emmanuel Deloget]

Hi everyone,

I got some time to try to fix all that stuff.

First,

On Sat, Mar 4, 2017 at 11:38 PM, David Sommerseth <
open...@sf.lists.topphemmelig.net> wrote:

> On 04/03/17 16:13, Steffan Karger wrote:
> > As a last resort, we could consider keeping the old code inside #if
> > OSSL_VER < 1.1.0 in release/2.4, but that might just create more
> > confusion...
>


​I think I found a solution -- see later in the email.​

I had a closer look to the corresponding code to see what are the
differences between the old check and the new one -- and indeed,
the discrepancies might only happen if the tested certificate is
kind-of-weird. For exemple, a client certificate:

* has the client bit set
* has no key, or has a key but the key usage is neither for
  signature nor for key agreement
* is not used to auth a client

​If a user creates a Frankenstein certificate that look like this,
it might be a good idea to warn him that such certificate is very
likely to be refused in the near future (one can have a similar
reasonning about server certificates).



>
> Just a very quick thought here ... I do dislike different behaviours
> depending on which OpenSSL version being used.  But given that this
> feature is already deprecated and even removed in OpenSSL-1.1, I think
> that gives us some options.
>
> I agree with Gert that breaking --ns-cert-type isn't good at all.
> However, consider when people upgrade from OpenSSL v1.0 to v1.1 - that
> is most commonly when there is major distro update.  It is not something
> which happens "mid-term", as OpenSSL is quite commonly used by lots of
> base system packages these days.
>
> Regardless of OpenSSL version, I agree to loudly deprecate
> --ns-cert-type, through documentation, --help and log files.
>
> But I think we need to carry the existing behaviour for --ns-cert-type
> when built against OpenSSL v1.0.  And we can solve through some
> compatibility wrapper when built against OpenSSL v1.1 - with even louder
> warnings in the logs that this may break apart.
>


​Fortunately, I think I found a solution that would help
when using OpenSSL 1.1. Idealy, this should be a call
to X509_check_purpose(.., X509_PURPOSE_SSL_*_WEAK, ...)​
but I don't think the OpenSSL developers will accept such
a change :)

It might be possible to use the X509_get_ext_d2i() call
like this:

void *ns = X509_get_ext_d2i(x, NID_netscape_cert_type,
NULL, NULL);

​In this case, the ​obtained ns object contains the data
we're interested in:

* if it exists, then EXFLAG_NSCERT is set
* if not null, ns is of type ASN1_BIT_STRING, and this
  type is not opaque (yep, sir).

​So the code would look like:

​ASN1_BIT_STRING *ns;
result_t result = FAILURE;
ns = X509_get_ext_d2i(x, NID_netscape_cert_type, NULL, NULL);
if (ns)
{
if (ns->length > 0)
{
int flags = ns->data[0];
if (flags & NS_SSL_CLIENT)
{
result = SUCCESS;
}
}
ASN1_BIT_STRING_free(ns);
}


​For the record, it's the code ​that is used within function
x509v3_cache_extensions() which builds the X509 flags we
are using so I'm failry confident it's the right thing to
do (but it's a bit convoluted and I don't like it much).

​Good news: the same code should work with nearly all the
previous versions of OpenSSL.



> ​
>
>
> --
> kind regards,
>
> David Sommerseth
> OpenVPN Technologies, Inc
>


I'll post my new patches as soon as I get over every issues
that have been talked on the ML (is that even a valid
sentence?)

​Best regards,

-- Emmanuel Deloget​
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH applied] Re: OpenSSL: don't use direct access to the internal of RSA_METHOD

[Emmanuel Deloget]

Hi,

On Sun, Mar 5, 2017 at 11:29 AM, Steffan Karger <stef...@karger.me> wrote:
>
> On 05-03-17 10:53, Gert Doering wrote:
>> Small side note: I assume that RSA_meth_new() can fail and return NULL
>> in OpenSSL 1.1?  Because for 1.0, the "check_malloc_return(rsa_meth)" call
>> isn't necessary, as ALLOC_OBJ_CLEAR() would call ALLOC_OBJ() and that
>> already checks...  (mentioning this here in case someone wonders and goes
>> to the list archives).
>
> For the archives: yes, RSA_meth_new() indeed returns NULL if it's
> internal malloc() call fails.

Yes, indeed. And that's the reason why I have a check_malloc_return()
here. I'm perfectly conscious that for OpenSSL < 1.1 we're checking
the pointer twice but on the other hand I would have missed the check
with OpenSSL 1.1. A solution would have been to use a direct
malloc()/calloc() call instead of ALLOC_OBJ_CLEAR() in the
compatibility code, but that would have looked weird. Another solution
would have been to encapsulate RSA_meth_new() but I don't think that
would have been a good idea (yet, I might be wrong on that one). So I
did this choice -- I don't like it much either but I cannot think of a
better solution.

> -Steffan

Best regards,

-- Emmanuel Deloget

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [RFC PATCH v1 05/15] OpenSSL: don't use direct access to the internal of X509

[Emmanuel Deloget]

Hello,

On Sat, Mar 4, 2017 at 4:13 PM, Steffan Karger <stef...@karger.me> wrote:
> Hi,
>
> On 02-03-17 22:26, Gert Doering wrote:
>> On Thu, Mar 02, 2017 at 09:36:32PM +0100, Steffan Karger wrote:
>>> So, what I propose instead is:
>>>  * remove all the nsCertType code (except the option in add_option())
>>>  * update the help strings and man page to indicate that --ns-cert-type
>>> is no longer supported and --remote-cert-tls should be used instead
>>>  * in add_option(), if the option is enabled in a config file, act as if
>>> --remote-cert-tls was specified correspondingly, and print a clear
>>> warning that --ns-cert-type is no longer supported and stricter checks
>>> are enabled instead.
>>
>> Mmmmh.  Is there a way to get the old behaviour with OpenSSL 1.1?
>>
>> We decided that we do want 1.1 compatibility in release/2.4, but what
>> you propose might break people's working config when upgrading from 2.4.1
>> to 2.4.2 - bad enough if we make mistakes, but if there is an alternative
>> to consciously changing cert validation behaviour in the middle of a
>> release train, we should look again...
>
> So I looked again, and there really seems to be no way to get the old
> behaviour with OpenSSL 1.1.  However, the exact behaviour of
> X509_check_purpose() is not as strict as I initially thought.  The
> current patch basically adds the following checks:
>  * if the cert has key usage set, it must be correct
>  * if the cert has extended key usage set, it must be correct
>  * if the cert has the CA flag set, it must be done correctly

When I coded this part, I searched for a long time before resorting to
using this function. It's true that the library does more check, yet I
didn't find any way to not do them, as the necessary members are not
available through getters.

However, my understanding is that OpenSSL does a full, correct check,
and that certificates that does not pass this check are probably not
correct - or at least, they hide some weird issues that could make
them a bit dangerous to use. Of course, I might be wrong. The
certificates I tested were valid w.r.t. this new code and I'm now sure
how I can generate certificates that do not pass.

> These are fairly low-risk.  I'd expect quite some issues if we would
> reject certs *without* (extended) key usage set, but if (e)ku is set,
> this will most likely be done correctly.  Or in other words, we might
> reject weird certificates, but not proper certificates.

Yet, rejecting certificates might pose problems in the organisations
that use them, and they should be warned by (at least) and
intermediate release that their certificates going to be rejected. So,
after a bit of thinking I would do :

1/ check the certificate with X209_check_purpose()
2/ on failure, for OpenSSL < 1.1.0, warn the user that the
certificates are probably not fully correct, and do the old check. If
it pass, continue

> All in all, I think the original patch (after fixing const correctness)
> is fine.
>
> As a last resort, we could consider keeping the old code inside #if
> OSSL_VER < 1.1.0 in release/2.4, but that might just create more
> confusion...

Unfortunately, I am overbooked right now and I'm not sure I'll be able
to do this fast (say, in less than 2 weeks). I'd be grateful of
someone else does it.

> -Steffan

Best regards,

-- Emmanuel Deloget

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v3 00/15] Add support for OpenSSL 1.1.x

[Emmanuel Deloget]

This is v3 of the remaining patches for the "Add support for OpenSSL 
1.1.x" series. This series is partial: only the modified patches are 
sent to the ML -- the other have not changed. The stats are a bit off 
so I don't include them in this mail.

They have been generated after a rebase from the master tree. Individual
commits can be viewed at

  https://github.com/emmanuel-deloget/openvpn/commits/openssl-1.1-v3

(This time, the branch name is correct :))

Changes v2 --> v3: 

* RSA_METHOD (04/15): rsa_meth->name is now a dup of the name parameter; 
  it's freed in RSA_meth_free(). 

* RSA (07/15): calling RSA_set_method() in rsa_finish() is both a Bad 
  Idea and not required so it has been removed.
  
Changes v1 --> v2:

* EVP_PKEY (06/15): add missing function EVP_PKEY_id() for 0.9.8.

* replace patch 15/15 with a new patch to use EVP_CipherInit_ex() 
  instead of EVP_CipherInit() when a full init is not needed.


Emmanuel Deloget (15):
  [commited] OpenSSL: don't use direct access to the internal of SSL_CTX
  [commited] OpenSSL: don't use direct access to the internal of X509_STORE
  [commited] OpenSSL: don't use direct access to the internal of X509_OBJECT
  OpenSSL: don't use direct access to the internal of RSA_METHOD
  OpenSSL: don't use direct access to the internal of X509
  OpenSSL: don't use direct access to the internal of EVP_PKEY
  OpenSSL: don't use direct access to the internal of RSA
  OpenSSL: don't use direct access to the internal of DSA
  [commited] OpenSSL: don't use direct access to the internal of X509_STORE_CTX
  OpenSSL: don't use direct access to the internal of EVP_MD_CTX
  OpenSSL: don't use direct access to the internal of EVP_CIPHER_CTX
  OpenSSL: don't use direct access to the internal of HMAC_CTX
  OpenSSL: SSLeay symbols are no longer available in OpenSSL 1.1
  OpenSSL: constify getbio() parameters
  OpenSSL: use EVP_CipherInit_ex() instead of EVP_CipherInit()

-- 
2.7.4


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v3 04/15] OpenSSL: don't use direct access to the internal of RSA_METHOD

[Emmanuel Deloget]

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including RSA_METHOD. We have to use the defined
functions to do so.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <log...@free.fr>
---
 configure.ac |   9 ++
 src/openvpn/openssl_compat.h | 190 +++
 src/openvpn/ssl_openssl.c|  22 ++---
 3 files changed, 210 insertions(+), 11 deletions(-)

diff --git a/configure.ac b/configure.ac
index 
0c55d78327597a0690fa9625e06adb8b055852b1..2406ad8d6bf10f202d3fc1e9b971bc8ec135bd4f
 100644
--- a/configure.ac
+++ b/configure.ac
@@ -905,6 +905,15 @@ if test "${enable_crypto}" = "yes" -a 
"${with_crypto_library}" = "openssl"; then
X509_STORE_get0_objects \
X509_OBJECT_free \
X509_OBJECT_get_type \
+   RSA_meth_new \
+   RSA_meth_free \
+   RSA_meth_set_pub_enc \
+   RSA_meth_set_pub_dec \
+   RSA_meth_set_priv_enc \
+   RSA_meth_set_priv_dec \
+   RSA_meth_set_init \
+   RSA_meth_set_finish \
+   RSA_meth_set0_app_data \
]
)
 
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index 
458a6adbe2b3fcd5ea63dcea6596cc24315d463c..e98e8dffc5773d73684398ae28215d4fdccac3c4
 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -41,6 +41,8 @@
 #include "config-msvc.h"
 #endif
 
+#include "buffer.h"
+
 #include 
 #include 
 
@@ -117,4 +119,192 @@ X509_OBJECT_get_type(const X509_OBJECT *obj)
 }
 #endif
 
+#if !defined(HAVE_RSA_METH_NEW)
+/**
+ * Allocate a new RSA method object
+ *
+ * @param name   The object name
+ * @param flags  Configuration flags
+ * @return   A new RSA method object
+ */
+static inline RSA_METHOD *
+RSA_meth_new(const char *name, int flags)
+{
+RSA_METHOD *rsa_meth = NULL;
+ALLOC_OBJ_CLEAR(rsa_meth, RSA_METHOD);
+rsa_meth->name = string_alloc(name, NULL);
+rsa_meth->flags = flags;
+return rsa_meth;
+}
+#endif
+
+#if !defined(HAVE_RSA_METH_FREE)
+/**
+ * Free an existing RSA_METHOD object
+ *
+ * @param meth   The RSA_METHOD object
+ */
+static inline void
+RSA_meth_free(RSA_METHOD *meth)
+{
+if (meth)
+{
+free(meth->name);
+free(meth);
+}
+}
+#endif
+
+#if !defined(HAVE_RSA_METH_SET_PUB_ENC)
+/**
+ * Set the public encoding function of an RSA_METHOD object
+ *
+ * @param meth   The RSA_METHOD object
+ * @param pub_encthe public encoding function
+ * @return   1 on success, 0 on error
+ */
+static inline int
+RSA_meth_set_pub_enc(RSA_METHOD *meth,
+ int (*pub_enc) (int flen, const unsigned char *from,
+ unsigned char *to, RSA *rsa,
+ int padding))
+{
+if (meth)
+{
+meth->rsa_pub_enc = pub_enc;
+return 1;
+}
+return 0;
+}
+#endif
+
+#if !defined(HAVE_RSA_METH_SET_PUB_DEC)
+/**
+ * Set the public decoding function of an RSA_METHOD object
+ *
+ * @param meth   The RSA_METHOD object
+ * @param pub_decthe public decoding function
+ * @return   1 on success, 0 on error
+ */
+static inline int
+RSA_meth_set_pub_dec(RSA_METHOD *meth,
+ int (*pub_dec) (int flen, const unsigned char *from,
+ unsigned char *to, RSA *rsa,
+ int padding))
+{
+if (meth)
+{
+meth->rsa_pub_dec = pub_dec;
+return 1;
+}
+return 0;
+}
+#endif
+
+#if !defined(HAVE_RSA_METH_SET_PRIV_ENC)
+/**
+ * Set the private encoding function of an RSA_METHOD object
+ *
+ * @param meth   The RSA_METHOD object
+ * @param priv_enc   the private encoding function
+ * @return   1 on success, 0 on error
+ */
+static inline int
+RSA_meth_set_priv_enc(RSA_METHOD *meth,
+  int (*priv_enc) (int flen, const unsigned char *from,
+   unsigned char *to, RSA *rsa,
+   int padding))
+{
+if (meth)
+{
+meth->rsa_priv_enc = priv_enc;
+return 1;
+}
+return 0;
+}
+#endif
+
+#if !defined(HAVE_RSA_METH_SET_PRIV_DEC)
+/**
+ * Set the private decoding function of an RSA_METHOD object
+ *
+ * @param meth   The RSA_METHOD object
+ * @param priv_dec   the private decoding function
+ * @return   1 on success, 0 on error
+ */
+static inline int
+RSA_meth_set_priv_dec(RSA_METHO

Re: [Openvpn-devel] [RFC PATCH v1 04/15] OpenSSL: don't use direct access to the internal of RSA_METHOD

[Emmanuel Deloget]

Hi Steffan,

On Wed, Feb 22, 2017 at 11:13 PM, Steffan Karger <stef...@karger.me> wrote:
> Hi,
>
> On 17-02-17 23:00, log...@free.fr wrote:
>> From: Emmanuel Deloget <log...@free.fr>
>>
>> OpenSSL 1.1 does not allow us to directly access the internal of
>> any data type, including RSA_METHOD. We have to use the defined
>> functions to do so.
>>
>> Compatibility with OpenSSL 1.0 is kept by defining the corresponding
>> functions when they are not found in the library.
>>
>> Signed-off-by: Emmanuel Deloget <log...@free.fr>
>> ---
>>  configure.ac |   9 +++
>>  src/openvpn/openssl_compat.h | 186 
>> +++
>>  src/openvpn/ssl_openssl.c|  22 ++---
>>  3 files changed, 206 insertions(+), 11 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 
>> 789ad08fbaa3b3fc4c95d2b7a22332c0a93aeab4..6f31609d0aeedd2c7841d271ecadd1aa6f3b11da
>>  100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -905,6 +905,15 @@ if test "${enable_crypto}" = "yes" -a 
>> "${with_crypto_library}" = "openssl"; then
>>   X509_STORE_get0_objects \
>>   X509_OBJECT_free \
>>   X509_OBJECT_get_type \
>> + RSA_meth_new \
>> + RSA_meth_free \
>> + RSA_meth_set_pub_enc \
>> + RSA_meth_set_pub_dec \
>> + RSA_meth_set_priv_enc \
>> + RSA_meth_set_priv_dec \
>> + RSA_meth_set_init \
>> + RSA_meth_set_finish \
>> + RSA_meth_set0_app_data \
>>   ],
>>   ,
>>   []
>> diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
>> index 
>> 458a6adbe2b3fcd5ea63dcea6596cc24315d463c..b1748754f821f472cf9ed7083ade918336c9b075
>>  100644
>> --- a/src/openvpn/openssl_compat.h
>> +++ b/src/openvpn/openssl_compat.h
>> @@ -41,6 +41,8 @@
>>  #include "config-msvc.h"
>>  #endif
>>
>> +#include "buffer.h"
>> +
>>  #include 
>>  #include 
>>
>> @@ -117,4 +119,188 @@ X509_OBJECT_get_type(const X509_OBJECT *obj)
>>  }
>>  #endif
>>
>> +#if !defined(HAVE_RSA_METH_NEW)
>> +/**
>> + * Allocate a new RSA method object
>> + *
>> + * @param name   The object name
>> + * @param flags  Configuration flags
>> + * @return   A new RSA method object
>> + */
>> +static inline RSA_METHOD *
>> +RSA_meth_new(const char *name, int flags)
>> +{
>> +RSA_METHOD *rsa_meth = NULL;
>> +ALLOC_OBJ_CLEAR(rsa_meth, RSA_METHOD);
>> +rsa_meth->name = name;
>> +rsa_meth->flags = flags;
>> +return rsa_meth;
>> +}
>> +#endif
>> +
>> +#if !defined(HAVE_RSA_METH_FREE)
>> +/**
>> + * Free an existing RSA_METHOD object
>> + *
>> + * @param meth   The RSA_METHOD object
>> + */
>> +static inline void
>> +RSA_meth_free(RSA_METHOD *meth)
>> +{
>> +free(meth);
>> +}
>> +#endif
>
> I think it would be nicer to more closely mimic the 1.1 behaviour in
> RSA_meth_{new,free}(), and copy the name string in new() and free it
> again in free().  That could prevent a future use-after-free that would
> occur for pre-1.1.0, but not 1.1.0+:

I failed to see that when I implemented my solution. I'll give a look
as soon as possible.

>   char *mystring = calloc(50, 1);
>   RSA_METHOD *meth = RSA_meth_new(mystring, 0);
>   free(mystring);
>
>   meth.smoke();
>^^ might cause problems
>
> (Hint: use string_alloc(x, NULL).)
>
>> +
>> +#if !defined(HAVE_RSA_METH_SET_PUB_ENC)
>> +/**
>> + * Set the public encoding function of an RSA_METHOD object
>> + *
>> + * @param meth   The RSA_METHOD object
>> + * @param pub_encthe public encoding function
>> + * @return   1 on success, 0 on error
>> + */
>> +static inline int
>> +RSA_meth_set_pub_enc(RSA_METHOD *meth,
>> + int (*pub_enc) (int flen, const unsigned char *from,
>> + unsigned char *to, RSA *rsa,
>> + int padding))
>> +{
>> +if (meth)
>> +{
>> +meth->rsa_pub_enc = pub_enc;
>> +retur

Re: [Openvpn-devel] [RFC PATCH v1 01/15] OpenSSL: don't use direct access to the internal of SSL_CTX

[Emmanuel Deloget]

Hello,

On Thu, Feb 23, 2017 at 10:23 AM, Gert Doering <g...@greenie.muc.de> wrote:
> Hi,
>
> On Thu, Feb 23, 2017 at 09:03:47AM +0100, Gert Doering wrote:
>> This patch brings two problems outside the "OpenSSL functionality"
>> part.
>>
>>  - openssl_compat.h is not included in the built tarballs, so mingw builds
>>fail (and "builds for anyone building from tarballs" would break) ->
>>findable by running "make distcheck"
>
> This has been fixed & pushed (so we can build windows snapshots again).
>
>>  - configure.ac does something to CentOS 6 / RHEL 6 which makes configure
>>explode:
>>
>> ...
>> checking for linux/if_tun.h... yes
>> checking tap-windows.h usability... no
>> checking tap-windows.h presence... no
>> checking for tap-windows.h... no
>> checking whether TUNSETPERSIST is declared... yes
>> checking for setcon in -lselinux... yes
>> checking for pam_start in -lpam... yes
>> checking for PKCS11_HELPER... no
>> ./configure: line 21440: syntax error near unexpected token `fi'
>> ./configure: line 21440: `fi'
>
> This still needs investigation.

I'm taking the time to install a CentOS 6 on a VM. I'll test this asap.

Best regards,

-- Emmanuel Deloget

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [RFC PATCH v1 00/15] Add support for OpenSSL 1.1.x

[Emmanuel Deloget]

On Mon, Feb 20, 2017 at 2:53 PM, Emmanuel Deloget <log...@free.fr> wrote:
> Hi again,
>
> On Mon, Feb 20, 2017 at 2:33 PM, Emmanuel Deloget <log...@free.fr> wrote:
>> Hi Christian,
>>
>> On Mon, Feb 20, 2017 at 1:29 PM, Christian Hesse <l...@eworm.de> wrote:
>>> That matches my findings. Built against openssl 1.1.0e (Arch Linux package
>>> openssl 1.1.0.e-1 [0]) the build itself succeeds, but 'make check' reports
>>> lots of cipher failures.
>>>
>>> Are your patches available from a public git repository?
>>
>> I will make my patches available on github ASAP.
>
> I did as fast as I could, here they are:
>
> https://github.com/emmanuel-deloget/openvpn/commits/openvpn-1.1

BTW, sorry for the branch name. I believe my fingers got stuck to a
limited number of characters. This should have been openssl-1.1 but
it's not too late to change it :)

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [RFC PATCH v2 06/15] OpenSSL: don't use direct access to the internal of EVP_PKEY

[Emmanuel Deloget]

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including EVP_PKEY. We have to use the defined
functions to do so.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <log...@free.fr>
---
 configure.ac |  3 +++
 src/openvpn/openssl_compat.h | 42 ++
 src/openvpn/ssl_openssl.c|  6 +++---
 3 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index c41db3e..8d99eb3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -906,6 +906,9 @@ if test "${enable_crypto}" = "yes" -a 
"${with_crypto_library}" = "openssl"; then
X509_STORE_get0_objects \
X509_OBJECT_free \
X509_OBJECT_get_type \
+   EVP_PKEY_id \
+   EVP_PKEY_get0_RSA \
+   EVP_PKEY_get0_DSA \
RSA_meth_new \
RSA_meth_free \
RSA_meth_set_pub_enc \
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index 6a89b91..72ed7ac 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -134,6 +134,48 @@ X509_OBJECT_get_type(const X509_OBJECT *obj)
 }
 #endif
 
+#if !defined(HAVE_EVP_PKEY_GET0_RSA)
+/**
+ * Get the RSA object of a public key
+ *
+ * @param pkeyPublic key object
+ * @returnThe underlying RSA object
+ */
+static inline RSA *
+EVP_PKEY_get0_RSA(EVP_PKEY *pkey)
+{
+return pkey ? pkey->pkey.rsa : NULL;
+}
+#endif
+
+#if !defined(HAVE_EVP_PKEY_ID)
+/**
+ * Get the PKEY type
+ *
+ * @param pkeyPublic key object
+ * @returnThe key type
+ */
+static inline int
+EVP_PKEY_id(const EVP_PKEY *pkey)
+{
+return pkey ? pkey->type : EVP_PKEY_NONE;
+}
+#endif
+
+#if !defined(HAVE_EVP_PKEY_GET0_DSA)
+/**
+ * Get the DSA object of a public key
+ *
+ * @param pkeyPublic key object
+ * @returnThe underlying DSA object
+ */
+static inline DSA *
+EVP_PKEY_get0_DSA(EVP_PKEY *pkey)
+{
+return pkey ? pkey->pkey.dsa : NULL;
+}
+#endif
+
 #if !defined(HAVE_RSA_METH_NEW)
 /**
  * Allocate a new RSA method object
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index b683961..dbeb868 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1075,7 +1075,7 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
 /* get the public key */
 EVP_PKEY *pkey = X509_get0_pubkey(cert);
 ASSERT(pkey); /* NULL before SSL_CTX_use_certificate() is called */
-pub_rsa = cert->cert_info->key->pkey->pkey.rsa;
+pub_rsa = EVP_PKEY_get0_RSA(pkey);
 
 /* initialize RSA object */
 rsa->n = BN_dup(pub_rsa->n);
@@ -1680,13 +1680,13 @@ print_details(struct key_state_ssl *ks_ssl, const char 
*prefix)
 EVP_PKEY *pkey = X509_get_pubkey(cert);
 if (pkey != NULL)
 {
-if (pkey->type == EVP_PKEY_RSA && pkey->pkey.rsa != NULL
+if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA && EVP_PKEY_get0_RSA(pkey) 
!= NULL
 && pkey->pkey.rsa->n != NULL)
 {
 openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA",
  BN_num_bits(pkey->pkey.rsa->n));
 }
-else if (pkey->type == EVP_PKEY_DSA && pkey->pkey.dsa != NULL
+else if (EVP_PKEY_id(pkey) == EVP_PKEY_DSA && 
EVP_PKEY_get0_DSA(pkey) != NULL
  && pkey->pkey.dsa->p != NULL)
 {
 openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
-- 
2.7.4


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [RFC PATCH v2 15/15] OpenSSL: use EVP_CipherInit_ex() instead of EVP_CipherInit()

[Emmanuel Deloget]

The behavior of EVP_CipherInit() changed in OpenSSL 1.1 -- instead
of clearing the context when the cipher parameter was !NULL, it now
clears the context unconditionnaly. As a result, subsequent calls
to the function with additional information now fails.

The bulk work is done by EVP_CipherInit_ex() which has been part of the
OpenSSL interface since the dawn of time (0.9.8 already has it). Thus,
the change allows us to get the old behavior back instead of relying
on dirty tricks.

Signed-off-by: Emmanuel Deloget <log...@free.fr>
---
 src/openvpn/crypto_openssl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 23de175..2bca88b 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -683,7 +683,7 @@ cipher_ctx_init(EVP_CIPHER_CTX *ctx, uint8_t *key, int 
key_len,
 crypto_msg(M_FATAL, "EVP set key size");
 }
 #endif
-if (!EVP_CipherInit(ctx, NULL, key, NULL, enc))
+if (!EVP_CipherInit_ex(ctx, NULL, NULL, key, NULL, enc))
 {
 crypto_msg(M_FATAL, "EVP cipher init #2");
 }
@@ -736,7 +736,7 @@ cipher_ctx_get_cipher_kt(const cipher_ctx_t *ctx)
 int
 cipher_ctx_reset(EVP_CIPHER_CTX *ctx, uint8_t *iv_buf)
 {
-return EVP_CipherInit(ctx, NULL, NULL, iv_buf, -1);
+return EVP_CipherInit_ex(ctx, NULL, NULL, NULL, iv_buf, -1);
 }
 
 int
-- 
2.7.4


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [RFC PATCH v2 00/15] Add support for OpenSSL 1.1.x

[Emmanuel Deloget]

This (limited) series replaces a few patches on the v1 series, namely: 

* "OpenSSL: don't use direct access to the internal of EVP_PKEY"
  This version replaces the previous version and adds function
  EVP_PKEY_id() which is present in 1.0.0 and later but not in
  0.9.8. 

* "OpenSSL: use EVP_CipherInit_ex() instead of EVP_CipherInit()"

This version has been compile-tested with the following versions:

* 0.9.8zh
* 1.0.0t
* 1.0.1u
* 1.0.2k
* 1.1.0-git

Each compilation test was followed by a connection test to an OpenVPN
server (v2.3). So far, everything seems to work. 

Emmanuel Deloget (15):
  OpenSSL: don't use direct access to the internal of SSL_CTX
  OpenSSL: don't use direct access to the internal of X509_STORE
  OpenSSL: don't use direct access to the internal of X509_OBJECT
  OpenSSL: don't use direct access to the internal of RSA_METHOD
  OpenSSL: don't use direct access to the internal of X509
  OpenSSL: don't use direct access to the internal of EVP_PKEY
  OpenSSL: don't use direct access to the internal of RSA
  OpenSSL: don't use direct access to the internal of DSA
  OpenSSL: don't use direct access to the internal of X509_STORE_CTX
  OpenSSL: don't use direct access to the internal of EVP_MD_CTX
  OpenSSL: don't use direct access to the internal of EVP_CIPHER_CTX
  OpenSSL: don't use direct access to the internal of HMAC_CTX
  OpenSSL: SSLeay symbols are no longer available in OpenSSL 1.1
  OpenSSL: constify getbio() parameters
  OpenSSL: use EVP_CipherInit_ex() instead of EVP_CipherInit()

 configure.ac |  38 +++
 src/openvpn/crypto.c |   8 +-
 src/openvpn/crypto_backend.h |  42 +++
 src/openvpn/crypto_mbedtls.c |  40 +++
 src/openvpn/crypto_openssl.c |  54 +++-
 src/openvpn/httpdigest.c |  78 ++---
 src/openvpn/misc.c   |  14 +-
 src/openvpn/ntlm.c   |  12 +-
 src/openvpn/openssl_compat.h | 623 +++
 src/openvpn/openvpn.h|   2 +-
 src/openvpn/push.c   |  11 +-
 src/openvpn/ssl.c|  38 +--
 src/openvpn/ssl_openssl.c|  94 +++---
 src/openvpn/ssl_verify_openssl.c |  55 ++--
 14 files changed, 963 insertions(+), 146 deletions(-)
 create mode 100644 src/openvpn/openssl_compat.h

-- 
2.7.4


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [RFC PATCH v1 00/15] Add support for OpenSSL 1.1.x

[Emmanuel Deloget]

Hi again,

On Mon, Feb 20, 2017 at 2:33 PM, Emmanuel Deloget <log...@free.fr> wrote:
> Hi Christian,
>
> On Mon, Feb 20, 2017 at 1:29 PM, Christian Hesse <l...@eworm.de> wrote:
>> That matches my findings. Built against openssl 1.1.0e (Arch Linux package
>> openssl 1.1.0.e-1 [0]) the build itself succeeds, but 'make check' reports
>> lots of cipher failures.
>>
>> Are your patches available from a public git repository?
>
> I will make my patches available on github ASAP.

I did as fast as I could, here they are:

https://github.com/emmanuel-deloget/openvpn/commits/openvpn-1.1

I post the PATCH V2 in a few minutes

-- Emmanuel Deloget

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [RFC PATCH v1 00/15] Add support for OpenSSL 1.1.x

[Emmanuel Deloget]

Hi,

On Mon, Feb 20, 2017 at 1:37 PM, Gert Doering <g...@greenie.muc.de> wrote:
>
> Interesting.  Anything useful in openvpn's logs?
>

Mon Feb 20 11:57:56 2017 us=371715 OpenSSL: error:0607B083:digital
envelope routines:EVP_CipherInit_ex:no cipher set
Mon Feb 20 11:57:56 2017 us=371746 EVP cipher init #2

I found the culprit: OpenSSL's EVP_CipherInit() changed way too much
for a 3 lines function. Prior to v1.1, the code did a check on cipher
parameter and cleared the EVP context only if cipher was not null. In
1.1, it clears the context unconditionnaly. Having to cope with
changes in the interface is not that fun, having to cope with behavior
changes is even worse :)

I'm producing an additional commit to work around that change (the
proposed change does not depend on the OpenSSL version).

>> I don't have much time to test with other OpenSSL versions but I guess
>> you have the infrastructure that will help.
>
> Well, *I* do not have specific "test across various OpenSSL versions"
> infrastructure, but compiling across our buildbot zoo gives us quite a
> bit of coverage...  and I assume Steffan has more coverage on SSL library
> versions.
>
> thanks for your work!
>
> gert

Well, thanks to everyone involved -- all of you have been really kind
with me (for now :))

Best regards,

-- Emmanuel Deloget

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [RFC PATCH v1 00/15] Add support for OpenSSL 1.1.x

[Emmanuel Deloget]

Hi Christian,

On Mon, Feb 20, 2017 at 1:29 PM, Christian Hesse <l...@eworm.de> wrote:
> That matches my findings. Built against openssl 1.1.0e (Arch Linux package
> openssl 1.1.0.e-1 [0]) the build itself succeeds, but 'make check' reports
> lots of cipher failures.
>
> Are your patches available from a public git repository?

I will make my patches available on github ASAP.

Best regards

-- Emmanuel Deloget

On Mon, Feb 20, 2017 at 1:29 PM, Christian Hesse <l...@eworm.de> wrote:
> Emmanuel Deloget <log...@free.fr> on Mon, 2017/02/20 12:45:
>> Hello,
>>
>> On Sun, Feb 19, 2017 at 6:49 PM, Gert Doering <g...@greenie.muc.de> wrote:
>> > Hi,
>> >
>> > On Sun, Feb 19, 2017 at 01:03:45PM +0100, Steffan Karger wrote:
>> >> Thank you very much.  You approach looks good to me, and quite closely
>> >> matches what I had in mind for when I would find the time to tackle
>> >> this.  (Which might have taken me a while, so really happy to see these
>> >> patches!)
>> > [..]
>> >> Also very good that this is split up into small and independently
>> >> reviewable patches.  I'll start review soon.
>> >
>> > While Steffan is our resident expert on nasty crypto libraries, I just
>> > want to echo the sentiment - having these "chunks" tackle one API function
>> > at a time, they are easily testable, and in case something explodes, it's
>> > much easier to bisect to find the problematic one.
>> >
>> > Now back to being a commit slave for Steffan's ACKs :-)  (I do not know
>> > the APIs well enough to properly comment on the changes, I can only run
>> > tests...)
>>
>> I resumed the work this morning. So far the results are :
>>
>> * 0.9.8zh --> EVP_PKEY_id() is not defined. I'm adding this to
>> openssl_compat.h and will provide a v2 patch with the change. Once
>> added, OpenVPN compiled successfully and was able to connect to my
>> /2.3 server.
>>
>> * 1.0.0t --> compile OK, connect OK
>>
>> * 1.0.1u --> compile OK, connect OK
>>
>> * 1.0.2.k --> compile OK, connect OK
>>
>> * 1.1.0-git --> compile OK, failure to connect. I'm currently
>> investigating this issue. I'll  provide a patch as soon as I fix this
>> (this is a bit ironic ; I may have forgotten something somewhere...).
>
> That matches my findings. Built against openssl 1.1.0e (Arch Linux package
> openssl 1.1.0.e-1 [0]) the build itself succeeds, but 'make check' reports
> lots of cipher failures.
>
> Are your patches available from a public git repository?
>
> [0] https://www.archlinux.org/packages/staging/x86_64/openssl/
> --
> main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
> "CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
> putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [RFC PATCH v1 00/15] Add support for OpenSSL 1.1.x

[Emmanuel Deloget]

Hello,

On Sun, Feb 19, 2017 at 6:49 PM, Gert Doering <g...@greenie.muc.de> wrote:
> Hi,
>
> On Sun, Feb 19, 2017 at 01:03:45PM +0100, Steffan Karger wrote:
>> Thank you very much.  You approach looks good to me, and quite closely
>> matches what I had in mind for when I would find the time to tackle
>> this.  (Which might have taken me a while, so really happy to see these
>> patches!)
> [..]
>> Also very good that this is split up into small and independently
>> reviewable patches.  I'll start review soon.
>
> While Steffan is our resident expert on nasty crypto libraries, I just
> want to echo the sentiment - having these "chunks" tackle one API function
> at a time, they are easily testable, and in case something explodes, it's
> much easier to bisect to find the problematic one.
>
> Now back to being a commit slave for Steffan's ACKs :-)  (I do not know
> the APIs well enough to properly comment on the changes, I can only run
> tests...)

I resumed the work this morning. So far the results are :

* 0.9.8zh --> EVP_PKEY_id() is not defined. I'm adding this to
openssl_compat.h and will provide a v2 patch with the change. Once
added, OpenVPN compiled successfully and was able to connect to my
/2.3 server.

* 1.0.0t --> compile OK, connect OK

* 1.0.1u --> compile OK, connect OK

* 1.0.2.k --> compile OK, connect OK

* 1.1.0-git --> compile OK, failure to connect. I'm currently
investigating this issue. I'll  provide a patch as soon as I fix this
(this is a bit ironic ; I may have forgotten something somewhere...).

I don't have much time to test with other OpenSSL versions but I guess
you have the infrastructure that will help.

> gert

Best regards,

-- Emmanuel Deloget

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [RFC PATCH v1 00/15] Add support for OpenSSL 1.1.x

[Emmanuel Deloget]

Hello,

On Sun, Feb 19, 2017 at 1:03 PM, Steffan Karger <stef...@karger.me> wrote:
>
> Hi Emmanuel,
>
> Thank you very much.  You approach looks good to me, and quite closely
> matches what I had in mind for when I would find the time to tackle
> this.  (Which might have taken me a while, so really happy to see these
> patches!)
>

Thanks Steffan,

> As discussed in other threads, we do want to support building on RHEL6,
> which is why we would prefer to be compatible with (patched) OpenSSL
> 0.9.8.  I haven't tested anything yet, but looking at the patches this
> might very well just work, or otherwise just needs some minor tweaking.

I haven't tested compilation with 0.9.8 but unless some massive
changes in the interface occured, this should not be a problem. I'd do
that at the beginning of next week.

> Also very good that this is split up into small and independently
> reviewable patches.  I'll start review soon.
>
> -Steffan

For the record, most of the patches deal with changing how the code
access to one selected OpenSSL type. I hope it will ease review -- in
the sense that people who are accustomed to the code might be able to
see if something is missing.

BR,

-- Emmanuel Deloget

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] build against openssl 1.1.0

[Emmanuel Deloget]

Hello,

On Fri, Feb 17, 2017 at 6:42 PM, Gert Doering <g...@greenie.muc.de> wrote:
> Hi,
>
> On Fri, Feb 17, 2017 at 06:37:04PM +0100, Emmanuel Deloget wrote:
>> I guess the answer to the riddle is: "how long will the 2.4 branch
>> live?". v2.3 shipped in May 2013. If we assume that v2.4 will be the
>> stable branch for two more years (I cannot find any roadmap, so this
>> is pure speculation) then it might make sense for 2.5 to at least
>> remove support for OpenSSL v0.9.8 (it would have been EoL'd for 3
>> years by then).
>
> We have *plans* to release 2.5 faster than "it takes another 3 years",
> but we said so when planning 2.4 as well.

That's good to know :) Is there any roadmap available to the general public?

> Since David pointed out already that RHEL5 is going to be EOLed soon,
> I do not thing 0.9.8 is an important target anymore.  Depending on the
> amount of #ifdef etc., it might make sense to drop 0.9.8 support in
> 2.4, but only add 1.1 support to master/2.5 - we're early in the 2.4
> cycle, which allows "somewhat larger" changes, but 500+ insertions
> sounds like a bit too intrusive.

I'm not targetting 2.4 -- my work is done on the current master. Adding
hundreds of lines to the current 2.4 for the purpose of supporting a library
which is not yet present on the user systems does not make much sense :)

BR,

-- Emmanuel Deloget

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] build against openssl 1.1.0

[Emmanuel Deloget]

Hello,

On Fri, Feb 17, 2017 at 5:41 PM, Gert Doering <g...@greenie.muc.de> wrote:
> Hi,
>
> On Fri, Feb 17, 2017 at 05:35:04PM +0100, Emmanuel Deloget wrote:
>> I understand that I'm the new guy in town, but can you allow me to
>> make the formal request to ditch OpenSSL 0.9.8, 1.0.0 and 1.0.1 and
>> require at least version 1.0.2?
>
> I'm not going to make a call on any of these versions, I just want
> to point out that we do need to (and *want* to) support older release
> of distributions that do not ship "most recent" OpenSSL versions yet.
>
> So we're somewhat caught in the middle between arch linux with 1.1.0
> and something like RHEL that ships seriously old OpenSSL (with patches).

My feeling is that RHEL6 and RHEL 7 are shipping v1.0.1 at least (both
updated the packages to 1.0.1e in March 2016). RHEL5 is still shipping
v0.9.8 (but then the installation of openvpn on RHEL 5 and Centos 5 is
fully manual as it seems there is no official packages for these
distrubutions). Of course, I might be wrong.

> This said, we need to regularily re-evaluate what the oldest distribution
> is that a given OpenVPN branch should support, and then we can drop support
> for older OpenSSL versions...

I guess the answer to the riddle is: "how long will the 2.4 branch
live?". v2.3 shipped in May 2013. If we assume that v2.4 will be the
stable branch for two more years (I cannot find any roadmap, so this
is pure speculation) then it might make sense for 2.5 to at least
remove support for OpenSSL v0.9.8 (it would have been EoL'd for 3
years by then). I must admit that the fact that I can build OpenVPN
against a security-focused library that haven't seen any evolution/bug
fix/security fix in one year makes me pretty shaky :)

>
> gert
>

BR,

-- Emmanuel Deloget

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] build against openssl 1.1.0

[Emmanuel Deloget]

Hello,

First, sorry for the inconvenience: this message is not attached to
the remaining of the discussion (I just joined the ML so I cannot
answer to a one week old message). That being said:

On Mon, Feb 13, 2017 at 08:17:58PM +0100, Christian Hesse wrote:
> Arch Linux is about to upgrade openssl to version 1.1.0. OpenVPN does not
> compile against this version. Did anybody start the work to support latest
> openssl versions?

I did (yesterday). So far, I made good progress on many front,
although I'm not sure the path I took is what you would expect.

One of the main change in OpenSSL 1.1 is that types are now opaque,
meaning that you need to access the internal fields using various
(mostly short) functions. For most of them, these functions has been
added to the API.

To make OpenVPN support OpenSSL, I decided to

  1. check whether the functions I need are in OpenSSL at configure
time. The function list is quite large.
  2. reimplement the missing functions as static inlines in an
openssl_compat.h header, using the OpenSSL prototypes.
  3. use the new interface in the OpenVPN code.
  4. when possible (i.e. when the interface already exists in OpenSSL
1.0), use this interface

The motivation behind this is to ease the porting of OpenVPN to a new
OpenSSL API -- if the code is already using the latest API, the next
changes are going to be less radical.

Having done 2/3 of the job, the patch set is about +900 insertion,
-200 deletion (or something like that). I still have a lot of things
to do and I should finish my first pass at the beginning of newt week.
I don't expect the patch set to be much longer than it already is.

Now, I have a question which is related to this. The way I'm doing
things, I will make sure that the new code is compatible with both
OpenSSL 1.0.x and OpenSSL 1.1. There is a good chance that it will be
compatible with version 0.9.8 as well, yet I can't stop wondering if
this is a good thing. OpenSSL 0.9.8 has been EoL'ed 12 month ago and I
believe it's OK to let it die. OpenVPN cannot rely on a dead SSL
library -- unless it wants to make sure that future vulnerabilities in
this old, deprecated version will affect it (and I'm not sure it's a
good thing). Same goes for OpenSSL 1.0.1 which has been declared out
of support in January 2017.

I understand that I'm the new guy in town, but can you allow me to
make the formal request to ditch OpenSSL 0.9.8, 1.0.0 and 1.0.1 and
require at least version 1.0.2?

Best regards,

-- Emmanuel Deloget

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel