Re: Review Request 26829: CPP Broker and client - disable SSL protocols SSLv2 and SSLv3
> On Oct. 20, 2014, 8:44 a.m., Gordon Sim wrote: > > Looks fine to me, though I am no expert on NSS. One thing just to note, is > > that when the SSL port and the plain TCP pport are the same, there is a > > different codepath used that includes some version checking (see > > isSslStream() in qpid/sys/ssl/SslSocket.cpp). That may be in addition to > > NSS checks, rather than instead of, so may not require any further fixes. Thanks for the head's up Gordon - I checked out that path and I think we're covered. The patch disables SSLv3 for the NSS library as a whole, so SSL sockets created via that version check path will have SSLv3 disabled (the version check also takes into account TLSv 1.0-1.2). - Kenneth --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26829/#review57310 --- On Oct. 16, 2014, 9:50 p.m., Kenneth Giusti wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26829/ > --- > > (Updated Oct. 16, 2014, 9:50 p.m.) > > > Review request for qpid and Gordon Sim. > > > Bugs: qpid-6160 > https://issues.apache.org/jira/browse/qpid-6160 > > > Repository: qpid > > > Description > --- > > Sets the minimum protocol level for SSL to TLSv1.0 > > > Diffs > - > > trunk/qpid/cpp/src/qpid/sys/ssl/util.cpp 1632383 > > Diff: https://reviews.apache.org/r/26829/diff/ > > > Testing > --- > > Used openssl to test for rejection, confirmed with wireshark traces. > > > Thanks, > > Kenneth Giusti > >
Re: Review Request 26829: CPP Broker and client - disable SSL protocols SSLv2 and SSLv3
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26829/#review57310 --- Ship it! Looks fine to me, though I am no expert on NSS. One thing just to note, is that when the SSL port and the plain TCP pport are the same, there is a different codepath used that includes some version checking (see isSslStream() in qpid/sys/ssl/SslSocket.cpp). That may be in addition to NSS checks, rather than instead of, so may not require any further fixes. - Gordon Sim On Oct. 16, 2014, 9:50 p.m., Kenneth Giusti wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26829/ > --- > > (Updated Oct. 16, 2014, 9:50 p.m.) > > > Review request for qpid and Gordon Sim. > > > Bugs: qpid-6160 > https://issues.apache.org/jira/browse/qpid-6160 > > > Repository: qpid > > > Description > --- > > Sets the minimum protocol level for SSL to TLSv1.0 > > > Diffs > - > > trunk/qpid/cpp/src/qpid/sys/ssl/util.cpp 1632383 > > Diff: https://reviews.apache.org/r/26829/diff/ > > > Testing > --- > > Used openssl to test for rejection, confirmed with wireshark traces. > > > Thanks, > > Kenneth Giusti > >
Re: Review Request 26829: CPP Broker and client - disable SSL protocols SSLv2 and SSLv3
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26829/#review57130 --- I run various tests that all passed, including: - test script from https://access.redhat.com/articles/1232123 (returns "SSL 3.0 disabled") - 3.1 (TLS 1.0) client works well - joint listening port for AMQP and AMQPS works well (both above scenarios tried) >From my perspective, the path is right. I would mark it as "Ship it", if I >knew SSL/NSS better. - Pavel Moravec On Oct. 16, 2014, 9:50 p.m., Kenneth Giusti wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26829/ > --- > > (Updated Oct. 16, 2014, 9:50 p.m.) > > > Review request for qpid and Gordon Sim. > > > Bugs: qpid-6160 > https://issues.apache.org/jira/browse/qpid-6160 > > > Repository: qpid > > > Description > --- > > Sets the minimum protocol level for SSL to TLSv1.0 > > > Diffs > - > > trunk/qpid/cpp/src/qpid/sys/ssl/util.cpp 1632383 > > Diff: https://reviews.apache.org/r/26829/diff/ > > > Testing > --- > > Used openssl to test for rejection, confirmed with wireshark traces. > > > Thanks, > > Kenneth Giusti > >
