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] Properly handle cross-signed CAs #33215

Closed
andrewvc opened this issue Sep 28, 2022 · 1 comment · Fixed by #33231
Closed

[Heartbeat] Properly handle cross-signed CAs #33215

andrewvc opened this issue Sep 28, 2022 · 1 comment · Fixed by #33231
Assignees
Labels
bug Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team

Comments

@andrewvc
Copy link
Contributor

andrewvc commented Sep 28, 2022

Let's encrypt currently cross-signs its certs, including a now expired CA for legacy compat, per this blog post.

We should handle this correctly, right now we don't. We currently pick the earliest / latest dates in the chain for validation, when instead we only need to make sure one of the roots is good, not all of them.

See https://www.sslshopper.com/ssl-checker.html#hostname=https://portals.edu.wyoming.gov/ for an example

@andrewvc andrewvc added bug Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team labels Sep 28, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/uptime (Team:Uptime)

andrewvc added a commit to andrewvc/beats that referenced this issue Sep 30, 2022
Fixes elastic#33215

This is a bit WIP in that I'm not certain this is the final logic we want. It appears the elastic agent libs override the default golang TLS implementation's verification and only set the cert chains attribute on the connection when strict validation is enabled, where the default is `full`. See https://www.elastic.co/guide/en/beats/heartbeat/current/configuration-ssl.html for definition of the various validation options.

I suspect we'd be best off making strict the default since this is what chrome does nowadays, requiring a SAN to be set, which is the only other distinction.

To test this use the following config, which returns a too-early expiration date without this patch:

```
- type: http
  enabled: true
  id: wyportal
  name: WYPortal
  schedule: "@every 1m"
  ssl.verification_mode: strict
  urls: "https://portals.edu.wyoming.gov/"
```

You can try playing with the verification mode as well
@andrewvc andrewvc self-assigned this Sep 30, 2022
andrewvc added a commit that referenced this issue Oct 11, 2022
* [Heartbeat] Fix expiration of cert chains calculation

Fixes #33215

This is a bit WIP in that I'm not certain this is the final logic we want. It appears the elastic agent libs override the default golang TLS implementation's verification and only set the cert chains attribute on the connection when strict validation is enabled, where the default is `full`. See https://www.elastic.co/guide/en/beats/heartbeat/current/configuration-ssl.html for definition of the various validation options.

I suspect we'd be best off making strict the default since this is what chrome does nowadays, requiring a SAN to be set, which is the only other distinction.

To test this use the following config, which returns a too-early expiration date without this patch:

```
- type: http
  enabled: true
  id: wyportal
  name: WYPortal
  schedule: "@every 1m"
  ssl.verification_mode: strict
  urls: "https://portals.edu.wyoming.gov/"
```

You can try playing with the verification mode as well

* Cleanup work

* Add changelog

* Fix cert calculations

* Fix lint
mergify bot pushed a commit that referenced this issue Oct 11, 2022
* [Heartbeat] Fix expiration of cert chains calculation

Fixes #33215

This is a bit WIP in that I'm not certain this is the final logic we want. It appears the elastic agent libs override the default golang TLS implementation's verification and only set the cert chains attribute on the connection when strict validation is enabled, where the default is `full`. See https://www.elastic.co/guide/en/beats/heartbeat/current/configuration-ssl.html for definition of the various validation options.

I suspect we'd be best off making strict the default since this is what chrome does nowadays, requiring a SAN to be set, which is the only other distinction.

To test this use the following config, which returns a too-early expiration date without this patch:

```
- type: http
  enabled: true
  id: wyportal
  name: WYPortal
  schedule: "@every 1m"
  ssl.verification_mode: strict
  urls: "https://portals.edu.wyoming.gov/"
```

You can try playing with the verification mode as well

* Cleanup work

* Add changelog

* Fix cert calculations

* Fix lint

(cherry picked from commit bd081b6)
mergify bot pushed a commit that referenced this issue Oct 11, 2022
* [Heartbeat] Fix expiration of cert chains calculation

Fixes #33215

This is a bit WIP in that I'm not certain this is the final logic we want. It appears the elastic agent libs override the default golang TLS implementation's verification and only set the cert chains attribute on the connection when strict validation is enabled, where the default is `full`. See https://www.elastic.co/guide/en/beats/heartbeat/current/configuration-ssl.html for definition of the various validation options.

I suspect we'd be best off making strict the default since this is what chrome does nowadays, requiring a SAN to be set, which is the only other distinction.

To test this use the following config, which returns a too-early expiration date without this patch:

```
- type: http
  enabled: true
  id: wyportal
  name: WYPortal
  schedule: "@every 1m"
  ssl.verification_mode: strict
  urls: "https://portals.edu.wyoming.gov/"
```

You can try playing with the verification mode as well

* Cleanup work

* Add changelog

* Fix cert calculations

* Fix lint

(cherry picked from commit bd081b6)

# Conflicts:
#	heartbeat/hbtest/hbtestutil.go
#	heartbeat/monitors/active/dialchain/tlsmeta/tlsmeta.go
#	heartbeat/monitors/active/dialchain/tlsmeta/tlsmeta_test.go
#	heartbeat/monitors/active/http/task.go
#	heartbeat/monitors/active/tcp/tcp.go
andrewvc added a commit that referenced this issue Oct 12, 2022
* [Heartbeat] Fix expiration of cert chains calculation

Fixes #33215

This is a bit WIP in that I'm not certain this is the final logic we want. It appears the elastic agent libs override the default golang TLS implementation's verification and only set the cert chains attribute on the connection when strict validation is enabled, where the default is `full`. See https://www.elastic.co/guide/en/beats/heartbeat/current/configuration-ssl.html for definition of the various validation options.

I suspect we'd be best off making strict the default since this is what chrome does nowadays, requiring a SAN to be set, which is the only other distinction.

To test this use the following config, which returns a too-early expiration date without this patch:

```
- type: http
  enabled: true
  id: wyportal
  name: WYPortal
  schedule: "@every 1m"
  ssl.verification_mode: strict
  urls: "https://portals.edu.wyoming.gov/"
```

You can try playing with the verification mode as well

* Cleanup work

* Add changelog

* Fix cert calculations

* Fix lint

(cherry picked from commit bd081b6)

Co-authored-by: Andrew Cholakian <[email protected]>
chrisberkhout pushed a commit that referenced this issue Jun 1, 2023
* [Heartbeat] Fix expiration of cert chains calculation

Fixes #33215

This is a bit WIP in that I'm not certain this is the final logic we want. It appears the elastic agent libs override the default golang TLS implementation's verification and only set the cert chains attribute on the connection when strict validation is enabled, where the default is `full`. See https://www.elastic.co/guide/en/beats/heartbeat/current/configuration-ssl.html for definition of the various validation options.

I suspect we'd be best off making strict the default since this is what chrome does nowadays, requiring a SAN to be set, which is the only other distinction.

To test this use the following config, which returns a too-early expiration date without this patch:

```
- type: http
  enabled: true
  id: wyportal
  name: WYPortal
  schedule: "@every 1m"
  ssl.verification_mode: strict
  urls: "https://portals.edu.wyoming.gov/"
```

You can try playing with the verification mode as well

* Cleanup work

* Add changelog

* Fix cert calculations

* Fix lint
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.

3 participants