mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Till Toenshoff <toensh...@me.com>
Subject Re: Review Request 60913: Adds support for OpenSSL's ECDH key exchange.
Date Thu, 27 Jul 2017 00:28:06 GMT


> On July 19, 2017, 11:50 a.m., Jan-Philip Gehrcke wrote:
> > Hey! This looks great.
> > 
> > I have a few remarks.
> > 
> > 1) The test in this patch seems to prove that the patch introduces support for the
_ephemeral_ elliptic curve Diffie-Hellman key exchange, usually abbreviated with ECDHE. Does
this patch also add ECDH (non-ephemeral key exchange, where an ever-static ECDH public key
is used) support? If it does not then I think this is good news :-) I'd appreciate a comment
on that in the patch description. If this can be controlled via external configuration through
the cipher suite string/list, then this is an appropriate answer.
> > 
> > 2) Even ECDHE can be used in two modes. Related to the first point we should stress
in the patch description and also via a code comment that this patch explicitly disables an
optimization called "ephemeral-static ECDH" via the SSL_OP_SINGLE_ECDH_USE option. I found
https://forum.nginx.org/read.php?29,241610,249058 insightful, but also https://eprint.iacr.org/2011/633.pdf
from which I'd like to quote:
> > 
> >     "2. Use of ephemeral ECDH-based cipher suites (e.g., ECDHE-ECDSA and ECDHE-RSA)
> >     in combination with the OpenSSL ephemeral-static ECDH optimisation. In such
cipher
> >     suites, and according to the TLS specification, a fresh ECDH public key should
be
> >     generated for each key exchange. However OpenSSL allows one-time generation
of said
> >     key when the TLS server is initialised, sharing it across an arbitrary number
of key
> >     exchanges thereafter."
> > 
> > 3) This patch contains a test that proves that RSA-signed key exchange works (i.e.
ECDHE_RSA type cipher suites). I assume that this patch also allows for ECDSA-signed key exchange
(i.e. ECDHE_ECDSA type cipher suites), but it just requires a corresponding private key type.
I'd appreciate to know if you agree with that or not :-).
> > 
> > 4) I think we should adjust the commit message saying "Adds support for OpenSSL's
ECDHE key exchange"

re 4: We commonly use past tense for the RR subject and present tense for the description.
We also always terminate with a period. Both the summary and the description form the commit
message as a whole. Hence it should be "Added support for OpenSSL's ECDHE key exchange."


- Till


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60913/#review180916
-----------------------------------------------------------


On July 26, 2017, 9:25 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60913/
> -----------------------------------------------------------
> 
> (Updated July 26, 2017, 9:25 a.m.)
> 
> 
> Review request for mesos, Jie Yu, James Peach, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7792
>     https://issues.apache.org/jira/browse/MESOS-7792
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds the configuration necesary so that the Elliptic Curve
> Diffie Hellman algorithm can be used for TLS key exchange if the
> OpenSSL version used supports it.
> 
> It also adds the ssl flag `LIBPROCESS_SSL_ECDH_CURVES` which allows
> for the specification of a specific elliptic curve (or set of curves).
> 
> Users will need to specify the TLS cipher suite that uses ECDH in order
> to enable the new key exchange. By default Mesos does not use any ECDH
> cipher suites.
> 
> Support for ephemeral ECDH public keys is the default, so that new
> public keys are generated for each exchange.
> 
> Note that in order to enable ECDSA ciphers an ECDSA is still necesary
> instead of the more traditionl RSA one.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 13fa7a0cc9d6d6d6849976a3ce383263c51504d7

>   3rdparty/libprocess/src/openssl.hpp 7ded2c74b2f92aacfa0f366bd27d5e0df2b8f25c 
>   3rdparty/libprocess/src/openssl.cpp e6f17e4591f573186e1dc9697e1e7b60a841fe4f 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 8a14dcb865dfab34fb4d0d51f42a28a913fb7ace

> 
> 
> Diff: https://reviews.apache.org/r/60913/diff/7/
> 
> 
> Testing
> -------
> 
> ```shell
> make check
> ```
> 
> Launched Mesos with only ECDHE handshake ciphers enabled
> 
> ```shell
> LIBPROCESS_SSL_ENABLED=1 \
> LIBPROCESS_SSL_KEY_FILE=/tmp/ssl/self-signed.key \
> LIBPROCESS_SSL_CERT_FILE=/tmp/ssl/self-signed.crt \
> LIBPROCESS_SSL_CIPHERS="ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES256-SHA"
\
> ./bin/mesos-master.sh \
>     --work_dir=/tmp/mesos/master \
>     --log_dir=/tmp/mesos/master/log
> ```
> 
> Then in another shell:
> 
> ```shell
> http -v --verify=no https://${MESOS_MASTER_IP}:5050/state
> 
> # Launches a browser.
> open https://${MESOS_MASTER_IP}:5050/state
> 
> # List the set of supported ciphers.
> # Expected output:
> # >  Starting Nmap 7.50 ( https://nmap.org ) at 2017-07-18 11:41 CEST
> # >  Nmap scan report for ${MESOS_MASTER_HOSTNAME} (${MESOS_MASTER_IP})
> # >  Host is up (0.13s latency).
> # >  rDNS record for ${MESOS_MASTER_IP}: ${MESOS_MASTER_HOSTNAME}
> # >  
> # >  PORT     STATE SERVICE
> # >  5050/tcp open  mmcc
> # >  | ssl-enum-ciphers:
> # >  |   TLSv1.2:
> # >  |     ciphers:
> # >  |       TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (secp256r1) - A
> # >  |       TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 (secp256r1) - A
> # >  |       TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (secp256r1) - A
> # >  |       TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (secp256r1) - A
> # >  |       TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 (secp256r1) - A
> # >  |       TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (secp256r1) - A
> # >  |     compressors:
> # >  |       NULL
> # >  |     cipher preference: server
> # >  |_  least strength: A
> # >  
> # >  Nmap done: 1 IP address (1 host up) scanned in 1.87 seconds
> wget https://svn.nmap.org/nmap/scripts/ssl-enum-ciphers.nse
> nmap --script ssl-enum-ciphers.nse -p 5050 ${MESOS_MASTER_IP}
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message