-
Notifications
You must be signed in to change notification settings - Fork 307
Standardize and add SSL settings #1118
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
Changes from all commits
953def7
6f3828b
03e6d71
0059d64
6814686
ef9bc18
e71f9a7
185cbe3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -107,38 +107,53 @@ def self.create_http_client(options) | |
end | ||
|
||
def self.setup_ssl(logger, params) | ||
params["ssl"] = true if params["hosts"].any? {|h| h.scheme == "https" } | ||
return {} if params["ssl"].nil? | ||
params["ssl_enabled"] = true if params["hosts"].any? {|h| h.scheme == "https" } | ||
return {} if params["ssl_enabled"].nil? | ||
|
||
return {:ssl => {:enabled => false}} if params["ssl"] == false | ||
return {:ssl => {:enabled => false}} if params["ssl_enabled"] == false | ||
|
||
cacert, truststore, truststore_password, keystore, keystore_password = | ||
params.values_at('cacert', 'truststore', 'truststore_password', 'keystore', 'keystore_password') | ||
ssl_certificate_authorities, ssl_truststore_path, ssl_certificate, ssl_keystore_path = params.values_at('ssl_certificate_authorities', 'ssl_truststore_path', 'ssl_certificate', 'ssl_keystore_path') | ||
|
||
if cacert && truststore | ||
raise(LogStash::ConfigurationError, "Use either \"cacert\" or \"truststore\" when configuring the CA certificate") if truststore | ||
if ssl_certificate_authorities && ssl_truststore_path | ||
raise LogStash::ConfigurationError, 'Use either "ssl_certificate_authorities/cacert" or "ssl_truststore_path/truststore" when configuring the CA certificate' | ||
end | ||
|
||
if ssl_certificate && ssl_keystore_path | ||
raise LogStash::ConfigurationError, 'Use either "ssl_certificate" or "ssl_keystore_path/keystore" when configuring client certificates' | ||
end | ||
|
||
ssl_options = {:enabled => true} | ||
|
||
if cacert | ||
ssl_options[:ca_file] = cacert | ||
elsif truststore | ||
ssl_options[:truststore_password] = truststore_password.value if truststore_password | ||
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 | ||
ssl_options[:ca_file] = ssl_certificate_authorities.first | ||
end | ||
|
||
ssl_options[:truststore] = truststore if truststore | ||
if keystore | ||
ssl_options[:keystore] = keystore | ||
ssl_options[:keystore_password] = keystore_password.value if keystore_password | ||
setup_ssl_store(ssl_options, 'truststore', params) | ||
setup_ssl_store(ssl_options, 'keystore', params) | ||
|
||
ssl_key = params["ssl_key"] | ||
if ssl_certificate | ||
raise LogStash::ConfigurationError, 'Using an "ssl_certificate" requires an "ssl_key"' unless ssl_key | ||
ssl_options[:client_cert] = ssl_certificate | ||
ssl_options[:client_key] = ssl_key | ||
elsif !ssl_key.nil? | ||
raise LogStash::ConfigurationError, 'An "ssl_certificate" is required when using an "ssl_key"' | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Message is a bit unclear (you don't actually have to set either, because you can use a keystore), and catches two distinct cases:
I prefer declaring it in a way that centers the single config that "activates" the feature, in this case
|
||
|
||
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 | ||
ssl_verification_mode = params["ssl_verification_mode"] | ||
unless ssl_verification_mode.nil? | ||
case ssl_verification_mode | ||
when 'none' | ||
logger.warn "You have enabled encryption but DISABLED certificate verification, " + | ||
"to make sure your data is secure set `ssl_verification_mode => full`" | ||
ssl_options[:verify] = :disable | ||
else | ||
ssl_options[:verify] = :strict | ||
end | ||
end | ||
|
||
ssl_options[:cipher_suites] = params["ssl_cipher_suites"] if params.include?("ssl_cipher_suites") | ||
ssl_options[:trust_strategy] = params["ssl_trust_strategy"] if params.include?("ssl_trust_strategy") | ||
|
||
protocols = params['ssl_supported_protocols'] | ||
|
@@ -147,6 +162,16 @@ def self.setup_ssl(logger, params) | |
{ ssl: ssl_options } | ||
end | ||
|
||
# @param kind is a string [truststore|keystore] | ||
def self.setup_ssl_store(ssl_options, kind, params) | ||
store_path = params["ssl_#{kind}_path"] | ||
if store_path | ||
ssl_options[kind.to_sym] = store_path | ||
ssl_options["#{kind}_type".to_sym] = params["ssl_#{kind}_type"] if params.include?("ssl_#{kind}_type") | ||
ssl_options["#{kind}_password".to_sym] = params["ssl_#{kind}_password"].value if params.include?("ssl_#{kind}_password") | ||
end | ||
end | ||
|
||
def self.setup_basic_auth(logger, params) | ||
user, password = params["user"], params["password"] | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this PR in the changelog correct? Shouldn't it be a link to this PR (#1118)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right, it should be #1118. I'll submit a fix! Thanks for the heads up!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @edmo. Remember that the link will be wrong for any release notes that include 11.14.0, and will require a manual fix. :-)