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

Add KeyUsage, ExtendedKeyUsage, CipherSuite & Protocol to SSL diagnostics #63784

Closed
tvernum opened this issue Oct 16, 2020 · 9 comments · Fixed by #65634
Closed

Add KeyUsage, ExtendedKeyUsage, CipherSuite & Protocol to SSL diagnostics #63784

tvernum opened this issue Oct 16, 2020 · 9 comments · Fixed by #65634
Labels
>enhancement good first issue low hanging fruit help wanted adoptme :Security/TLS SSL/TLS, Certificates Team:Security Meta label for security team

Comments

@tvernum
Copy link
Contributor

tvernum commented Oct 16, 2020

Per https://discuss.elastic.co/t/ldaps-and-chain-of-certificates/250724 it's possible to get an SSL failure & diagnostic when the cipher requires certain key usage that is not permitted by the certificate.

To assist in such diagnostics, it would be of assistance to print out the ceritficate's KeyUsage and the session's Cipher suite in the message.
While we're doing that, the cert's ExtendedKeyUsage and session Protocol are probably worth including as well.

@tvernum tvernum added >enhancement good first issue low hanging fruit help wanted adoptme :Security/TLS SSL/TLS, Certificates labels Oct 16, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (:Security/Network)

@elasticmachine elasticmachine added the Team:Security Meta label for security team label Oct 16, 2020
@AGZain
Copy link

AGZain commented Oct 16, 2020

I'd like to start contributing to this issue.
Any hints on which part of the code I should start with, for this issue?

@Moe82
Copy link

Moe82 commented Oct 28, 2020

Hi, @tvernum . I'd like to work on this issue. Can you please provide a general format for how you want the 4 new strings to appear?

@tvernum
Copy link
Contributor Author

tvernum commented Oct 30, 2020

@AGZain , @Moe82 you'll need to coordinate between yourselves as to who is looking at this.

The class that needs to change is SslDiagnostics but I haven't looked at this in enough detail to know exactly how we should represent these value, or where we should slot them in.

@Moe82
Copy link

Moe82 commented Oct 30, 2020

I assumed the other user was no longer interested. @AGZain , let me know if you still want to work on it.

@Moe82
Copy link

Moe82 commented Nov 12, 2020

Ok, seems like AGZain is no longer interested. I'll pick it up.

@sindhusp
Copy link
Contributor

Hi @Moe82, are you working on this?

@Moe82
Copy link

Moe82 commented Nov 30, 2020

hi @sindhusp , not at the moment. Was planning on resuming mid December after my school semester ends. But feel free to work on it. If you can't figure it out by then, please let me know.

@sindhusp
Copy link
Contributor

sindhusp commented Nov 30, 2020

@tvernum Can you please review my PR for this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement good first issue low hanging fruit help wanted adoptme :Security/TLS SSL/TLS, Certificates Team:Security Meta label for security team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants