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.

Sign up for GitHub

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

Jump to bottom

[Heartbeat] Certificate Expiry not recorded with forward proxies #15797

Closed
andrewvc opened this issue Jan 23, 2020 · 1 comment · Fixed by #22190
Closed

[Heartbeat] Certificate Expiry not recorded with forward proxies #15797

andrewvc opened this issue Jan 23, 2020 · 1 comment · Fixed by #22190
Assignees
@andrewvc
Labels
bug Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team test-plan Add this PR to be manual test plan test-plan-ok This PR passed manual testing v7.11.0

Comments

@andrewvc
Copy link
Contributor

Go's HTTP transport uses a different code path when an HTTP forward proxy is used. When fixing #15516 it was discovered that we don't record x509 cert expiry when an HTTP proxy is used.

The implementation here is a little wonky, we'll probably have to instantiate a new HTTP client per request when using a forward proxy.
@andrewvc andrewvc added bug Heartbeat [zube]: Backlog Team:obs-ds-hosted-services Label for the Observability Hosted Services team labels Jan 23, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/uptime (:uptime)

@andrewvc andrewvc mentioned this issue Jan 23, 2020
Merged
andrewvc added a commit that referenced this issue Jan 27, 2020
@andrewvc
[heartbeat][libbeat][metricbeat][filebeat] Pass TLS options to forwar…
0e57f6b 
…d proxies (#15516)

Fixes a bug where TLS options would not be passed to forward proxies. This fixes a reported issue where x509 client cert auth doesn't work due to the cert parameters not being passed in.

Since this doesn't give us a full dialer we can't easily record TLS certificate expiry with proxies in heartbeat with this approach, so those fields would be missing. I think it makes sense to merge this change as-is and fix that secondary bug in a subsequent PR. I've opened a separate issue ( #15797 ) to cover that.

I've tested this manually against with the following config: https://github.com/elastic/uptime-contrib/tree/master/testing/configs/client-auth , note that you'll need the included client certs in that directory, and will need to alter the YAML to point at their actual path.

I think in this scenario I'm fine with only having unit tests because the complexity of setting of a forward proxy + a custom TLS backend is very high.

Fixes #15524
andrewvc added a commit to andrewvc/beats that referenced this issue Jan 27, 2020
@andrewvc
[heartbeat][libbeat][metricbeat][filebeat] Pass TLS options to forwar…
7a4e5b9 
…d proxies (elastic#15516)

Fixes a bug where TLS options would not be passed to forward proxies. This fixes a reported issue where x509 client cert auth doesn't work due to the cert parameters not being passed in.

Since this doesn't give us a full dialer we can't easily record TLS certificate expiry with proxies in heartbeat with this approach, so those fields would be missing. I think it makes sense to merge this change as-is and fix that secondary bug in a subsequent PR. I've opened a separate issue ( elastic#15797 ) to cover that.

I've tested this manually against with the following config: https://github.com/elastic/uptime-contrib/tree/master/testing/configs/client-auth , note that you'll need the included client certs in that directory, and will need to alter the YAML to point at their actual path.

I think in this scenario I'm fine with only having unit tests because the complexity of setting of a forward proxy + a custom TLS backend is very high.

Fixes elastic#15524

(cherry picked from commit 0e57f6b)
@andrewvc andrewvc mentioned this issue Jan 27, 2020
Merged
5 tasks
andrewvc added a commit to andrewvc/beats that referenced this issue Jan 27, 2020
@andrewvc
[heartbeat][libbeat][metricbeat][filebeat] Pass TLS options to forwar…
dae2792 
…d proxies (elastic#15516)

Fixes a bug where TLS options would not be passed to forward proxies. This fixes a reported issue where x509 client cert auth doesn't work due to the cert parameters not being passed in.

Since this doesn't give us a full dialer we can't easily record TLS certificate expiry with proxies in heartbeat with this approach, so those fields would be missing. I think it makes sense to merge this change as-is and fix that secondary bug in a subsequent PR. I've opened a separate issue ( elastic#15797 ) to cover that.

I've tested this manually against with the following config: https://github.com/elastic/uptime-contrib/tree/master/testing/configs/client-auth , note that you'll need the included client certs in that directory, and will need to alter the YAML to point at their actual path.

I think in this scenario I'm fine with only having unit tests because the complexity of setting of a forward proxy + a custom TLS backend is very high.

Fixes elastic#15524

(cherry picked from commit 0e57f6b)
@andrewvc andrewvc mentioned this issue Jan 27, 2020
Merged
5 tasks
andrewvc added a commit that referenced this issue Jan 27, 2020
@andrewvc
[7.x][heartbeat][libbeat][metricbeat][filebeat] (#15874)
f8022d5 
Fixes a bug where TLS options would not be passed to forward proxies. This fixes a reported issue where x509 client cert auth doesn't work due to the cert parameters not being passed in.

Since this doesn't give us a full dialer we can't easily record TLS certificate expiry with proxies in heartbeat with this approach, so those fields would be missing. I think it makes sense to merge this change as-is and fix that secondary bug in a subsequent PR. I've opened a separate issue ( #15797 ) to cover that.

I've tested this manually against with the following config: https://github.com/elastic/uptime-contrib/tree/master/testing/configs/client-auth , note that you'll need the included client certs in that directory, and will need to alter the YAML to point at their actual path.

I think in this scenario I'm fine with only having unit tests because the complexity of setting of a forward proxy + a custom TLS backend is very high.

Fixes #15524

(cherry picked from commit 0e57f6b)
andrewvc added a commit that referenced this issue Jan 27, 2020
@andrewvc
[7.6][heartbeat][libbeat][metricbeat][filebeat] (#15875)
711a2cb 
Fixes a bug where TLS options would not be passed to forward proxies. This fixes a reported issue where x509 client cert auth doesn't work due to the cert parameters not being passed in.

Since this doesn't give us a full dialer we can't easily record TLS certificate expiry with proxies in heartbeat with this approach, so those fields would be missing. I think it makes sense to merge this change as-is and fix that secondary bug in a subsequent PR. I've opened a separate issue ( #15797 ) to cover that.

I've tested this manually against with the following config: https://github.com/elastic/uptime-contrib/tree/master/testing/configs/client-auth , note that you'll need the included client certs in that directory, and will need to alter the YAML to point at their actual path.

I think in this scenario I'm fine with only having unit tests because the complexity of setting of a forward proxy + a custom TLS backend is very high.

Fixes #15524

(cherry picked from commit 0e57f6b)
adriansr added a commit to adriansr/beats that referenced this issue Oct 27, 2020
@adriansr
Heartbeat: Add tls fields when connecting through proxy
c9ff368 
This updates Heartbeat to enrich an event with TLS information when the
connection has been established via an HTTP proxy.

Closes elastic#15797
@adriansr adriansr mentioned this issue Oct 27, 2020
Merged
6 tasks
adriansr added a commit that referenced this issue Dec 3, 2020
@adriansr
[Heartbeat] Add tls fields when connecting through proxy (#22190)
cd16ca0 
This updates Heartbeat to enrich an event with TLS information when the
connection has been established via an HTTP proxy.

Closes #15797
adriansr added a commit to adriansr/beats that referenced this issue Dec 3, 2020
@adriansr
[Heartbeat] Add tls fields when connecting through proxy (elastic#22190)
3887484 
This updates Heartbeat to enrich an event with TLS information when the
connection has been established via an HTTP proxy.

Closes elastic#15797

(cherry picked from commit cd16ca0)
@adriansr adriansr mentioned this issue Dec 3, 2020
Merged
6 tasks
adriansr added a commit that referenced this issue Dec 3, 2020
@adriansr
[Heartbeat] Add tls fields when connecting through proxy (#22190) (#2…
5c6aedf 
…2881)

This updates Heartbeat to enrich an event with TLS information when the
connection has been established via an HTTP proxy.

Closes #15797

(cherry picked from commit cd16ca0)
@paulb-elastic paulb-elastic added test-plan Add this PR to be manual test plan v7.11.0 labels Dec 16, 2020
@paulb-elastic paulb-elastic added the test-plan-ok This PR passed manual testing label Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@andrewvc andrewvc

Labels
bug Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team test-plan Add this PR to be manual test plan test-plan-ok This PR passed manual testing v7.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Heartbeat] Add tls fields when connecting through proxy adriansr/beats
3 participants
@andrewvc @elasticmachine @paulb-elastic