Mark deprecated SSL settings as obsolete (#228)

* Mark deprecated SSL settings as obsolete

This commit marks the following SSL settings as obsolete:

`ssl_cert`, which should be replaced by `ssl_certificate`
`ssl_enable`, which should be replaced by `ssl_enabled`
`ssl_verify`, which should be replaced by `ssl_client_authentication` when `mode` is `server` or `ssl_verification_mode`when mode is `client`

---------

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

Author: 3 people

CHANGELOG.md

@@ -1,3 +1,11 @@
+## 7.0.0
+  - SSL settings that were marked deprecated in version `6.4.0` are now marked obsolete, and will prevent the plugin from starting.
+  - These settings are:
+    - `ssl_cert`, which should be replaced by `ssl_certificate`
+    - `ssl_enable`, which should be replaced by `ssl_enabled`
+    - `ssl_verify`, which should be replaced by `ssl_client_authentication` when `mode` is `server` or `ssl_verification_mode`when mode is `client`
+    - [228](https://github.com/logstash-plugins/logstash-input-tcp/pull/228)
+
 ## 6.4.4
   - update netty to 4.1.115 [#227](https://github.com/logstash-plugins/logstash-input-tcp/pull/227)
 

docs/index.asciidoc

@@ -121,6 +121,10 @@ filter {
 
 This plugin supports the following configuration options plus the <<plugins-{type}s-{plugin}-common-options>> described later.
 
+NOTE: As of version `7.0.0` of this plugin, a number of previously deprecated settings related to SSL have been removed. Please see the
+<<plugins-{type}s-{plugin}-obsolete-options>> for more details.
+
+
 [cols="<,<,<",options="header",]
 |=======================================================================
 |Setting |Input type|Required
@@ -130,19 +134,16 @@ This plugin supports the following configuration options plus the <<plugins-{typ
 | <<plugins-{type}s-{plugin}-mode>> |<<string,string>>, one of `["server", "client"]`|No
 | <<plugins-{type}s-{plugin}-port>> |<<number,number>>|Yes
 | <<plugins-{type}s-{plugin}-proxy_protocol>> |<<boolean,boolean>>|No
-| <<plugins-{type}s-{plugin}-ssl_cert>> |a valid filesystem path|__Deprecated__
 | <<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>> |<<string,string>>|No
 | <<plugins-{type}s-{plugin}-ssl_client_authentication>> |<<string,string>>, one of `["none", "optional", "required"]`|No
-| <<plugins-{type}s-{plugin}-ssl_enable>> |<<boolean,boolean>>|__Deprecated__
 | <<plugins-{type}s-{plugin}-ssl_enabled>> |<<boolean,boolean>>|No
 | <<plugins-{type}s-{plugin}-ssl_extra_chain_certs>> |<<array,array>>|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>> |<<string,string>>|No
 | <<plugins-{type}s-{plugin}-ssl_verification_mode>> |<<string,string>>, one of `["full", "none"]`|No
-| <<plugins-{type}s-{plugin}-ssl_verify>> |<<boolean,boolean>>|__Deprecated__
 | <<plugins-{type}s-{plugin}-tcp_keep_alive>> |<<boolean,boolean>>|No
 |=======================================================================
 
@@ -212,16 +213,6 @@ When mode is `client`, the port to connect to.
 Proxy protocol support, only v1 is supported at this time
 http://www.haproxy.org/download/1.5/doc/proxy-protocol.txt
 
-[id="plugins-{type}s-{plugin}-ssl_cert"]
-===== `ssl_cert` 
-deprecated[6.4.0, Replaced by <<plugins-{type}s-{plugin}-ssl_certificate>>]
-
-  * Value type is <<path,path>>
-  * There is no default value for this setting.
-
-Path to certificate in PEM format. This certificate will be presented
-to the connecting clients.
-
 [id="plugins-{type}s-{plugin}-ssl_certificate"]
 ===== `ssl_certificate`
 
@@ -268,14 +259,6 @@ Please note that the server does not validate the client certificate CN (Common
 
 NOTE: This setting can be used only if <<plugins-{type}s-{plugin}-mode>> is `server` and <<plugins-{type}s-{plugin}-ssl_certificate_authorities>> is set.
 
-[id="plugins-{type}s-{plugin}-ssl_enable"]
-===== `ssl_enable` 
-deprecated[6.4.0, Replaced by <<plugins-{type}s-{plugin}-ssl_enabled>>]
-
-  * Value type is <<boolean,boolean>>
-  * Default value is `false`
-
-Enable SSL (must be set for other `ssl_` options to take effect).
 
 [id="plugins-{type}s-{plugin}-ssl_enabled"]
 ===== `ssl_enabled`
@@ -343,16 +326,6 @@ This setting can be used only if <<plugins-{type}s-{plugin}-mode>> is `client`.
 
 WARNING: Setting certificate verification to `none` disables many security benefits of SSL/TLS, which is very dangerous. For more information on disabling certificate verification please read https://www.cs.utexas.edu/~shmat/shmat_ccs12.pdf
 
-[id="plugins-{type}s-{plugin}-ssl_verify"]
-===== `ssl_verify`
-deprecated[6.4.0, Replaced by <<plugins-{type}s-{plugin}-ssl_client_authentication>> and <<plugins-{type}s-{plugin}-ssl_verification_mode>>]
-
-  * Value type is <<boolean,boolean>>
-  * Default value is `true`
-
-Verify the identity of the other end of the SSL connection against the CA.
-For input, sets the field `sslsubject` to that of the client certificate.
-
 [id="plugins-{type}s-{plugin}-tcp_keep_alive"]
 ===== `tcp_keep_alive`
 
@@ -363,6 +336,21 @@ Instruct the socket to use TCP keep alive. If it's `true` then the underlying so
 will use the OS defaults settings for keep alive. If it's `false` it doesn't configure any
 keep alive setting for the underlying socket.
 
+[id="plugins-{type}s-{plugin}-obsolete-options"]
+==== TCP Input Obsolete Configuration Options
+
+WARNING: As of version `7.0.0` of this plugin, some configuration options have been replaced.
+The plugin will fail to start if it contains any of these obsolete options. 
+
+
+[cols="<,<",options="header",]
+|=======================================================================
+|Setting|Replaced by
+| ssl_cert |<<plugins-{type}s-{plugin}-ssl_certificate>>
+| ssl_enable |<<plugins-{type}s-{plugin}-ssl_enabled>>
+| ssl_verify |<<plugins-{type}s-{plugin}-ssl_client_authentication>> in `server` mode and <<plugins-{type}s-{plugin}-ssl_verification_mode>> in `client` mode
+|=======================================================================
+
 
 [id="plugins-{type}s-{plugin}-common-options"]
 include::{include_path}/{type}.asciidoc[]

lib/logstash/inputs/tcp.rb

@@ -6,7 +6,6 @@
 require "logstash/util/socket_peer"
 require "logstash-input-tcp_jars"
 require 'logstash/plugin_mixins/ecs_compatibility_support'
-require "logstash/plugin_mixins/normalize_config_support"
 
 require "socket"
 require "openssl"
@@ -69,8 +68,6 @@ class LogStash::Inputs::Tcp < LogStash::Inputs::Base
   # ecs_compatibility option, provided by Logstash core or the support adapter.
   include LogStash::PluginMixins::ECSCompatibilitySupport(:disabled, :v1, :v8 => :v1)
 
-  include LogStash::PluginMixins::NormalizeConfigSupport
-
   config_name "tcp"
 
   default :codec, "line"
@@ -91,8 +88,6 @@ class LogStash::Inputs::Tcp < LogStash::Inputs::Base
   # http://www.haproxy.org/download/1.5/doc/proxy-protocol.txt
   config :proxy_protocol, :validate => :boolean, :default => false
 
-  # Enable SSL (must be set for other `ssl_` options to take effect).
-  config :ssl_enable, :validate => :boolean, :default => false, :deprecated => "Use 'ssl_enabled' instead."
 
   # Enable SSL (must be set for other `ssl_` options to take effect).
   config :ssl_enabled, :validate => :boolean, :default => false
@@ -104,9 +99,6 @@ class LogStash::Inputs::Tcp < LogStash::Inputs::Base
   # This option needs to be used with `ssl_certificate_authorities` and a defined list of CAs.
   config :ssl_client_authentication, :validate => %w[none optional required], :default => 'required'
 
-  # Verify the identity of the other end of the SSL connection against the CA.
-  # For input, sets the field `sslsubject` to that of the client certificate.
-  config :ssl_verify, :validate => :boolean, :default => true, :deprecated => "Use 'ssl_client_authentication' when mode is 'server' or 'ssl_verification_mode' when mode is 'client'"
 
   # Options to verify the server's certificate.
   # "full": validates that the provided certificate has an issue date that’s within the not_before and not_after dates;
@@ -116,8 +108,6 @@ class LogStash::Inputs::Tcp < LogStash::Inputs::Base
   config :ssl_verification_mode, :validate => %w[full none], :default => 'full'
 
   # SSL certificate path
-  config :ssl_cert, :validate => :path, :deprecated => "Use 'ssl_certificate' instead."
-
   # SSL certificate path
   config :ssl_certificate, :validate => :path
 
@@ -148,6 +138,13 @@ class LogStash::Inputs::Tcp < LogStash::Inputs::Base
   # Option to allow users to avoid DNS Reverse Lookup.
   config :dns_reverse_lookup_enabled, :validate => :boolean, :default => true
 
+  # Obsolete SSL Settings
+  config :ssl_enable, :obsolete => "Use 'ssl_enabled' instead."
+  config :ssl_verify, :obsolete => "Use 'ssl_client_authentication' when mode is 'server' or 'ssl_verification_mode' when mode is 'client'"
+  config :ssl_cert, :obsolete => "Use 'ssl_certificate' instead."
+
+
+
   # Monkey patch TCPSocket and SSLSocket to include socket peer
   # @private
   def self.patch_socket_peer!
@@ -163,7 +160,6 @@ def initialize(*args)
     super(*args)
 
     setup_fields!
-    setup_ssl_params!
 
     self.class.patch_socket_peer!
 
@@ -368,35 +364,6 @@ def provided_ssl_enabled_config_name
     original_params.include?('ssl_enable') ? 'ssl_enable' : 'ssl_enabled'
   end
 
-  def setup_ssl_params!
-    @ssl_enabled = normalize_config(:ssl_enabled) do |normalizer|
-      normalizer.with_deprecated_alias(:ssl_enable)
-    end
-
-    @ssl_certificate = normalize_config(:ssl_certificate) do |normalizer|
-      normalizer.with_deprecated_alias(:ssl_cert)
-    end
-
-    if server?
-      @ssl_client_authentication = normalize_config(:ssl_client_authentication) do |normalizer|
-        normalizer.with_deprecated_mapping(:ssl_verify) do |ssl_verify|
-          ssl_verify == true ? "required" : "none"
-        end
-      end
-    else
-      @ssl_verification_mode = normalize_config(:ssl_verification_mode) do |normalize|
-        normalize.with_deprecated_mapping(:ssl_verify) do |ssl_verify|
-          ssl_verify == true ? "full" : "none"
-        end
-      end
-    end
-
-    params['ssl_enabled'] = @ssl_enabled unless @ssl_enabled.nil?
-    params['ssl_certificate'] = @ssl_certificate unless @ssl_certificate.nil?
-    params['ssl_verification_mode'] = @ssl_verification_mode unless @ssl_verification_mode.nil?
-    params['ssl_client_authentication'] = @ssl_client_authentication unless @ssl_client_authentication.nil?
-  end
-
   def server?
     @mode == "server"
   end

logstash-input-tcp.gemspec

@@ -22,7 +22,6 @@ Gem::Specification.new do |s|
   # Gem dependencies
   s.add_runtime_dependency "logstash-core-plugin-api", ">= 1.60", "<= 2.99"
   s.add_runtime_dependency 'logstash-mixin-ecs_compatibility_support', '~>1.2'
-  s.add_runtime_dependency 'logstash-mixin-normalize_config_support', '~>1.0'
 
   s.add_runtime_dependency 'logstash-core', '>= 8.1.0'
 

spec/inputs/tcp_spec.rb

@@ -54,6 +54,25 @@ def get_port
     end
   end
 
+  ['client', 'server'].each do | mode|
+    describe "handling obsolete settings for #{mode} mode" do
+      [{:name => 'ssl_cert', :replacement => 'ssl_certificate', :sample_value => "certificate_path"},
+       {:name => 'ssl_enable', :replacement => 'ssl_enabled', :sample_value => true},
+       {:name => 'ssl_verify', :replacement => 'ssl_client_authentication', :sample_value => 'peer'}].each do | obsolete_setting |
+        context "with obsolete #{obsolete_setting[:name]}" do
+          let(:config) { { "mode" => mode, "port" => port } }
+          let (:deprecated_config) do
+            config.merge({obsolete_setting[:name] => obsolete_setting[:sample_value]})
+          end
+
+          it "should raise a config error with the appropriate message" do
+            expect { LogStash::Inputs::Tcp.new(deprecated_config).register }.to raise_error LogStash::ConfigurationError, /The setting `#{obsolete_setting[:name]}` in plugin `tcp` is obsolete and is no longer available. Use '#{obsolete_setting[:replacement]}'/i
+          end
+        end
+      end
+    end
+  end
+
   ecs_compatibility_matrix(:disabled,:v1, :v8 => :v1) do |ecs_select|
     before(:each) do
       allow_any_instance_of(described_class).to receive(:ecs_compatibility).and_return(ecs_compatibility)
@@ -602,17 +621,6 @@ def get_port
             end
           end
 
-          context "with deprecated ssl_verify = true and no ssl_certificate_authorities" do
-            let(:config) { super().merge(
-              'ssl_verify' => true,
-              'ssl_certificate_authorities' => []
-            ) }
-
-            it "should register without errors" do
-              expect { subject.register }.to_not raise_error
-            end
-          end
-
           %w[required optional].each do |ssl_client_authentication|
             context "with ssl_client_authentication = `#{ssl_client_authentication}` and no ssl_certificate_authorities" do
               let(:config) { super().merge(
@@ -636,70 +644,6 @@ def get_port
             end
           end
         end
-
-        context "with deprecated settings" do
-          let(:ssl_verify) { true }
-          let(:certificate_path) { File.expand_path('../fixtures/small.crt', File.dirname(__FILE__)) }
-          let(:config) do
-            {
-              "host" => "127.0.0.1",
-              "port" => port,
-              "ssl_enable" => true,
-              "ssl_cert" => certificate_path,
-              "ssl_key" => File.expand_path('../fixtures/small.key', File.dirname(__FILE__)),
-              "ssl_verify" => ssl_verify
-            }
-          end
-
-          context "and mode is server" do
-            let(:config) { super().merge("mode" => 'server') }
-            [true, false].each do |verify|
-              context "and ssl_verify is #{verify}" do
-                let(:ssl_verify) { verify }
-
-                it "should set new configs params" do
-                  subject.register
-                  expect(subject.params).to match hash_including(
-                    "ssl_enabled" => true,
-                    "ssl_certificate" => certificate_path,
-                    "ssl_client_authentication" => verify ? 'required' : 'none')
-                end
-
-                it "should set new configs variables" do
-                  subject.register
-                  expect(subject.instance_variable_get(:@ssl_enabled)).to eql(true)
-                  expect(subject.instance_variable_get(:@ssl_client_authentication)).to eql(verify ? 'required' : 'none')
-                  expect(subject.instance_variable_get(:@ssl_certificate)).to eql(certificate_path)
-                end
-              end
-            end
-          end
-
-          context "and mode is client" do
-            let(:config) { super().merge("mode" => 'client') }
-            [true, false].each do |verify|
-              context "and ssl_verify is #{verify}" do
-                let(:ssl_verify) { verify }
-
-                it "should set new configs params" do
-                  subject.register
-                  expect(subject.params).to match hash_including(
-                    "ssl_enabled" => true,
-                    "ssl_certificate" => certificate_path,
-                    "ssl_verification_mode" => verify ? 'full' : 'none'
-                  )
-                end
-
-                it "should set new configs variables" do
-                  subject.register
-                  expect(subject.instance_variable_get(:@ssl_enabled)).to eql(true)
-                  expect(subject.instance_variable_get(:@ssl_verification_mode)).to eql(verify ? 'full' : 'none')
-                  expect(subject.instance_variable_get(:@ssl_certificate)).to eql(certificate_path)
-                end
-              end
-            end
-          end
-        end
       end
     end
 
@@ -747,7 +691,7 @@ def get_port
 
           context "with a non encrypted private key" do
             let(:config) do
-              base_config.merge "ssl_verify" => true
+              base_config.merge "ssl_client_authentication" => "required"
             end
             it "should be able to connect and write data" do
               result = TcpHelpers.pipelineless_input(subject, 1) do

version

@@ -1 +1 @@
-6.4.4
+7.0.0