qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gordon Sim" <g...@redhat.com>
Subject Re: Review Request 22890: Allow SSL hostname verification to be disabled in c++ client
Date Tue, 24 Jun 2014 07:32:36 GMT


> On June 23, 2014, 9:21 p.m., Andrew Stitcher wrote:
> > This mostly looks fine - just a few comments below:
> > 
> > Unless you are planning to do this work for windows too you should raise a JIRA
for this work on that platform. Is there a JIRA that this relates to?

Not yet, but I'll raise one.


> On June 23, 2014, 9:21 p.m., Andrew Stitcher wrote:
> > /trunk/qpid/cpp/src/qpid/client/amqp0_10/ConnectionImpl.cpp, line 158
> > <https://reviews.apache.org/r/22890/diff/1/?file=615200#file615200line158>
> >
> >     [Personal opinion] I think that it would be easier to remember the option if
it began with "ssl-" like all the other ssl related options. I'd suggest "ssl-ignore-hostname-verification-failure".


> On June 23, 2014, 9:21 p.m., Andrew Stitcher wrote:
> > /trunk/qpid/cpp/src/qpid/sys/ssl/SslSocket.cpp, line 147
> > <https://reviews.apache.org/r/22890/diff/1/?file=615204#file615204line147>
> >
> >     I'm not sure this should be at warning level. Since the user had to specifically
request this behaviour it is expected, so warning might be too noisy - perhaps info?
> >     
> >     BTW In the failure case do we log enough information to really figure out what
is wrong? If not then maybe we should always go through this callback with the verification
flag passed in here somehow. This way we can log a better error message.

With the wrong hostname at present (or with the option off), the error message is 'Unable
to communicate securely with peer: requested domain name does not match the server's certificate.'


- Gordon


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


On June 23, 2014, 9:03 p.m., Gordon Sim wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22890/
> -----------------------------------------------------------
> 
> (Updated June 23, 2014, 9:03 p.m.)
> 
> 
> Review request for qpid, Andrew Stitcher and Pavel Moravec.
> 
> 
> Repository: qpid
> 
> 
> Description
> -------
> 
> Adds a connection option ignore_ssl_hostname_verification_failure, which if set to true
will cause a connect attempt to proceed even if the hostname connecting to does not match
the peers certificate.
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/client/ConnectionSettings.h 1604917 
>   /trunk/qpid/cpp/src/qpid/client/ConnectionSettings.cpp 1604917 
>   /trunk/qpid/cpp/src/qpid/client/SslConnector.cpp 1604917 
>   /trunk/qpid/cpp/src/qpid/client/amqp0_10/ConnectionImpl.cpp 1604917 
>   /trunk/qpid/cpp/src/qpid/messaging/ConnectionOptions.cpp 1604917 
>   /trunk/qpid/cpp/src/qpid/messaging/amqp/SslTransport.cpp 1604917 
>   /trunk/qpid/cpp/src/qpid/sys/ssl/SslSocket.h 1604917 
>   /trunk/qpid/cpp/src/qpid/sys/ssl/SslSocket.cpp 1604917 
> 
> Diff: https://reviews.apache.org/r/22890/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gordon Sim
> 
>


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