Mark deprecated SSL settings as obsolete (#58)

* 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_cacert`, which should be replaced by `ssl_certificate_authorities`
`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.2.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_cacert`, which should be replaced by `ssl_certificate_authorities`
+      - `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`
+      - [58](https://github.com/logstash-plugins/logstash-output-tcp/pull/58)
 ## 6.2.1
   - Document correct default plugin codec [#54](https://github.com/logstash-plugins/logstash-output-tcp/pull/54)
 

docs/index.asciidoc

@@ -33,26 +33,26 @@ depending on `mode`.
 
 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
 | <<plugins-{type}s-{plugin}-host>> |<<string,string>>|Yes
 | <<plugins-{type}s-{plugin}-mode>> |<<string,string>>, one of `["server", "client"]`|No
 | <<plugins-{type}s-{plugin}-port>> |<<number,number>>|Yes
 | <<plugins-{type}s-{plugin}-reconnect_interval>> |<<number,number>>|No
-| <<plugins-{type}s-{plugin}-ssl_cacert>> |a valid filesystem path|__Deprecated__
-| <<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_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>>|No
 |=======================================================================
 
 Also see <<plugins-{type}s-{plugin}-common-options>> for a list of options supported by all
@@ -97,24 +97,6 @@ When mode is `client`, the port to connect to.
 
 When connect failed,retry interval in sec.
 
-[id="plugins-{type}s-{plugin}-ssl_cacert"]
-===== `ssl_cacert` 
-deprecated[6.2.0, Replaced by <<plugins-{type}s-{plugin}-ssl_certificate_authorities>>]
-
-  * Value type is <<path,path>>
-  * There is no default value for this setting.
-
-The SSL CA certificate, chainfile or CA path. The system CA path is automatically included.
-
-[id="plugins-{type}s-{plugin}-ssl_cert"]
-===== `ssl_cert` 
-deprecated[6.2.0, Replaced by <<plugins-{type}s-{plugin}-ssl_certificate>>]
-
-  * Value type is <<path,path>>
-  * There is no default value for this setting.
-
-SSL certificate path
-
 [id="plugins-{type}s-{plugin}-ssl_certificate"]
 ===== `ssl_certificate`
 
@@ -160,15 +142,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.2.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`
 
@@ -223,15 +196,21 @@ has a hostname or IP address that matches the names within the certificate.
 
 NOTE: This setting can be used only if <<plugins-{type}s-{plugin}-mode>> is `client`.
 
-[id="plugins-{type}s-{plugin}-ssl_verify"]
-===== `ssl_verify` 
-deprecated[6.2.0, Replaced by <<plugins-{type}s-{plugin}-ssl_client_authentication>> and <<plugins-{type}s-{plugin}-ssl_verification_mode>>]
+[id="plugins-{type}s-{plugin}-obsolete-options"]
+==== TCP Output Obsolete Configuration Options
+
+WARNING: As of version `6.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. 
 
-  * Value type is <<boolean,boolean>>
-  * Default value is `false`
 
-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.
+[cols="<,<",options="header",]
+|=======================================================================
+|Setting|Replaced by
+| ssl_cacert |<<plugins-{type}s-{plugin}-ssl_certificate_authorities>>
+| 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/outputs/tcp.rb

@@ -3,7 +3,6 @@
 require "logstash/namespace"
 require "thread"
 require "logstash/util/socket_peer"
-require "logstash/plugin_mixins/normalize_config_support"
 
 # Write events over a TCP socket.
 #
@@ -13,8 +12,6 @@
 # depending on `mode`.
 class LogStash::Outputs::Tcp < LogStash::Outputs::Base
 
-  include LogStash::PluginMixins::NormalizeConfigSupport
-
   config_name "tcp"
   concurrency :single
 
@@ -35,9 +32,6 @@ class LogStash::Outputs::Tcp < LogStash::Outputs::Base
   # `client` connects to a server.
   config :mode, :validate => ["server", "client"], :default => "client"
 
-  # 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
 
@@ -48,27 +42,18 @@ class LogStash::Outputs::Tcp < LogStash::Outputs::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 => 'none'
 
-  # 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 => false, :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;
   # chains to a trusted Certificate Authority (CA); has a hostname or IP address that matches the names within the certificate.
   # "certificate": Validates the provided certificate and verifies that it’s signed by a trusted authority (CA), but does’t check the certificate hostname.
   # "none": performs no certificate validation. Disabling this severely compromises security (https://www.cs.utexas.edu/~shmat/shmat_ccs12.pdf)
   config :ssl_verification_mode, :validate => %w[full none], :default => 'full'
 
-  # The SSL CA certificate, chainfile or CA path. The system CA path is automatically included.
-  config :ssl_cacert, :validate => :path, :deprecated => "Use 'ssl_certificate_authorities' instead."
 
   # Validate client certificate or certificate chain against these authorities. You can define multiple files.
   # All the certificates will be read and added to the trust store.
   config :ssl_certificate_authorities, :validate => :path, :list => true
 
-  # SSL certificate path
-  config :ssl_cert, :validate => :path, :deprecated => "Use 'ssl_certificate' instead."
-
   # SSL certificate path
   config :ssl_certificate, :validate => :path
 
@@ -84,6 +69,11 @@ class LogStash::Outputs::Tcp < LogStash::Outputs::Base
   # The list of ciphers suite to use
   config :ssl_cipher_suites, :validate => :string, :list => true
 
+  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_cacert, :obsolete => "Use 'ssl_certificate_authorities' instead."
+  config :ssl_cert, :obsolete => "Use 'ssl_certificate' instead."
+
   class Client
 
     ##
@@ -189,11 +179,6 @@ def load_cert_store
   end
   private :load_cert_store
 
-  def initialize(*args)
-    super(*args)
-    setup_ssl_params!
-  end
-
   # @overload Base#register
   def register
     require "socket"
@@ -405,47 +390,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
-
-      # Keep backwards compatibility with the default :ssl_verify value (false)
-      if !original_params.include?('ssl_verify') && !original_params.include?('ssl_verification_mode')
-        @ssl_verification_mode = 'none'
-      end
-    end
-
-    @ssl_certificate_authorities = normalize_config(:ssl_certificate_authorities) do |normalize|
-      normalize.with_deprecated_mapping(:ssl_cacert) do |ssl_cacert|
-        if File.directory?(ssl_cacert)
-          Dir.children(ssl_cacert)
-          .map{ |f| File.join(ssl_cacert, f) }
-          .reject{ |f| File.directory?(f) || File.basename(f).start_with?('.') }
-        else
-          [ssl_cacert]
-        end
-      end
-    end
-  end
-
   def server?
     @mode == "server"
   end # def server?

logstash-output-tcp.gemspec

@@ -1,7 +1,7 @@
 Gem::Specification.new do |s|
 
   s.name            = 'logstash-output-tcp'
-  s.version         = '6.2.1'
+  s.version         = '7.0.0'
   s.licenses        = ['Apache License (2.0)']
   s.summary         = "Writes events over a TCP socket"
   s.description     = "This gem is a Logstash plugin required to be installed on top of the Logstash core pipeline using $LS_HOME/bin/logstash-plugin install gemname. This gem is not a stand-alone program"
@@ -24,7 +24,6 @@ Gem::Specification.new do |s|
   s.add_runtime_dependency 'logstash-core', '>= 8.1.0'
   s.add_runtime_dependency 'logstash-codec-json'
   s.add_runtime_dependency 'stud'
-  s.add_runtime_dependency 'logstash-mixin-normalize_config_support', '~>1.0'
 
   s.add_runtime_dependency 'jruby-openssl', '>= 0.12.2' # 0.12 supports TLSv1.3
 

spec/outputs/tcp_spec.rb

@@ -24,6 +24,25 @@
 
   let(:event) { LogStash::Event.new('message' => 'foo bar') }
 
+  ['server', 'client'].each do |mode|
+    describe "handling obsolete settings for #{mode} mode" do
+      [{:name => 'ssl_cert', :replacement => 'ssl_certificate', :sample_value => "certificate_path"},
+       {:name => 'ssl_cacert', :replacement => 'ssl_certificate_authorities', :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 (:deprecated_config) do
+            config.merge({'mode' => mode, obsolete_setting[:name] => obsolete_setting[:sample_value]})
+          end
+
+          it "should raise a config error with the appropriate message" do
+            expect { LogStash::Outputs::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
+
   context 'failing to connect' do
 
     before { subject.register }
@@ -214,7 +233,7 @@
 
       context 'with supported protocol' do
 
-        let(:config) { super().merge("ssl_supported_protocols" => ['TLSv1.2']) }
+        let(:config) { super().merge("ssl_supported_protocols" => ['TLSv1.2'], "ssl_verification_mode" => "none") }
 
         let(:server_min_version) { 'TLS1_2' }
 
@@ -277,7 +296,7 @@
     context "and protocol is TLSv1.3" do
       let(:key_file) { File.join(FIXTURES_PATH, 'plaintext/instance.key') }
       let(:crt_file) { File.join(FIXTURES_PATH, 'plaintext/instance.crt') }
-      let(:config) { super().merge("ssl_certificate" => crt_file, "ssl_key" => key_file) }
+      let(:config) { super().merge("ssl_certificate" => crt_file, "ssl_key" => key_file, "ssl_verification_mode" => "none") }
 
       let(:secure_server) do
         ssl_context = OpenSSL::SSL::SSLContext.new
@@ -374,16 +393,6 @@
         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
@@ -409,53 +418,6 @@
       end
     end
 
-    context "with deprecated settings" do
-      let(:ssl_verify) { true }
-      let(:certificate_path) { File.join(FIXTURES_PATH, 'plaintext/instance.crt') }
-      let(:config) do
-        {
-          "host" => "127.0.0.1",
-          "port" => port,
-          "ssl_enable" => true,
-          "ssl_cert" => certificate_path,
-          "ssl_key" => File.join(FIXTURES_PATH, 'plaintext/instance.key'),
-          "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 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 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
-
     context "with ssl_client_authentication" do
       let(:config) do
         super().merge 'ssl_client_authentication' => 'required'