mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan-Philip Gehrcke via Review Board <nore...@reviews.apache.org>
Subject Re: Review Request 60913: Adds support for OpenSSL's ECDH handshake.
Date Wed, 19 Jul 2017 11:50:34 GMT

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



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"

- Jan-Philip Gehrcke


On July 18, 2017, 12:13 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60913/
> -----------------------------------------------------------
> 
> (Updated July 18, 2017, 12:13 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Till Toenshoff.
> 
> 
> Bugs: MESOS-7792
>     https://issues.apache.org/jira/browse/MESOS-7792
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support for Elliptic Curve Diffie Hellman algorithm requires extra
> configuration parameters which weren't part of Mesos.
> 
> This patch enables the extra configuration to Mesos in order to
> support ECDH algorithm, it also adds the ssl flag
> `LIBPROCESS_SSL_ECDH_CURVES` which allows for the specification of
> a specific elliptic curve.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 13fa7a0cc9d6d6d6849976a3ce383263c51504d7

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

> 
> 
> Diff: https://reviews.apache.org/r/60913/diff/3/
> 
> 
> 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