-
Notifications
You must be signed in to change notification settings - Fork 521
Fix CA startup when configured with SCEP and Google Cloud CAS #2517
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
Conversation
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.
Pull request overview
This PR fixes a startup issue when the Certificate Authority (CA) is configured with SCEP and Google Cloud CAS. The fix properly handles intermediate certificates in the certificate chain, including cases where only a root certificate exists.
Key changes:
- Modified
GetCertificateAuthorityto parse and return intermediate certificates from the certificate chain - Added guard to prevent nil dereference when no intermediate certificates are present in SCEP configuration
- Enhanced test coverage with additional test cases for different certificate chain scenarios
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cas/cloudcas/cloudcas.go | Added parsing of intermediate certificates and updated error handling in GetCertificateAuthority |
| cas/cloudcas/cloudcas_test.go | Added new test helper functions and test cases to cover root-only and multiple intermediate certificate scenarios |
| authority/authority.go | Added guard to prevent accessing intermediates array when empty in SCEP options initialization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
8ec0e97 to
548717c
Compare
maraino
left a comment
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.
Looks good to me, but can you make sure GET /intermediates.pem works properly, looking at the code it looks it will.
|
@maraino that indeed works. The logic is in line with what we did for Vault now. Users of the endpoint will now possibly get more certificates in the response when using GCP CAS, but it should be in line with what one expects from that endpoint. I could follow up with an integration test in a new PR for this behavior? The current tests for the API endpoints are using a minimal mock authority, so it requires some more boilerplate to get an actual authority with a (fake) CAS setup. |
Fixes: #2509