Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support SSL connections #3

Closed
iconara opened this issue May 15, 2014 · 12 comments
Closed

Support SSL connections #3

iconara opened this issue May 15, 2014 · 12 comments

Comments

@iconara
Copy link
Owner

iconara commented May 15, 2014

Just make sure that this is not an issue, but it actually looks ok.

@iconara
Copy link
Owner Author

iconara commented May 24, 2014

It seems like IO.select is partially broken when it comes to OpenSSL::SSL::SSLSocket. If a #read_nonblock doesn't read all available data IO.select will still block – even though there is data available to read.

One solution is to loop until #read_nonblock raises OpenSSL::SSL::SSLError (it doesn't raise Errno::EWOULDBLOCK or Errno::EAGAIN like you would expect), but this feels wrong and inefficient, we want to avoid that exception being raised. We're using IO.select to not have to do flow control with exceptions.

Another way, which seems to work (in a very limited test) is to use OpenSSL::SSL::SSLSocket#pending. The docs say this is the "number of bytes that are immediately available for reading". Unfortunately it is zero before #read_nonblock has been called… it seems that its the call to #read_nonblock that populates it. But it can still be useful since we can do something like this:

while true
  r, _, _ = IO.select([c])
  r && r.each do |s|
    read_size = 2**16
    data = ''
    while read_size > 0
      data << s.read_nonblock(read_size)
      read_size = s.pending
    end
    # process data
  end
end

I.e. read an initial chunk, which is what we would have done anyway, and then read another chunk if #pending returns non-zero. With non-SSL sockets we would have relied on IO.select returning again immediately if there was more than the initial chunk of data available.

@iconara iconara mentioned this issue May 24, 2014
@iconara
Copy link
Owner Author

iconara commented May 24, 2014

Turns out that it's a bit more complicated: JRuby doesn't support #pending. I decided on falling back on exceptions in JRuby: https://github.com/iconara/ione/blob/ssl_support/lib/ione/io/ssl_connection.rb#L41-L50

@iconara
Copy link
Owner Author

iconara commented May 26, 2014

Another fun fact: OpenSSL::SSL::SSLSocket#connect_nonblock raises IO::WaitWritable in MRI and IO::WaitReadable in JRuby: jruby/jruby#1716

@jasonmk
Copy link

jasonmk commented Sep 4, 2014

What's the most recent status on this issue? Are there any plans to merge it into master any time soon? Is there anything specific I could help with to make that happen?

Thanks!

@iconara
Copy link
Owner Author

iconara commented Sep 5, 2014

I'd really like to, and if I get some help testing it I can get it out in v1.2, but as I said in iconara/cql-rb#110 I really need people to test it, and no one has stepped up. It's very hard to automate testing for this, and I just don't feel confident that the limited testing that I can do myself is enough. I'd like it for people who actually need to run Cassandra, for example, with encryption to test it and see that it works for them.

@jasonmk
Copy link

jasonmk commented Sep 5, 2014

Ok, that makes sense. I'm finally getting time to start looking at adding encryption to my project, so I'll definitely be pulling it in. I'm still on Cassandra 1.2 for now, but I don't think the encryption support has changed much between that and 2.0. I'll let you know what I find. Has the ssl_support branch been kept up to date with the other 1.2 changes?

@iconara
Copy link
Owner Author

iconara commented Sep 5, 2014

It should be fine to run cql-rb with the encryption_support branch and ione with the ssl_support branch with Cassandra 1.2. I haven't rebased these on top of the latest, but there's no reason it shouldn't work. There's been no incompatibile changes.

@iconara
Copy link
Owner Author

iconara commented Sep 5, 2014

I just rebased ssl_support on top of master and pushed it up as ssl_support_on_1.2 if you want to try it.

@iconara
Copy link
Owner Author

iconara commented Sep 5, 2014

And thanks for helping out!

@jasonmk
Copy link

jasonmk commented Sep 5, 2014

Thanks for the rebase. I was going to attempt it, but I'm far from familiar with the code. Integrating it now and will let you know what I find.

@iconara
Copy link
Owner Author

iconara commented Sep 9, 2014

For anyone else watching this, the SSL features have been released with v1.2.0.pre2

@iconara
Copy link
Owner Author

iconara commented Oct 31, 2014

This has been released with v1.2.0.

@iconara iconara closed this as completed Oct 31, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants