- Notifications
You must be signed in to change notification settings - Fork622
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Fix cryptographic certificates for post go 1.19#3161
base:master
Are you sure you want to change the base?
Conversation
Looks like generated files need an update (also golangci-lint failing; could be out of memory?) |
ca/config_test.go Outdated
| //defer close(actualErrChan) | ||
| require.Error(t, err) | ||
| err = <-actualErrChan | ||
| require.Error(t, err) | ||
| require.IsType(t, x509.UnknownAuthorityError{}, err) | ||
| require.ErrorAs(t, err, &x509.UnknownAuthorityError{}) | ||
| // require.IsType(t, x509.UnknownAuthorityError{}, err) | ||
| _, actualErrChan, err = tlsGRPCDial(tc.Context, l.Addr().String(), tcConfig.ClientTLSCreds) | ||
| defer close(actualErrChan) | ||
| //defer close(actualErrChan) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Are these commented-out lines temporary? Or forgot to remove?
Something is broken with the linter. Even on my local machine, running |
Wondering if it needs settings tweaked; if it's a case of it running in a container and trying to spawn too many parallel things 🤔 I know I had to tweak GOMAXPROCS and some other things some years back for moby/moby |
Go 1.19 changed the way cryptographic certificates were verified, whichbroke a certain edge case of root CA rotation. This edge case is nowdisallowed.Signed-off-by: Drew Erny <derny@mirantis.com>
codecov-commenter commentedApr 3, 2024
Codecov Report
Additional details and impacted files@@ Coverage Diff @@## master #3161 +/- ##==========================================+ Coverage 57.28% 57.35% +0.07%========================================== Files 144 144 Lines 30598 30603 +5 ==========================================+ Hits 17528 17553 +25+ Misses 11732 11709 -23- Partials 1338 1341 +3 |
- What I did
Go 1.19 changed the way cryptographic certificates were verified, which broke a certain edge case of root CA rotation. This edge case is now disallowed.
- How I did it
As of go 1.19, the logic for certificate trust chain validation changed, and a chain including two certs with the same key will not validate. This case would usually occur when reissuing the same cert with a later expiration date. Because of this validation failure, our root rotation algorithm fails. While it might be possible to adjust the rotation procedure to accommodate such a cert change, it is somewhat of an edge case, and, more importantly, we do not currently possess the cryptographic expertise to safely make such a change. So, as a result, this operation is disallowed. The new root cert must have a new key.
- How to test it
Test changes within.