Skip to content

helm: Lower default hubble.tls.auto.certValidityDuration to 365 days #35630

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

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

chancez
Copy link
Contributor

@chancez chancez commented Oct 29, 2024

Hubble CLI will fail to validate certificates with an expiration that's too large on recent MacOS versions with the following error:

rpc error: code = Unavailable desc = connection error: desc = "transport: authentication handshake failed: tls: failed to verify certificate: x509: “*.hubble-relay.cilium.io” certificate is not standards compliant"

This is due to https://support.apple.com/en-us/103769 which specifies:

All TLS server certificates must comply with these new security requirements in iOS 13 and macOS 10.15:
...
Additionally, all TLS server certificates issued after July 1, 2019 (as indicated in the NotBefore field of the certificate) must follow these guidelines:
...
TLS server certificates must have a validity period of 825 days or fewer (as expressed in the NotBefore and NotAfter fields of the certificate).

Since we have many users using MacOS and are likely using the default values with Hubble when starting with TLS, we should adjust the default value to work with recent MacOS versions. Users who wish to preserve the existing expiration validity can set hubble.tls.auto.certValidityDuration to the previous default value.

helm: Lower default `hubble.tls.auto.certValidityDuration` to 365 days

@chancez chancez added this to the 1.17 milestone Oct 29, 2024
@chancez chancez self-assigned this Oct 29, 2024
@chancez chancez requested review from a team as code owners October 29, 2024 23:08
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 29, 2024
@chancez chancez requested a review from qmonnet October 29, 2024 23:09
Copy link
Contributor

@michi-covalent michi-covalent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems reasonable

@michi-covalent michi-covalent added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Oct 29, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 29, 2024
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the context in the PR's description; I'd recommend keeping it in the commit description as well (for next time).

Looks good to me

@qmonnet
Copy link
Member

qmonnet commented Oct 30, 2024

It seems the spell-checker does not recognise “expirations” 🤦

@qmonnet qmonnet removed the request for review from nathanjsweet October 30, 2024 11:29
Hubble CLI will fail to validate certificates with an expiration that's too large on recent MacOS versions with the following error:
```
rpc error: code = Unavailable desc = connection error: desc = "transport: authentication handshake failed: tls: failed to verify certificate: x509: “*.hubble-relay.cilium.io” certificate is not standards compliant"
```

This is due to https://support.apple.com/en-us/103769 which specifies:

> All TLS server certificates must comply with these new security requirements in iOS 13 and macOS 10.15:
> ...
> Additionally, all TLS server certificates issued after July 1, 2019 (as indicated in the NotBefore field of the certificate) must follow these guidelines:
> ...
> TLS server certificates must have a validity period of 825 days or fewer (as expressed in the NotBefore and NotAfter fields of the certificate).

Since we have many users using MacOS and are likely using the default values with Hubble when starting with TLS, we should adjust the default value to work with recent MacOS versions. Users who wish to preserve the existing expiration validity can set `hubble.tls.auto.certValidityDuration` to the previous default value.

Signed-off-by: Chance Zibolski <chance.zibolski@gmail.com>
@chancez chancez force-pushed the pr/chancez/hubble_tls_auto_cert_validity_duration branch from dfab7aa to c29cfc7 Compare October 30, 2024 22:59
@qmonnet
Copy link
Member

qmonnet commented Oct 31, 2024

/test

@qmonnet qmonnet enabled auto-merge October 31, 2024 09:50
@qmonnet qmonnet added this pull request to the merge queue Oct 31, 2024
Merged via the queue into main with commit 13d3c46 Oct 31, 2024
266 checks passed
@qmonnet qmonnet deleted the pr/chancez/hubble_tls_auto_cert_validity_duration branch October 31, 2024 15:35
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Oct 31, 2024
@chancez
Copy link
Contributor Author

chancez commented Oct 31, 2024

I just realized too: we should probably backport to 1.16 at least?

@chancez chancez added the needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch label Oct 31, 2024
@joamaki joamaki mentioned this pull request Nov 5, 2024
23 tasks
@joamaki joamaki added backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. and removed needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch labels Nov 5, 2024
@github-actions github-actions bot added backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. and removed backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. labels Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants