mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Benjamin Hindman" <b...@berkeley.edu>
Subject Re: Review Request 31207: Support for SSL and non-ssl traffic simultaneously.
Date Mon, 29 Jun 2015 01:37:58 GMT

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


This is an incomplete review but I wanted to publish it sot that we can deal with the 'link_connect'
and 'send_connect' issue.


3rdparty/libprocess/src/openssl.cpp (line 336)
<https://reviews.apache.org/r/31207/#comment142369>

    s/plain HTTP/a non-SSL socket/
    
    Because someone using the socket might not want to use it for HTTP! ;-) Only for libprocess
will we expect the data to be HTTP.
    
    Feel free to check as much in process::initialize and print out a big warning there too?



3rdparty/libprocess/src/process.cpp (line 1245)
<https://reviews.apache.org/r/31207/#comment142371>

    How do you know that the ProcessBase pointer is still going to be valid across this async
callback? A ProcessReference here might be better suited.



3rdparty/libprocess/src/process.cpp (lines 1263 - 1264)
<https://reviews.apache.org/r/31207/#comment142370>

    We can't "close this socket" yet because it will generate "exited" events as though this
link was unsuccessful.
    
    One suggestion for now is to add a 'bool generate_exited' parameter to SocketManager::close
that only generates the exited events if true (the default).
    
    This general pattern of "closing" the socket is still unfortunate, however, because it'll
mean we may queue up some data to send (Encoders) that we then close even though the downgraded
connection may work correctly. This could start to introduce some funky things in practice,
as in, some messages actually look like they're getting dropped! So the real way to fix this
is to probably grab the SocketManager mutex and actually update all of the data structures
with the "new" socket. Or alternatively, could we swap the "implementation" of a Socket and
reuse the same file descriptor?


- Benjamin Hindman


On June 29, 2015, 1:22 a.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31207/
> -----------------------------------------------------------
> 
> (Updated June 29, 2015, 1:22 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-2085
>     https://issues.apache.org/jira/browse/MESOS-2085
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add a flag SSL_SUPPORT_DOWNGRADE which allows:
> 1. an SSL accepting socket to peek at the incoming data. If the hello handshake bits
are not set, then accept as a Socket::POLL socket instead.
> 2. When calling Process::link or Process:send(Message), if a new connection is required,
allow a second attempt using Socket::POLL if an SSL connection was first attempted.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/socket.hpp f53d2e1dbb31e135c8951145d379cbbff3044448

>   3rdparty/libprocess/src/libevent_ssl_socket.hpp 4f2cd357bfdb5268d2bae2df5d7138ff14064bf6

>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 2920e0e1a5643118b14911d77fb682e60958b4e6

>   3rdparty/libprocess/src/openssl.hpp 60c7b078b891e09d53d82508bb2965addf359d68 
>   3rdparty/libprocess/src/openssl.cpp 6ff4adb4c9792ff10d8c6ed2f3b2f3d8d0d7f1a8 
>   3rdparty/libprocess/src/poll_socket.hpp 553aa641525d587a44608d7c6c4f16b09b47cfe0 
>   3rdparty/libprocess/src/process.cpp 52649fb90cdbefb495b68d0beb8c7f7e5ef6888e 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp c077aaeaecbe2cdcdad2b042741eeb8906699a22

> 
> Diff: https://reviews.apache.org/r/31207/diff/
> 
> 
> Testing
> -------
> 
> Running with:
> 1) An SSL master
>   - connect a non-ssl slave
>   - connect a non-ssl framework
>   - connect an ssl slave
>   - connect an ssl framework
> 2) A non-ssl master
>   - connect a non-ssl slave
>   - connect a non-ssl framework
>   - connect an ssl slave
>   - connect an ssl framework
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


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