Skip to content
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

Support TLS certificate & key pair #998

Closed
wants to merge 1 commit into from

Conversation

UXabre
Copy link

@UXabre UXabre commented Mar 4, 2021

Integrated the support for a "regular" certificate & key pair (since it is supported on Manticore)
The certificate only supports PEM, the private key only support PKCS#8

Background info: My use case is to predominantly use this in combination with Hashicorp Vault, where the creation of both these (a pem cert and pkcs8 key) comes out of the box

Addresses #672 to some extend

@cla-checker-service
Copy link

cla-checker-service bot commented Mar 4, 2021

💚 CLA has been signed

@UXabre UXabre force-pushed the mtls_with_pem branch 4 times, most recently from ba418f1 to 7e562ad Compare March 4, 2021 13:59
@UXabre
Copy link
Author

UXabre commented Mar 25, 2021

Hi, anything that needs to done to push this one forward?

@UXabre
Copy link
Author

UXabre commented Aug 26, 2021

Anything on the triaging of this PR? (I see it now has conflicts which would need to be resolved first, but I wanted to learn if it would be wise to do it now?)

@UXabre
Copy link
Author

UXabre commented Oct 15, 2021

Resolved conflicts :-)

@kares
Copy link
Contributor

kares commented Nov 25, 2021

Hey Aren, this is looking great 👍

I'll be looking into testing TLS 1.3 support and since that might introduce an option for specifying the protocol we might want to align the naming. Haven't decided the naming strategy yet but I am going to keep this PR in mind and eventually provide feedback.

Thank your for your efforts so far!

@kares kares self-requested a review November 25, 2021 11:57
@UXabre
Copy link
Author

UXabre commented Nov 25, 2021

Hi @kares , thanks for your input! Completely agree to align on these things, I'll be awaiting your input and take actions based on those :-)

@kares
Copy link
Contributor

kares commented May 30, 2022

Sorry, I forgot about this - for now we seem to be following the naming as established by ES/Beats.
In this case ssl.key and ssl.certificate is used in ES which would end up as ssl_key and ssl_certificate options in Logstash.

There's still one gotcha when introducing official alignment with the rest of the stack we should primarily support the same format - Manticore supports PKCS#8 explicitly but I think, as the test suggest if I am not wrong, PKCS#12 format works implicitly. Thus the docs should probably advertise using .p12.

@UXabre
Copy link
Author

UXabre commented Jul 3, 2022

Hi @kares ,
Thanks for getting back on this subject.
I'll change the names accordingly.

To be honest, I don't completely understand the second bit on documentation though, I only really tested with PKCS#8, I could try and expand the tests with PKCS#12 to be sure that it works. If so, I can add that both these are supported, but that's probably not what you meant?

@kares
Copy link
Contributor

kares commented Jul 4, 2022

The concern is when introducing ssl_certificate we should make sure it works with .p12 files.
PKCS#8 is fine to support but we should be aligned with the rest of the Elastic stack.

I'll be off after a few days so feel free to ping someone else from the team when the PR is ready for review.

@kares kares removed their request for review July 4, 2022 05:50
@edmocosta
Copy link
Contributor

Hey Arend! Thank you very much for taking the time to open this PR and sorry for the long waiting! We recently started working on standardizing the SSL settings across all the plugins (elastic/logstash#14905), and this feature was incorporated into the SSL normalization PR (#1118). Thanks!

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

Successfully merging this pull request may close these issues.

4 participants