stompgem / stomp

A ruby gem for sending and receiving messages from a Stomp protocol compliant message queue. Includes: failover logic, ssl support.
http://stomp.github.com
Apache License 2.0
152 stars 80 forks source link

Add ability to take cert_text and key_text as SSL params #154

Closed krsibbald closed 5 years ago

krsibbald commented 5 years ago

Adds support to take a param cert_text and key_text for SSL param creation, instead of just being able to load that information from a file.

gmallard commented 5 years ago

Good idea. I am surprised this has not been previously requested. Good start. However, so far ..........

Item 1. Attempting to do a little regression test from your branch gets me:

$ ruby -I  ../../../lib ssl_uc3_from_files.rb 
SSLUC3 Connect host: localhost, port: 61612
/ad3/gma/ad3/stomp-repos/stompgem/lib/stomp/sslparams.rb:70:in `initialize': undefined method `blank?' for nil:NilClass (NoMethodError)
    from ssl_uc3_from_files.rb:45:in `new'
    from ssl_uc3_from_files.rb:45:in `run'
    from ssl_uc3_from_files.rb:69:in `<main>'

I am pretty sure #blank? is added by Rail's Active Support. I do not run Rails here, and many people do not.

You are also using .blank? in netio.rb.

Please figure out a different way to do that check.

Item 2. I am thinking that SSL processing should allow a mix and match of file and text. That is:

or

I do not think current logic will allow that. Please think about adding that capability.

Regards, Guy

gmallard commented 5 years ago

In ssl_test.rb the word BEGIN is misspelled.

krsibbald commented 5 years ago

@gmallard Thanks! I'll get working on those changes

gmallard commented 5 years ago

One thought. Active support defines blank? like this:

class Object
    def blank?
        respond_to?(:empty?) ? !!empty? : !self
    end   
end

You might think about just copying that.

gmallard commented 5 years ago

So now when I try and run the files example with your latest, I get:

$ ruby -I ../../../lib/ ssl_uc3_from_files.rb 
SSLUC3 Connect host: localhost, port: 61612
SSLOPTS: #<Stomp::SSLParams:0x000000014d78e8 @ts_files=nil, @cert_file="/ad3/gma/ad3/sslwork/2017-01/client.crt", @key_file="/ad3/gma/ad3/sslwork/2017-01/client.key", @cert_text=nil, @key_text=nil, @key_password=nil, @ciphers=nil, @use_ruby_ciphers=false, @ssl_ctxopts=nil>
Connect starts, SSL Use Case 3
/ad3/gma/ad3/stomp-repos/stompgem/lib/connection/netio.rb:374:in `open_ssl_socket'
/ad3/gma/ad3/stomp-repos/stompgem/lib/connection/netio.rb:482:in `open_socket'
/ad3/gma/ad3/stomp-repos/stompgem/lib/connection/utils.rb:116:in `block in socket'
/ad3/gma/ad3/stomp-repos/stompgem/lib/connection/utils.rb:109:in `synchronize'
/ad3/gma/ad3/stomp-repos/stompgem/lib/connection/utils.rb:109:in `socket'
/ad3/gma/ad3/stomp-repos/stompgem/lib/stomp/connection.rb:173:in `initialize'
ssl_uc3_from_files.rb:59:in `new'
ssl_uc3_from_files.rb:59:in `run'
ssl_uc3_from_files.rb:69:in `<main>'
/ad3/gma/ad3/stomp-repos/stompgem/lib/connection/netio.rb:374:in `open_ssl_socket': certificate and key files are both required (Stomp::Error::SSLClientParamsError)
    from /ad3/gma/ad3/stomp-repos/stompgem/lib/connection/netio.rb:482:in `open_socket'
    from /ad3/gma/ad3/stomp-repos/stompgem/lib/connection/utils.rb:116:in `block in socket'
    from /ad3/gma/ad3/stomp-repos/stompgem/lib/connection/utils.rb:109:in `synchronize'
    from /ad3/gma/ad3/stomp-repos/stompgem/lib/connection/utils.rb:109:in `socket'
    from /ad3/gma/ad3/stomp-repos/stompgem/lib/stomp/connection.rb:173:in `initialize'
    from ssl_uc3_from_files.rb:59:in `new'
    from ssl_uc3_from_files.rb:59:in `run'
    from ssl_uc3_from_files.rb:69:in `<main>'
gmallard commented 5 years ago

And additionally the SSL tests fail:

$ STOMP_TESTSSL=y ruby -I lib test/test_ssl.rb 
Loaded suite test/test_ssl
Started
.....F
================================================================================
Failure: <Stomp::Error::SSLClientParamsError> exception expected but none was thrown.
test_ssl_0050_certtext(TestSSL)
test/test_ssl.rb:104:in `test_ssl_0050_certtext'
     101:     fake_cert
     102:     ------END CERTIFICATE-----'
     103: 
  => 104:     assert_raise(Stomp::Error::SSLClientParamsError) {
     105:       _ = Stomp::SSLParams.new(:cert_text => cert_text, :key_text => key_text)
     106:     }
     107: 
================================================================================

Finished in 1.472830993 seconds.
--------------------------------------------------------------------------------
6 tests, 11 assertions, 1 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
83.3333% passed
--------------------------------------------------------------------------------
4.07 tests/s, 7.47 assertions/s
krsibbald commented 5 years ago

@gmallard I updated the ssl test and the code behind the connection example. Could you try running those and let me know if they work for you now?

gmallard commented 5 years ago

@krsibbald You are very close.

Please make these changes.

Number 1. In ssl_uc3_without_files.rb, change this:

:cert_text => cli_cert().to_s      # the client's signed certificate 

to this:

:cert_text => cli_cert_text().to_s      # the client's signed certificate 

Number 2. In ssl_uc3_without_files_ciphers.rb, change this:

      :key_file => cli_key_text().to_s,         # the client's private key, private data
      :cert_file => cli_cert_text().to_s,       # the client's signed certificate

to this:

      :key_text => cli_key_text().to_s,         # the client's private key, private data
      :cert_text => cli_cert_text().to_s,       # the client's signed certificate

Number 3. I discovered during testing that Ruby seems to add extra tabs to multi-line strings. This seems to affect key_text but not cert_text. So, in netio.rb, change this:

            if @ssl.key_text
              ctx.key  = OpenSSL::PKey::RSA.new(@ssl.key_text, @ssl.key_password)
            end

to this:

            if @ssl.key_text
              nt = @ssl.key_text.gsub(/\t/, "")
              ctx.key  = OpenSSL::PKey::RSA.new(nt, @ssl.key_password)
           end

After those changes I will test again. But this is very close to done.

Thanks, Guy

krsibbald commented 5 years ago

@gmallard Thank you for tracking down those additional errors in my tests. I fixed those errors and added the code to remove tabs from the key_text. Let me know if any additional changes are needed.