mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benno Evers <bev...@mesosphere.com>
Subject Re: Review Request 70562: Add LIBPROCESS_SSL_ENABLE_TLS_V1_3
Date Mon, 29 Apr 2019 13:49:54 GMT

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


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