Re: Review Request 70562: Add LIBPROCESS_SSL_ENABLE_TLS_V1_3
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70562/#review214949 --- Ship it! Thanks so much Stephane. Looks slick -- Benno will go ahead and land this with the small fix he mentioned already (thanks Benno :)) - Till Toenshoff On April 29, 2019, 6:43 a.m., Stéphane Cottin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70562/ > --- > > (Updated April 29, 2019, 6:43 a.m.) > > > Review request for mesos and Benno Evers. > > > Bugs: MESOS-9730 > https://issues.apache.org/jira/browse/MESOS-9730 > > > Repository: mesos > > > Description > --- > > When building mesos with libopenssl >= 1.1.1, TLS1.3 is enabled by default. > This causes major communication issues between executors and agents. > > This patch adds new `LIBPROCESS_SSL_ENABLE_TLS_V1_3` env var, disabled by > default. > Should be changed to enabled by default when full openssl >= 1.1 support will > land. > > > Diffs > - > > 3rdparty/libprocess/include/process/ssl/flags.hpp 3806266fb > 3rdparty/libprocess/include/process/ssl/gtest.hpp e173b3224 > 3rdparty/libprocess/src/openssl.hpp 0c4192f90 > 3rdparty/libprocess/src/openssl.cpp a4d503642 > 3rdparty/libprocess/src/tests/ssl_tests.cpp 5e994498d > > > Diff: https://reviews.apache.org/r/70562/diff/1/ > > > Testing > --- > > > Thanks, > > Stéphane Cottin > >
Re: Review Request 70562: Add LIBPROCESS_SSL_ENABLE_TLS_V1_3
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70562/#review214946
---
Hey,
thanks, the patches looks great!
I just ran them through our CI, and on Ubuntu 16.04 the
`TEST_P(SSLProtocolTest, Mismatch)` failed when both server and client protocol
is set to `LIBPROCESS_SSL_ENABLE_TLS_V1_3`.
>From looking at the test, it looks like the test expects a TLS1.3 client to be
>able to talk to a TLS1.3 server, but the openssl on that platform doesn't
>support TLS 1.3 so the connection fails.
It seems like straight-forward solution would be to guard this protcol with
`#ifdef`s as shown below, similar to how we handle
`LIBPROCESS_SSL_ENABLE_SSL_V3` in the same test. I'll confirm that with Till
and commit the modified patch if he agrees.
```
--- a/3rdparty/libprocess/src/tests/ssl_tests.cpp
+++ b/3rdparty/libprocess/src/tests/ssl_tests.cpp
@@ -122,7 +122,11 @@ static const vector protocols = {
"LIBPROCESS_SSL_ENABLE_TLS_V1_0",
"LIBPROCESS_SSL_ENABLE_TLS_V1_1",
"LIBPROCESS_SSL_ENABLE_TLS_V1_2",
- "LIBPROCESS_SSL_ENABLE_TLS_V1_3"
+// On some platforms, we need to build against OpenSSL versions that
+// do not support TLS 1.3 yet.
+#ifdef SSL_OP_NO_TLSv1_3
+ "LIBPROCESS_SSL_ENABLE_TLS_V1_3",
+#endif
};
```
- Benno Evers
On April 29, 2019, 6:43 a.m., Stéphane Cottin wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70562/
> ---
>
> (Updated April 29, 2019, 6:43 a.m.)
>
>
> Review request for mesos and Benno Evers.
>
>
> Bugs: MESOS-9730
> https://issues.apache.org/jira/browse/MESOS-9730
>
>
> Repository: mesos
>
>
> Description
> ---
>
> When building mesos with libopenssl >= 1.1.1, TLS1.3 is enabled by default.
> This causes major communication issues between executors and agents.
>
> This patch adds new `LIBPROCESS_SSL_ENABLE_TLS_V1_3` env var, disabled by
> default.
> Should be changed to enabled by default when full openssl >= 1.1 support will
> land.
>
>
> Diffs
> -
>
> 3rdparty/libprocess/include/process/ssl/flags.hpp 3806266fb
> 3rdparty/libprocess/include/process/ssl/gtest.hpp e173b3224
> 3rdparty/libprocess/src/openssl.hpp 0c4192f90
> 3rdparty/libprocess/src/openssl.cpp a4d503642
> 3rdparty/libprocess/src/tests/ssl_tests.cpp 5e994498d
>
>
> Diff: https://reviews.apache.org/r/70562/diff/1/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Stéphane Cottin
>
>
Review Request 70562: Add LIBPROCESS_SSL_ENABLE_TLS_V1_3
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70562/ --- Review request for mesos. Bugs: MESOS-9730 https://issues.apache.org/jira/browse/MESOS-9730 Repository: mesos Description --- When building mesos with libopenssl >= 1.1.1, TLS1.3 is enabled by default. This causes major communication issues between executors and agents. This patch adds new `LIBPROCESS_SSL_ENABLE_TLS_V1_3` env var, disabled by default. Should be changed to enabled by default when full openssl >= 1.1 support will land. Diffs - 3rdparty/libprocess/include/process/ssl/flags.hpp 3806266fb 3rdparty/libprocess/include/process/ssl/gtest.hpp e173b3224 3rdparty/libprocess/src/openssl.hpp 0c4192f90 3rdparty/libprocess/src/openssl.cpp a4d503642 3rdparty/libprocess/src/tests/ssl_tests.cpp 5e994498d Diff: https://reviews.apache.org/r/70562/diff/1/ Testing --- Thanks, Stéphane Cottin
