> On June 14, 2019, 12:44 a.m., Till Toenshoff wrote: > > 3rdparty/libprocess/src/openssl.cpp > > Line 738 (original), 745 (patched) > > <https://reviews.apache.org/r/70748/diff/2/?file=2147980#file2147980line745> > > > > This means that when a certificate gets provided, we always validate > > it. Entirely straightforward but may be too much of a potential problem > > source in the real world, I fear. > > > > IIUC then client certificate validation is exactly a non goal of this > > review-chain as client certificates in real world scenarios often times do > > not match. My feeling is that we have a great chance of reducing potential > > problem sources by working more like this; > > > > CLIENT mode (we initiate a connection) > > - when we receive a peer certificate, we verify it > > - when we receive no peer certificate, we error out > > SERVER mode (we accept a connection) > > - `require` is set to false > > - when we receive a peer certificate, we ignore it > > - when we receive no peer certificate, we ignore it > > > > We would return early as done for verify above like this; > > ``` > > if (mode == Mode::SERVER && !ssl_flags->require_cert) { > > return Nothing(); > > } > > ``` > > > > Then relax this part towards; > > ``` > > if (cert == nullptr) { > > return Error("Peer did not provide certificate"); > > } > > ```
Sounds good to me. I've only implemented the minimal change for now, to see if this is enough in principle. Before we actually commit this, we should also change the implication during flag initialization (i.e. `require_cert` => `verify_cert`) and look at the other usages of verify/require. - Benno ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70748/#review215878 ----------------------------------------------------------- On June 14, 2019, 11:40 a.m., Benno Evers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70748/ > ----------------------------------------------------------- > > (Updated June 14, 2019, 11:40 a.m.) > > > Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, > and Till Toenshoff. > > > Bugs: MESOS-9810 > https://issues.apache.org/jira/browse/MESOS-9810 > > > Repository: mesos > > > Description > ------- > > When in SSL client mode and `LIBPROCESS_SSL_VERIFY_CERT=true` has > been set, enforce that the server actually presents a certificate > that can be verified. > > Note that in most cases, the TLS stack would rejected the connection > before the code ever reaches `openssl::verify()`, since the TLS > specification that a server MUST always send a certificate unless > an anonymous cipher is used. > > > Diffs > ----- > > 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/ssl_tests.cpp > 6b8496aeeed79ae1bd39d7013f4f403b248fdd4c > > > Diff: https://reviews.apache.org/r/70748/diff/3/ > > > Testing > ------- > > > Thanks, > > Benno Evers > >
