You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benno Evers <be...@mesosphere.com> on 2019/06/06 23:15:51 UTC
Re: Review Request 70749: WIP: Introduced optional new algorithm for
hostname validation.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70749/
-----------------------------------------------------------
(Updated June 6, 2019, 11:15 p.m.)
Review request for mesos, Alexander Rukletsov and Joseph Wu.
Changes
-------
v2, merged with reverse DNS review.
Summary (updated)
-----------------
WIP: Introduced optional new algorithm for hostname validation.
Bugs: MESOS-9809
https://issues.apache.org/jira/browse/MESOS-9809
Repository: mesos
Description (updated)
-------
This commit introduces a new libprocess SSL flag
`hostname_validation_algorithm`, which can be used to select
between the previous hostname validation behaviour and a new
option to use standardized OpenSSL algorithms to handle
hostname validation as part of the
As a nice side-effect, the new algorithm gets rid of reverse DNS
lookups during TLS connection establishment, which used to be
a common source of hard-to-debug unresponsiveness in Mesos
components.
See `docs/ssl.md` in the follow-up commit for details of and
differences between the algorithms.
Diffs (updated)
-----
3rdparty/libprocess/include/process/address.hpp e740e840c38381bafd7a1a7fcde5f963832ac1fb
3rdparty/libprocess/include/process/ssl/flags.hpp f3483f97f93bb29117b2c78f0f2ed9735d9c4b3a
3rdparty/libprocess/src/http.cpp 3e73ee936f5c6329f41704a179f3d88ab65dfb6d
3rdparty/libprocess/src/openssl.hpp 17bec246e516261f8d772f1647c17f092fae82d1
3rdparty/libprocess/src/openssl.cpp e7dbd67913fa8e7fbbf60dee428e7e38895f86ce
3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 29a1bf71c1df9d80370455a6269ecea0ec4193b0
3rdparty/libprocess/src/tests/http_tests.cpp 97aaf3ed3d4fab6d717d5c9b6d12402562ac6b46
3rdparty/libprocess/src/tests/ssl_tests.cpp 6b8496aeeed79ae1bd39d7013f4f403b248fdd4c
Diff: https://reviews.apache.org/r/70749/diff/2/
Changes: https://reviews.apache.org/r/70749/diff/1-2/
Testing (updated)
-------
Todo!
Thanks,
Benno Evers
Re: Review Request 70749: Introduced optional new scheme for hostname
validation.
Posted by Benno Evers <be...@mesosphere.com>.
> On June 21, 2019, 1:32 a.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/src/openssl.cpp
> > Lines 808 (patched)
> > <https://reviews.apache.org/r/70749/diff/4/?file=2151450#file2151450line808>
> >
> > I know you just moved it, but where do these 100ms come from and how could we be more explicit about that choice?
> >
> > I would suggest to use a const with some explaining comment how that value was chosen - can we please? :D
I'm afraid I have no idea where the 100ms come from. The suggestion sounds good, but I think the changes will fit better in a separate review, since they're not really related to hostname validation.
> On June 21, 2019, 1:32 a.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/src/openssl.cpp
> > Lines 984-985 (patched)
> > <https://reviews.apache.org/r/70749/diff/4/?file=2151450#file2151450line984>
> >
> > This sounds like a great idea worth spending 1 more cycle on -- can we create and reference a ticket that explains this jazz as nicely as we do here in the code?
> >
> > My thought is that being open about this idea in JIRA, we would provide more chances of getting community support for it.
Opened https://issues.apache.org/jira/browse/MESOS-9855
> On June 21, 2019, 1:32 a.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/src/openssl.cpp
> > Lines 989 (patched)
> > <https://reviews.apache.org/r/70749/diff/4/?file=2151450#file2151450line989>
> >
> > Shall we explain why this is the `right` way - aka best practices?
Huh, I actually removed this now: I originally had a look at the OpenSSL example code and at RFC6125, but missed that partial wildcards are only disallowed in *internationalized* domain names. (and for these, openssl already does the correct thing regardless of this flag.)
With this removed, we're a bit more loose than what a web browser would accept, but Mesos is not a web browser so that seems fine.
- Benno
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70749/#review216017
-----------------------------------------------------------
On June 20, 2019, 5:48 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70749/
> -----------------------------------------------------------
>
> (Updated June 20, 2019, 5:48 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9809
> https://issues.apache.org/jira/browse/MESOS-9809
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a new libprocess SSL flag
> `hostname_validation_scheme`, which can be used to select
> between the previous hostname validation behaviour and a new
> option to use standardized OpenSSL algorithms to handle
> hostname validation as part of the handshake.
>
> As a nice side-effect, the new scheme gets rid of reverse DNS
> lookups during TLS connection establishment, which used to be
> a common source of hard-to-debug unresponsiveness in Mesos
> components.
>
> See `docs/ssl.md` in the follow-up commit for details of and
> differences between the schemes.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/ssl/flags.hpp f3483f97f93bb29117b2c78f0f2ed9735d9c4b3a
> 3rdparty/libprocess/src/openssl.hpp 17bec246e516261f8d772f1647c17f092fae82d1
> 3rdparty/libprocess/src/openssl.cpp e7dbd67913fa8e7fbbf60dee428e7e38895f86ce
> 3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp 6ef5a86566af3439cfe0b06ab3576076623f7be0
> 3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 29a1bf71c1df9d80370455a6269ecea0ec4193b0
>
>
> Diff: https://reviews.apache.org/r/70749/diff/4/
>
>
> Testing
> -------
>
> Todo!
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 70749: Introduced optional new scheme for hostname
validation.
Posted by Benno Evers <be...@mesosphere.com>.
> On June 21, 2019, 1:32 a.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/src/openssl.cpp
> > Lines 808 (patched)
> > <https://reviews.apache.org/r/70749/diff/4/?file=2151450#file2151450line808>
> >
> > I know you just moved it, but where do these 100ms come from and how could we be more explicit about that choice?
> >
> > I would suggest to use a const with some explaining comment how that value was chosen - can we please? :D
>
> Benno Evers wrote:
> I'm afraid I have no idea where the 100ms come from. The suggestion sounds good, but I think the changes will fit better in a separate review, since they're not really related to hostname validation.
Fixed in https://reviews.apache.org/r/70933/
- Benno
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70749/#review216017
-----------------------------------------------------------
On June 21, 2019, 3:32 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70749/
> -----------------------------------------------------------
>
> (Updated June 21, 2019, 3:32 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9809
> https://issues.apache.org/jira/browse/MESOS-9809
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a new libprocess SSL flag
> `hostname_validation_scheme`, which can be used to select
> between the previous hostname validation behaviour and a new
> option to use standardized OpenSSL algorithms to handle
> hostname validation as part of the
>
> As a nice side-effect, the new scheme gets rid of reverse DNS
> lookups during TLS connection establishment, which used to be
> a common source of hard-to-debug unresponsiveness in Mesos
> components.
>
> See `docs/ssl.md` in the follow-up commit for details of and
> differences between the schemes.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/ssl/flags.hpp f3483f97f93bb29117b2c78f0f2ed9735d9c4b3a
> 3rdparty/libprocess/src/openssl.hpp 17bec246e516261f8d772f1647c17f092fae82d1
> 3rdparty/libprocess/src/openssl.cpp e7dbd67913fa8e7fbbf60dee428e7e38895f86ce
> 3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp 6ef5a86566af3439cfe0b06ab3576076623f7be0
> 3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 29a1bf71c1df9d80370455a6269ecea0ec4193b0
>
>
> Diff: https://reviews.apache.org/r/70749/diff/5/
>
>
> Testing
> -------
>
> Todo!
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 70749: Introduced optional new scheme for hostname
validation.
Posted by Till Toenshoff via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70749/#review216017
-----------------------------------------------------------
3rdparty/libprocess/src/openssl.cpp
Lines 147 (patched)
<https://reviews.apache.org/r/70749/#comment302961>
s/algorithm/scheme/?
3rdparty/libprocess/src/openssl.cpp
Lines 153 (patched)
<https://reviews.apache.org/r/70749/#comment302962>
s/algorithm/scheme/?
Also, we should tell the user which version he needs at least here, nice touch ;)
3rdparty/libprocess/src/openssl.cpp
Lines 565-566 (patched)
<https://reviews.apache.org/r/70749/#comment302963>
Should we
```
EXIT(EXIT_FAILURE)
<< "Unknown value for hostname_validation_scheme: "
<< ssl_flags->hostname_validation_scheme;
```
instead here and below to be perfectly explicit and consistent?
3rdparty/libprocess/src/openssl.cpp
Lines 576 (patched)
<https://reviews.apache.org/r/70749/#comment302964>
s/algorithm/scheme/?
3rdparty/libprocess/src/openssl.cpp
Line 755 (original), 788 (patched)
<https://reviews.apache.org/r/70749/#comment302965>
Maybe
```
// When using the OpenSSL built-in scheme,...
```
3rdparty/libprocess/src/openssl.cpp
Lines 795 (patched)
<https://reviews.apache.org/r/70749/#comment302966>
s/algorithm/scheme/
3rdparty/libprocess/src/openssl.cpp
Lines 808 (patched)
<https://reviews.apache.org/r/70749/#comment302967>
I know you just moved it, but where do these 100ms come from and how could we be more explicit about that choice?
I would suggest to use a const with some explaining comment how that value was chosen - can we please? :D
3rdparty/libprocess/src/openssl.cpp
Lines 984-985 (patched)
<https://reviews.apache.org/r/70749/#comment302968>
This sounds like a great idea worth spending 1 more cycle on -- can we create and reference a ticket that explains this jazz as nicely as we do here in the code?
My thought is that being open about this idea in JIRA, we would provide more chances of getting community support for it.
3rdparty/libprocess/src/openssl.cpp
Lines 989 (patched)
<https://reviews.apache.org/r/70749/#comment302969>
Shall we explain why this is the `right` way - aka best practices?
3rdparty/libprocess/src/openssl.cpp
Lines 1016 (patched)
<https://reviews.apache.org/r/70749/#comment302970>
s/ip/IP/
3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp
Lines 444-448 (patched)
<https://reviews.apache.org/r/70749/#comment302999>
Great comment - thanks!
3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp
Lines 569 (patched)
<https://reviews.apache.org/r/70749/#comment303000>
Now that we log the `peer_hostname`, I wonder if it made sense to also log the `peer_ip` like this - what do you think? If you agree, then we should even try to render a single log-line out of those two - it will be noisy enough on level 2.
3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp
Lines 1129 (patched)
<https://reviews.apache.org/r/70749/#comment303002>
I feel `peername` comes in a bit surprising here if one does not know about `getpeername()`.
Maybe s/peername/IP address/?
3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp
Lines 1136 (patched)
<https://reviews.apache.org/r/70749/#comment303001>
As mentioned in some other place of this chain, let's add a neat JIRA on this plan.
- Till Toenshoff
On June 20, 2019, 5:48 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70749/
> -----------------------------------------------------------
>
> (Updated June 20, 2019, 5:48 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9809
> https://issues.apache.org/jira/browse/MESOS-9809
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a new libprocess SSL flag
> `hostname_validation_scheme`, which can be used to select
> between the previous hostname validation behaviour and a new
> option to use standardized OpenSSL algorithms to handle
> hostname validation as part of the handshake.
>
> As a nice side-effect, the new scheme gets rid of reverse DNS
> lookups during TLS connection establishment, which used to be
> a common source of hard-to-debug unresponsiveness in Mesos
> components.
>
> See `docs/ssl.md` in the follow-up commit for details of and
> differences between the schemes.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/ssl/flags.hpp f3483f97f93bb29117b2c78f0f2ed9735d9c4b3a
> 3rdparty/libprocess/src/openssl.hpp 17bec246e516261f8d772f1647c17f092fae82d1
> 3rdparty/libprocess/src/openssl.cpp e7dbd67913fa8e7fbbf60dee428e7e38895f86ce
> 3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp 6ef5a86566af3439cfe0b06ab3576076623f7be0
> 3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 29a1bf71c1df9d80370455a6269ecea0ec4193b0
>
>
> Diff: https://reviews.apache.org/r/70749/diff/4/
>
>
> Testing
> -------
>
> Todo!
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 70749: Introduced optional new scheme for hostname
validation.
Posted by Jan-Philip Gehrcke via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70749/#review215999
-----------------------------------------------------------
Fix it, then Ship it!
Just saw that I did not yet submit this review, it's from yesterday's conversation. Hope it still applies. Feel free to ignore.
3rdparty/libprocess/src/openssl.cpp
Lines 563 (patched)
<https://reviews.apache.org/r/70749/#comment302937>
Let's maybe emit a warning when VERIFY_CERT is set to true, but ALLOW_DOWNGRADE is also set to true
- Jan-Philip Gehrcke
On June 20, 2019, 5:48 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70749/
> -----------------------------------------------------------
>
> (Updated June 20, 2019, 5:48 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9809
> https://issues.apache.org/jira/browse/MESOS-9809
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a new libprocess SSL flag
> `hostname_validation_scheme`, which can be used to select
> between the previous hostname validation behaviour and a new
> option to use standardized OpenSSL algorithms to handle
> hostname validation as part of the handshake.
>
> As a nice side-effect, the new scheme gets rid of reverse DNS
> lookups during TLS connection establishment, which used to be
> a common source of hard-to-debug unresponsiveness in Mesos
> components.
>
> See `docs/ssl.md` in the follow-up commit for details of and
> differences between the schemes.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/ssl/flags.hpp f3483f97f93bb29117b2c78f0f2ed9735d9c4b3a
> 3rdparty/libprocess/src/openssl.hpp 17bec246e516261f8d772f1647c17f092fae82d1
> 3rdparty/libprocess/src/openssl.cpp e7dbd67913fa8e7fbbf60dee428e7e38895f86ce
> 3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp 6ef5a86566af3439cfe0b06ab3576076623f7be0
> 3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 29a1bf71c1df9d80370455a6269ecea0ec4193b0
>
>
> Diff: https://reviews.apache.org/r/70749/diff/4/
>
>
> Testing
> -------
>
> Todo!
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 70749: Introduced RFC6125-compliant hostname
validation scheme.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70749/
-----------------------------------------------------------
(Updated July 4, 2019, 6:52 p.m.)
Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Joseph Wu, and Till Toenshoff.
Changes
-------
Renamed 'libprocess' -> 'legacy'
Bugs: MESOS-9809
https://issues.apache.org/jira/browse/MESOS-9809
Repository: mesos
Description (updated)
-------
This commit introduces a new libprocess SSL flag
`hostname_validation_scheme`, which can be set to 'legacy'
to select the previous hostname validation behaviour or to
'openssl' to use standardized OpenSSL algorithms to handle
hostname validation as part of the TLS handshake.
As a nice side-effect, the new scheme gets rid of reverse DNS
lookups during TLS connection establishment, which used to be
a common source of hard-to-debug unresponsiveness in Mesos
components.
See `docs/ssl.md` in the follow-up commit for details of and
differences between the schemes.
Diffs (updated)
-----
3rdparty/libprocess/include/process/ssl/flags.hpp f3483f97f93bb29117b2c78f0f2ed9735d9c4b3a
3rdparty/libprocess/src/openssl.hpp 17bec246e516261f8d772f1647c17f092fae82d1
3rdparty/libprocess/src/openssl.cpp 19d25a89f7dda1f6c66dd1ffc5051e35457d26b0
3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp 6ef5a86566af3439cfe0b06ab3576076623f7be0
3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 7e2229a9ed815727500bd457356e5531607fa6cf
Diff: https://reviews.apache.org/r/70749/diff/9/
Changes: https://reviews.apache.org/r/70749/diff/8-9/
Testing
-------
See added unit tests later in this chain.
Thanks,
Benno Evers
Re: Review Request 70749: Introduced RFC6125-compliant hostname
validation scheme.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70749/
-----------------------------------------------------------
(Updated July 2, 2019, 5:49 p.m.)
Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Joseph Wu, and Till Toenshoff.
Changes
-------
Rebased onto latest master; changed title.
Summary (updated)
-----------------
Introduced RFC6125-compliant hostname validation scheme.
Bugs: MESOS-9809
https://issues.apache.org/jira/browse/MESOS-9809
Repository: mesos
Description
-------
This commit introduces a new libprocess SSL flag
`hostname_validation_scheme`, which can be used to select
between the previous hostname validation behaviour and a new
option to use standardized OpenSSL algorithms to handle
hostname validation as part of the
As a nice side-effect, the new scheme gets rid of reverse DNS
lookups during TLS connection establishment, which used to be
a common source of hard-to-debug unresponsiveness in Mesos
components.
See `docs/ssl.md` in the follow-up commit for details of and
differences between the schemes.
Diffs (updated)
-----
3rdparty/libprocess/include/process/ssl/flags.hpp f3483f97f93bb29117b2c78f0f2ed9735d9c4b3a
3rdparty/libprocess/src/openssl.hpp 17bec246e516261f8d772f1647c17f092fae82d1
3rdparty/libprocess/src/openssl.cpp 19d25a89f7dda1f6c66dd1ffc5051e35457d26b0
3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp 6ef5a86566af3439cfe0b06ab3576076623f7be0
3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 7e2229a9ed815727500bd457356e5531607fa6cf
Diff: https://reviews.apache.org/r/70749/diff/8/
Changes: https://reviews.apache.org/r/70749/diff/7-8/
Testing (updated)
-------
See added unit tests later in this chain.
Thanks,
Benno Evers
Re: Review Request 70749: Introduced optional new scheme for hostname
validation.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70749/
-----------------------------------------------------------
(Updated June 21, 2019, 3:32 p.m.)
Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
Bugs: MESOS-9809
https://issues.apache.org/jira/browse/MESOS-9809
Repository: mesos
Description (updated)
-------
This commit introduces a new libprocess SSL flag
`hostname_validation_scheme`, which can be used to select
between the previous hostname validation behaviour and a new
option to use standardized OpenSSL algorithms to handle
hostname validation as part of the
As a nice side-effect, the new scheme gets rid of reverse DNS
lookups during TLS connection establishment, which used to be
a common source of hard-to-debug unresponsiveness in Mesos
components.
See `docs/ssl.md` in the follow-up commit for details of and
differences between the schemes.
Diffs (updated)
-----
3rdparty/libprocess/include/process/ssl/flags.hpp f3483f97f93bb29117b2c78f0f2ed9735d9c4b3a
3rdparty/libprocess/src/openssl.hpp 17bec246e516261f8d772f1647c17f092fae82d1
3rdparty/libprocess/src/openssl.cpp e7dbd67913fa8e7fbbf60dee428e7e38895f86ce
3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp 6ef5a86566af3439cfe0b06ab3576076623f7be0
3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 29a1bf71c1df9d80370455a6269ecea0ec4193b0
Diff: https://reviews.apache.org/r/70749/diff/5/
Changes: https://reviews.apache.org/r/70749/diff/4-5/
Testing
-------
Todo!
Thanks,
Benno Evers
Re: Review Request 70749: Introduced optional new scheme for hostname
validation.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70749/
-----------------------------------------------------------
(Updated June 20, 2019, 5:48 p.m.)
Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
Changes
-------
Renamed algorithm -> scheme
Summary (updated)
-----------------
Introduced optional new scheme for hostname validation.
Bugs: MESOS-9809
https://issues.apache.org/jira/browse/MESOS-9809
Repository: mesos
Description (updated)
-------
This commit introduces a new libprocess SSL flag
`hostname_validation_scheme`, which can be used to select
between the previous hostname validation behaviour and a new
option to use standardized OpenSSL algorithms to handle
hostname validation as part of the
As a nice side-effect, the new scheme gets rid of reverse DNS
lookups during TLS connection establishment, which used to be
a common source of hard-to-debug unresponsiveness in Mesos
components.
See `docs/ssl.md` in the follow-up commit for details of and
differences between the schemes.
Diffs (updated)
-----
3rdparty/libprocess/include/process/ssl/flags.hpp f3483f97f93bb29117b2c78f0f2ed9735d9c4b3a
3rdparty/libprocess/src/openssl.hpp 17bec246e516261f8d772f1647c17f092fae82d1
3rdparty/libprocess/src/openssl.cpp e7dbd67913fa8e7fbbf60dee428e7e38895f86ce
3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp 6ef5a86566af3439cfe0b06ab3576076623f7be0
3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 29a1bf71c1df9d80370455a6269ecea0ec4193b0
Diff: https://reviews.apache.org/r/70749/diff/4/
Changes: https://reviews.apache.org/r/70749/diff/3-4/
Testing
-------
Todo!
Thanks,
Benno Evers
Re: Review Request 70749: Introduced optional new algorithm for
hostname validation.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70749/
-----------------------------------------------------------
(Updated June 19, 2019, 2:45 p.m.)
Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
Summary (updated)
-----------------
Introduced optional new algorithm for hostname validation.
Bugs: MESOS-9809
https://issues.apache.org/jira/browse/MESOS-9809
Repository: mesos
Description
-------
This commit introduces a new libprocess SSL flag
`hostname_validation_algorithm`, which can be used to select
between the previous hostname validation behaviour and a new
option to use standardized OpenSSL algorithms to handle
hostname validation as part of the
As a nice side-effect, the new algorithm gets rid of reverse DNS
lookups during TLS connection establishment, which used to be
a common source of hard-to-debug unresponsiveness in Mesos
components.
See `docs/ssl.md` in the follow-up commit for details of and
differences between the algorithms.
Diffs (updated)
-----
3rdparty/libprocess/include/process/ssl/flags.hpp f3483f97f93bb29117b2c78f0f2ed9735d9c4b3a
3rdparty/libprocess/src/openssl.hpp 17bec246e516261f8d772f1647c17f092fae82d1
3rdparty/libprocess/src/openssl.cpp e7dbd67913fa8e7fbbf60dee428e7e38895f86ce
3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp 6ef5a86566af3439cfe0b06ab3576076623f7be0
3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 29a1bf71c1df9d80370455a6269ecea0ec4193b0
Diff: https://reviews.apache.org/r/70749/diff/3/
Changes: https://reviews.apache.org/r/70749/diff/2-3/
Testing
-------
Todo!
Thanks,
Benno Evers
Re: Review Request 70749: WIP: Introduced optional new algorithm for
hostname validation.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70749/#review215736
-----------------------------------------------------------
Some notes:
First, this has become quite a big review, since I merged it with the other review about removing reverse DNS calls for `connect()`.
The reason I did this was because it turns out that getting rid of rDNS will actually be a noticable behavioural change, so it should be hidden behind a feature flag. On the other hand, this review introduces a good candidate for such a flag, and I didn't want to introduce another one that is required just for one commit.
Second, the semantics of the flag actually have changed compared to the first revision. I'll update the design doc shortly, but in the meantime be sure to read the detailed description in the follow-up commit.
Third, this is still a bit theoretical and requires additional testing to figure out how practical the `openssl` algorithm is right now, but I felt it's better if other people are able to look at the code as well.
3rdparty/libprocess/include/process/address.hpp
Lines 70 (patched)
<https://reviews.apache.org/r/70749/#comment302553>
The renaming was mostly done to ensure I catch all usages of `hostname()`, I'm happy to revert if you think this doesn't belong in this review or breaks API.
However, given that several users of this function seemed to be unaware that calling this would involve a network operation, I'd say renaming is in principle a good idea.
- Benno Evers
On June 6, 2019, 11:15 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70749/
> -----------------------------------------------------------
>
> (Updated June 6, 2019, 11:15 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
>
>
> Bugs: MESOS-9809
> https://issues.apache.org/jira/browse/MESOS-9809
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit introduces a new libprocess SSL flag
> `hostname_validation_algorithm`, which can be used to select
> between the previous hostname validation behaviour and a new
> option to use standardized OpenSSL algorithms to handle
> hostname validation as part of the
>
> As a nice side-effect, the new algorithm gets rid of reverse DNS
> lookups during TLS connection establishment, which used to be
> a common source of hard-to-debug unresponsiveness in Mesos
> components.
>
> See `docs/ssl.md` in the follow-up commit for details of and
> differences between the algorithms.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/address.hpp e740e840c38381bafd7a1a7fcde5f963832ac1fb
> 3rdparty/libprocess/include/process/ssl/flags.hpp f3483f97f93bb29117b2c78f0f2ed9735d9c4b3a
> 3rdparty/libprocess/src/http.cpp 3e73ee936f5c6329f41704a179f3d88ab65dfb6d
> 3rdparty/libprocess/src/openssl.hpp 17bec246e516261f8d772f1647c17f092fae82d1
> 3rdparty/libprocess/src/openssl.cpp e7dbd67913fa8e7fbbf60dee428e7e38895f86ce
> 3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 29a1bf71c1df9d80370455a6269ecea0ec4193b0
> 3rdparty/libprocess/src/tests/http_tests.cpp 97aaf3ed3d4fab6d717d5c9b6d12402562ac6b46
> 3rdparty/libprocess/src/tests/ssl_tests.cpp 6b8496aeeed79ae1bd39d7013f4f403b248fdd4c
>
>
> Diff: https://reviews.apache.org/r/70749/diff/2/
>
>
> Testing
> -------
>
> Todo!
>
>
> Thanks,
>
> Benno Evers
>
>