-
Notifications
You must be signed in to change notification settings - Fork 551
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
Add support for Fulcio username identity in SAN #2291
Conversation
We have changed the format of the username identity to not look like an email, and so we also needed to change which SAN the username was set in. Now using OtherName, I've added some custom unmarshalling to extract the OtherName SAN, because Go doesn't support this SAN type. Note in verify.go, I had to handle when the extension was critical. Since Go doesn't handle the extension, but it must be marked critical according to RFC5280, the cert will fail verification. We can simply remove the extension from the list of unhandled extensions before verifying. Signed-off-by: Hayden Blauzvern <[email protected]>
fyi @woodruffw, I'm not sure if the Python crypto library will have the same issue of not supporting less well known SAN types. |
Signed-off-by: Hayden Blauzvern <[email protected]>
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.
Nice! Mostly nitpicks 🙂
BTW tests in certextensions_test.go look really great, thanks for the excellent comments
Signed-off-by: Hayden Blauzvern <[email protected]>
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.
LGTM!
This is good to merge! |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [github.com/sigstore/cosign](https://togithub.com/sigstore/cosign) | require | minor | `v1.12.1` -> `v1.13.0` | --- ### Release Notes <details> <summary>sigstore/cosign</summary> ### [`v1.13.0`](https://togithub.com/sigstore/cosign/blob/HEAD/CHANGELOG.md#v1130) [Compare Source](https://togithub.com/sigstore/cosign/compare/v1.12.1...v1.13.0) > # Highlights > > - For users who have deployed a private instance of Fulcio release v0.6.x and issue certificates with the Username identity, you will need to upgrade to use this version." #### Enhancements - Add support for Fulcio username identity in SAN ([https://github.com/sigstore/cosign/pull/2291](https://togithub.com/sigstore/cosign/pull/2291)) - Data race in FetchSignaturesForReference ([https://github.com/sigstore/cosign/pull/2283](https://togithub.com/sigstore/cosign/pull/2283)) - Check error on chain verification failure ([https://github.com/sigstore/cosign/pull/2284](https://togithub.com/sigstore/cosign/pull/2284)) - feat: improve the verification message ([https://github.com/sigstore/cosign/pull/2268](https://togithub.com/sigstore/cosign/pull/2268)) - feat: use stdin as an input for predicate ([https://github.com/sigstore/cosign/pull/2269](https://togithub.com/sigstore/cosign/pull/2269)) #### Bug Fixes - fix: make tlog entry lookups for online verification shard-aware ([https://github.com/sigstore/cosign/pull/2297](https://togithub.com/sigstore/cosign/pull/2297)) - Fix: Create a static copy of signatures as part of verification. ([https://github.com/sigstore/cosign/pull/2287](https://togithub.com/sigstore/cosign/pull/2287)) - Fix: Remove an extra registry request from verification path. ([https://github.com/sigstore/cosign/pull/2285](https://togithub.com/sigstore/cosign/pull/2285)) - fix pivtool generate key touch policy ([https://github.com/sigstore/cosign/pull/2282](https://togithub.com/sigstore/cosign/pull/2282)) #### Others - use scaffolding 0.4.8 for tests. ([https://github.com/sigstore/cosign/pull/2280](https://togithub.com/sigstore/cosign/pull/2280)) #### Contributors - Asra Ali ([@​asraa](https://togithub.com/asraa)) - Batuhan Apaydın ([@​developer-guy](https://togithub.com/developer-guy)) - Carlos Tadeu Panato Junior ([@​cpanato](https://togithub.com/cpanato)) - Hayden Blauzvern ([@​haydentherapper](https://togithub.com/haydentherapper)) - Matt Moore ([@​mattmoor](https://togithub.com/mattmoor)) - Ross Tannenbaum ([@​RTann](https://togithub.com/RTann)) - Ville Aikas ([@​vaikas](https://togithub.com/vaikas)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/defenseunicorns/zarf). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4yMjIuMyIsInVwZGF0ZWRJblZlciI6IjMyLjIyMi4zIn0=--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [github.com/sigstore/cosign](https://togithub.com/sigstore/cosign) | require | minor | `v1.12.1` -> `v1.13.0` | --- ### Release Notes <details> <summary>sigstore/cosign</summary> ### [`v1.13.0`](https://togithub.com/sigstore/cosign/blob/HEAD/CHANGELOG.md#v1130) [Compare Source](https://togithub.com/sigstore/cosign/compare/v1.12.1...v1.13.0) > # Highlights > > - For users who have deployed a private instance of Fulcio release v0.6.x and issue certificates with the Username identity, you will need to upgrade to use this version." #### Enhancements - Add support for Fulcio username identity in SAN ([https://github.com/sigstore/cosign/pull/2291](https://togithub.com/sigstore/cosign/pull/2291)) - Data race in FetchSignaturesForReference ([https://github.com/sigstore/cosign/pull/2283](https://togithub.com/sigstore/cosign/pull/2283)) - Check error on chain verification failure ([https://github.com/sigstore/cosign/pull/2284](https://togithub.com/sigstore/cosign/pull/2284)) - feat: improve the verification message ([https://github.com/sigstore/cosign/pull/2268](https://togithub.com/sigstore/cosign/pull/2268)) - feat: use stdin as an input for predicate ([https://github.com/sigstore/cosign/pull/2269](https://togithub.com/sigstore/cosign/pull/2269)) #### Bug Fixes - fix: make tlog entry lookups for online verification shard-aware ([https://github.com/sigstore/cosign/pull/2297](https://togithub.com/sigstore/cosign/pull/2297)) - Fix: Create a static copy of signatures as part of verification. ([https://github.com/sigstore/cosign/pull/2287](https://togithub.com/sigstore/cosign/pull/2287)) - Fix: Remove an extra registry request from verification path. ([https://github.com/sigstore/cosign/pull/2285](https://togithub.com/sigstore/cosign/pull/2285)) - fix pivtool generate key touch policy ([https://github.com/sigstore/cosign/pull/2282](https://togithub.com/sigstore/cosign/pull/2282)) #### Others - use scaffolding 0.4.8 for tests. ([https://github.com/sigstore/cosign/pull/2280](https://togithub.com/sigstore/cosign/pull/2280)) #### Contributors - Asra Ali ([@​asraa](https://togithub.com/asraa)) - Batuhan Apaydın ([@​developer-guy](https://togithub.com/developer-guy)) - Carlos Tadeu Panato Junior ([@​cpanato](https://togithub.com/cpanato)) - Hayden Blauzvern ([@​haydentherapper](https://togithub.com/haydentherapper)) - Matt Moore ([@​mattmoor](https://togithub.com/mattmoor)) - Ross Tannenbaum ([@​RTann](https://togithub.com/RTann)) - Ville Aikas ([@​vaikas](https://togithub.com/vaikas)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/defenseunicorns/zarf). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4yMjIuMyIsInVwZGF0ZWRJblZlciI6IjMyLjIyMi4zIn0=--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
We have changed the format of the username identity to not look like an email, and so we also needed to change which SAN the username was set in. Now using OtherName, I've added some custom unmarshalling to extract the OtherName SAN, because Go doesn't support this SAN type.
Note in verify.go, I had to handle when the extension was critical. Since Go doesn't handle the extension, but it must be marked critical according to RFC5280, the cert will fail verification. We can simply remove the extension from the list of unhandled extensions before verifying.
Signed-off-by: Hayden Blauzvern [email protected]
Summary
Release Note
Added support for new Fulcio username identity format
Documentation