logstash-plugins / logstash-input-tcp

Apache License 2.0
35 stars 75 forks source link

SSL/TLS Settings #47

Open harbulot opened 8 years ago

harbulot commented 8 years ago

This is a follow up of issue #9. (PR #28 aimed to fix it, but didn't and introduced more confusion unfortunately.)

History

Before PR #28:

Issue #9 was asking for:

What PR #28 did:

There seems to have been some confusion regarding what the certificate chain and what the trust anchors are.

Those are fundamentally different roles, which should not be mixed up. (For a Java point of view, those would be the roles of the KeyManager and TrustManager, respectively.)

OpenSSL (at least the way it is used or emulated here), has ways to fall back to other default options when some settings are not set. This may have cause some confusion when implementing these features.

Here is how these options are mapped to the ssl_context used in the code:

These settings can apply to both the client and the server side, except ssl_context.client_ca which is server-specific. Setting ssl_context.ca_file, ssl_context.ca_path or ssl_context.cert_store on the server only makes sense on the server (a) if you're using client-certificate authentication or (b) if you don't set ssl_context.extra_chain_cert and you want the server certificate chain to be built automatically from your trust anchors.

Why separating those settings is required

This all has to do with the way client-certificate authentication is used in practice. Verifying the remote certificate when you are a server has a very different purpose from doing it when you are a client.

As a client

ssl_verify => true by default makes perfect sense here (and too often, some code forgets to verify the hostname anyway, since there are two trust aspects to check here).

In this context, it can make sense to have a reasonably large set of trusted CAs (although you don't necessarily want it to be too large):

This is far from a perfect scenario, but it's manageable.

As a server (which is the case in logstash-input-tcp)

Those who use ssl_verify => false (as a server) are willing to let anyone connect to their service.

Those who use ssl_verify => true want client-certification enabled (and that's generally an active step because you need to set up the PKI to do that), but more importantly, they're really after authorisation (or possibly audit, i.e. traceability of the genuine entity that made the connection).

Considering that there is no authorisation layer in logstash-input-tcp, anything that is authenticated is effectively authorised. In this context, letting in client-certs issued by a dozen of pre-installed CAs becomes a problem.

Basically, by trusting all the CAs installed by default on the system, you're letting anyone willing to pay for the cheapest cert from any widely trusted CA in. That, of course, is a no-no.

Another aspect to take into consideration is that a server doesn't necessarily want to advertise all the CA certificates it trusts when its system behaves as a client in the CertificateRequest TLS message:

At the moment, that list is empty anyway, but this might change if jruby/jruby-openssl#85 is fixed (this has more to do with ssl_context.client_ca otherwise, but it's not used in logstash-input-tcp at the moment).

How to fix?

Here is an incomplete patch (but that should sort ca_file and ca_path at least):

@@ -38,7 +38,7 @@
   config :ssl_verify, :validate => :boolean, :default => true

   # The SSL CA certificate, chainfile or CA path. The system CA path is automatically included.
-  config :ssl_cacert, :validate => :path, :deprecated => "This setting is deprecated in favor of ssl_extra_chain_certs as it sets a more clear expectation to add more X509 certificates to the store"
+  config :ssl_cacert, :validate => :path, :deprecated => "This setting is deprecated in favor of ssl_ca_file and ssl_ca_path"

   # SSL certificate path
   config :ssl_cert, :validate => :path
@@ -49,9 +49,14 @@
   # SSL key passphrase
   config :ssl_key_passphrase, :validate => :password, :default => nil

-  # An Array of extra X509 certificates to be added to the certificate chain.
-  # Useful when the CA chain is not necessary in the system store.
-  config :ssl_extra_chain_certs, :validate => :array, :default => []
+  # Not in use
+  config :ssl_extra_chain_certs, :validate => :array, :default => [], :deprecated => "Not in use"
+
+  # Bundle of CA certificates in PEM format in a single file to use as trust anchors
+  config :ssl_ca_file, :validate => :path
+
+  # Directory containing CA certificates in PEM format to use as trust anchors (check OpenSSL naming conventions)
+  config :ssl_ca_path, :validate => :path

   def initialize(*args)
     super(*args)
@@ -213,7 +218,13 @@
       @ssl_context.cert = OpenSSL::X509::Certificate.new(File.read(@ssl_cert))
       @ssl_context.key = OpenSSL::PKey::RSA.new(File.read(@ssl_key),@ssl_key_passphrase)
       if @ssl_verify
-        @ssl_context.cert_store  = load_cert_store
+        if @ssl_ca_file
+          @ssl_context.ca_file = @ssl_ca_file
+        end
+        if @ssl_ca_path
+          @ssl_context.ca_path = @ssl_ca_path
+        end
+
         @ssl_context.verify_mode = OpenSSL::SSL::VERIFY_PEER|OpenSSL::SSL::VERIFY_FAIL_IF_NO_PEER_CERT
       end
     rescue => e
@@ -224,20 +235,6 @@
     @ssl_context
   end

-  def load_cert_store
-    cert_store = OpenSSL::X509::Store.new
-    cert_store.set_default_paths
-    if File.directory?(@ssl_cacert)
-      cert_store.add_path(@ssl_cacert)
-    else
-      cert_store.add_file(@ssl_cacert)
-    end if @ssl_cacert
-    @ssl_extra_chain_certs.each do |cert|
-      cert_store.add_file(cert)
-    end
-    cert_store
-  end
-
   def new_server_socket
     @logger.info("Starting tcp input listener", :address => "#{@host}:#{@port}")
     begin

I don't know well enough how this project is implemented and how much it depends on JRuby-OpenSSL. Considering that it seems to be expected to run in a Java environment, it might be worth considering using JSSE settings instead of all this (i.e. keystores and truststore). Having additional options to build a keystore from PEM files (to be compatible with the current key and cert settings) would not be too difficult to implement.


If hope these details can help. I have not submitted a PR because this patch is incomplete (the main thing that is missing is an option that sets ssl_context.extra_chain_cert). I don't have enough spare time to learn Ruby, but I guess loading a number of PEM-encoded certificates from a file and turning them into some sort of collection that can be passed to ssl_context.extra_chain_cert shouldn't be too difficult for someone who knows Ruby.

harbulot commented 8 years ago

It looks like setting ssl_context.extra_chain_cert has already been done in other ELK projects: