Skip to content

Feat: add ssl_supported_protocols option #1055

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 31 commits into from
Apr 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
52e35ee
Refactor: memoize hash-es to avoid allocation
kares Nov 29, 2021
5ed7a4c
Feat: ssl_enabled_protocols support (draft)
kares Nov 29, 2021
bbbd02d
CI: test with xpack.security.http.ssl.supported_protocols
kares Nov 29, 2021
b41c955
Test: adjust a test expectation (new protocols: ...)
kares Nov 29, 2021
c17bac9
Test: integration spec with TLSv1.2 vs TLSv1.3
kares Nov 29, 2021
e8d0abd
CI: pass down $ES_SSL_SUPPORTED_PROTOCOLS
kares Nov 29, 2021
90736a6
Temp: debug LS run
kares Nov 30, 2021
eca80e6
Temp: second service
kares Nov 30, 2021
5705182
restore running with CURL_OPTS
kares Dec 1, 2021
90d2e6a
Test: refactor - multi_receive might block
kares Dec 1, 2021
a7d754e
restore back initial dummy multi_receive []
kares Dec 1, 2021
abaa296
Test: restore TLS 1.3 retry looping spec
kares Dec 1, 2021
73e4a22
make sure index exists
kares Dec 1, 2021
cacdd6b
CI: disable debug logging LS
kares Dec 1, 2021
d8e8c27
adjust spec for using curl instead of manticore
kares Jan 24, 2022
ecdfa40
Test: run with curl --tlsv1.2 --tls-max 1.3
kares Jan 24, 2022
bd1ec71
rename new option to ssl_supported_protocols
kares Jan 25, 2022
4f9ad28
bump + changelog
kares Jan 26, 2022
944e5d7
revert precedence on TLSv1.3 being first
kares Jan 26, 2022
3883483
Docs: the new configuration option
kares Jan 26, 2022
76b87eb
Test: curl -s -S
kares Jan 26, 2022
f32f319
Test: more curl-ing - detect http status code
kares Jan 26, 2022
96c4a66
Merge branch 'main' into tls13
kares Mar 14, 2022
82d4487
only support TLSv1.1+ and validate enumeration
kares Mar 15, 2022
877377e
adust docs
kares Mar 15, 2022
e7c6fb4
Docs: align docs with rest of plugins
kares Mar 28, 2022
0655b45
Temp: compare against 7.x
kares Mar 28, 2022
d126939
Test: a default super-user in ES
kares Mar 28, 2022
09e4301
Test: proper status code detection from curl
kares Mar 28, 2022
37912d3
Test: always require auth for _refresh etc (due 8.2)
kares Mar 28, 2022
8ae8452
Revert "Temp: compare against 7.x"
kares Mar 28, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .ci/Dockerfile.elasticsearch
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ ARG es_path=/usr/share/elasticsearch
ARG es_yml=$es_path/config/elasticsearch.yml
ARG SECURE_INTEGRATION
ARG ES_SSL_KEY_INVALID
ARG ES_SSL_SUPPORTED_PROTOCOLS

RUN rm -f $es_path/config/scripts

Expand All @@ -25,6 +26,10 @@ RUN if [ "$SECURE_INTEGRATION" = "true" ] ; then \
fi \
fi
RUN if [ "$SECURE_INTEGRATION" = "true" ] ; then echo "xpack.security.http.ssl.certificate_authorities: [ '$es_path/config/test_certs/ca.crt' ]" >> $es_yml; fi
RUN if [ "$SECURE_INTEGRATION" = "true" ] && [ ! -z "$ES_SSL_SUPPORTED_PROTOCOLS" ] ; then echo "xpack.security.http.ssl.supported_protocols: ${ES_SSL_SUPPORTED_PROTOCOLS}" >> $es_yml; fi

RUN cat $es_yml

RUN if [ "$SECURE_INTEGRATION" = "true" ] ; then $es_path/bin/elasticsearch-users useradd admin -p elastic -r superuser; fi
RUN if [ "$SECURE_INTEGRATION" = "true" ] ; then $es_path/bin/elasticsearch-users useradd simpleuser -p abc123 -r superuser; fi
RUN if [ "$SECURE_INTEGRATION" = "true" ] ; then $es_path/bin/elasticsearch-users useradd 'f@ncyuser' -p 'ab%12#' -r superuser; fi
4 changes: 4 additions & 0 deletions .ci/docker-compose.override.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ services:
- INTEGRATION=${INTEGRATION:-false}
- SECURE_INTEGRATION=${SECURE_INTEGRATION:-false}
- ES_SSL_KEY_INVALID=${ES_SSL_KEY_INVALID:-false}
- ES_SSL_SUPPORTED_PROTOCOLS=$ES_SSL_SUPPORTED_PROTOCOLS

elasticsearch:
build:
Expand All @@ -22,6 +23,9 @@ services:
- INTEGRATION=${INTEGRATION:-false}
- SECURE_INTEGRATION=${SECURE_INTEGRATION:-false}
- ES_SSL_KEY_INVALID=${ES_SSL_KEY_INVALID:-false}
- ES_SSL_SUPPORTED_PROTOCOLS=$ES_SSL_SUPPORTED_PROTOCOLS
environment:
- ES_JAVA_OPTS=-Xms640m -Xmx640m
command: /usr/share/elasticsearch/elasticsearch-run.sh
tty: true
ports:
Expand Down
16 changes: 11 additions & 5 deletions .ci/logstash-run.sh
Original file line number Diff line number Diff line change
@@ -1,26 +1,32 @@
#!/bin/bash

env

set -ex

export PATH=$BUILD_DIR/gradle/bin:$PATH

if [[ "$SECURE_INTEGRATION" == "true" ]]; then
ES_URL="https://elasticsearch:9200 -k"
ES_URL="https://elasticsearch:9200"
else
ES_URL="http://elasticsearch:9200"
fi

# CentOS 7 using curl defaults does not enable TLSv1.3
CURL_OPTS="-k --tlsv1.2 --tls-max 1.3"

wait_for_es() {
count=120
while ! curl -s $ES_URL >/dev/null && [[ $count -ne 0 ]]; do
while ! curl $CURL_OPTS $ES_URL >/dev/null && [[ $count -ne 0 ]]; do
count=$(( $count - 1 ))
[[ $count -eq 0 ]] && exit 1
sleep 1
done
echo $(curl -s $ES_URL | python -c "import sys, json; print(json.load(sys.stdin)['version']['number'])")
echo $(curl $CURL_OPTS -vi $ES_URL | python -c "import sys, json; print(json.load(sys.stdin)['version']['number'])")
}

if [[ "$INTEGRATION" != "true" ]]; then
bundle exec rspec -fd spec/unit -t ~integration -t ~secure_integration
jruby -rbundler/setup -S rspec -fd spec/unit -t ~integration -t ~secure_integration
else

