Skip to content

Conversation

edmocosta
Copy link
Contributor

@edmocosta edmocosta commented Mar 7, 2023

What this PR does?

Added the following SSL settings:
ssl_enabled: Enable/disable the SSL settings. If not provided, the value is inferred from the hosts scheme
ssl_certificate: OpenSSL-style X.509 certificate file to authenticate the client
ssl_key: OpenSSL-style RSA private key that corresponds to the ssl_certificate
ssl_truststore_path: he JKS truststore to validate the server's certificate
ssl_truststore_type: The format of the truststore file
ssl_truststore_password: The truststore password
ssl_keystore_path: The keystore used to present a certificate to the server
ssl_keystore_type: The format of the keystore file
ssl_keystore_password: The keystore password
ssl_cipher_suites: The list of cipher suites to use
ssl_supported_protocols: Supported protocols with versions
ssl_verification_mode: Defines how to verify the certificates presented by another party in the TLS connection

Reviewed and deprecated SSL settings to comply with Logstash's naming convention
ssl in favor of ssl_enabled
ca_file in favor of ssl_certificate_authorities
keystore in favor of ssl_keystore_path
keystore_password in favor of ssl_keystore_password


Closes elastic/logstash#14923
Closes #164
Closes #49

@edmocosta edmocosta changed the title [WIP] Deprecated and added new SSL settings [WIP] Standardize and add SSL settings Mar 8, 2023
@edmocosta edmocosta changed the title [WIP] Standardize and add SSL settings Standardize and add SSL settings Mar 8, 2023
@edmocosta edmocosta marked this pull request as ready for review March 8, 2023 15:14
@roaksoax roaksoax requested a review from andsel March 9, 2023 02:51
Copy link
Contributor

@andsel andsel left a comment

Choose a reason for hiding this comment

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

Left some comments, that are quite similar to the observations done for output and input.

@edmocosta edmocosta requested a review from andsel March 9, 2023 21:30
Copy link
Contributor

@andsel andsel left a comment

Choose a reason for hiding this comment

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

Left a nitpick style note

@edmocosta
Copy link
Contributor Author

Left a nitpick style note

Thank you again for reviewing it, @andsel. I've addressed your suggestions plus the suggestion #1 and suggestion #2 made on the output plugin.

@edmocosta edmocosta requested a review from andsel March 10, 2023 10:56
Copy link
Contributor

@andsel andsel left a comment

Choose a reason for hiding this comment

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

LGTM

Just update the

deprecated[8.8.0, Replaced by

to refer to the plugin's version instead of Logstash one.


ssl_key = params["ssl_key"]
if ssl_certificate
raise LogStash::ConfigurationError, 'Using an "ssl_certificate" requires an "ssl_key"' unless ssl_key
Copy link
Contributor

Choose a reason for hiding this comment

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

replace an by a

Copy link
Contributor Author

@edmocosta edmocosta Mar 10, 2023

Choose a reason for hiding this comment

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

Thanks for reviewing, @kaisecheng. Although it looks wrong, I think the article "an" is correct here, we pronounce the "s" as "ess" (vowel sound).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm gonna move on with this PR and merge it! If that's a mistake I can submit another PR to fix it! Thanks!

@edmocosta edmocosta merged commit 4da7f62 into logstash-plugins:main Mar 10, 2023
@edmocosta edmocosta deleted the standardize_ssl_settings branch March 10, 2023 16:53
@djschny
Copy link
Contributor

djschny commented Apr 6, 2023

How can I tell what version of Elastic stack this is in?

@andsel
Copy link
Contributor

andsel commented Apr 6, 2023

Hi @djschny actually is not yet bundled with any Logstash release. However you can update this plugin to version 3.15.0 in your local installation, following the instruction described here.

@djschny
Copy link
Contributor

djschny commented Apr 6, 2023

Did the trick, thank you!

@tommycahir
Copy link

tommycahir commented Sep 12, 2023

I am just running v3.15.3 and I am seeing an error saying " Unknown setting 'ssl_certificate_verification' for elasticsearch" when using this filter

@edmocosta
Copy link
Contributor Author

Hi @tommycahir!

The ssl_certificate_verification setting does not exist for this filter. I think the correct one is ssl_verification_mode :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

6 participants