From 40bd3cfbb0ba10a1c6dae4c1e220c351702593ca Mon Sep 17 00:00:00 2001 From: Rob Bavey Date: Mon, 25 Nov 2024 16:40:14 -0500 Subject: [PATCH 1/6] 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` --- CHANGELOG.md | 8 +++ docs/index.asciidoc | 50 +++++++---------- lib/logstash/inputs/tcp.rb | 46 +++------------ spec/inputs/tcp_spec.rb | 111 ++++++++++++------------------------- version | 2 +- 5 files changed, 71 insertions(+), 146 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ef4c3d..c6a5981 100644 --- a/CHANGELOG.md +++ b/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` + - [xxx](https://github.com/logstash-plugins/logstash-input-tcp/pull/xxx) + ## 6.4.4 - update netty to 4.1.115 [#227](https://github.com/logstash-plugins/logstash-input-tcp/pull/227) diff --git a/docs/index.asciidoc b/docs/index.asciidoc index 3d8756c..e6bd6df 100644 --- a/docs/index.asciidoc +++ b/docs/index.asciidoc @@ -121,6 +121,10 @@ filter { This plugin supports the following configuration options plus the <> 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 +<> for more details. + + [cols="<,<,<",options="header",] |======================================================================= |Setting |Input type|Required @@ -130,19 +134,16 @@ This plugin supports the following configuration options plus the <> |<>, one of `["server", "client"]`|No | <> |<>|Yes | <> |<>|No -| <> |a valid filesystem path|__Deprecated__ | <> |a valid filesystem path|No | <> |<>|No | <> |<>|No | <> |<>, one of `["none", "optional", "required"]`|No -| <> |<>|__Deprecated__ | <> |<>|No | <> |<>|No | <> |a valid filesystem path|No | <> |<>|No | <> |<>|No | <> |<>, one of `["full", "none"]`|No -| <> |<>|__Deprecated__ | <> |<>|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 <>] - - * Value type is <> - * 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 <> is `server` and <> is set. -[id="plugins-{type}s-{plugin}-ssl_enable"] -===== `ssl_enable` -deprecated[6.4.0, Replaced by <>] - - * Value type is <> - * 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 <> 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 <> and <>] - - * Value type is <> - * 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 `6.0.0` of this plugin, the following configuration options are no longer available, +and have been replaced by the following options: + + +[cols="<,<",options="header",] +|======================================================================= +|Setting|Replaced by +| ssl_cert |<> +| ssl_enable |<> +| ssl_verify |<> in `server` mode and <> in `client` mode +|======================================================================= + [id="plugins-{type}s-{plugin}-common-options"] include::{include_path}/{type}.asciidoc[] diff --git a/lib/logstash/inputs/tcp.rb b/lib/logstash/inputs/tcp.rb index df5ba49..0e888d6 100644 --- a/lib/logstash/inputs/tcp.rb +++ b/lib/logstash/inputs/tcp.rb @@ -91,8 +91,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 +102,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 +111,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 +141,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 +163,6 @@ def initialize(*args) super(*args) setup_fields! - setup_ssl_params! self.class.patch_socket_peer! @@ -367,36 +366,7 @@ def validate_server_ssl_config! 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 diff --git a/spec/inputs/tcp_spec.rb b/spec/inputs/tcp_spec.rb index ce7d801..87ac02b 100644 --- a/spec/inputs/tcp_spec.rb +++ b/spec/inputs/tcp_spec.rb @@ -54,6 +54,40 @@ def get_port end end + describe 'handling obsolete settings for client 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) { { "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 + + describe 'handling obsolete settings for server 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) { { "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 + 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) @@ -600,17 +634,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( @@ -634,70 +657,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 @@ -745,7 +704,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 diff --git a/version b/version index 49df80b..66ce77b 100644 --- a/version +++ b/version @@ -1 +1 @@ -6.4.4 +7.0.0 From c2b7bd3bbfb754912f422d7274b5540346d398bd Mon Sep 17 00:00:00 2001 From: Rob Bavey Date: Tue, 26 Nov 2024 14:37:09 -0500 Subject: [PATCH 2/6] Remove unused dependency --- lib/logstash/inputs/tcp.rb | 5 +---- logstash-input-tcp.gemspec | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/logstash/inputs/tcp.rb b/lib/logstash/inputs/tcp.rb index 0e888d6..2d41bfa 100644 --- a/lib/logstash/inputs/tcp.rb +++ b/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" @@ -366,7 +363,7 @@ def validate_server_ssl_config! def provided_ssl_enabled_config_name original_params.include?('ssl_enable') ? 'ssl_enable' : 'ssl_enabled' end - + def server? @mode == "server" end diff --git a/logstash-input-tcp.gemspec b/logstash-input-tcp.gemspec index 6c524b1..1b8407c 100644 --- a/logstash-input-tcp.gemspec +++ b/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' From ec5663b0e564314819d3ef64203565a54600afd3 Mon Sep 17 00:00:00 2001 From: Rob Bavey Date: Tue, 3 Dec 2024 08:56:40 -0500 Subject: [PATCH 3/6] Fix travis build --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index e618b27..7813d29 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,5 +4,5 @@ import: env: jobs: - - ELASTIC_STACK_VERSION=8.x DOCKER_ENV=dockerjdk17.env - - SNAPSHOT=true ELASTIC_STACK_VERSION=8.x DOCKER_ENV=dockerjdk17.env + - ELASTIC_STACK_VERSION=8.x DOCKER_ENV=dockerjdk21.env + - SNAPSHOT=true ELASTIC_STACK_VERSION=8.x DOCKER_ENV=dockerjdk21.env From 21a5c7fe0351614c2a97cba8dd9963dcb4d10ccb Mon Sep 17 00:00:00 2001 From: Rob Bavey Date: Tue, 3 Dec 2024 09:55:24 -0500 Subject: [PATCH 4/6] Use new standard Travis jobs --- .travis.yml | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index 7813d29..88596ce 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,2 @@ import: -- logstash-plugins/.ci:travis/defaults.yml@1.x -- logstash-plugins/.ci:travis/exec.yml@1.x - -env: - jobs: - - ELASTIC_STACK_VERSION=8.x DOCKER_ENV=dockerjdk21.env - - SNAPSHOT=true ELASTIC_STACK_VERSION=8.x DOCKER_ENV=dockerjdk21.env + - logstash-plugins/.ci:travis/travis.yml@1.x From 7b4252a3da468e502c4589f2894aa7c5b2dbaf2d Mon Sep 17 00:00:00 2001 From: Rob Bavey Date: Mon, 9 Dec 2024 16:38:51 -0500 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Cas Donoghue --- CHANGELOG.md | 2 +- docs/index.asciidoc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6a5981..07e0415 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ - `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` - - [xxx](https://github.com/logstash-plugins/logstash-input-tcp/pull/xxx) + - [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) diff --git a/docs/index.asciidoc b/docs/index.asciidoc index e6bd6df..e43be2c 100644 --- a/docs/index.asciidoc +++ b/docs/index.asciidoc @@ -339,7 +339,7 @@ keep alive setting for the underlying socket. [id="plugins-{type}s-{plugin}-obsolete-options"] ==== TCP Input Obsolete Configuration Options -WARNING: As of version `6.0.0` of this plugin, the following configuration options are no longer available, +WARNING: As of version `7.0.0` of this plugin, the following configuration options are no longer available, and have been replaced by the following options: From 0f133d05f0b48619cc1afbb742f7d65c823ff9b9 Mon Sep 17 00:00:00 2001 From: Rob Bavey Date: Tue, 10 Dec 2024 15:04:11 -0500 Subject: [PATCH 6/6] Separate client and server tests properly --- spec/inputs/tcp_spec.rb | 41 +++++++++++++---------------------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/spec/inputs/tcp_spec.rb b/spec/inputs/tcp_spec.rb index 20795f2..fd94789 100644 --- a/spec/inputs/tcp_spec.rb +++ b/spec/inputs/tcp_spec.rb @@ -54,40 +54,25 @@ def get_port end end - describe 'handling obsolete settings for client 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) { { "port" => port } } - let (:deprecated_config) do - config.merge({obsolete_setting[:name] => obsolete_setting[:sample_value]}) - 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 + 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 - describe 'handling obsolete settings for server 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) { { "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 - 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)