[pve-devel] [PATCH manager] Add ECDH curves to use with modern ciphers

[Fabian Grünbichler]

On Wed, Nov 02, 2016 at 03:15:00PM +0100, Jos Ewert wrote:
> Hi, 
> 
> Thanks for your help. 
> 
> I have a few more questions and comments:
> 
> > OpenSSL API methods, which are at least fairly easy to grasp in
> > the OpenSSL code base (although the whole ECDH subsystem seems to be
> > rather under-documented there as well).
> Unfortunately the recommended way by openssl [1] is to use the high level 
> EVP_PKEY_CTX functions and not the low level EC_KEY ones. unfortunately I 
> could not find perl bindings for the EVP functions. SSLeay has some of them, 
> but it seems not the ones that are used in the openssl example. 
> IO::Socket::ssl has SSL_ecdh_curve , but I'm not sure that can be used 
> together with anyevent::TLS.
> So I think the only way to do it is to use the EC_KEY functions. These follow 
> the openssl example closely. get curve, get key. set key/do key exchange. 
> 
> [1] https://wiki.openssl.org/index.php/Elliptic_Curve_Diffie_Hellman

yep, seems like that's the only way to go (for now, maybe we can
reevaluate when updating to a newer OpenSSL/Net::SSLeay).

> > - IMHO, the comment and if can be dropped since Jessie has
> >   libnet-ssleay-perl 1.65 and openssl 1.0.1 (or replaced with a TODO for
> >   25519, see above)
> Yes that's what I'm thinking too, done.
> 
> > - there is an option to regenerate the key on each handshake, not sure
> >   about the performance implications
> I'm not sure where I would do this. do you mean when anyevent is waiting for 
> and accepting new connections? ($self->{socket_watch}). If I understand that 
> correctly, anyevent at this point transparently uses TLS, so I don't know how 
> I would hook into the TLS handshake at that point to change ECDH keys. But yes 
> it's not exactly ephemeral to set up the key when the httpserver is started.
> 

sorry for being not clear enough here - there is an OpenSSL option
(which should by supported by Net::SSLeay) called
"(SSL_)OP_SINGLE_ECDH_USE", which should be settable on the TLS context.

for regular DH this is exposed by anyevent as well it seems (but not
currently enabled by us), so it might be worth it to leave that as a
second, separate patch for further testing regarding performance.

> > - testing with all supported browsers to check for problems (especially
> >   the Microsoft ones can be quite buggy regarding TLS negotiation)
> I tested it with Chrome 54 Linux, Firefox 49 Linux, IE 11 .
> 
> > - Perl style[3], indentation and spaces around parenthesis
> Done (hopefully)
> 
> > - the patch needs a "Signed-Off-By" line and we need a signed copy of
> >   the CLA before we could merge it[4]
> The Signed-Off-By, is by me?

yes (your name + email address)

> 
> I'll send the CLA soon.
> 

thanks

----- End forwarded message -----