Feat: review and deprecate ssl protocol/cipher settings

CHANGELOG.md

@@ -1,3 +1,6 @@
+## 3.6.0
+ - Feat: review and deprecate ssl protocol/cipher related settings [#151](https://github.com/logstash-plugins/logstash-input-http/pull/151)
+
 ## 3.5.1
  - Fix: codecs provided with `additional_codecs` now correctly run in the pipeline's context, which means that they respect the `pipeline.ecs_compatibility` setting [#152](https://github.com/logstash-plugins/logstash-input-http/pull/152)
 

VERSION

@@ -1 +1 @@
-3.5.1
+3.6.0

docs/index.asciidoc

@@ -104,9 +104,11 @@ This plugin supports the following configuration options plus the <<plugins-{typ
 | <<plugins-{type}s-{plugin}-ssl>> |<<boolean,boolean>>|No
 | <<plugins-{type}s-{plugin}-ssl_certificate>> |a valid filesystem path|No
 | <<plugins-{type}s-{plugin}-ssl_certificate_authorities>> |<<array,array>>|No
+| <<plugins-{type}s-{plugin}-ssl_cipher_suites>> |<<array,array>>|No
 | <<plugins-{type}s-{plugin}-ssl_handshake_timeout>> |<<number,number>>|No
 | <<plugins-{type}s-{plugin}-ssl_key>> |a valid filesystem path|No
 | <<plugins-{type}s-{plugin}-ssl_key_passphrase>> |<<password,password>>|No
+| <<plugins-{type}s-{plugin}-ssl_supported_protocols>> |<<array,array>>|No
 | <<plugins-{type}s-{plugin}-ssl_verify_mode>> |<<string,string>>, one of `["none", "peer", "force_peer"]`|No
 | <<plugins-{type}s-{plugin}-threads>> |<<number,number>>|No
 | <<plugins-{type}s-{plugin}-tls_max_version>> |<<number,number>>|No
@@ -134,20 +136,20 @@ and no codec for the request's content-type is found
 ===== `cipher_suites` 
 
   * Value type is <<array,array>>
-  * Default value is `[TLS_AES_128_GCM_SHA256, TLS_AES_256_GCM_SHA384, TLS_CHACHA20_POLY1305_SHA256, TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384, TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384, TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256]`
+  * This option is deprecated
 
 The list of cipher suites to use, listed by priorities.
-This default list applies for OpenJDK 11.0.14 and higher.
-For older JDK versions, the default list includes only suites supported by that version.
-For example, the ChaCha20 family of ciphers is not supported in older versions.
+
+NOTE: This option is deprecated and it will be removed in the next major version of Logstash.
+Use `ssl_cipher_suites` instead.
 
 [id="plugins-{type}s-{plugin}-ecs_compatibility"]
 ===== `ecs_compatibility`
 
-* Value type is <<string,string>>
-* Supported values are:
-** `disabled`: unstructured connection metadata added at root level
-** `v1`,`v8`: headers added under `[@metadata][http][header]`. Some are copied to structured ECS fields `http`, `url`, `user_agent` and `host`
+  * Value type is <<string,string>>
+  * Supported values are:
+    ** `disabled`: unstructured connection metadata added at root level
+    ** `v1`,`v8`: headers added under `[@metadata][http][header]`. Some are copied to structured ECS fields `http`, `url`, `user_agent` and `host`
 
 Controls this plugin's compatibility with the
 {ecs-ref}[Elastic Common Schema (ECS)].
@@ -345,6 +347,17 @@ be read and added to the trust store. You need to configure the `ssl_verify_mode
 to `peer` or `force_peer` to enable the verification.
 
 
+[id="plugins-{type}s-{plugin}-ssl_cipher_suites"]
+===== `ssl_cipher_suites`
+
+  * Value type is <<array,array>>
+  * Default value is `['TLS_AES_256_GCM_SHA384', 'TLS_AES_128_GCM_SHA256', 'TLS_CHACHA20_POLY1305_SHA256', 'TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384', 'TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384', 'TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256', 'TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256', 'TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256', 'TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256', 'TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384', 'TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384', 'TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256', 'TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256']`
+
+The list of cipher suites to use, listed by priorities.
+This default list applies for OpenJDK 11.0.14 and higher.
+For older JDK versions, the default list includes only suites supported by that version.
+For example, the ChaCha20 family of ciphers is not supported in older versions.
+
 [id="plugins-{type}s-{plugin}-ssl_handshake_timeout"]
 ===== `ssl_handshake_timeout` 
 
@@ -371,6 +384,23 @@ for more information.
 
 SSL key passphrase to use.
 
+[id="plugins-{type}s-{plugin}-ssl_supported_protocols"]
+===== `ssl_supported_protocols`
+
+  * Value type is <<array,array>>
+  * Allowed values are: `'TLSv1.1'`, `'TLSv1.2'`, `'TLSv1.3'`
+  * Default depends on the JDK being used. With up-to-date Logstash, the default is `['TLSv1.2', 'TLSv1.3']`.
+    `'TLSv1.1'` is not considered secure and is only provided for legacy applications.
+
+List of allowed SSL/TLS versions to use when establishing a connection to the HTTP endpoint.
+
+For Java 8 `'TLSv1.3'` is supported  only since **8u262** (AdoptOpenJDK), but requires that you set the
+`LS_JAVA_OPTS="-Djdk.tls.client.protocols=TLSv1.3"` system property in Logstash.
+
+NOTE: If you configure the plugin to use `'TLSv1.1'` on any recent JVM, such as the one packaged with Logstash,
+the protocol is disabled by default and needs to be enabled manually by changing `jdk.tls.disabledAlgorithms` in
+the *$JDK_HOME/conf/security/java.security* configuration file. That is, `TLSv1.1` needs to be removed from the list.
+
 [id="plugins-{type}s-{plugin}-ssl_verify_mode"]
 ===== `ssl_verify_mode` 
 
@@ -399,7 +429,7 @@ Number of threads to use for both accepting connections and handling requests
 ===== `tls_max_version` 
 
   * Value type is <<number,number>>
-  * Default value is `1.3`
+  * This option is deprecated
 
 The maximum TLS version allowed for the encrypted connections.
 The value must be the one of the following: 1.1 for TLS 1.1, 1.2 for TLS 1.2, 1.3 for TLSv1.3
@@ -408,7 +438,7 @@ The value must be the one of the following: 1.1 for TLS 1.1, 1.2 for TLS 1.2, 1.
 ===== `tls_min_version` 
 
   * Value type is <<number,number>>
-  * Default value is `1.2`
+  * This option is deprecated
 
 The minimum TLS version allowed for the encrypted connections.
 The value must be one of the following: 1.1 for TLS 1.1, 1.2 for TLS 1.2, 1.3 for TLSv1.3
@@ -430,7 +460,7 @@ Username for basic authorization
 
 Set the client certificate verification method. Valid methods: none, peer, force_peer
 
-Note: This option is deprecated and it will be removed in the next major version of Logstash.
+NOTE: This option is deprecated and it will be removed in the next major version of Logstash.
 Use `ssl_verify_mode` instead.
 
 

lib/logstash/inputs/http.rb

@@ -30,6 +30,7 @@ class LogStash::Inputs::Http < LogStash::Inputs::Base
   require "logstash/inputs/http/tls"
 
   java_import "io.netty.handler.codec.http.HttpUtil"
+  java_import 'org.logstash.plugins.inputs.http.util.SslSimpleBuilder'
 
   config_name "http"
 
@@ -86,16 +87,11 @@ class LogStash::Inputs::Http < LogStash::Inputs::Base
   # Time in milliseconds for an incomplete ssl handshake to timeout
   config :ssl_handshake_timeout, :validate => :number, :default => 10000
 
-  # The minimum TLS version allowed for the encrypted connections. The value must be one of the following:
-  # 1.0 for TLS 1.0, 1.1 for TLS 1.1, 1.2 for TLS 1.2, 1.3 for TLS 1.3
-  config :tls_min_version, :validate => :number, :default => TLS.min.version
-
-  # The maximum TLS version allowed for the encrypted connections. The value must be the one of the following:
-  # 1.0 for TLS 1.0, 1.1 for TLS 1.1, 1.2 for TLS 1.2, 1.3 for TLS 1.3
-  config :tls_max_version, :validate => :number, :default => TLS.max.version
-
   # The list of ciphers suite to use, listed by priorities.
-  config :cipher_suites, :validate => :array, :default => org.logstash.plugins.inputs.http.util.SslSimpleBuilder.getDefaultCiphers
+  config :ssl_cipher_suites, :validate => SslSimpleBuilder::SUPPORTED_CIPHERS.to_a,
+                             :default => SslSimpleBuilder.getDefaultCiphers, :list => true
+
+  config :ssl_supported_protocols, :validate => ['TLSv1.1', 'TLSv1.2', 'TLSv1.3'], :default => ['TLSv1.2', 'TLSv1.3'], :list => true
 
   # Apply specific codecs for specific content types.
   # The default codec will be applied only after this list is checked
@@ -118,14 +114,23 @@ class LogStash::Inputs::Http < LogStash::Inputs::Base
   config :max_content_length, :validate => :number, :required => false, :default => 100 * 1024 * 1024
 
   config :response_code, :validate => [200, 201, 202, 204], :default => 200
+
   # Deprecated options
 
   # The JKS keystore to validate the client's certificates
   config :keystore, :validate => :path, :deprecated => "Set 'ssl_certificate' and 'ssl_key' instead."
   config :keystore_password, :validate => :password, :deprecated => "Set 'ssl_key_passphrase' instead."
 
-  config :verify_mode, :validate => ['none', 'peer', 'force_peer'], :default => 'none',
-     :deprecated => "Set 'ssl_verify_mode' instead."
+  config :verify_mode, :validate => ['none', 'peer', 'force_peer'], :default => 'none', :deprecated => "Set 'ssl_verify_mode' instead."
+  config :cipher_suites, :validate => :array, :default => [], :deprecated => "Set 'ssl_cipher_suites' instead."
+
+  # The minimum TLS version allowed for the encrypted connections. The value must be one of the following:
+  # 1.0 for TLS 1.0, 1.1 for TLS 1.1, 1.2 for TLS 1.2, 1.3 for TLS 1.3
+  config :tls_min_version, :validate => :number, :default => TLS.min.version, :deprecated => "Set 'ssl_supported_protocols' instead."
+
+  # The maximum TLS version allowed for the encrypted connections. The value must be the one of the following:
+  # 1.0 for TLS 1.0, 1.1 for TLS 1.1, 1.2 for TLS 1.2, 1.3 for TLS 1.3
+  config :tls_max_version, :validate => :number, :default => TLS.max.version, :deprecated => "Set 'ssl_supported_protocols' instead."
 
   attr_reader :codecs
 
@@ -233,24 +238,45 @@ def validate_ssl_settings!
       @logger.warn("SSL Certificate will not be used") if @ssl_certificate
       @logger.warn("SSL Key will not be used") if @ssl_key
       @logger.warn("SSL Java Key Store will not be used") if @keystore
-    elsif !(ssl_key_configured? || ssl_jks_configured?)
+      return # code bellow assumes `ssl => true`
+    end
+
+    if !(ssl_key_configured? || ssl_jks_configured?)
       raise LogStash::ConfigurationError, "Certificate or JKS must be configured"
     end
 
-    if @ssl && (original_params.key?("verify_mode") && original_params.key?("ssl_verify_mode"))
-        raise LogStash::ConfigurationError, "Both 'ssl_verify_mode' and 'verify_mode' were set. Use only 'ssl_verify_mode'."
+    if original_params.key?("verify_mode") && original_params.key?("ssl_verify_mode")
+      raise LogStash::ConfigurationError, "Both `ssl_verify_mode` and (deprecated) `verify_mode` were set. Use only `ssl_verify_mode`."
     elsif original_params.key?("verify_mode")
       @ssl_verify_mode_final = @verify_mode
-    elsif original_params.key?("ssl_verify_mode")
-      @ssl_verify_mode_final = @ssl_verify_mode
     else
       @ssl_verify_mode_final = @ssl_verify_mode
     end
 
-    if @ssl && require_certificate_authorities? && !client_authentication?
-      raise LogStash::ConfigurationError, "Using `ssl_verify_mode` or `verify_mode` set to PEER or FORCE_PEER, requires the configuration of `ssl_certificate_authorities`"
-    elsif @ssl && !require_certificate_authorities? && client_authentication?
-      raise LogStash::ConfigurationError, "The configuration of `ssl_certificate_authorities` requires setting `ssl_verify_mode` or `verify_mode` to PEER or FORCE_PEER"
+    if original_params.key?('cipher_suites') && original_params.key?('ssl_cipher_suites')
+      raise LogStash::ConfigurationError, "Both `ssl_cipher_suites` and (deprecated) `cipher_suites` were set. Use only `ssl_cipher_suites`."
+    elsif original_params.key?('cipher_suites')
+      @ssl_cipher_suites_final = @cipher_suites
+    else
+      @ssl_cipher_suites_final = @ssl_cipher_suites
+    end
+
+    if original_params.key?('tls_min_version') && original_params.key?('ssl_supported_protocols')
+      raise LogStash::ConfigurationError, "Both `ssl_supported_protocols` and (deprecated) `tls_min_ciphers` were set. Use only `ssl_supported_protocols`."
+    elsif original_params.key?('tls_max_version') && original_params.key?('ssl_supported_protocols')
+      raise LogStash::ConfigurationError, "Both `ssl_supported_protocols` and (deprecated) `tls_max_ciphers` were set. Use only `ssl_supported_protocols`."
+    else
+      if original_params.key?('tls_min_version') || original_params.key?('tls_max_version')
+        @ssl_supported_protocols_final = TLS.get_supported(tls_min_version..tls_max_version).map(&:name)
+      else
+        @ssl_supported_protocols_final = @ssl_supported_protocols
+      end
+    end
+
+    if require_certificate_authorities? && !client_authentication?
+      raise LogStash::ConfigurationError, "Using `ssl_verify_mode` (or `verify_mode`) set to PEER or FORCE_PEER, requires the configuration of `ssl_certificate_authorities`"
+    elsif !require_certificate_authorities? && client_authentication?
+      raise LogStash::ConfigurationError, "The configuration of `ssl_certificate_authorities` requires setting `ssl_verify_mode` (or `verify_mode`) to PEER or FORCE_PEER"
     end
   end
 
@@ -268,7 +294,7 @@ def build_ssl_params
       begin
         ssl_builder = org.logstash.plugins.inputs.http.util.SslSimpleBuilder
                           .new(@ssl_certificate, @ssl_key, @ssl_key_passphrase.nil? ? nil : @ssl_key_passphrase.value)
-                          .setCipherSuites(normalized_ciphers)
+                          .setCipherSuites(normalized_cipher_suites)
       rescue java.lang.IllegalArgumentException => e
         @logger.error("SSL configuration invalid", error_details(e))
         raise LogStash::ConfigurationError, e
@@ -300,19 +326,15 @@ def require_certificate_authorities?
 
   private
 
-  def normalized_ciphers
-    @cipher_suites.map(&:upcase)
-  end
-
-  def convert_protocols
-    TLS.get_supported(@tls_min_version..@tls_max_version).map(&:name)
+  def normalized_cipher_suites
+    @ssl_cipher_suites_final.map(&:upcase)
   end
 
   def new_ssl_handshake_provider(ssl_builder)
     begin
       ssl_handler_provider = org.logstash.plugins.inputs.http.util.SslHandlerProvider.new(ssl_builder.build())
       ssl_handler_provider.setVerifyMode(@ssl_verify_mode_final.upcase)
-      ssl_handler_provider.setProtocols(convert_protocols)
+      ssl_handler_provider.setProtocols(@ssl_supported_protocols_final)
       ssl_handler_provider.setHandshakeTimeoutMilliseconds(@ssl_handshake_timeout)
       ssl_handler_provider
     rescue java.lang.IllegalArgumentException => e

spec/inputs/http_spec.rb

@@ -169,10 +169,10 @@
 
         let(:config) do
           super().merge 'ssl' => true,
-                      'ssl_certificate_authorities' => [ File.join(certs_dir, 'root.crt') ],
-                      'ssl_certificate' => File.join(certs_dir, 'server_from_root.crt'),
-                      'ssl_key' => File.join(certs_dir, 'server_from_root.key.pkcs8'),
-                      'ssl_verify_mode' => 'peer'
+                        'ssl_certificate_authorities' => [ File.join(certs_dir, 'root.crt') ],
+                        'ssl_certificate' => File.join(certs_dir, 'server_from_root.crt'),
+                        'ssl_key' => File.join(certs_dir, 'server_from_root.key.pkcs8'),
+                        'ssl_verify_mode' => 'peer'
         end
 
         let(:client_options) do
@@ -219,7 +219,21 @@
 
           context 'enforced TLSv1.3 in plugin' do
 
-            let(:config) { super().merge 'tls_min_version' => '1.3', 'cipher_suites' => [ 'TLS_AES_128_GCM_SHA256' ] }
+            let(:config) { super().merge 'ssl_supported_protocols' => ['TLSv1.3'],
+                                         'ssl_cipher_suites' => [ 'TLS_AES_128_GCM_SHA256' ] }
+
+            it "should parse the json body" do
+              expect(response.code).to eq(200)
+              event = logstash_queue.pop
+              expect(event.get("message")).to eq("Hello")
+            end
+
+          end
+
+          context 'enforced TLSv1.3 (deprecated options)' do
+
+            let(:config) { super().merge 'tls_min_version' => 1.3,
+                                         'cipher_suites' => [ 'TLS_AES_128_GCM_SHA256' ] }
 
             it "should parse the json body" do
               expect(response.code).to eq(200)
@@ -537,6 +551,12 @@ def setup_server_client(url = self.url)
         expect { subject.register }.to raise_exception(LogStash::ConfigurationError)
       end
     end
+    context "with invalid cipher suites" do
+      it "should raise a configuration error" do
+        invalid_config = config.merge("ssl_cipher_suites" => "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA38")
+        expect { LogStash::Inputs::Http.new(invalid_config) }.to raise_error(LogStash::ConfigurationError)
+      end
+    end
     context "with :ssl_certificate" do
       let(:ssc) { SelfSignedCertificate.new }
       let(:ssl_certificate) { ssc.certificate }
@@ -595,18 +615,6 @@ def setup_server_client(url = self.url)
         end
       end
 
-      context "with invalid cipher_suites" do
-        let(:config) { super().merge("cipher_suites" => "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA38") }
-
-        it "should raise a configuration error" do
-          expect( subject.logger ).to receive(:error) do |msg, opts|
-            expect( msg ).to match /.*?configuration invalid/
-            expect( opts[:message] ).to match /TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA38.*? not available/
-          end
-          expect { subject.register }.to raise_error(LogStash::ConfigurationError)
-        end
-      end
-
       context "with invalid ssl certificate" do
         before do
           cert = File.readlines path = config["ssl_certificate"]
@@ -654,6 +662,47 @@ def setup_server_client(url = self.url)
         end
       end
 
+      context "with both verify_mode options set" do
+        let(:config) do
+          super().merge('ssl_verify_mode' => 'peer', 'verify_mode' => 'none')
+        end
+
+        it "should raise a configuration error" do
+          expect { subject.register }.to raise_error LogStash::ConfigurationError, /Use only .?ssl_verify_mode.?/i
+        end
+      end
+
+      context "with ssl_cipher_suites and cipher_suites set" do
+        let(:config) do
+          super().merge('ssl_cipher_suites' => ['TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384'],
+                        'cipher_suites' => ['TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384'])
+        end
+
+        it "should raise a configuration error" do
+          expect { subject.register }.to raise_error LogStash::ConfigurationError, /Use only .?ssl_cipher_suites.?/i
+        end
+      end
+
+      context "with ssl_supported_protocols and tls_min_version set" do
+        let(:config) do
+          super().merge('ssl_supported_protocols' => ['TLSv1.2'], 'tls_min_version' => 1.0)
+        end
+
+        it "should raise a configuration error" do
+          expect { subject.register }.to raise_error LogStash::ConfigurationError, /Use only .?ssl_supported_protocols.?/i
+        end
+      end
+
+      context "with ssl_supported_protocols and tls_max_version set" do
+        let(:config) do
+          super().merge('ssl_supported_protocols' => ['TLSv1.2'], 'tls_max_version' => 1.2)
+        end
+
+        it "should raise a configuration error" do
+          expect { subject.register }.to raise_error LogStash::ConfigurationError, /Use only .?ssl_supported_protocols.?/i
+        end
+      end
+
     end
   end
 end