qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Stitcher" <astitc...@apache.org>
Subject Re: Review Request 22890: Allow SSL hostname verification to be disabled in c++ client
Date Mon, 23 Jun 2014 21:21:03 GMT

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


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?


/trunk/qpid/cpp/src/qpid/client/amqp0_10/ConnectionImpl.cpp
<https://reviews.apache.org/r/22890/#comment81844>

    [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".



/trunk/qpid/cpp/src/qpid/sys/ssl/SslSocket.cpp
<https://reviews.apache.org/r/22890/#comment81843>

    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.


- Andrew Stitcher


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