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

[Heartbeat] TLS options ignored when forward proxy used #15524

Closed
andrewvc opened this issue Jan 14, 2020 · 1 comment · Fixed by #15516
Closed

[Heartbeat] TLS options ignored when forward proxy used #15524

andrewvc opened this issue Jan 14, 2020 · 1 comment · Fixed by #15516
Assignees
Labels
bug Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team

Comments

@andrewvc
Copy link
Contributor

For configs where an HTTP proxy is passed into the proxy_url setting and custom TLS options are used the TLS options are ignored.

@andrewvc andrewvc added bug Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team labels Jan 14, 2020
@andrewvc andrewvc self-assigned this Jan 14, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/uptime (:uptime)

andrewvc added a commit that referenced this issue Jan 27, 2020
…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
…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 added a commit to andrewvc/beats that referenced this issue Jan 27, 2020
…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 added a commit that referenced this issue Jan 27, 2020
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
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants