----------------------------------------------------------- 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<string> 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 > >