Feat: review and deprecate ssl protocol/cipher settings (#450)

Co-authored-by: Karen Metts <[email protected]>

Author: kares

CHANGELOG.md

@@ -1,3 +1,6 @@
+## 6.4.0
+ - Feat: review and deprecate ssl protocol/cipher settings [#450](https://github.com/logstash-plugins/logstash-input-beats/pull/450)
+
 ## 6.3.1
  - Fix: Removed use of deprecated `import` of java classes in ruby [#449](https://github.com/logstash-plugins/logstash-input-beats/pull/449)
 

VERSION

@@ -1 +1 @@
-6.3.1
+6.4.0

docs/index.asciidoc

@@ -159,8 +159,8 @@ This plugin supports the following configuration options plus the <<plugins-{typ
 [cols="<,<,<",options="header",]
 |=======================================================================
 |Setting |Input type|Required
-| <<plugins-{type}s-{plugin}-add_hostname>> |<<boolean,boolean>>|No
-| <<plugins-{type}s-{plugin}-cipher_suites>> |<<array,array>>|No
+| <<plugins-{type}s-{plugin}-add_hostname>> |<<boolean,boolean>>|__Deprecated__
+| <<plugins-{type}s-{plugin}-cipher_suites>> |<<array,array>>|__Deprecated__
 | <<plugins-{type}s-{plugin}-client_inactivity_timeout>> |<<number,number>>|No
 | <<plugins-{type}s-{plugin}-ecs_compatibility>> | <<string,string>>|No
 | <<plugins-{type}s-{plugin}-executor_threads>> |<<number,number>>|No
@@ -173,10 +173,11 @@ This plugin supports the following configuration options plus the <<plugins-{typ
 | <<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_verify_mode>> |<<string,string>>, one of `["none", "peer", "force_peer"]`|No
 | <<plugins-{type}s-{plugin}-ssl_peer_metadata>> |<<boolean,boolean>>|No
-| <<plugins-{type}s-{plugin}-tls_max_version>> |<<number,number>>|No
-| <<plugins-{type}s-{plugin}-tls_min_version>> |<<number,number>>|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}-tls_max_version>> |<<number,number>>|__Deprecated__
+| <<plugins-{type}s-{plugin}-tls_min_version>> |<<number,number>>|__Deprecated__
 |=======================================================================
 
 Also see <<plugins-{type}s-{plugin}-common-options>> for a list of options supported by all
@@ -194,17 +195,13 @@ input plugins.
 
 Flag to determine whether to add `host` field to event using the value supplied by the {plugin-singular} in the `hostname` field.
 
-
 [id="plugins-{type}s-{plugin}-cipher_suites"]
 ===== `cipher_suites`
+deprecated[6.4.0, Replaced by <<plugins-{type}s-{plugin}-ssl_cipher_suites>>]
 
   * Value type is <<array,array>>
-  * Default value is `java.lang.String[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]@459cfcca`
 
-The list of ciphers suite 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.
+The list of cipher suites to use, listed by priorities.
 
 [id="plugins-{type}s-{plugin}-client_inactivity_timeout"]
 ===== `client_inactivity_timeout`
@@ -217,14 +214,14 @@ Close Idle clients after X seconds of inactivity.
 [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`: structured connection metadata added under ECS v1 compliant namespaces
-** `v8`: structured connection metadata added under ECS v8 compliant namespaces
-* Default value depends on which version of Logstash is running:
-** When Logstash provides a `pipeline.ecs_compatibility` setting, its value is used as the default
-** Otherwise, the default value is `disabled`.
+  * Value type is <<string,string>>
+  * Supported values are:
+    ** `disabled`: unstructured connection metadata added at root level
+    ** `v1`: structured connection metadata added under ECS v1 compliant namespaces
+    ** `v8`: structured connection metadata added under ECS v8 compliant namespaces
+  * Default value depends on which version of Logstash is running:
+    ** When Logstash provides a `pipeline.ecs_compatibility` setting, its value is used as the default
+    ** Otherwise, the default value is `disabled`.
 
 Refer to <<plugins-{type}s-{plugin}-ecs_metadata,ECS mapping>> for detailed information.
 
@@ -300,6 +297,16 @@ You can define multiple files or paths. All the certificates will
 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`
@@ -333,6 +340,33 @@ openssl pkcs8 -inform PEM -in path/to/logstash.key -topk8 -nocrypt -outform PEM
 
 SSL key passphrase to use.
 
+[id="plugins-{type}s-{plugin}-ssl_peer_metadata"]
+===== `ssl_peer_metadata`
+
+  * Value type is <<boolean,boolean>>
+  * Default value is `false`
+
+Enables storing client certificate information in event's metadata.
+
+This option is only valid when `ssl_verify_mode` is set to `peer` or `force_peer`.
+
+[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`
 
@@ -349,33 +383,23 @@ If the client doesn't provide a certificate, the connection will be closed.
 
 This option needs to be used with `ssl_certificate_authorities` and a defined list of CAs.
 
-[id="plugins-{type}s-{plugin}-ssl_peer_metadata"]
-===== `ssl_peer_metadata`
-
-  * Value type is <<boolean,boolean>>
-  * Default value is `false`
-
-Enables storing client certificate information in event's metadata.
-
-This option is only valid when `ssl_verify_mode` is set to `peer` or `force_peer`.
-
 [id="plugins-{type}s-{plugin}-tls_max_version"]
 ===== `tls_max_version`
+deprecated[6.4.0, Replaced by <<plugins-{type}s-{plugin}-ssl_supported_protocols>>]
 
   * Value type is <<number,number>>
-  * Default value is `1.3`
 
-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
+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
 
 [id="plugins-{type}s-{plugin}-tls_min_version"]
 ===== `tls_min_version`
+deprecated[6.4.0, Replaced by <<plugins-{type}s-{plugin}-ssl_supported_protocols>>]
 
   * Value type is <<number,number>>
-  * Default value is `1`
 
-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
+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 TLS 1.3
 
 
 

lib/logstash/inputs/beats.rb

@@ -51,6 +51,8 @@ class LogStash::Inputs::Beats < LogStash::Inputs::Base
   require "logstash/inputs/beats/message_listener"
   require "logstash/inputs/beats/tls"
 
+  java_import 'org.logstash.netty.SslContextBuilder'
+
   # adds ecs_compatibility config which could be :disabled or :v1
   include LogStash::PluginMixins::ECSCompatibilitySupport(:disabled,:v1, :v8 => :v1)
 
@@ -89,9 +91,6 @@ class LogStash::Inputs::Beats < LogStash::Inputs::Base
   # 
   config :ssl_certificate_authorities, :validate => :array, :default => []
 
-  # Flag to determine whether to add host information (provided by the beat in the 'hostname' field) to the event
-  config :add_hostname, :validate => :boolean, :default => false, :deprecated => 'This option will be removed in the future as beats determine the event schema'
-
   # By default the server doesn't do any client verification.
   # 
   # `peer` will make the server ask the client to provide a certificate. 
@@ -112,22 +111,31 @@ class LogStash::Inputs::Beats < 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
-  config :tls_min_version, :validate => :number, :default => TLS.min.version
+  config :ssl_cipher_suites, :validate => SslContextBuilder::SUPPORTED_CIPHERS.to_a,
+         :default => SslContextBuilder.getDefaultCiphers, :list => true
 
-  # 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
-  config :tls_max_version, :validate => :number, :default => TLS.max.version
+  config :ssl_supported_protocols, :validate => ['TLSv1.1', 'TLSv1.2', 'TLSv1.3'], :default => ['TLSv1.2', 'TLSv1.3'], :list => true
 
-  # The list of ciphers suite to use, listed by priorities.
-  config :cipher_suites, :validate => :array, :default => org.logstash.netty.SslContextBuilder.getDefaultCiphers
   # Close Idle clients after X seconds of inactivity.
   config :client_inactivity_timeout, :validate => :number, :default => 60
 
   # Beats handler executor thread
   config :executor_threads, :validate => :number, :default => LogStash::Config::CpuCoreStrategy.maximum
 
+  # Flag to determine whether to add host information (provided by the beat in the 'hostname' field) to the event
+  config :add_hostname, :validate => :boolean, :default => false, :deprecated => 'This option will be removed in the future as beats determine the event schema'
+
+  # The list of ciphers suite to use, listed by priorities.
+  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
+  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
+  config :tls_max_version, :validate => :number, :default => TLS.max.version, :deprecated => "Set 'ssl_supported_protocols' instead."
+
   attr_reader :field_hostname, :field_hostip
   attr_reader :field_tls_protocol_version, :field_tls_peer_subject, :field_tls_cipher
 
@@ -156,6 +164,26 @@ def register
       if client_authentication_metadata? && !require_certificate_authorities?
         configuration_error "Configuring ssl_peer_metadata => true requires ssl_verify_mode => to be configured with 'peer' or 'force_peer'"
       end
+
+      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
     else
       @logger.warn("configured ssl_certificate => #{@ssl_certificate.inspect} will not be used") if @ssl_certificate
       @logger.warn("configured ssl_key => #{@ssl_key.inspect} will not be used") if @ssl_key
@@ -184,9 +212,9 @@ def create_server
       ssl_context_builder = new_ssl_context_builder
       if client_authentification?
         if @ssl_verify_mode == "force_peer"
-          ssl_context_builder.setVerifyMode(org.logstash.netty.SslContextBuilder::SslClientVerifyMode::FORCE_PEER)
+          ssl_context_builder.setVerifyMode(SslContextBuilder::SslClientVerifyMode::FORCE_PEER)
         elsif @ssl_verify_mode == "peer"
-          ssl_context_builder.setVerifyMode(org.logstash.netty.SslContextBuilder::SslClientVerifyMode::VERIFY_PEER)
+          ssl_context_builder.setVerifyMode(SslContextBuilder::SslClientVerifyMode::VERIFY_PEER)
         end
         ssl_context_builder.setCertificateAuthorities(@ssl_certificate_authorities)
       end
@@ -247,20 +275,16 @@ def new_ssl_context_builder
     passphrase = @ssl_key_passphrase.nil? ? nil : @ssl_key_passphrase.value
     begin
       org.logstash.netty.SslContextBuilder.new(@ssl_certificate, @ssl_key, passphrase)
-          .setProtocols(convert_protocols)
-          .setCipherSuites(normalized_ciphers)
+          .setProtocols(@ssl_supported_protocols_final)
+          .setCipherSuites(normalized_cipher_suites)
     rescue java.lang.IllegalArgumentException => e
       @logger.error("SSL configuration invalid", error_details(e))
       raise LogStash::ConfigurationError, e
     end
   end
 
-  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 configuration_error(message)

spec/inputs/beats_spec.rb

@@ -47,6 +47,9 @@
     end
 
     context "with ssl enabled" do
+
+      let(:config) { { "ssl" => true, "port" => port, "ssl_key" => certificate.ssl_key, "ssl_certificate" => certificate.ssl_cert } }
+
       context "without certificate configuration" do
         let(:config) { { "port" => 0, "ssl" => true, "ssl_key" => certificate.ssl_key, "type" => "example" } }
 
@@ -78,7 +81,7 @@
       end
 
       context "with invalid ciphers" do
-        let(:config) { super().merge("ssl" => true, "cipher_suites" => "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA38") }
+        let(:config) { super().merge("cipher_suites" => "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA38") }
 
         it "should raise a configuration error" do
           plugin = LogStash::Inputs::Beats.new(config)
@@ -92,7 +95,7 @@
 
       context "verify_mode" do
         context "verify_mode configured to PEER" do
-          let(:config) { super().merge("ssl" => true, "ssl_verify_mode" => "peer") }
+          let(:config) { super().merge("ssl_verify_mode" => "peer") }
 
           it "raise a ConfigurationError when certificate_authorities is not set" do
             plugin = LogStash::Inputs::Beats.new(config)
@@ -107,7 +110,7 @@
         end
 
         context "verify_mode configured to FORCE_PEER" do
-          let(:config) { super().merge("ssl" => true, "ssl_verify_mode" => "force_peer") }
+          let(:config) { super().merge("ssl_verify_mode" => "force_peer") }
 
           it "raise a ConfigurationError when certificate_authorities is not set" do
             plugin = LogStash::Inputs::Beats.new(config)
@@ -120,6 +123,40 @@
             expect {plugin.register}.not_to raise_error
           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
+            plugin = LogStash::Inputs::Beats.new(config)
+            expect { plugin.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.2)
+          end
+
+          it "should raise a configuration error" do
+            plugin = LogStash::Inputs::Beats.new(config)
+            expect { plugin.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
+            plugin = LogStash::Inputs::Beats.new(config)
+            expect { plugin.register }.to raise_error LogStash::ConfigurationError, /Use only .?ssl_supported_protocols.?/i
+          end
+        end
       end
     end