Fix tls options #22
Fix tls options #22
Conversation
| @@ -40,9 +40,6 @@ def setup_rabbitmq_connection_config | |||
| # Specify TLS version if using TLS e.g "TLSv1.2" | |||
| config :ssl, :validate => :string | |||
|
|
|||
| # Validate SSL certificate | |||
| config :verify_ssl, :validate => :boolean, :default => false | |||
There was a problem hiding this comment.
even if this did nothing, should we go thorugh the deprecated-then-obsolete process to help users (who have this setting) not be confused at its disappearance?
We could keep it and mark it :deprecated => "This setting never actually did anything. <insert recommendation here>" -- thoughts?
There was a problem hiding this comment.
@jordansissel I had the same thought at first. My worry though is that if someone sets it to true and it does nothing that's actually dangerous, so we should probably just remove it.
A larger problem we should tackle (in a different PR) is perhaps making SSL validation the default. This is actually kind of tricky in RabbitMQ Java, so I think its not something we should do immediately. But we should do it.
Your thoughts?
|
@jordansissel I made a variety of changes that should address all your concerns. |
| s[:tls_certificate_path] = @tls_certificate_path || "" | ||
| s[:tls_certificate_password] = @tls_certificate_password || "" | ||
|
|
||
| # This MUST be passed as a string or the cert options will be ignored |
There was a problem hiding this comment.
I think you can remove this comment now, right? or just make it set on the s[:tls] line instead.
|
Made one small comment. LGTM once resolved! <3 |
This removes the non-functionaly verify_ssl option. It also makes the tls_certificate_path and tls_certificate_password options work correctly when 'true' is passed as the TLS parameter instead of an explicit string value.
|
Thanks for the review @jordansissel ! |
This removes the non-functional verify_ssl option.
It also makes the tls_certificate_path and tls_certificate_password options work
correctly when 'true' is passed as the TLS parameter instead of an explicit string value.
This also puts in some test files for manually testing SSL stuff with rabbitmq