You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2019/10/23 23:21:02 UTC

Review Request 71664: SSL Wrapper: Implemented BIO for SSL socket wrapper.

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

Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till Toenshoff.


Bugs: MESOS-10009
    https://issues.apache.org/jira/browse/MESOS-10009


Repository: mesos


Description
-------

This implements the OpenSSL basic I/O abstraction based on the
libprocess event loop.  This BIO wraps a socket and handles the
reading/writing, using io::read and io::write.

This BIO can be passed into an SSL context to enable usage of
SSL translation functions like SSL_read and SSL_write.


Diffs
-----

  3rdparty/libprocess/src/ssl/socket_wrapper.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/71664/diff/1/


Testing
-------

cmake --build . --target process

A tiny bit of testing next patch.


Thanks,

Joseph Wu


Re: Review Request 71664: SSL Wrapper: Implemented BIO for SSL socket wrapper.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71664/#review218806
-----------------------------------------------------------




3rdparty/libprocess/src/ssl/socket_wrapper.cpp
Lines 218-236 (patched)
<https://reviews.apache.org/r/71664/#comment306676>

    Do we need these? How can we gain some confidence regarding which control commands need to be supported by our custom BIO and which do not?


- Greg Mann


On Nov. 19, 2019, 8:16 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71664/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2019, 8:16 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-10009
>     https://issues.apache.org/jira/browse/MESOS-10009
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This implements the OpenSSL basic I/O abstraction based on the
> libprocess event loop.  This BIO wraps a socket and handles the
> reading/writing, using io::read and io::write.
> 
> This BIO can be passed into an SSL context to enable usage of
> SSL translation functions like SSL_read and SSL_write.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/ssl/socket_wrapper.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71664/diff/3/
> 
> 
> Testing
> -------
> 
> cmake --build . --target process
> 
> A tiny bit of testing next patch.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 71664: SSL Wrapper: Implemented BIO for SSL socket wrapper.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Nov. 26, 2019, 11:38 a.m., Greg Mann wrote:
> > 3rdparty/libprocess/src/ssl/socket_wrapper.cpp
> > Lines 212-215 (patched)
> > <https://reviews.apache.org/r/71664/diff/3/?file=2174350#file2174350line212>
> >
> >     Are we sure that there are no ill effects of immediately returning 1 here, even when there may still be data left to be flushed?

Looking through the BIO implementations within OpenSSL itself, there are a couple which implement the "default" return value for this method:
https://github.com/openssl/openssl/blob/f059e4cc435b7b850cfc8188d265a8925edff0bd/crypto/bio/bss_sock.c#L185
https://github.com/openssl/openssl/blob/e7fb44e7c3f7a37ff83a6b69ba51a738e549bf5c/crypto/bio/bss_mem.c#L314
https://github.com/openssl/openssl/blob/e7fb44e7c3f7a37ff83a6b69ba51a738e549bf5c/crypto/bio/bss_null.c#L61

Also, the header seems to imply the method is optional:
https://github.com/openssl/openssl/blob/54a0d4ceb28d53f5b00a27fc5ca8ff8f0ddf9036/include/openssl/bio.h#L87
```
# define BIO_CTRL_FLUSH 11/* opt - 'flush' buffered output */
```


- Joseph


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


On Nov. 19, 2019, 12:16 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71664/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2019, 12:16 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-10009
>     https://issues.apache.org/jira/browse/MESOS-10009
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This implements the OpenSSL basic I/O abstraction based on the
> libprocess event loop.  This BIO wraps a socket and handles the
> reading/writing, using io::read and io::write.
> 
> This BIO can be passed into an SSL context to enable usage of
> SSL translation functions like SSL_read and SSL_write.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/ssl/socket_wrapper.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71664/diff/3/
> 
> 
> Testing
> -------
> 
> cmake --build . --target process
> 
> A tiny bit of testing next patch.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 71664: SSL Wrapper: Implemented BIO for SSL socket wrapper.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71664/#review218800
-----------------------------------------------------------




