Add diagnostics callback for tlscommon configs and httptransport #207

michel-laterman · 2024-05-28T00:32:42Z

What does this PR do?

Add diagnostics callbacks for TLS configuration to make it easier to
include in diagnostics bundles. Diagnostics info will geneally verify
that the certs or cas are able to load, and display some basic info such
as cert names, expiry times, fingerprints, etc. httptransport callback
will attempt an HTTP request and return information collected from
httptrace about the request.

Why is it important?

Allows more information about TLS to be present in diagnostics to help troubleshoot any certificate issues.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have added tests that prove my fix is effective or that my feature works

Add diagnostics callbacks for TLS configuration to make it easier to include in diagnostics bundles. Diagnostics info will geneally verify that the certs or cas are able to load, and display some basic info such as cert names, expiry times, fingerprints, etc. httptransport callback will attempt an HTTP request and return information collected from httptrace about the request.

michel-laterman · 2024-05-29T00:31:36Z

Example of the hooks functions and outputs added to a (draft) fleet-server pr: elastic/fleet-server#3587

michel-laterman

I would like feedback, specifically if we need more information to help troubleshoot issues.

transport/httpcommon/diag.go

michel-laterman · 2024-05-29T18:53:34Z

transport/tlscommon/diag.go

+		diagCertificate(logger, &c.Certificate)
+		diagCAs(logger, c.CAs)
+
+		return b.Bytes()


From the fleet-server demo, output is:

tlscommon.ServerConfig: Start diagnostics 2024-05-29 00:18:41.569890423 +0000 UTC tlscommon.ServerConfig: verification_mode=full tlscommon.ServerConfig: client_auth=none tlscommon.ServerConfig: ca_sha256=[] tlscommon.ServerConfig: CertificateSettings: checking certificate keypair tlscommon.ServerConfig: CertificateSettings: certificate keypair OK. tlscommon.ServerConfig: CertificateSettings: cert 0 - Subject: CN=fleet-server-dev,O=elastic-fleet Issuer: CN=localhost,O=elastic-fleet NotBefore: 2024-05-29 00:18:15 +0000 UTC NotAfter: 2034-05-29 00:18:15 +0000 UTC Fingerprint: EvAr8erNJZJ0BoXor1jnCl5X2WCU79ISMgwMITOEJBs= SAN IP: [] SAN DNS: [fleet-server-dev] tlscommon.ServerConfig: CertificateAuthorities: certificate_authorities provided. tlscommon.ServerConfig: CertificateAuthorities: - cert 0 Subject: CN=localhost,O=elastic-fleet IsCa: true BasicConstraintsValid: true NotBefore: 2024-05-29 00:18:15 +0000 UTC NotAfter: 2034-05-29 00:18:15 +0000 UTC Fingerprint: cSawoNEa2cSPV4Mwce4mRysrcBGtHBGcSeoBL+Cr48Q=

We want to make sure that certificate & ca information is provided in the diagnostics

Slight update after I added CertDiagString

tlscommon.ServerConfig: Start diagnostics 2024-05-29 22:10:07.333735518 +0000 UTC tlscommon.ServerConfig: verification_mode=full tlscommon.ServerConfig: client_auth=none tlscommon.ServerConfig: ca_sha256=[] tlscommon.ServerConfig: CertificateSettings: checking certificate keypair tlscommon.ServerConfig: CertificateSettings: certificate keypair OK. tlscommon.ServerConfig: CertificateSettings: cert 0 Subject=CN=fleet-server-dev,O=elastic-fleet Issuer=CN=localhost,O=elastic-fleet IsCA=false BasicConstrintsValid=false NotBefore=2024-05-29 22:08:32 +0000 UTC NotAfter=2034-05-29 22:08:32 +0000 UTC Fingerprint=w04CVUAd7Tfnd/AZ/A8YB9bCUAImGeGXBAUPnhEvGgM= SAN IP=[] SAN DNS=[fleet-server-dev] SAN URI=[] tlscommon.ServerConfig: CertificateAuthorities: certificate_authorities provided. tlscommon.ServerConfig: CertificateAuthorities: - cert 0 Subject=CN=localhost,O=elastic-fleet Issuer=CN=localhost,O=elastic-fleet IsCA=true BasicConstrintsValid=true NotBefore=2024-05-29 22:08:23 +0000 UTC NotAfter=2034-05-29 22:08:23 +0000 UTC Fingerprint=QHVJrVpzEHBJTTr6F9RW6TnYbj9ckPD2BQ5JphTzh7M= SAN IP=[] SAN DNS=[localhost] SAN URI=[]

