From 0e65a7697eacdab2bba958e1b0f5284c8c847b1a Mon Sep 17 00:00:00 2001 From: Edmo Vamerlatti Costa <11836452+edmocosta@users.noreply.github.com> Date: Wed, 30 Aug 2023 17:21:50 +0200 Subject: [PATCH] Standardised SSL settings (#42) This commit adds standardized SSL settings and deprecates their non-standard counterparts. Deprecated settings will continue to work, and will provide pipeline maintainers with guidance toward using their standardized counterparts: - Adds new `ssl_truststore_path`, `ssl_truststore_password`, and `ssl_truststore_type` settings for configuring SSL-trust using a PKCS-12 or JKS trust store, deprecating their `truststore`, `truststore_password`, and `truststore_type` counterparts. - Adds new `ssl_certificate_authorities` setting for configuring SSL-trust using a PEM-formated list certificate authorities, deprecating its `cacert` counterpart. - Adds new `ssl_keystore_path`, `ssl_keystore_password`, and `ssl_keystore_type` settings for configuring SSL-identity using a PKCS-12 or JKS key store, deprecating their `keystore`, `keystore_password`, and `keystore_type` counterparts. - Adds new `ssl_certificate` and `ssl_key` settings for configuring SSL-identity using a PEM-formatted certificate/key pair, deprecating their `client_cert` and `client_key` counterparts. Other changes: - Added a way for plugin maintainers to include this mixin _without_ supporting the now-deprecated SSL options. - Added the `ssl_cipher_suites` option --------- Co-authored-by: Ry Biesemeyer --- .travis.yml | 20 +- CHANGELOG.md | 9 + Gemfile | 8 + lib/logstash/plugin_mixins/http_client.rb | 337 +++++++------- .../deprecated_ssl_config_support.rb | 74 +++ logstash-mixin-http_client.gemspec | 3 +- spec/plugin_mixin/http_client_spec.rb | 177 +------ spec/plugin_mixin/http_client_ssl_spec.rb | 438 ++++++++++++++++++ 8 files changed, 716 insertions(+), 350 deletions(-) create mode 100644 lib/logstash/plugin_mixins/http_client/deprecated_ssl_config_support.rb create mode 100644 spec/plugin_mixin/http_client_ssl_spec.rb diff --git a/.travis.yml b/.travis.yml index 6e328f6..074287d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,19 +1,3 @@ --- -sudo: false -language: ruby -cache: bundler - -script: bundle exec rspec spec -jdk: openjdk8 -matrix: - include: - - rvm: jruby-9.2.20.1 - env: LOGSTASH_BRANCH=8.0 - - rvm: jruby-9.2.20.1 - env: LOGSTASH_BRANCH=7.16 - - rvm: jruby-9.1.13.0 - env: LOGSTASH_BRANCH=6.7 - - rvm: jruby-9.2.7.0 - env: LOGSTASH_BRANCH=6.8 - fast_finish: true -before_install: gem install bundler -v '< 2' +import: + - logstash-plugins/.ci:travis/travis.yml@1.x \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b1deb8..01ecfce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,12 @@ +## 7.3.0 + - Adds standardized SSL settings and deprecates their non-standard counterparts. Deprecated settings will continue to work, and will provide pipeline maintainers with guidance toward using their standardized counterparts [#42](https://github.com/logstash-plugins/logstash-mixin-http_client/pull/42) + - Adds new `ssl_truststore_path`, `ssl_truststore_password`, and `ssl_truststore_type` settings for configuring SSL-trust using a PKCS-12 or JKS trust store, deprecating their `truststore`, `truststore_password`, and `truststore_type` counterparts. + - Adds new `ssl_certificate_authorities` setting for configuring SSL-trust using a PEM-formated list certificate authorities, deprecating its `cacert` counterpart. + - Adds new `ssl_keystore_path`, `ssl_keystore_password`, and `ssl_keystore_type` settings for configuring SSL-identity using a PKCS-12 or JKS key store, deprecating their `keystore`, `keystore_password`, and `keystore_type` counterparts. + - Adds new `ssl_certificate` and `ssl_key` settings for configuring SSL-identity using a PEM-formatted certificate/key pair, deprecating their `client_cert` and `client_key` counterparts. + - Added a way for plugin maintainers to include this mixin _without_ supporting the now-deprecated SSL options. + - Added the `ssl_cipher_suites` option + ## 7.2.0 - Feat: add `ssl_supported_protocols` option [#40](https://github.com/logstash-plugins/logstash-mixin-http_client/pull/40) diff --git a/Gemfile b/Gemfile index 2b03d18..4cf9869 100644 --- a/Gemfile +++ b/Gemfile @@ -2,3 +2,11 @@ source 'https://rubygems.org' # Specify your gem's dependencies in logstash-mass_effect.gemspec gemspec + +logstash_path = ENV["LOGSTASH_PATH"] || "../../logstash" +use_logstash_source = ENV["LOGSTASH_SOURCE"] && ENV["LOGSTASH_SOURCE"].to_s == "1" + +if Dir.exist?(logstash_path) && use_logstash_source + gem 'logstash-core', :path => "#{logstash_path}/logstash-core" + gem 'logstash-core-plugin-api', :path => "#{logstash_path}/logstash-core-plugin-api" +end \ No newline at end of file diff --git a/lib/logstash/plugin_mixins/http_client.rb b/lib/logstash/plugin_mixins/http_client.rb index 26c1772..b1b689a 100644 --- a/lib/logstash/plugin_mixins/http_client.rb +++ b/lib/logstash/plugin_mixins/http_client.rb @@ -7,203 +7,230 @@ module LogStash::PluginMixins::HttpClient class InvalidHTTPConfigError < StandardError; end + def self.[](**a) + Adapter.new(**a) + end + def self.included(base) - require 'manticore' - base.extend(self) - base.setup_http_client_config + # TODO: deprecate the act of including this mixin directly, + # in a way that turns focus to plugin maintainers since + # an end-user cannot act to resolve the issue. + base.include(Adapter.new(with_deprecated: true)) end - public - def setup_http_client_config - # Timeout (in seconds) for the entire request - config :request_timeout, :validate => :number, :default => 60 + class Adapter < Module + def initialize(with_deprecated: false) + @include_dep = with_deprecated + end - # Timeout (in seconds) to wait for data on the socket. Default is `10s` - config :socket_timeout, :validate => :number, :default => 10 + def included(base) + base.include(Implementation) + if @include_dep + require_relative 'http_client/deprecated_ssl_config_support' + base.include(DeprecatedSslConfigSupport) + end + nil + end + end + private_constant :Adapter - # Timeout (in seconds) to wait for a connection to be established. Default is `10s` - config :connect_timeout, :validate => :number, :default => 10 + module Implementation + def self.included(base) + require 'manticore' - # Should redirects be followed? Defaults to `true` - config :follow_redirects, :validate => :boolean, :default => true + # Timeout (in seconds) for the entire request + base.config :request_timeout, :validate => :number, :default => 60 - # Max number of concurrent connections. Defaults to `50` - config :pool_max, :validate => :number, :default => 50 + # Timeout (in seconds) to wait for data on the socket. Default is `10s` + base.config :socket_timeout, :validate => :number, :default => 10 - # Max number of concurrent connections to a single host. Defaults to `25` - config :pool_max_per_route, :validate => :number, :default => 25 + # Timeout (in seconds) to wait for a connection to be established. Default is `10s` + base.config :connect_timeout, :validate => :number, :default => 10 - # Turn this on to enable HTTP keepalive support. We highly recommend setting `automatic_retries` to at least - # one with this to fix interactions with broken keepalive implementations. - config :keepalive, :validate => :boolean, :default => true + # Should redirects be followed? Defaults to `true` + base.config :follow_redirects, :validate => :boolean, :default => true - # How many times should the client retry a failing URL. We highly recommend NOT setting this value - # to zero if keepalive is enabled. Some servers incorrectly end keepalives early requiring a retry! - # Note: if `retry_non_idempotent` is set only GET, HEAD, PUT, DELETE, OPTIONS, and TRACE requests will be retried. - config :automatic_retries, :validate => :number, :default => 1 + # Max number of concurrent connections. Defaults to `50` + base.config :pool_max, :validate => :number, :default => 50 - # If `automatic_retries` is enabled this will cause non-idempotent HTTP verbs (such as POST) to be retried. - config :retry_non_idempotent, :validate => :boolean, :default => false + # Max number of concurrent connections to a single host. Defaults to `25` + base.config :pool_max_per_route, :validate => :number, :default => 25 - # How long to wait before checking if the connection is stale before executing a request on a connection using keepalive. - # # You may want to set this lower, possibly to 0 if you get connection errors regularly - # Quoting the Apache commons docs (this client is based Apache Commmons): - # 'Defines period of inactivity in milliseconds after which persistent connections must be re-validated prior to being leased to the consumer. Non-positive value passed to this method disables connection validation. This check helps detect connections that have become stale (half-closed) while kept inactive in the pool.' - # See https://hc.apache.org/httpcomponents-client-ga/httpclient/apidocs/org/apache/http/impl/conn/PoolingHttpClientConnectionManager.html#setValidateAfterInactivity(int)[these docs for more info] - config :validate_after_inactivity, :validate => :number, :default => 200 + # Turn this on to enable HTTP keepalive support. We highly recommend setting `automatic_retries` to at least + # one with this to fix interactions with broken keepalive implementations. + base.config :keepalive, :validate => :boolean, :default => true - # If you need to use a custom X.509 CA (.pem certs) specify the path to that here - config :cacert, :validate => :path + # How many times should the client retry a failing URL. We highly recommend NOT setting this value + # to zero if keepalive is enabled. Some servers incorrectly end keepalives early requiring a retry! + # Note: if `retry_non_idempotent` is set only GET, HEAD, PUT, DELETE, OPTIONS, and TRACE requests will be retried. + base.config :automatic_retries, :validate => :number, :default => 1 - # If you'd like to use a client certificate (note, most people don't want this) set the path to the x509 cert here - config :client_cert, :validate => :path - # If you're using a client certificate specify the path to the encryption key here - config :client_key, :validate => :path + # If `automatic_retries` is enabled this will cause non-idempotent HTTP verbs (such as POST) to be retried. + base.config :retry_non_idempotent, :validate => :boolean, :default => false - # If you need to use a custom keystore (`.jks`) specify that here. This does not work with .pem keys! - config :keystore, :validate => :path + # How long to wait before checking if the connection is stale before executing a request on a connection using keepalive. + # # You may want to set this lower, possibly to 0 if you get connection errors regularly + # Quoting the Apache commons docs (this client is based Apache Commmons): + # 'Defines period of inactivity in milliseconds after which persistent connections must be re-validated prior to being leased to the consumer. Non-positive value passed to this method disables connection validation. This check helps detect connections that have become stale (half-closed) while kept inactive in the pool.' + # See https://hc.apache.org/httpcomponents-client-ga/httpclient/apidocs/org/apache/http/impl/conn/PoolingHttpClientConnectionManager.html#setValidateAfterInactivity(int)[these docs for more info] + base.config :validate_after_inactivity, :validate => :number, :default => 200 - # Specify the keystore password here. - # Note, most .jks files created with keytool require a password! - config :keystore_password, :validate => :password + # If you need to use a custom X.509 CA (.pem certs) specify the path to that here + base.config :ssl_certificate_authorities, :validate => :path, :list => :true - # Specify the keystore type here. One of `JKS` or `PKCS12`. Default is `JKS` - config :keystore_type, :validate => :string, :default => "JKS" + # If you'd like to use a client certificate (note, most people don't want this) set the path to the x509 cert here + base.config :ssl_certificate, :validate => :path - # Naming aligned with the Elastic stack. - # full: verifies that the provided certificate is signed by a trusted authority (CA) and also verifies that the - # server’s hostname (or IP address) matches the names identified within the certificate - # none: no verification of the server’s certificate - config :ssl_verification_mode, :validate => ['full', 'none'], :default => 'full' + # If you're using a client certificate specify the path to the encryption key here + base.config :ssl_key, :validate => :path - # NOTE: the default setting [] uses Java SSL engine defaults. - config :ssl_supported_protocols, :validate => ['TLSv1.1', 'TLSv1.2', 'TLSv1.3'], :default => [], :list => true + # If you need to use a custom keystore (`.jks`) specify that here. This does not work with .pem keys! + base.config :ssl_keystore_path, :validate => :path - # If you need to use a custom truststore (`.jks`) specify that here. This does not work with .pem certs! - config :truststore, :validate => :path + # Specify the keystore password here. + # Note, most .jks files created with keytool require a password! + base.config :ssl_keystore_password, :validate => :password - # Specify the truststore password here. - # Note, most .jks files created with keytool require a password! - config :truststore_password, :validate => :password + # Specify the keystore type here. One of `jks` or `pkcs12`. + # The default value is inferred from the filename. + # Note: If it's unable to determine the type based on the filename, it uses the + # `keystore.type` security property, or "jks" as default value. + base.config :ssl_keystore_type, :validate => %w(pkcs12 jks) - # Specify the truststore type here. One of `JKS` or `PKCS12`. Default is `JKS` - config :truststore_type, :validate => :string, :default => "JKS" + # Naming aligned with the Elastic stack. + # full: verifies that the provided certificate is signed by a trusted authority (CA) and also verifies that the + # server’s hostname (or IP address) matches the names identified within the certificate + # none: no verification of the server’s certificate + base.config :ssl_verification_mode, :validate => ['full', 'none'], :default => 'full' - # Enable cookie support. With this enabled the client will persist cookies - # across requests as a normal web browser would. Enabled by default - config :cookies, :validate => :boolean, :default => true + # The list of cipher suites to use, listed by priorities. + # Supported cipher suites vary depending on which version of Java is used. + base.config :ssl_cipher_suites, :validate => :string, :list => true - # If you'd like to use an HTTP proxy . This supports multiple configuration syntaxes: - # - # 1. Proxy host in form: `http://proxy.org:1234` - # 2. Proxy host in form: `{host => "proxy.org", port => 80, scheme => 'http', user => 'username@host', password => 'password'}` - # 3. Proxy host in form: `{url => 'http://proxy.org:1234', user => 'username@host', password => 'password'}` - config :proxy + # NOTE: the default setting [] uses Java SSL engine defaults. + base.config :ssl_supported_protocols, :validate => ['TLSv1.1', 'TLSv1.2', 'TLSv1.3'], :default => [], :list => true - # Username to use for HTTP auth. - config :user, :validate => :string + # If you need to use a custom truststore (`.jks`) specify that here. This does not work with .pem certs! + base.config :ssl_truststore_path, :validate => :path - # Password to use for HTTP auth - config :password, :validate => :password - end + # Specify the truststore password here. + # Note, most .jks files created with keytool require a password! + base.config :ssl_truststore_password, :validate => :password + + # Specify the truststore type here. One of `JKS` or `PKCS12`. + # The default value is inferred from the filename. + # Note: If it's unable to determine the type based on the filename, it uses the + # `keystore.type` security property, or "jks" as default value. + base.config :ssl_truststore_type, :validate => %w(pkcs12 jks) + + # Enable cookie support. With this enabled the client will persist cookies + # across requests as a normal web browser would. Enabled by default + base.config :cookies, :validate => :boolean, :default => true - public - def client_config - c = { - connect_timeout: @connect_timeout, - socket_timeout: @socket_timeout, - request_timeout: @request_timeout, - follow_redirects: @follow_redirects, - automatic_retries: @automatic_retries, - retry_non_idempotent: @retry_non_idempotent, - check_connection_timeout: @validate_after_inactivity, - pool_max: @pool_max, - pool_max_per_route: @pool_max_per_route, - cookies: @cookies, - keepalive: @keepalive - } - - if @proxy - # Symbolize keys if necessary - c[:proxy] = @proxy.is_a?(Hash) ? - @proxy.reduce({}) {|memo,(k,v)| memo[k.to_sym] = v; memo} : - @proxy + # If you'd like to use an HTTP proxy . This supports multiple configuration syntaxes: + # + # 1. Proxy host in form: `http://proxy.org:1234` + # 2. Proxy host in form: `{host => "proxy.org", port => 80, scheme => 'http', user => 'username@host', password => 'password'}` + # 3. Proxy host in form: `{url => 'http://proxy.org:1234', user => 'username@host', password => 'password'}` + base.config :proxy + + # Username to use for HTTP auth. + base.config :user, :validate => :string + + # Password to use for HTTP auth + base.config :password, :validate => :password end - if @user - if !@password || !@password.value - raise ::LogStash::ConfigurationError, "User '#{@user}' specified without password!" + public + + def client_config + c = { + connect_timeout: @connect_timeout, + socket_timeout: @socket_timeout, + request_timeout: @request_timeout, + follow_redirects: @follow_redirects, + automatic_retries: @automatic_retries, + retry_non_idempotent: @retry_non_idempotent, + check_connection_timeout: @validate_after_inactivity, + pool_max: @pool_max, + pool_max_per_route: @pool_max_per_route, + cookies: @cookies, + keepalive: @keepalive + } + + if @proxy + # Symbolize keys if necessary + c[:proxy] = @proxy.is_a?(Hash) ? + @proxy.reduce({}) {|memo,(k,v)| memo[k.to_sym] = v; memo} : + @proxy end - # Symbolize keys if necessary - c[:auth] = { - :user => @user, - :password => @password.value, - :eager => true - } - end + if @user + if !@password || !@password.value + raise ::LogStash::ConfigurationError, "User '#{@user}' specified without password!" + end + + # Symbolize keys if necessary + c[:auth] = { + :user => @user, + :password => @password.value, + :eager => true + } + end - c[:ssl] = {} - if @cacert - c[:ssl][:ca_file] = @cacert + c[:ssl] = ssl_options + + c end - if @truststore - c[:ssl].merge!( - :truststore => @truststore, - :truststore_type => @truststore_type, - :truststore_password => @truststore_password.value - ) - - if c[:ssl][:truststore_password].nil? - raise LogStash::ConfigurationError, "Truststore declared without a password! This is not valid, please set the 'truststore_password' option" + private + + def ssl_options + + options = {} + if @ssl_certificate_authorities&.any? + raise LogStash::ConfigurationError, 'Multiple values on `ssl_certificate_authorities` are not supported by this plugin' if @ssl_certificate_authorities.size > 1 + + options[:ca_file] = @ssl_certificate_authorities.first end - end - if @keystore - c[:ssl].merge!( - :keystore => @keystore, - :keystore_type => @keystore_type, - :keystore_password => @keystore_password.value - ) + if @ssl_truststore_path + options[:truststore] = @ssl_truststore_path + options[:truststore_type] = @ssl_truststore_type if @ssl_truststore_type + options[:truststore_password] = @ssl_truststore_password.value if @ssl_truststore_password + elsif @ssl_truststore_password + fail LogStash::ConfigurationError, "An `ssl_truststore_password` cannot be specified unless `ssl_truststore_path` is also provided." + end - if c[:ssl][:keystore_password].nil? - raise LogStash::ConfigurationError, "Keystore declared without a password! This is not valid, please set the 'keystore_password' option" + if @ssl_keystore_path + options[:keystore] = @ssl_keystore_path + options[:keystore_type] = @ssl_keystore_type if @ssl_keystore_type + options[:keystore_password] = @ssl_keystore_password.value if @ssl_keystore_password + elsif @ssl_keystore_password + fail LogStash::ConfigurationError, "An `ssl_keystore_password` cannot be specified unless `ssl_keystore_path` is also provided." end - end - if @client_cert && @client_key - c[:ssl][:client_cert] = @client_cert - c[:ssl][:client_key] = @client_key - elsif !!@client_cert ^ !!@client_key - raise InvalidHTTPConfigError, "You must specify both client_cert and client_key for an HTTP client, or neither!" - end + if @ssl_certificate && @ssl_key + options[:client_cert] = @ssl_certificate + options[:client_key] = @ssl_key + elsif !!@ssl_certificate ^ !!@ssl_key + raise InvalidHTTPConfigError, "You must specify both `ssl_certificate` and `ssl_key` for an HTTP client, or neither!" + end - case @ssl_verification_mode - when 'full' - # NOTE: would make sense to have :browser here but historically we've used the (:strict) default - # - # supporting `ssl_verification_mode => strict` the same way ES does might require upstream Manticore - # changes -> as in ES/Beats setting `strict` means: "if the SAN is empty return an error" - c[:ssl][:verify] = :strict # :default - when 'none' - c[:ssl][:verify] = :disable - end + options[:verify] = @ssl_verification_mode == 'full' ? :strict : :disable + options[:protocols] = @ssl_supported_protocols if @ssl_supported_protocols&.any? + options[:cipher_suites] = @ssl_cipher_suites if @ssl_cipher_suites&.any? - if @ssl_supported_protocols && @ssl_supported_protocols.any? - c[:ssl][:protocols] = @ssl_supported_protocols + options end - c - end - - private - def make_client - Manticore::Client.new(client_config) - end + def make_client + Manticore::Client.new(client_config) + end - public - def client - @client ||= make_client + public + def client + @client ||= make_client + end end -end +end \ No newline at end of file diff --git a/lib/logstash/plugin_mixins/http_client/deprecated_ssl_config_support.rb b/lib/logstash/plugin_mixins/http_client/deprecated_ssl_config_support.rb new file mode 100644 index 0000000..25cb41f --- /dev/null +++ b/lib/logstash/plugin_mixins/http_client/deprecated_ssl_config_support.rb @@ -0,0 +1,74 @@ +module LogStash::PluginMixins::HttpClient + module DeprecatedSslConfigSupport + def self.included(base) + fail ArgumentError unless base <= LogStash::PluginMixins::HttpClient::Implementation + + require 'logstash/plugin_mixins/normalize_config_support' + base.include(LogStash::PluginMixins::NormalizeConfigSupport) + + # If you need to use a custom X.509 CA (.pem certs) specify the path to that here + base.config :cacert, :validate => :path, :deprecated => 'Use `ssl_certificate_authorities` instead' + # If you'd like to use a client certificate (note, most people don't want this) set the path to the x509 cert here + base.config :client_cert, :validate => :path, :deprecated => 'Use `ssl_certificate` instead' + # If you're using a client certificate specify the path to the encryption key here + base.config :client_key, :validate => :path, :deprecated => 'Use `ssl_key` instead' + # If you need to use a custom keystore (`.jks`) specify that here. This does not work with .pem keys! + base.config :keystore, :validate => :path, :deprecated => 'Use `ssl_keystore_path` instead' + # Specify the keystore password here. + # Note, most .jks files created with keytool require a password! + base.config :keystore_password, :validate => :password, :deprecated => 'Use `ssl_keystore_password` instead' + # Specify the keystore type here. One of `JKS` or `PKCS12`. Default is `JKS` + base.config :keystore_type, :validate => :string, :default => 'JKS', :deprecated => 'Use `ssl_keystore_type` instead' + # If you need to use a custom truststore (`.jks`) specify that here. This does not work with .pem certs! + base.config :truststore, :validate => :path, :deprecated => 'Use `ssl_truststore_path` instead' + # Specify the truststore password here. + # Note, most .jks files created with keytool require a password! + base.config :truststore_password, :validate => :password, :deprecated => 'Use `ssl_truststore_password` instead' + # Specify the truststore type here. One of `JKS` or `PKCS12`. Default is `JKS` + base.config :truststore_type, :validate => :string, :default => 'JKS', :deprecated => 'Use `ssl_truststore_type` instead' + # NOTE: the default setting [] uses Java SSL engine defaults. + end + + def initialize(*a) + super + + @ssl_certificate_authorities = normalize_config(:ssl_certificate_authorities) do |normalize| + normalize.with_deprecated_mapping(:cacert) do |cacert| + [cacert] + end + end + + @ssl_certificate = normalize_config(:ssl_certificate) do |normalize| + normalize.with_deprecated_alias(:client_cert) + end + + @ssl_key = normalize_config(:ssl_key) do |normalize| + normalize.with_deprecated_alias(:client_key) + end + + %w[keystore truststore].each do |store| + %w[path type password].each do |variable| + config_name = "ssl_#{store}_#{variable}" + normalized_value = normalize_config(config_name) do |normalize| + deprecated_config_alias = variable == 'path' ? store : "#{store}_#{variable}" + normalize.with_deprecated_alias(deprecated_config_alias.to_sym) + end + instance_variable_set("@#{config_name}", normalized_value) + end + end + end + + def ssl_options + fail(InvalidHTTPConfigError, "When `client_cert` is provided, `client_key` must also be provided") if @client_cert && !@client_key + fail(InvalidHTTPConfigError, "A `client_key` is not allowed unless a `client_cert` is provided") if @client_key && !@client_cert + + fail(LogStash::ConfigurationError, "When `keystore` is provided, `keystore_password` must also be provided") if @keystore && !@keystore_password + fail(LogStash::ConfigurationError, "A `keystore_password` is not allowed unless a `keystore` is provided") if @keystore_password && !@keystore + + fail(LogStash::ConfigurationError, "When `truststore` is provided, `truststore_password` must also be provided") if @truststore && !@truststore_password + fail(LogStash::ConfigurationError, "A `truststore_password` is not allowed unless a `truststore` is provided") if @truststore_password && !@truststore + + super + end + end +end \ No newline at end of file diff --git a/logstash-mixin-http_client.gemspec b/logstash-mixin-http_client.gemspec index 6f7f04e..efe6fba 100644 --- a/logstash-mixin-http_client.gemspec +++ b/logstash-mixin-http_client.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |s| s.name = 'logstash-mixin-http_client' - s.version = '7.2.0' + s.version = '7.3.0' s.licenses = ['Apache License (2.0)'] s.summary = "AWS mixins to provide a unified interface for Amazon Webservice" 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" @@ -19,6 +19,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency "logstash-core-plugin-api", ">= 1.60", "<= 2.99" s.add_runtime_dependency 'logstash-codec-plain' s.add_runtime_dependency 'manticore', '>= 0.8.0', '< 1.0.0' + s.add_runtime_dependency 'logstash-mixin-normalize_config_support', '~>1.0' s.add_development_dependency 'logstash-devutils' s.add_development_dependency 'stud' diff --git a/spec/plugin_mixin/http_client_spec.rb b/spec/plugin_mixin/http_client_spec.rb index 9dc4eb3..140ea5c 100644 --- a/spec/plugin_mixin/http_client_spec.rb +++ b/spec/plugin_mixin/http_client_spec.rb @@ -5,6 +5,7 @@ class Dummy < LogStash::Inputs::Base include LogStash::PluginMixins::HttpClient + config_name 'dummy' end describe LogStash::PluginMixins::HttpClient do @@ -29,51 +30,6 @@ class Dummy < LogStash::Inputs::Base expect(impl.send(:client)).to eql(impl.client) end - shared_examples "setting ca bundles" do |key, type| - subject { Dummy.new(conf).send(:client_config) } - - it "should correctly set the path" do - expect(subject[:ssl][key]).to eql(path), "Expected to find path for #{key}" - end - - if type == :jks - let(:store_password) { conf["#{key}_password"] } - let(:store_type) { conf["#{key}_type"]} - - it "should set the bundle password" do - expect(subject[:ssl]["#{key}_password".to_sym]).to eql(store_password) - end - - it "should set the bundle type" do - expect(subject[:ssl]["#{key}_type".to_sym]).to eql(store_type) - end - end - end - - describe "with a custom ssl bundle" do - let(:file) { Stud::Temporary.file } - let(:path) { file.path } - after { File.unlink(path)} - - context "with x509" do - let(:conf) { basic_config.merge("cacert" => path) } - - include_examples("setting ca bundles", :ca_file) - end - - context "with JKS" do - let(:conf) { - basic_config.merge( - "truststore" => path, - "truststore_password" => "foobar", - "truststore_type" => "jks" - ) - } - - include_examples("setting ca bundles", :truststore, :jks) - end - end - describe "with a custom validate_after_activity" do subject { Dummy.new(client_config).send(:client_config) } @@ -120,135 +76,4 @@ class Dummy < LogStash::Inputs::Base end end end - - ["keystore", "truststore"].each do |store| - describe "with a custom #{store}" do - let(:file) { Stud::Temporary.file } - let(:path) { file.path } - let(:password) { "foo" } - after { File.unlink(path)} - - let(:conf) { - basic_config.merge( - store => path, - "#{store}_password" => password, - "#{store}_type" => "jks" - ) - } - - include_examples("setting ca bundles", store.to_sym, :jks) - - context "with no password set" do - let(:password) { nil } - - it "should raise an error" do - expect do - Dummy.new(conf).client_config - end.to raise_error(LogStash::ConfigurationError) - end - end - end - end - - describe "with a client cert" do - let(:file) { Stud::Temporary.file } - let(:path) { file.path } - after { File.unlink(path)} - - context "with correct client certs" do - let(:conf) { basic_config.merge("client_cert" => path, "client_key" => path) } - - it "should create without error" do - expect { - Dummy.new(conf).client_config - }.not_to raise_error - end - end - - shared_examples("raising a configuration error") do - it "should raise an error error" do - expect { - Dummy.new(conf).client_config - }.to raise_error(LogStash::PluginMixins::HttpClient::InvalidHTTPConfigError) - end - end - - context "without a key" do - let(:conf) { basic_config.merge("client_cert" => path) } - - include_examples("raising a configuration error") - end - - context "without a cert" do - let(:conf) { basic_config.merge("client_key" => path) } - - include_examples("raising a configuration error") - end - end - - describe "with verify mode" do - let(:file) { Stud::Temporary.file } - let(:path) { file.path } - after { File.unlink(path)} - - context "default" do - let(:conf) { basic_config } - - it "sets manticore verify to :strict" do - expect( Dummy.new(conf).client_config[:ssl] ).to include :verify => :strict - end - end - - context "'full'" do - let(:conf) { basic_config.merge("ssl_verification_mode" => 'full') } - - it "sets manticore verify to :strict" do - expect( Dummy.new(conf).client_config[:ssl] ).to include :verify => :strict - end - end - - context "'none'" do - let(:conf) { basic_config.merge("ssl_verification_mode" => 'none') } - - it "sets manticore verify to :disable" do - expect( Dummy.new(conf).client_config[:ssl] ).to include :verify => :disable - end - end - - end - - describe "with supported protocols" do - context "default" do - let(:conf) { basic_config } - - it "does not set manticore protocols option" do - expect( Dummy.new(conf).client_config[:ssl] ).to_not include :protocols - end - end - - context "empty" do - let(:conf) { basic_config.merge("ssl_supported_protocols" => []) } - - it "does not set manticore protocols option" do - expect( Dummy.new(conf).client_config[:ssl] ).to_not include :protocols - end - end - - context "'TLSv1.3'" do - let(:conf) { basic_config.merge("ssl_supported_protocols" => ['TLSv1.3']) } - - it "sets manticore protocols option" do - expect( Dummy.new(conf).client_config[:ssl] ).to include :protocols => ['TLSv1.3'] - end - end - - context "'TLSv1.2' and 'TLSv1.3'" do - let(:conf) { basic_config.merge("ssl_supported_protocols" => ['TLSv1.3', 'TLSv1.2']) } - - it "sets manticore protocols option" do - expect( Dummy.new(conf).client_config[:ssl] ).to include :protocols => ['TLSv1.3', 'TLSv1.2'] - end - end - - end end diff --git a/spec/plugin_mixin/http_client_ssl_spec.rb b/spec/plugin_mixin/http_client_ssl_spec.rb new file mode 100644 index 0000000..281e8b2 --- /dev/null +++ b/spec/plugin_mixin/http_client_ssl_spec.rb @@ -0,0 +1,438 @@ +require 'logstash/devutils/rspec/spec_helper' +require 'logstash/plugin_mixins/http_client' +require 'logstash/plugin_mixins/http_client/deprecated_ssl_config_support' +require 'stud/temporary' + +shared_examples 'setting ca bundles' do |key, type| + subject(:client_config) { plugin_class.new(conf).send(:client_config) } + + it 'should correctly set the path' do + expect(client_config[:ssl][key]).to eql(path), "Expected to find path for #{key}" + end + + if type == :jks + let(:store_password) { conf["#{use_deprecated_config ? '' : 'ssl_'}#{key}_password"] } + let(:store_type) { conf["#{use_deprecated_config ? '' : 'ssl_'}#{key}_type"]} + + it 'should set the bundle password' do + expect(client_config[:ssl]["#{key}_password".to_sym]).to eql(store_password) + end + + it 'should set the bundle type' do + expect(client_config[:ssl]["#{key}_type".to_sym]).to eql(store_type) + end + end +end + +shared_examples 'a deprecated setting with guidance' do |deprecations_and_guidance| + + let(:logger_stub) { double('Logger').as_null_object } + + before(:each) do + allow(plugin_class).to receive(:logger).and_return(logger_stub) + end + + deprecations_and_guidance.each do |deprecated_setting_name, canonical_setting_name| + it "emits a warning about the setting `#{deprecated_setting_name}` being deprecated and provides guidance to use `#{canonical_setting_name}`" do + plugin_class.new(conf) + + deprecation_text = "deprecated config setting \"#{deprecated_setting_name}\" set" + guidance_text = "Use `#{canonical_setting_name}` instead" + + expect(logger_stub).to have_received(:warn).with(a_string_including(deprecation_text).and(including(guidance_text)), anything) + end + end +end + +shared_examples 'with common ssl options' do + describe 'with verify mode' do + let(:file) { Stud::Temporary.file } + let(:path) { file.path } + after { File.unlink(path)} + + context 'default' do + let(:conf) { basic_config } + + it 'sets manticore verify to :strict' do + expect(plugin_class.new(conf).client_config[:ssl]).to include :verify => :strict + end + end + + context "'full'" do + let(:conf) { basic_config.merge('ssl_verification_mode' => 'full') } + + it 'sets manticore verify to :strict' do + expect(plugin_class.new(conf).client_config[:ssl]).to include :verify => :strict + end + end + + context "'none'" do + let(:conf) { basic_config.merge('ssl_verification_mode' => 'none') } + + it 'sets manticore verify to :disable' do + expect(plugin_class.new(conf).client_config[:ssl]).to include :verify => :disable + end + end + end + + describe 'with supported protocols' do + context 'default' do + let(:conf) { basic_config } + + it 'does not set manticore protocols option' do + expect(plugin_class.new(conf).client_config[:ssl]).to_not include :protocols + end + end + + context 'empty' do + let(:conf) { basic_config.merge('ssl_supported_protocols' => []) } + + it 'does not set manticore protocols option' do + expect(plugin_class.new(conf).client_config[:ssl]).to_not include :protocols + end + end + + context "'TLSv1.3'" do + let(:conf) { basic_config.merge('ssl_supported_protocols' => ['TLSv1.3']) } + + it 'sets manticore protocols option' do + expect(plugin_class.new(conf).client_config[:ssl]).to include :protocols => ['TLSv1.3'] + end + end + + context "'TLSv1.2' and 'TLSv1.3'" do + let(:conf) { basic_config.merge('ssl_supported_protocols' => ['TLSv1.3', 'TLSv1.2']) } + + it 'sets manticore protocols option' do + expect(plugin_class.new(conf).client_config[:ssl]).to include :protocols => ['TLSv1.3', 'TLSv1.2'] + end + end + end + + describe 'with ssl_cipher_suites' do + context 'default' do + let(:conf) { basic_config } + + it 'does not set manticore cipher_suites option' do + expect(plugin_class.new(conf).client_config[:ssl]).to_not include :cipher_suites + end + end + + context 'empty' do + let(:conf) { basic_config.merge('ssl_cipher_suites' => []) } + + it 'does not set manticore cipher_suites option' do + expect(plugin_class.new(conf).client_config[:ssl]).to_not include :cipher_suites + end + end + + context "set to ['TLS_AES_256_GCM_SHA384']" do + let(:conf) { basic_config.merge('ssl_cipher_suites' => ['TLS_AES_256_GCM_SHA384']) } + + it 'sets manticore cipher_suites option' do + expect(plugin_class.new(conf).client_config[:ssl]).to include :cipher_suites => ['TLS_AES_256_GCM_SHA384'] + end + end + end +end + +shared_examples("raise an http config error") do |message| + it "should raise an error error" do + expect { + plugin_class.new(conf).client_config + }.to raise_error(LogStash::PluginMixins::HttpClient::InvalidHTTPConfigError, message) + end +end + +shared_examples 'a client with deprecated ssl options' do + describe LogStash::PluginMixins::HttpClient do + let(:basic_config) { {} } + let(:impl) { plugin_class.new(basic_config) } + let(:use_deprecated_config) { true } + + include_examples 'with common ssl options' + + describe 'with a custom ssl bundle' do + let(:file) { Stud::Temporary.file } + let(:path) { file.path } + after { File.unlink(path)} + + context 'with x509' do + let(:conf) { basic_config.merge('cacert' => path) } + + include_examples('setting ca bundles', :ca_file) + + it_behaves_like('a deprecated setting with guidance', + 'cacert' => 'ssl_certificate_authorities') + end + + context 'with JKS' do + let(:conf) { + basic_config.merge( + 'truststore' => path, + 'truststore_password' => 'foobar', + 'truststore_type' => 'jks' + ) + } + + include_examples('setting ca bundles', :truststore, :jks) + + it_behaves_like('a deprecated setting with guidance', + 'truststore' => 'ssl_truststore_path', + 'truststore_password' => 'ssl_truststore_password', + 'truststore_type' => 'ssl_truststore_type') + end + end + + describe 'with a client cert' do + let(:file) { Stud::Temporary.file } + let(:path) { file.path } + after { File.unlink(path)} + + context 'with correct client certs' do + let(:conf) { basic_config.merge('client_cert' => path, 'client_key' => path) } + + it 'should create without error' do + expect { + plugin_class.new(conf).client_config + }.not_to raise_error + end + + it_behaves_like('a deprecated setting with guidance', + 'client_cert' => 'ssl_certificate', + 'client_key' => 'ssl_key') + end + + context 'without a key' do + let(:conf) { basic_config.merge('client_cert' => path) } + + include_examples('raise an http config error', 'When `client_cert` is provided, `client_key` must also be provided') + end + + context 'without a cert' do + let(:conf) { basic_config.merge('client_key' => path) } + + include_examples('raise an http config error', 'A `client_key` is not allowed unless a `client_cert` is provided') + end + end + + %w[keystore truststore].each do |store| + describe "with a custom #{store}" do + let(:file) { Stud::Temporary.file } + let(:path) { file.path } + let(:password) { "foo" } + after { File.unlink(path)} + + let(:conf) { + basic_config.merge( + store => path, + "#{store}_password" => password, + "#{store}_type" => "jks" + ).compact + } + + include_examples("setting ca bundles", store.to_sym, :jks) + + it_behaves_like('a deprecated setting with guidance', + "#{store}" => "ssl_#{store}_path", + "#{store}_password" => "ssl_#{store}_password", + "#{store}_type" => "ssl_#{store}_type") + + context "with no password set" do + let(:password) { nil } + + it "should raise an error" do + expect do + plugin_class.new(conf).client_config + end.to raise_error(LogStash::ConfigurationError) + end + end + end + end + end +end + +shared_examples 'a client with standardized ssl options' do + describe LogStash::PluginMixins::HttpClient do + let(:basic_config) { {} } + let(:impl) { plugin_class.new(basic_config) } + let(:use_deprecated_config) { false } + + include_examples 'with common ssl options' + + describe 'with a custom ssl bundle' do + let(:file) { Stud::Temporary.file } + let(:path) { file.path } + after { File.unlink(path)} + + context 'with x509' do + let(:conf) { basic_config.merge('ssl_certificate_authorities' => path) } + + include_examples('setting ca bundles', :ca_file) + end + + context 'with JKS' do + let(:conf) { + basic_config.merge( + 'ssl_truststore_path' => path, + 'ssl_truststore_password' => 'foobar', + 'ssl_truststore_type' => 'jks' + ) + } + + include_examples('setting ca bundles', :truststore, :jks) + end + end + + describe 'with a client cert' do + let(:file) { Stud::Temporary.file } + let(:path) { file.path } + after { File.unlink(path)} + + context 'with correct client certs' do + let(:conf) { basic_config.merge('ssl_certificate' => path, 'ssl_key' => path) } + + it 'should create without error' do + expect { + plugin_class.new(conf).client_config + }.not_to raise_error + end + end + + context 'without an ssl_key' do + let(:conf) { basic_config.merge('ssl_certificate' => path) } + + include_examples('raise an http config error', 'You must specify both `ssl_certificate` and `ssl_key` for an HTTP client, or neither!') + end + + context 'without an ssl_certificate' do + let(:conf) { basic_config.merge('ssl_key' => path) } + include_examples('raise an http config error', 'You must specify both `ssl_certificate` and `ssl_key` for an HTTP client, or neither!') + end + end + + %w[keystore truststore].each do |store| + describe "with a custom #{store}" do + let(:file) { Stud::Temporary.file } + let(:path) { file.path } + let(:password) { "foo" } + after { File.unlink(path)} + + let(:conf) { + basic_config.merge( + "ssl_#{store}_path" => path, + "ssl_#{store}_password" => password, + "ssl_#{store}_type" => "jks" + ).compact + } + + include_examples("setting ca bundles", store.to_sym, :jks) + + context "with no password set" do + let(:password) { nil } + + it "should not raise an error" do + expect do + plugin_class.new(conf).client_config + end.to_not raise_error + end + end + end + end + end +end + +class PluginWithNoModuleConfig < LogStash::Inputs::Base + include LogStash::PluginMixins::HttpClient + config_name 'no_config' +end + +class PluginWithDeprecatedTrue < LogStash::Inputs::Base + include LogStash::PluginMixins::HttpClient[:with_deprecated => true] + config_name 'with_deprecated' +end + +class PluginWithDeprecatedFalse < LogStash::Inputs::Base + include LogStash::PluginMixins::HttpClient[:with_deprecated => false] + config_name 'without_deprecated' +end + +describe PluginWithNoModuleConfig do + let(:plugin_class) { PluginWithNoModuleConfig } + + it_behaves_like 'a client with deprecated ssl options' + + it 'includes DeprecatedSslConfigSupport module' do + expect(plugin_class.ancestors).to include(LogStash::PluginMixins::HttpClient::DeprecatedSslConfigSupport) + end +end + +describe PluginWithDeprecatedFalse do + let(:plugin_class) { PluginWithDeprecatedFalse } + + it_behaves_like 'a client with standardized ssl options' + + it 'does not include DeprecatedSslConfigSupport module' do + expect(plugin_class.ancestors).to_not include(LogStash::PluginMixins::HttpClient::DeprecatedSslConfigSupport) + end +end + +describe PluginWithDeprecatedTrue do + let(:plugin_class) { PluginWithDeprecatedTrue } + + it_behaves_like 'a client with deprecated ssl options' + + it_behaves_like 'a client with standardized ssl options' + + context 'setting deprecate configs' do + let(:cacert) { Stud::Temporary.file.path } + let(:client_cert) { Stud::Temporary.file.path } + let(:client_key) { Stud::Temporary.file.path } + let(:keystore) { Stud::Temporary.file.path } + let(:keystore_type) { 'pkcs12' } + let(:keystore_password) { 'bar' } + let(:truststore) { Stud::Temporary.file.path } + let(:truststore_type) { 'pkcs12' } + let(:truststore_password) { 'foo' } + + let(:settings) do + { + 'cacert' => cacert, + 'client_cert' => client_cert, + 'client_key' => client_key, + 'keystore' => keystore, + 'keystore_password' => keystore_password, + 'keystore_type' => keystore_type, + 'truststore' => truststore, + 'truststore_password' => truststore_password, + 'truststore_type' => truststore_type + } + end + + subject(:plugin_instance) { plugin_class.new(settings) } + + after do + File.unlink(cacert) + File.unlink(client_cert) + File.unlink(client_key) + File.unlink(keystore) + File.unlink(truststore) + end + + it 'normalizes deprecated settings' do + expect(plugin_instance.ssl_certificate_authorities).to eq([cacert]) + expect(plugin_instance.ssl_certificate).to eq(client_cert) + expect(plugin_instance.ssl_key).to eq(client_key) + expect(plugin_instance.ssl_keystore_path).to eq(keystore) + expect(plugin_instance.ssl_keystore_password.value).to eq(keystore_password) + expect(plugin_instance.ssl_keystore_type).to eq(keystore_type) + expect(plugin_instance.ssl_truststore_path).to eq(truststore) + expect(plugin_instance.ssl_truststore_password.value).to eq(truststore_password) + expect(plugin_instance.ssl_truststore_type).to eq(truststore_type) + end + end + + it 'includes DeprecatedSslConfigSupport module' do + expect(plugin_class.ancestors).to include(LogStash::PluginMixins::HttpClient::DeprecatedSslConfigSupport) + end +end \ No newline at end of file