3rdparty/libprocess/src/ssl/socket_wrapper.cpp
Lines 115 (patched)
<https://reviews.apache.org/r/71664/#comment306665>

    Nit: end line with a period.



3rdparty/libprocess/src/ssl/socket_wrapper.cpp
Lines 166-167 (patched)
<https://reviews.apache.org/r/71664/#comment306669>

    This is a bit hard for me to understand, maybe something like "This assumption allows us to avoid copying the read bytes every time this function is called."



3rdparty/libprocess/src/ssl/socket_wrapper.cpp
Lines 212-215 (patched)
<https://reviews.apache.org/r/71664/#comment306670>

    Are we sure that there are no ill effects of immediately returning 1 here, even when there may still be data left to be flushed?


- Greg Mann


On Nov. 19, 2019, 8:16 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71664/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2019, 8:16 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-10009
>     https://issues.apache.org/jira/browse/MESOS-10009
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This implements the OpenSSL basic I/O abstraction based on the
> libprocess event loop.  This BIO wraps a socket and handles the
> reading/writing, using io::read and io::write.
> 
> This BIO can be passed into an SSL context to enable usage of
> SSL translation functions like SSL_read and SSL_write.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/ssl/socket_wrapper.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71664/diff/3/
> 
> 
> Testing
> -------
> 
> cmake --build . --target process
> 
> A tiny bit of testing next patch.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 71664: SSL Wrapper: Implemented BIO for SSL socket wrapper.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Nov. 26, 2019, 1:59 p.m., Greg Mann wrote:
> > 3rdparty/libprocess/src/ssl/socket_wrapper.cpp
> > Lines 212-215 (patched)
> > <https://reviews.apache.org/r/71664/diff/3/?file=2174350#file2174350line212>
> >
> >     I wonder if openssl's internal calls to `BIO_flush()` will retry if the call fails... perhaps we could synchronously return zero here if there are pending writes, and return 1 if everything has been written?

No.  If this call returns 0 (or anything less than 1), the SSL library will consider this a `SSL_ERROR_SYSCALL` and will refuse to make any further progress, thereby shutting down the socket.  I haven't found any way around this, but I also have not seen any consequences from lying about the flush either.


- Joseph


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


On Nov. 19, 2019, 12:16 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71664/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2019, 12:16 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-10009
>     https://issues.apache.org/jira/browse/MESOS-10009
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This implements the OpenSSL basic I/O abstraction based on the
> libprocess event loop.  This BIO wraps a socket and handles the
> reading/writing, using io::read and io::write.
> 
> This BIO can be passed into an SSL context to enable usage of
> SSL translation functions like SSL_read and SSL_write.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/ssl/socket_wrapper.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71664/diff/3/
> 
> 
> Testing
> -------
> 
> cmake --build . --target process
> 
> A tiny bit of testing next patch.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 71664: SSL Wrapper: Implemented BIO for SSL socket wrapper.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71664/#review218804
-----------------------------------------------------------




3rdparty/libprocess/src/ssl/socket_wrapper.cpp
Lines 212-215 (patched)
<https://reviews.apache.org/r/71664/#comment306672>

    I wonder if openssl's internal calls to `BIO_flush()` will retry if the call fails... perhaps we could synchronously return zero here if there are pending writes, and return 1 if everything has been written?


- Greg Mann


On Nov. 19, 2019, 8:16 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71664/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2019, 8:16 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-10009
>     https://issues.apache.org/jira/browse/MESOS-10009
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This implements the OpenSSL basic I/O abstraction based on the
> libprocess event loop.  This BIO wraps a socket and handles the
> reading/writing, using io::read and io::write.
> 
> This BIO can be passed into an SSL context to enable usage of
> SSL translation functions like SSL_read and SSL_write.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/ssl/socket_wrapper.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71664/diff/3/
> 
> 
> Testing
> -------
> 
> cmake --build . --target process
> 
> A tiny bit of testing next patch.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 71664: SSL Wrapper: Implemented BIO for SSL socket wrapper.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71664/#review219045
-----------------------------------------------------------