leehinman · 2024-05-29T20:47:06Z

I would like feedback, specifically if we need more information to help troubleshoot issues.

This may or may not be helpful, so feel free to disregard.

I'd like to see some failure scenarios, paired with the new diagnostics showing how they give the info needed to correctly diagnose the problem. I'm thinking of things like:

User configures NonTLS client but connects to TLS server
User configures TLS client but connects to NonTLS server
User configures TLS client, no client cert, private key but connects to TLS server that requires TLS client authentication
User configures TLS client but connects to server that has expired certificate
User configures TLS client but doesn't have CA that signed TLS server cert in it's trust store
User configures TLS client but server requires an algorithm that isn't supported by the client
User configures TLS client but connects to Proxy that doesn't have the server name as a SAN

Add some error parsing to HTTPSettings.DiagRequests to attempt to give a human readable statement about the cause of any errors when gathering request diagnostics.

michel-laterman · 2024-05-30T20:18:57Z

transport/httpcommon/diag.go

+// isGoHTTPResp detects if the response is one that a go http.Server sends if an HTTP request is made to an HTTPS server.
+// non Go servers may return a net.OpError instead.
+func isGoHTTPResp(r *http.Response) bool {


This behaviour is a part of go servers, and also occurs for our current cloud deplyments

michel-laterman · 2024-05-30T20:20:24Z

transport/httpcommon/diag.go

+// diagError tries to diagnose the error and return a cause/possible cause in a human readable format.
+// If no matching errors are found err.Error is returned.
+func diagError(err error) string {


@leehinman, I've tried to add this to explain errors based on the scenarios you provided (with test cases for most of them). we can add more in the future as well

transport/httpcommon/diag.go

transport/httpcommon/diag_test.go

transport/tlscommon/diag.go

Co-authored-by: Tiago Queiroz <me@tiago.life>

michel-laterman · 2024-06-04T16:47:31Z

@lucabelluccini is there anything I should add/change to help with your troubleshooting efforts?

leehinman

LGTM. Thanks for adding the test cases.

lucabelluccini · 2024-06-10T13:40:45Z

Hello, thanks for the heads up on this one and thank you for those efforts into troubleshooting TLS.
I have few questions:

We will have 1 TLS diagnostic per output (e.g. the integrations output, the monitoring output, fleet server connection)? How will it work?
We will have a documentation section to cover some example failures a user might encounter?

Example about (1).
Imagine a user has 1 Elastic Agent.
The Elastic Agent also runs the System integration.
The EA sends logs & metrics to Elasticsearch.

So we have:

connection to ES for monitoring (logs & metrics of EA)
connection to ES for integrations (system integration)
connection to Fleet Server (Fleet Server enroll/checkins)

We will get 3 TLS diags?

elasticmachine · 2024-06-10T14:33:20Z

💚 Build Succeeded

Buildkite Build
Commit: debc0d8

History

💚 Build #316 succeeded 3e958fa
💚 Build #314 succeeded 2b1ec36
💚 Build #313 succeeded 565c2d6
💚 Build #301 succeeded 81a2ad3
💚 Build #299 succeeded cb684e2
💚 Build #298 succeeded fcbf25d

michel-laterman · 2024-06-10T14:36:16Z

@lucabelluccini

yes, we'll have a request trace file per destination type; these will need to be manually added in future PRs. The implemenation issue for the elastic-agent is: Include TLS information in diagnostics bundle elastic-agent#4880.
just note the fleet-server implementation: Add tlscommon and httpcommon diagnostics hooks fleet-server#3587 currently only tests the connection to ES; it does not check for monitoring output; but I can add that check in the pr
I added diagError to try to turn an error that occurs in the connection into a human-readable cause such as %v: possible cause: HTTP schema used for HTTPS server., or %v: caused by no trusted client CA. but we can make a KB article to help track the causes of (connection) failures

lucabelluccini · 2024-06-10T15:00:52Z

Lovely - Thank you @michel-laterman & team ❤️

Add custom hooks to use diag hooks added in elastic/elastic-agent-libs#207 to provide additional files that contain information about the TLS certs used by the server's API, TLS infomation used when connecting to elasticsearch, and a full trace to each specified elasticsearch host.

michel-laterman added enhancement New feature or request Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team labels May 28, 2024

michel-laterman force-pushed the tlsdiag branch from 0e0f732 to 37f7458 Compare May 28, 2024 18:40

michel-laterman changed the title ~~Add diagnostics callback for tlscommon configs~~ Add diagnostics callback for tlscommon configs and httptransport May 28, 2024

michel-laterman force-pushed the tlsdiag branch 2 times, most recently from 378e0f7 to 54aeb42 Compare May 28, 2024 19:05

michel-laterman force-pushed the tlsdiag branch from 54aeb42 to 308c6a7 Compare May 28, 2024 19:25

put all diag funcs in dedicated file

6e1ff3f

michel-laterman force-pushed the tlsdiag branch from 4795e79 to 6e1ff3f Compare May 29, 2024 00:16

michel-laterman mentioned this pull request May 29, 2024

Add tlscommon and httpcommon diagnostics hooks elastic/fleet-server#3587

Merged

3 tasks

michel-laterman added 2 commits May 28, 2024 17:33

close resp body

7337cd9

add unit tests

44605d3

michel-laterman force-pushed the tlsdiag branch from 87f0b0f to 44605d3 Compare May 29, 2024 18:46

michel-laterman commented May 29, 2024

View reviewed changes

michel-laterman marked this pull request as ready for review May 29, 2024 18:54

michel-laterman requested a review from a team as a code owner May 29, 2024 18:54

michel-laterman requested review from belimawr and rdner and removed request for a team May 29, 2024 18:54

fix sonarr issues

cb684e2

michel-laterman force-pushed the tlsdiag branch from fcbf25d to cb684e2 Compare May 29, 2024 19:19

michel-laterman added 2 commits May 29, 2024 15:18

Print peer certs and chains in httpcommon diag

81a2ad3

Add some error diagnostics

565c2d6

Add some error parsing to HTTPSettings.DiagRequests to attempt to give a human readable statement about the cause of any errors when gathering request diagnostics.

michel-laterman commented May 30, 2024

View reviewed changes

Silence linter false positives

2b1ec36

belimawr reviewed Jun 3, 2024

View reviewed changes

transport/httpcommon/diag.go Show resolved Hide resolved

transport/httpcommon/diag_test.go Show resolved Hide resolved

transport/tlscommon/diag.go Outdated Show resolved Hide resolved

Update transport/tlscommon/diag.go

3e958fa

Co-authored-by: Tiago Queiroz <me@tiago.life>

belimawr approved these changes Jun 7, 2024

View reviewed changes

leehinman approved these changes Jun 7, 2024

View reviewed changes

Merge branch 'main' into tlsdiag

debc0d8

michel-laterman merged commit 89e2295 into elastic:main Jun 10, 2024
5 of 6 checks passed

michel-laterman deleted the tlsdiag branch June 10, 2024 14:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add diagnostics callback for tlscommon configs and httptransport #207

Add diagnostics callback for tlscommon configs and httptransport #207

michel-laterman commented May 28, 2024 •

edited

Loading

michel-laterman commented May 29, 2024

michel-laterman left a comment

michel-laterman May 29, 2024

michel-laterman May 29, 2024

leehinman commented May 29, 2024

michel-laterman May 30, 2024

michel-laterman May 30, 2024

michel-laterman commented Jun 4, 2024

leehinman left a comment

lucabelluccini commented Jun 10, 2024

elasticmachine commented Jun 10, 2024

michel-laterman commented Jun 10, 2024

lucabelluccini commented Jun 10, 2024

Add diagnostics callback for tlscommon configs and httptransport #207

Add diagnostics callback for tlscommon configs and httptransport #207

Conversation

michel-laterman commented May 28, 2024 • edited Loading

What does this PR do?

Why is it important?

Checklist

michel-laterman commented May 29, 2024

michel-laterman left a comment

Choose a reason for hiding this comment

michel-laterman May 29, 2024

Choose a reason for hiding this comment

michel-laterman May 29, 2024

Choose a reason for hiding this comment

leehinman commented May 29, 2024

michel-laterman May 30, 2024

Choose a reason for hiding this comment

michel-laterman May 30, 2024

Choose a reason for hiding this comment

michel-laterman commented Jun 4, 2024

leehinman left a comment

Choose a reason for hiding this comment

lucabelluccini commented Jun 10, 2024

elasticmachine commented Jun 10, 2024

💚 Build Succeeded

History

michel-laterman commented Jun 10, 2024

lucabelluccini commented Jun 10, 2024

michel-laterman commented May 28, 2024 •

edited

Loading