TLS Peer verification: ssl_verify_peer does not support multiple CAs

[h4xnoodle]

Currently using 0.8.2

In the client here: https://github.com/nats-io/ruby-nats/blob/master/lib/nats/client.rb#L830, only the first pem found in options[:tls][:ca_file] is used for OpenSSL::X509::Certificate. Therefore, only the first CA in a file of potentially more than one, is used to verify the incoming cert here.

I would also recommend that we use the OpenSSL::X509::Store to verify like so:

def ssl_verify_peer(cert)
  incoming = OpenSSL::X509::Certificate.new(cert)
  store = OpenSSL::X509::Store.new
  store.set_default_paths
  store.add_file @options[:tls][:ca_file]
  result = store.verify incoming
  err_cb.call(NATS::ConnectError.new("TLS Verification failed checking issuer based on CA %s" % @options[:tls][:ca_file])) unless result
  result
end

For further context, a use case for this is certificate rotation. mTLS fails easily when more than one CA is not considered during the transitional stage.

[wallyqs]

Thanks for the report, I will take a look

[h4xnoodle]

I'm currently writing tests for a PR that I could be ready to submit pretty soon. I've been using the same code as a monkey-patch in the real world and it's working out well.

[wallyqs]

That's great, thanks a lot!

[wallyqs]

Fixed via #152

[pivotal-jamil-shamy]

@wallyqs would there be a possibility to have a new release of the GEM that includes this fix sometime soon ?

thanks

[wallyqs]

yes will be doing a release this week

[h4xnoodle]

Left the same comment in the PR:

One small gotcha that we're finding with this, is that if the CAs in the CA file have the same SANs/CN, then OpenSSL::X509::Store add_file or the verification process seems to ignore the second CA with the same SAN.

I'm working on a solution that either allows duplicates easily at the store level, or if openssl config is required.

This PR still helps in the case where there are CAs with different SANs, which is very common anyways.

@pivotal-jamil-shamy because we use the same SANs we'll need to figure out what the proper solution is. The PR is still useful for others without the same SANs.

[wallyqs]

@h4xnoodle @pivotal-jamil-shamy new gem is published now with the changes:
https://github.com/nats-io/ruby-nats/releases/tag/v0.9.2
Thanks again for the contribution! 🙇

[pivotal-jamil-shamy]

@wallyqs thank you !