You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Gordon Sim <gs...@redhat.com> on 2016/11/18 13:57:07 UTC
Review Request 53876: add explicit sni option
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53876/
-----------------------------------------------------------
Review request for qpid, Alan Conway, Justin Ross, Kenneth Giusti, and Robbie Gemmell.
Repository: qpid-proton-git
Description
-------
Ulf's fix for https://issues.apache.org/jira/browse/PROTON-1355 allows you to control the sni via the existing virtual host option. For cases where you want the sni to be different from the hostname in the open frame, this patch adds an explicit option.
Diffs
-----
proton-c/bindings/python/proton/reactor.py 3562aa9
Diff: https://reviews.apache.org/r/53876/diff/
Testing
-------
Thanks,
Gordon Sim
Re: Review Request 53876: add explicit sni option
Posted by Robbie Gemmell <ro...@gmail.com>.
> On Nov. 18, 2016, 2:48 p.m., Alan Conway wrote:
> > Do we need the explicit sni setting? i.e. are there use cases where you really need to set SSL SNI = X and AMQP/SASL host = Y where X != Y? If there are then ship it. If not then I'd prefer to drop the ssl_sni option and make virtual_host the One True Fake Hostname for all security purposes. This story is confusing enough already.
>
> Alan Conway wrote:
> To justify: I think embedding SSL in proton was a mistake from the get go, in future it should be handled externally, using the user's choice of tools, configured as normal for those tools. Proton should make it easy to integrate by providing access to relevant *AMQP* settings (virtual_host, SASL user/pass etc.) but *not* by exporting random config settings from a randomly-chosen SSL tool (openssl) as part of proton's top-level API. Openssl config is not suitable for windows SSL, the Go TLS stack, native python/ruby SSL etc. I know SNI is common to all SSL stacks, but SSL is not the only encryption layer in the world. I want to get proton back to doing just AMQP and provide extension points to integrate whatever protocol layers the user wants. It's not realistic now so if we need this feature short term go ahead. However someday I will cut the Gordian Knot so I'd like to avoid expanding the amount of pesky non-AMQP config I will have to untangle from the top-level API.
I think Gordon was trying to account for my mentioning the previous change was altering the existing behaviour in a way that it would no longer be possible to have the connection hostname and SNI hostname details match while changing the AMQP sasl/open host, quite possibly making the python bits act differently from everything else (including the .net client Ken referenced). I agree it would often make sense for them all to be the same, though at that point I'd also tend toward having the connection host match so that you didnt need to set anything else at all.
- Robbie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53876/#review156298
-----------------------------------------------------------
On Nov. 18, 2016, 3:26 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53876/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2016, 3:26 p.m.)
>
>
> Review request for qpid, Alan Conway, Chug Rolke, Justin Ross, Kenneth Giusti, and Robbie Gemmell.
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> Ulf's fix for https://issues.apache.org/jira/browse/PROTON-1355 allows you to control the sni via the existing virtual host option. For cases where you want the sni to be different from the hostname in the open frame, this patch adds an explicit option.
>
>
> Diffs
> -----
>
> proton-c/bindings/python/proton/reactor.py 3562aa9
>
> Diff: https://reviews.apache.org/r/53876/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 53876: add explicit sni option
Posted by Alan Conway <ac...@redhat.com>.
> On Nov. 18, 2016, 2:48 p.m., Alan Conway wrote:
> > Do we need the explicit sni setting? i.e. are there use cases where you really need to set SSL SNI = X and AMQP/SASL host = Y where X != Y? If there are then ship it. If not then I'd prefer to drop the ssl_sni option and make virtual_host the One True Fake Hostname for all security purposes. This story is confusing enough already.
To justify: I think embedding SSL in proton was a mistake from the get go, in future it should be handled externally, using the user's choice of tools, configured as normal for those tools. Proton should make it easy to integrate by providing access to relevant *AMQP* settings (virtual_host, SASL user/pass etc.) but *not* by exporting random config settings from a randomly-chosen SSL tool (openssl) as part of proton's top-level API. Openssl config is not suitable for windows SSL, the Go TLS stack, native python/ruby SSL etc. I know SNI is common to all SSL stacks, but SSL is not the only encryption layer in the world. I want to get proton back to doing just AMQP and provide extension points to integrate whatever protocol layers the user wants. It's not realistic now so if we need this feature short term go ahead. However someday I will cut the Gordian Knot so I'd like to avoid expanding the amount of pesky non-AMQP config I will have to untangle from the top-level API.
- Alan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53876/#review156298
-----------------------------------------------------------
On Nov. 18, 2016, 1:58 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53876/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2016, 1:58 p.m.)
>
>
> Review request for qpid, Alan Conway, Justin Ross, Kenneth Giusti, and Robbie Gemmell.
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> Ulf's fix for https://issues.apache.org/jira/browse/PROTON-1355 allows you to control the sni via the existing virtual host option. For cases where you want the sni to be different from the hostname in the open frame, this patch adds an explicit option.
>
>
> Diffs
> -----
>
> proton-c/bindings/python/proton/reactor.py 3562aa9
>
> Diff: https://reviews.apache.org/r/53876/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 53876: add explicit sni option
Posted by Alan Conway <ac...@redhat.com>.
> On Nov. 18, 2016, 2:48 p.m., Alan Conway wrote:
> > Do we need the explicit sni setting? i.e. are there use cases where you really need to set SSL SNI = X and AMQP/SASL host = Y where X != Y? If there are then ship it. If not then I'd prefer to drop the ssl_sni option and make virtual_host the One True Fake Hostname for all security purposes. This story is confusing enough already.
>
> Alan Conway wrote:
> To justify: I think embedding SSL in proton was a mistake from the get go, in future it should be handled externally, using the user's choice of tools, configured as normal for those tools. Proton should make it easy to integrate by providing access to relevant *AMQP* settings (virtual_host, SASL user/pass etc.) but *not* by exporting random config settings from a randomly-chosen SSL tool (openssl) as part of proton's top-level API. Openssl config is not suitable for windows SSL, the Go TLS stack, native python/ruby SSL etc. I know SNI is common to all SSL stacks, but SSL is not the only encryption layer in the world. I want to get proton back to doing just AMQP and provide extension points to integrate whatever protocol layers the user wants. It's not realistic now so if we need this feature short term go ahead. However someday I will cut the Gordian Knot so I'd like to avoid expanding the amount of pesky non-AMQP config I will have to untangle from the top-level API.
>
> Robbie Gemmell wrote:
> I think Gordon was trying to account for my mentioning the previous change was altering the existing behaviour in a way that it would no longer be possible to have the connection hostname and SNI hostname details match while changing the AMQP sasl/open host, quite possibly making the python bits act differently from everything else (including the .net client Ken referenced). I agree it would often make sense for them all to be the same, though at that point I'd also tend toward having the connection host match so that you didnt need to set anything else at all.
Let's say C is socket.connect address, A is the AMQP/SASL hostname, S is the SSL hostname.
The likely scenarios in my mind are:
- direct connection, working DNS: C = A = S = DNS name
- indirect connection (router) or broken client DNS: C is not the target host, or not a name. A = S = DNS (server rarely doesn't have at least a pseudo-DNS name)
I can't think of a reason why you would configure the *same service* using *different* hostnames for SSL certs, AMQP/SASL and DNS. But I concede I may lack imagination.
SHIP IT.
- Alan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53876/#review156298
-----------------------------------------------------------
On Nov. 18, 2016, 3:26 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53876/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2016, 3:26 p.m.)
>
>
> Review request for qpid, Alan Conway, Chug Rolke, Justin Ross, Kenneth Giusti, and Robbie Gemmell.
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> Ulf's fix for https://issues.apache.org/jira/browse/PROTON-1355 allows you to control the sni via the existing virtual host option. For cases where you want the sni to be different from the hostname in the open frame, this patch adds an explicit option.
>
>
> Diffs
> -----
>
> proton-c/bindings/python/proton/reactor.py 3562aa9
>
> Diff: https://reviews.apache.org/r/53876/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 53876: add explicit sni option
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53876/#review156298
-----------------------------------------------------------
Do we need the explicit sni setting? i.e. are there use cases where you really need to set SSL SNI = X and AMQP/SASL host = Y where X != Y? If there are then ship it. If not then I'd prefer to drop the ssl_sni option and make virtual_host the One True Fake Hostname for all security purposes. This story is confusing enough already.
- Alan Conway
On Nov. 18, 2016, 1:58 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53876/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2016, 1:58 p.m.)
>
>
> Review request for qpid, Alan Conway, Justin Ross, Kenneth Giusti, and Robbie Gemmell.
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> Ulf's fix for https://issues.apache.org/jira/browse/PROTON-1355 allows you to control the sni via the existing virtual host option. For cases where you want the sni to be different from the hostname in the open frame, this patch adds an explicit option.
>
>
> Diffs
> -----
>
> proton-c/bindings/python/proton/reactor.py 3562aa9
>
> Diff: https://reviews.apache.org/r/53876/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 53876: add explicit sni option
Posted by Robbie Gemmell <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53876/#review156297
-----------------------------------------------------------
Seems good to me.
- Robbie Gemmell
On Nov. 18, 2016, 1:58 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53876/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2016, 1:58 p.m.)
>
>
> Review request for qpid, Alan Conway, Justin Ross, Kenneth Giusti, and Robbie Gemmell.
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> Ulf's fix for https://issues.apache.org/jira/browse/PROTON-1355 allows you to control the sni via the existing virtual host option. For cases where you want the sni to be different from the hostname in the open frame, this patch adds an explicit option.
>
>
> Diffs
> -----
>
> proton-c/bindings/python/proton/reactor.py 3562aa9
>
> Diff: https://reviews.apache.org/r/53876/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 53876: add explicit sni option
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53876/#review156299
-----------------------------------------------------------
Looks good to me.
Can you add Chuck Rolke to the reviewer's list? He's found issues with how hostname is used w.r.t. Azure - might be a good idea to have him take a look at it (though I doubt it will affect things - better safe than sorry...)
- Kenneth Giusti
On Nov. 18, 2016, 1:58 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53876/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2016, 1:58 p.m.)
>
>
> Review request for qpid, Alan Conway, Justin Ross, Kenneth Giusti, and Robbie Gemmell.
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> Ulf's fix for https://issues.apache.org/jira/browse/PROTON-1355 allows you to control the sni via the existing virtual host option. For cases where you want the sni to be different from the hostname in the open frame, this patch adds an explicit option.
>
>
> Diffs
> -----
>
> proton-c/bindings/python/proton/reactor.py 3562aa9
>
> Diff: https://reviews.apache.org/r/53876/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 53876: add explicit sni option
Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53876/
-----------------------------------------------------------
(Updated Nov. 18, 2016, 1:58 p.m.)
Review request for qpid, Alan Conway, Justin Ross, Kenneth Giusti, and Robbie Gemmell.
Repository: qpid-proton-git
Description
-------
Ulf's fix for https://issues.apache.org/jira/browse/PROTON-1355 allows you to control the sni via the existing virtual host option. For cases where you want the sni to be different from the hostname in the open frame, this patch adds an explicit option.
Diffs (updated)
-----
proton-c/bindings/python/proton/reactor.py 3562aa9
Diff: https://reviews.apache.org/r/53876/diff/
Testing
-------
Thanks,
Gordon Sim