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

[Fabian Grünbichler]

On Mon, Oct 31, 2016 at 07:16:41PM +0100, Jos Ewert wrote:
> The TLS_ECDHE_* ciphers will automatically be used be the proxy
> as they are in the HIGH ciphersuite.
> ---
>  PVE/HTTPServer.pm | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/PVE/HTTPServer.pm b/PVE/HTTPServer.pm
> index 1e27bba..e38542b 100755
> --- a/PVE/HTTPServer.pm
> +++ b/PVE/HTTPServer.pm
> @@ -1625,6 +1625,13 @@ sub new {
>      if ($self->{ssl}) {
>  	$self->{tls_ctx} = AnyEvent::TLS->new(%{$self->{ssl}});
>  	Net::SSLeay::CTX_set_options($self->{tls_ctx}->{ctx}, &Net::SSLeay::OP_NO_COMPRESSION);
> +        # ECDH curve ( Net-SSLeay >= 1.56, openssl >= 1.0.0 )
> +        if ( exists &Net::SSLeay::CTX_set_tmp_ecdh ) {
> +            my $curve = Net::SSLeay::OBJ_txt2nid('prime256v1');
> +            my $ecdh  = Net::SSLeay::EC_KEY_new_by_curve_name($curve);
> +            Net::SSLeay::CTX_set_tmp_ecdh( $self->{tls_ctx}->{ctx}, $ecdh );
> +            Net::SSLeay::EC_KEY_free($ecdh);
> +        }
>      }
>  
>      if ($self->{spiceproxy}) {
> -- 
> 2.7.4

the EC_KEY_new_by_curve_name and CTX_set_tmp_ecdh low-level API methods
are undocumented upstream[1], which is a bit unfortunate for crypto
related code. fortunately they are straightforward wrappers of the
actual 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 it seems like we'd need to limit ourselves to one curve -
I'd like to switch to the 25519 one[2] as soon as we switch to OpenSSL
1.1.0 (so probably with Stretch).

for the patch itself:
- 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)
- there is an option to regenerate the key on each handshake, not sure
  about the performance implications
- testing with all supported browsers to check for problems (especially
  the Microsoft ones can be quite buggy regarding TLS negotiation)
- Perl style[3], indentation and spaces around parenthesis
- the patch needs a "Signed-Off-By" line and we need a signed copy of
  the CLA before we could merge it[4]

the last one is of course the most important, but since you need to send
a v2 for that anyway, it would be nice to at least fix the style issues,
I can check the browser support and regenerate option myself before
applying (but if you do some or all of it yourself, I am happy ;)).

1: http://search.cpan.org/~mikem/Net-SSLeay/lib/Net/SSLeay.pod#Low_level_API:_EC_related_functions
2: https://en.wikipedia.org/wiki/Curve25519
3: http://pve.proxmox.com/wiki/Perl_Style_Guide#Spacing_and_syntax_usage
4: http://pve.proxmox.com/wiki/Developer_Documentation#Software_License_and_Copyright