Expose SSLContext#ssl_protocol as SSLParams#protocol

[richardc]

With this commit we allow the choice of ssl protocol to be specified by the
user. This allows for mitigation of protocol-based attacks such as the current
SSLv3 POODLE by specifying SSLParams.new(:protocol => :TLSv1_2, ...)

[richardc]

On more careful thought I'm not sure if this is general enough, as really what I would to do is the equivalent of SSL_CTX_set_options(ctx, OpenSSL::SSL::OP_NO_SSLv2 | OpenSSL::SSL::OP_NO_SSLv3) in order to ask openssl to simply not negotiate SSLv2 and SSLv3.

Maybe supplying a Proc that's called on the ctx, enabling something like SSLParams.new(:customize => Proc.new { |ctx| ctx.options |= OpenSSL::SSL::OP_NO_SSLv2 | OpenSSL::SSL::OP_NO_SSLv3 }) fits. But it may be exposing OpenSSL specifics too tightly.

[gmallard]

ACK that this has been seen, and is being considered.

Something must have happened with an app that you work on to trigger this, correct? Are you in a position to elaborate?

Rule 1: it must seem likely that existing users will not be affected by any change (keep all defaults).
Rule 2: see Rule 1.

To date, we claim support for Ruby 1.8.5 .... 2.x

Do all of those SSL constants exist in all Ruby versions? The current thought is that a check that the user is doing something sane might be added.

So, raise if the user has done:

... SSLParams.new(:protocol => "junk", ... )

Because that is very likely to happen.

I will try and run some sanity check / tests this weekend.

[richardc]

It's intended as a defence against SSLv3/POODLE attacks https://securityblog.redhat.com/2014/10/15/poodle-a-ssl3-vulnerability-cve-2014-3566/

The intent is to be able to express 'don't ever negotiate to use the SSLv3 protocol', as it's the simplest mitigation to this, and the other known/suspected flaws in SSLv3.

I'm not sure what the best way of exposing the world of openssl options to the user is, this seemed the most obvious first pass, but with reflection it started to feel too specific.

[gmallard]

You are not enthusiastic about this solution.

Neither am I at this point.

Will you close this PR? Or do you intend to enhance it?

[richardc]

I was hoping for some feedback from you on what would make exposing the SSLContext for this level of configuration fit with the current interface of the stomp gem. We've worked around this for our app with a somewhat unpleasant monkey patch, so we don't need it for now.

Closing.

[gmallard]

How about providing a way for the gem user to pass parameters to
OpenSSL::SSL::SSLContext.new ?

Would that work for you?

I am still puzzling over your original PR. One line of code is:

ctx.ssl_protocol = @ssl.protocol

I do not see OpenSSL::SSL::SSLContext#ssl_protocol in any of the 7 or 8
Ruby builds I have. Where did you get that variable name?

Also, what does your 'unpleasant monkey patch' look like ?

On 11/03/2014 06:20 AM, Richard Clamp wrote:

I was hoping for some feedback from you on what would make exposing
the SSLContext for this level of configuration fit with the current
interface of the stomp gem. We've worked around this for our app with
a somewhat unpleasant monkey patch, so we don't need it for now.

Closing.

—
Reply to this email directly or view it on GitHub
#104 (comment).

[gmallard]

Yeah, when attempting to use your PR from one of my test clients, I get:

D, [2014-11-03T12:21:14.223888 #16046] DEBUG -- : SSL Connect Fail
Message: undefined method `ssl_protocol=' for
#OpenSSL::SSL::SSLContext:0x7f3f88f397f0

On 11/03/2014 06:20 AM, Richard Clamp wrote:

I was hoping for some feedback from you on what would make exposing
the SSLContext for this level of configuration fit with the current
interface of the stomp gem. We've worked around this for our app with
a somewhat unpleasant monkey patch, so we don't need it for now.

Closing.

—
Reply to this email directly or view it on GitHub
#104 (comment).

[gmallard]

@richardc - Check out the latest on the 'dev' branch. In particular:

73a654d

96336e4

There may be something there for you on this subject.