Ship it!




Ship It!

- Greg Mann


On Dec. 16, 2019, 9:56 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71664/
> -----------------------------------------------------------
> 
> (Updated Dec. 16, 2019, 9:56 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-10009
>     https://issues.apache.org/jira/browse/MESOS-10009
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This implements the OpenSSL basic I/O abstraction based on the
> libprocess event loop.  This BIO wraps a socket and handles the
> reading/writing, using io::read and io::write.
> 
> This BIO can be passed into an SSL context to enable usage of
> SSL translation functions like SSL_read and SSL_write.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/ssl/openssl_socket.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71664/diff/5/
> 
> 
> Testing
> -------
> 
> cmake --build . --target process
> 
> A tiny bit of testing next patch.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 71664: SSL Wrapper: Implemented BIO for SSL socket wrapper.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71664/
-----------------------------------------------------------

(Updated Dec. 16, 2019, 1:56 p.m.)


Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till Toenshoff.


Changes
-------

Mostly commenting changes.


Bugs: MESOS-10009
    https://issues.apache.org/jira/browse/MESOS-10009


Repository: mesos


Description
-------

This implements the OpenSSL basic I/O abstraction based on the
libprocess event loop.  This BIO wraps a socket and handles the
reading/writing, using io::read and io::write.

This BIO can be passed into an SSL context to enable usage of
SSL translation functions like SSL_read and SSL_write.


Diffs (updated)
-----

  3rdparty/libprocess/src/ssl/openssl_socket.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/71664/diff/5/

Changes: https://reviews.apache.org/r/71664/diff/4-5/


Testing
-------

cmake --build . --target process

A tiny bit of testing next patch.


Thanks,

Joseph Wu


Re: Review Request 71664: SSL Wrapper: Implemented BIO for SSL socket wrapper.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71664/#review219032
-----------------------------------------------------------




3rdparty/libprocess/src/ssl/openssl_socket.cpp
Lines 45 (patched)
<https://reviews.apache.org/r/71664/#comment307062>

    Maybe rename this now that the implementation has been renamed? A couple random ideas: `OpenSSLBIOData`, `LibprocessBIOData`



3rdparty/libprocess/src/ssl/openssl_socket.cpp
Lines 48 (patched)
<https://reviews.apache.org/r/71664/#comment307064>

    s/SSLSocketWrapper/OpenSSLSocketImpl/
    
    Here and elsewhere



3rdparty/libprocess/src/ssl/openssl_socket.cpp
Lines 165-170 (patched)
<https://reviews.apache.org/r/71664/#comment307067>

    Who provides this guarantee? Might this break if the libprocess callsites change in the future?



3rdparty/libprocess/src/ssl/openssl_socket.cpp
Lines 166-167 (patched)
<https://reviews.apache.org/r/71664/#comment307066>

    s/onto same/onto the same/



3rdparty/libprocess/src/ssl/openssl_socket.cpp
Lines 188 (patched)
<https://reviews.apache.org/r/71664/#comment307065>

    Could you include a comment in/around this function explaining a bit more how we determined which enums needed to be handled here, and which didn't?


- Greg Mann


On Dec. 10, 2019, 11:52 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71664/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2019, 11:52 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-10009
>     https://issues.apache.org/jira/browse/MESOS-10009
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This implements the OpenSSL basic I/O abstraction based on the
> libprocess event loop.  This BIO wraps a socket and handles the
> reading/writing, using io::read and io::write.
> 
> This BIO can be passed into an SSL context to enable usage of
> SSL translation functions like SSL_read and SSL_write.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/ssl/openssl_socket.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71664/diff/4/
> 
> 
> Testing
> -------
> 
> cmake --build . --target process
> 
> A tiny bit of testing next patch.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 71664: SSL Wrapper: Implemented BIO for SSL socket wrapper.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71664/
-----------------------------------------------------------

(Updated Dec. 10, 2019, 3:52 p.m.)


Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till Toenshoff.


Changes
-------

Addressed comments...
Most notably deleted some BIO_ctrl methods which are unnecessary on further investigation.


Bugs: MESOS-10009
    https://issues.apache.org/jira/browse/MESOS-10009


Repository: mesos


Description
-------

This implements the OpenSSL basic I/O abstraction based on the
libprocess event loop.  This BIO wraps a socket and handles the
reading/writing, using io::read and io::write.

This BIO can be passed into an SSL context to enable usage of
SSL translation functions like SSL_read and SSL_write.


Diffs (updated)
-----

  3rdparty/libprocess/src/ssl/openssl_socket.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/71664/diff/4/

Changes: https://reviews.apache.org/r/71664/diff/3-4/


Testing
-------

cmake --build . --target process

A tiny bit of testing next patch.


Thanks,

Joseph Wu


Re: Review Request 71664: SSL Wrapper: Implemented BIO for SSL socket wrapper.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71664/
-----------------------------------------------------------

(Updated Nov. 19, 2019, 12:16 p.m.)


Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till Toenshoff.


Changes
-------

Removed blocking `BIO_flush`, and addressed comments about BIO_METHOD creation.


Bugs: MESOS-10009
    https://issues.apache.org/jira/browse/MESOS-10009


Repository: mesos


Description
-------

This implements the OpenSSL basic I/O abstraction based on the
libprocess event loop.  This BIO wraps a socket and handles the
reading/writing, using io::read and io::write.

This BIO can be passed into an SSL context to enable usage of
SSL translation functions like SSL_read and SSL_write.


Diffs (updated)
-----

  3rdparty/libprocess/src/ssl/socket_wrapper.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/71664/diff/3/

Changes: https://reviews.apache.org/r/71664/diff/2-3/


Testing
-------

cmake --build . --target process

A tiny bit of testing next patch.


Thanks,

Joseph Wu


Re: Review Request 71664: SSL Wrapper: Implemented BIO for SSL socket wrapper.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71664/#review218608
-----------------------------------------------------------



Notes from an offline review session:


3rdparty/libprocess/src/ssl/socket_wrapper.cpp
Lines 213-221 (patched)
<https://reviews.apache.org/r/71664/#comment306399>

    Since this is a blocking call, consider not implementing it.
    
    In tests, `BIO_flush` seems to be used in the handshake code within OpenSSL.  Since we already have retry logic waiting on these futures asynchronously, a flush would negatively affect the thread, with no real benefit.



3rdparty/libprocess/src/ssl/socket_wrapper.cpp
Lines 254-275 (patched)
<https://reviews.apache.org/r/71664/#comment306415>

    An alternative implementation would involve this set of methods:
    https://www.openssl.org/docs/man1.1.1/man3/BIO_f_ssl.html
    
    This patch chain transforms the libprocess I/O into a BIO-flavoured object consumable by OpenSSL as a source/sink.
    
    A `BIO_f_ssl` would give us a different interface, where the OpenSSL library is a filter object used by libprocess instead.


- Joseph Wu


On Oct. 30, 2019, 6:29 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71664/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2019, 6:29 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-10009
>     https://issues.apache.org/jira/browse/MESOS-10009
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This implements the OpenSSL basic I/O abstraction based on the
> libprocess event loop.  This BIO wraps a socket and handles the
> reading/writing, using io::read and io::write.
> 
> This BIO can be passed into an SSL context to enable usage of
> SSL translation functions like SSL_read and SSL_write.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/ssl/socket_wrapper.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71664/diff/2/
> 
> 
> Testing
> -------
> 
> cmake --build . --target process
> 
> A tiny bit of testing next patch.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 71664: SSL Wrapper: Implemented BIO for SSL socket wrapper.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Nov. 19, 2019, 5:28 a.m., Benno Evers wrote:
> > 3rdparty/libprocess/src/ssl/socket_wrapper.cpp
> > Lines 83 (patched)
> > <https://reviews.apache.org/r/71664/diff/2/?file=2170941#file2170941line83>
> >
> >     It seems like `synchronized()` for atomic flags is implemented as a busy wait, was this choice intentional?

Yes, the logic of this SSL socket is expected to run on multiple threads (the creator of the socket's thread and the event loop thread, and [soon] libprocess worker threads), so we need some guards here.


- Joseph


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


On Nov. 19, 2019, 12:16 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71664/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2019, 12:16 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-10009
>     https://issues.apache.org/jira/browse/MESOS-10009
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This implements the OpenSSL basic I/O abstraction based on the
> libprocess event loop.  This BIO wraps a socket and handles the
> reading/writing, using io::read and io::write.
> 
> This BIO can be passed into an SSL context to enable usage of
> SSL translation functions like SSL_read and SSL_write.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/ssl/socket_wrapper.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71664/diff/3/
> 
> 
> Testing
> -------
> 
> cmake --build . --target process
> 
> A tiny bit of testing next patch.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 71664: SSL Wrapper: Implemented BIO for SSL socket wrapper.

Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71664/#review218661
-----------------------------------------------------------


Fix it, then Ship it!





3rdparty/libprocess/src/ssl/socket_wrapper.cpp
Lines 44 (patched)
<https://reviews.apache.org/r/71664/#comment306518>

    We can replace the 57 by `BIO_get_new_index()` to avoid the conflict with libevent. (Even if we can't for some reason, we should at least choose 58 here to avoid conflict.)



3rdparty/libprocess/src/ssl/socket_wrapper.cpp
Lines 83 (patched)
<https://reviews.apache.org/r/71664/#comment306519>

    It seems like `synchronized()` for atomic flags is implemented as a busy wait, was this choice intentional?



3rdparty/libprocess/src/ssl/socket_wrapper.cpp
Lines 258 (patched)
<https://reviews.apache.org/r/71664/#comment306538>

    Should this be `get_libprocess_BIO_METHOD()` for consistency?


- Benno Evers


On Oct. 31, 2019, 1:29 a.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71664/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2019, 1:29 a.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-10009
>     https://issues.apache.org/jira/browse/MESOS-10009
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This implements the OpenSSL basic I/O abstraction based on the
> libprocess event loop.  This BIO wraps a socket and handles the
> reading/writing, using io::read and io::write.
> 
> This BIO can be passed into an SSL context to enable usage of
> SSL translation functions like SSL_read and SSL_write.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/ssl/socket_wrapper.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71664/diff/2/
> 
> 
> Testing
> -------
> 
> cmake --build . --target process
> 
> A tiny bit of testing next patch.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 71664: SSL Wrapper: Implemented BIO for SSL socket wrapper.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71664/
-----------------------------------------------------------

(Updated Oct. 30, 2019, 6:29 p.m.)


Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till Toenshoff.


Changes
-------

Reimplemented bio_libprocess_write and bio_libprocess_read to surface backpressure to the client/peer.  Instead of having a read loop, we simply buffer a single read at once.  And instead of queueing all writes indiscriminately, we only allow one pending write at once.


Bugs: MESOS-10009
    https://issues.apache.org/jira/browse/MESOS-10009


Repository: mesos


Description
-------

This implements the OpenSSL basic I/O abstraction based on the
libprocess event loop.  This BIO wraps a socket and handles the
reading/writing, using io::read and io::write.

This BIO can be passed into an SSL context to enable usage of
SSL translation functions like SSL_read and SSL_write.


Diffs (updated)
-----

  3rdparty/libprocess/src/ssl/socket_wrapper.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/71664/diff/2/

Changes: https://reviews.apache.org/r/71664/diff/1-2/


Testing
-------

cmake --build . --target process

A tiny bit of testing next patch.


Thanks,

Joseph Wu