Hello,
On Wed, Jan 3, 2018 at 9:34 AM, Arne Schwabe <[email protected]> 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