if [[ "$SECURE_INTEGRATION" == "true" ]]; then
Expand All @@ -32,5 +38,5 @@ else
echo "Waiting for elasticsearch to respond..."
ES_VERSION=$(wait_for_es)
echo "Elasticsearch $ES_VERSION is Up!"
bundle exec rspec -fd $extra_tag_args --tag update_tests:painless --tag es_version:$ES_VERSION spec/integration
jruby -rbundler/setup -S rspec -fd $extra_tag_args --tag update_tests:painless --tag es_version:$ES_VERSION spec/integration
fi
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ env:
- SECURE_INTEGRATION=true INTEGRATION=true ELASTIC_STACK_VERSION=8.x SNAPSHOT=true LOG_LEVEL=info
- SECURE_INTEGRATION=true INTEGRATION=true ELASTIC_STACK_VERSION=7.x LOG_LEVEL=info
- SECURE_INTEGRATION=true INTEGRATION=true ELASTIC_STACK_VERSION=7.x ES_SSL_KEY_INVALID=true LOG_LEVEL=info
- SECURE_INTEGRATION=true INTEGRATION=true ELASTIC_STACK_VERSION=7.x ES_SSL_SUPPORTED_PROTOCOLS=TLSv1.3 LOG_LEVEL=info
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
## 11.5.0
- Feat: add ssl_supported_protocols option [#1055](https://github.com/logstash-plugins/logstash-output-elasticsearch/pull/1055)

## 11.4.2
- [DOC] Add `v8` to supported values for ecs_compatiblity defaults [#1059](https://github.com/logstash-plugins/logstash-output-elasticsearch/pull/1059)

Expand Down
22 changes: 20 additions & 2 deletions docs/index.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ This plugin supports the following configuration options plus the
| <<plugins-{type}s-{plugin}-sniffing_path>> |<<string,string>>|No
| <<plugins-{type}s-{plugin}-ssl>> |<<boolean,boolean>>|No
| <<plugins-{type}s-{plugin}-ssl_certificate_verification>> |<<boolean,boolean>>|No
| <<plugins-{type}s-{plugin}-ssl_supported_protocols>> |<<string,string>>|No
| <<plugins-{type}s-{plugin}-template>> |a valid filesystem path|No
| <<plugins-{type}s-{plugin}-template_name>> |<<string,string>>|No
| <<plugins-{type}s-{plugin}-template_overwrite>> |<<boolean,boolean>>|No
Expand Down Expand Up @@ -1004,6 +1005,23 @@ Option to validate the server's certificate. Disabling this severely compromises
For more information on disabling certificate verification please read
https://www.cs.utexas.edu/~shmat/shmat_ccs12.pdf

[id="plugins-{type}s-{plugin}-ssl_supported_protocols"]
===== `ssl_supported_protocols`

* Value type is <<string,string>>
* Allowed values are: `'TLSv1.1'`, `'TLSv1.2'`, `'TLSv1.3'`
* Default depends on the JDK being used. With up-to-date Logstash, the default is `['TLSv1.2', 'TLSv1.3']`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it makes sense to have a Logstash page equivalent to https://www.elastic.co/guide/en/elasticsearch/reference/current/jdk-tls-versions.html for Logstash for a single point of reference, given that this is copied across multiple plugins.
cc @karenzone

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does @robbavey ! That refactor should happen separately from that PR, but I agree we should centralize this information. Pls open a docs issue so the idea isn't lost.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue created - elastic/logstash#13962

`'TLSv1.1'` is not considered secure and is only provided for legacy applications.

List of allowed SSL/TLS versions to use when establishing a connection to the Elasticsearch cluster.

For Java 8 `'TLSv1.3'` is supported only since **8u262** (AdoptOpenJDK), but requires that you set the
`LS_JAVA_OPTS="-Djdk.tls.client.protocols=TLSv1.3"` system property in Logstash.

NOTE: If you configure the plugin to use `'TLSv1.1'` on any recent JVM, such as the one packaged with Logstash,
the protocol is disabled by default and needs to be enabled manually by changing `jdk.tls.disabledAlgorithms` in
the *$JDK_HOME/conf/security/java.security* configuration file. That is, `TLSv1.1` needs to be removed from the list.

[id="plugins-{type}s-{plugin}-template"]
===== `template`

Expand All @@ -1018,8 +1036,8 @@ If not set, the included template will be used.

* Value type is <<string,string>>
* Default value depends on whether <<plugins-{type}s-{plugin}-ecs_compatibility>> is enabled:
** ECS Compatibility disabled: `logstash`
** ECS Compatibility enabled: `ecs-logstash`
** ECS Compatibility disabled: `logstash`
** ECS Compatibility enabled: `ecs-logstash`


This configuration option defines how the template is named inside Elasticsearch.
Expand Down
4 changes: 2 additions & 2 deletions lib/logstash/outputs/elasticsearch/http_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -283,11 +283,11 @@ def uris
end

def client_settings
@options[:client_settings] || {}
@_client_settings ||= @options[:client_settings] || {}
end

def ssl_options
client_settings.fetch(:ssl, {})
@_ssl_options ||= client_settings.fetch(:ssl, {})
end

def http_compression
Expand Down
5 changes: 5 additions & 0 deletions lib/logstash/outputs/elasticsearch/http_client_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,16 @@ def self.setup_ssl(logger, params)
ssl_options[:keystore] = keystore
ssl_options[:keystore_password] = keystore_password.value if keystore_password
end

if !params["ssl_certificate_verification"]
logger.warn "You have enabled encryption but DISABLED certificate verification, " +
"to make sure your data is secure remove `ssl_certificate_verification => false`"
ssl_options[:verify] = :disable # false accepts self-signed but still validates hostname
end

protocols = params['ssl_supported_protocols']
ssl_options[:protocols] = protocols if protocols && protocols.any?

{ ssl: ssl_options }
end

Expand Down
2 changes: 2 additions & 0 deletions lib/logstash/plugin_mixins/elasticsearch/api_configs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ module APIConfigs
# Set the keystore password
:keystore_password => { :validate => :password },

:ssl_supported_protocols => { :validate => ['TLSv1.1', 'TLSv1.2', 'TLSv1.3'], :default => [], :list => true },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES also (still) supports: SSLv3, TLSv1 and SSLv2Hello but since these are disabled by default in an up-to-date Java I do not think we need to support them give even an old 6.0 cluster is expected to run on at least Java 8.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly wary of removing something that is a valid option with Elasticsearch, maybe we should keep it but warn?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we had a discussion with @jsvd on the topic and part of that outcome was that we start with >= TLSv1.1 (for all plugins), in this case despite ES' support for legacy SSLv3 and TLSv1.
existing plugin versions using TLS <= 1.1, on an up-to-date JDK, need tampering with the java.security files so if they're upgrading they either upgrade to secure TLS or we'll hear them complaining on the feature per plugin basis (the default - not setting the option is expected to work for them as before).


# This setting asks Elasticsearch for the list of all cluster nodes and adds them to the hosts list.
# Note: This will return ALL nodes with HTTP enabled (including master nodes!). If you use
# this with master nodes, you probably want to disable HTTP on them by setting
Expand Down
2 changes: 1 addition & 1 deletion logstash-output-elasticsearch.gemspec
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Gem::Specification.new do |s|
s.name = 'logstash-output-elasticsearch'
s.version = '11.4.2'
s.version = '11.5.0'
s.licenses = ['apache-2.0']
s.summary = "Stores logs in Elasticsearch"
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"
Expand Down
67 changes: 59 additions & 8 deletions spec/integration/outputs/index_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,25 +60,48 @@

let(:curl_opts) { nil }

let(:es_admin) { 'admin' } # default user added in ES -> 8.x requires auth credentials for /_refresh etc
let(:es_admin_pass) { 'elastic' }

def curl_and_get_json_response(url, method: :get); require 'open3'
cmd = "curl -s -v --show-error #{curl_opts} -X #{method.to_s.upcase} -k #{url}"
begin
stdout, status = Open3.capture2("curl #{curl_opts} -X #{method.to_s.upcase} -k #{url}")
out, err, status = Open3.capture3(cmd)
rescue Errno::ENOENT
fail "curl not available, make sure curl binary is installed and available on $PATH"
end

if status.success?
LogStash::Json.load(stdout)
http_status = err.match(/< HTTP\/1.1 (\d+)/)[1] || '0' # < HTTP/1.1 200 OK\r\n

if http_status.strip[0].to_i > 2
error = (LogStash::Json.load(out)['error']) rescue nil
if error
fail "#{cmd.inspect} received an error: #{http_status}\n\n#{error.inspect}"
else
warn out
fail "#{cmd.inspect} unexpected response: #{http_status}\n\n#{err}"
end
end

LogStash::Json.load(out)
else
fail "curl failed: #{status}\n #{stdout}"
warn out
fail "#{cmd.inspect} process failed: #{status}\n\n#{err}"
end
end

let(:initial_events) { [] }

before do
subject.register
subject.multi_receive([])
subject.multi_receive(initial_events) if initial_events
end


after do
subject.do_close
end

shared_examples "an indexer" do |secure|
it "ships events" do
subject.multi_receive(events)
Expand Down Expand Up @@ -146,17 +169,17 @@ def curl_and_get_json_response(url, method: :get); require 'open3'
let(:user) { "simpleuser" }
let(:password) { "abc123" }
let(:cacert) { "spec/fixtures/test_certs/ca.crt" }
let(:es_url) {"https://elasticsearch:9200"}
let(:es_url) { "https://#{get_host_port}" }
let(:config) do
{
"hosts" => ["elasticsearch:9200"],
"hosts" => [ get_host_port ],
"user" => user,
"password" => password,
"ssl" => true,
"cacert" => cacert,
"index" => index
}
end
end

let(:curl_opts) { "-u #{user}:#{password}" }

Expand Down Expand Up @@ -197,6 +220,8 @@ def curl_and_get_json_response(url, method: :get); require 'open3'

else

let(:curl_opts) { "#{super()} --tlsv1.2 --tls-max 1.3 -u #{es_admin}:#{es_admin_pass}" } # due ES 8.x we need user/password

it_behaves_like("an indexer", true)

describe "with a password requiring escaping" do
Expand All @@ -219,6 +244,32 @@ def curl_and_get_json_response(url, method: :get); require 'open3'
include_examples("an indexer", true)
end

context 'with enforced TLSv1.3 protocol' do
let(:config) { super().merge 'ssl_supported_protocols' => [ 'TLSv1.3' ] }

it_behaves_like("an indexer", true)
end

context 'with enforced TLSv1.2 protocol (while ES only enabled TLSv1.3)' do
let(:config) { super().merge 'ssl_supported_protocols' => [ 'TLSv1.2' ] }

let(:initial_events) { nil }

it "does not ship events" do
curl_and_get_json_response index_url, method: :put # make sure index exists
Thread.start { subject.multi_receive(events) } # we'll be stuck in a retry loop
sleep 2.5

curl_and_get_json_response "#{es_url}/_refresh", method: :post

result = curl_and_get_json_response "#{index_url}/_count?q=*"
cur_count = result["count"]
expect(cur_count).to eq(0) # ES output keeps re-trying but ends up with a
# [Manticore::ClientProtocolException] Received fatal alert: protocol_version
end

end if ENV['ES_SSL_SUPPORTED_PROTOCOLS'] == 'TLSv1.3'

end

end
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/outputs/elasticsearch_ssl_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@

it "should pass the flag to the ES client" do
expect(::Manticore::Client).to receive(:new) do |args|
expect(args[:ssl]).to eq(:enabled => true, :verify => :disable)
expect(args[:ssl]).to match hash_including(:enabled => true, :verify => :disable)
end.and_return(manticore_double)

subject.register
Expand Down