> 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
> 
>

Reply via email to