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.

Sign up for GitHub

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

Jump to bottom

Check error on chain verification failure #2284

Merged
merged 2 commits into from Sep 27, 2022
Merged

Check error on chain verification failure #2284

merged 2 commits into from Sep 27, 2022

Conversation

haydentherapper
Copy link
Contributor

This fails certificate verification at the correct step.

err was declared in the else scope, but was not checked before exiting the conditional. Therefore, if a failure occurred, we didn't return an error. However, later in the code, in verifyBlob, we do an extra check for if the certificate is valid. Because we haven't set the root and intermediate certs yet, cert verification fails.

Signed-off-by: Hayden Blauzvern hblauzvern@google.com

Summary

Release Note

Documentation

@haydentherapper
Check error on chain verification failure
e61e47d 
This fails certificate verification at the correct step.

err was declared in the else scope, but was not checked before exiting
the conditional. Therefore, if a failure occurred, we didn't return
an error. However, later in the code, in verifyBlob, we do an extra
check for if the certificate is valid. Because we haven't set the root
and intermediate certs yet, cert verification fails.

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@haydentherapper
Update error messages
2e58b74 
Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@codecov-commenter
Copy link

codecov-commenter commented Sep 27, 2022 •
edited

Codecov Report

Merging #2284 (2e58b74) into main (d720f04) will increase coverage by 0.01%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##             main    #2284      +/-   ##
==========================================
+ Coverage   29.02%   29.04%   +0.01%     
==========================================
  Files         131      131              
  Lines        7872     7878       +6     
==========================================
+ Hits         2285     2288       +3     
- Misses       5275     5277       +2     
- Partials      312      313       +1
Impacted Files Coverage Δ
cmd/cosign/cli/verify/verify_blob.go 45.15% <85.71%> (+0.06%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@dlorenc dlorenc merged commit f449fa8 into sigstore:main Sep 27, 2022
@github-actions github-actions bot added this to the v1.13.0 milestone Sep 27, 2022
jeff-mccoy pushed a commit to defenseunicorns/zarf that referenced this pull request Oct 7, 2022
@renovate
Update module github.com/sigstore/cosign to v1.13.0 (#863)
6379653 
[![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
([sigstore/cosign#2291)
- Data race in FetchSignaturesForReference
([sigstore/cosign#2283)
- Check error on chain verification failure
([sigstore/cosign#2284)
- feat: improve the verification message
([sigstore/cosign#2268)
- feat: use stdin as an input for predicate
([sigstore/cosign#2269)

#### Bug Fixes

- fix: make tlog entry lookups for online verification shard-aware
([sigstore/cosign#2297)
- Fix: Create a static copy of signatures as part of verification.
([sigstore/cosign#2287)
- Fix: Remove an extra registry request from verification path.
([sigstore/cosign#2285)
- fix pivtool generate key touch policy
([sigstore/cosign#2282)

#### Others

- use scaffolding 0.4.8 for tests.
([sigstore/cosign#2280)

#### Contributors

-   Asra Ali ([@&#8203;asraa](https://togithub.com/asraa))
- Batuhan Apaydın
([@&#8203;developer-guy](https://togithub.com/developer-guy))
- Carlos Tadeu Panato Junior
([@&#8203;cpanato](https://togithub.com/cpanato))
- Hayden Blauzvern
([@&#8203;haydentherapper](https://togithub.com/haydentherapper))
-   Matt Moore ([@&#8203;mattmoor](https://togithub.com/mattmoor))
-   Ross Tannenbaum ([@&#8203;RTann](https://togithub.com/RTann))
-   Ville Aikas ([@&#8203;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>
@haydentherapper haydentherapper deleted the fix-err-check branch January 10, 2023 23:34
Noxsios pushed a commit to defenseunicorns/zarf that referenced this pull request Mar 8, 2023
@renovate
Update module github.com/sigstore/cosign to v1.13.0 (#863)
2dec3c4 
[![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
([sigstore/cosign#2291)
- Data race in FetchSignaturesForReference
([sigstore/cosign#2283)
- Check error on chain verification failure
([sigstore/cosign#2284)
- feat: improve the verification message
([sigstore/cosign#2268)
- feat: use stdin as an input for predicate
([sigstore/cosign#2269)

#### Bug Fixes

- fix: make tlog entry lookups for online verification shard-aware
([sigstore/cosign#2297)
- Fix: Create a static copy of signatures as part of verification.
([sigstore/cosign#2287)
- Fix: Remove an extra registry request from verification path.
([sigstore/cosign#2285)
- fix pivtool generate key touch policy
([sigstore/cosign#2282)

#### Others

- use scaffolding 0.4.8 for tests.
([sigstore/cosign#2280)

#### Contributors

-   Asra Ali ([@&#8203;asraa](https://togithub.com/asraa))
- Batuhan Apaydın
([@&#8203;developer-guy](https://togithub.com/developer-guy))
- Carlos Tadeu Panato Junior
([@&#8203;cpanato](https://togithub.com/cpanato))
- Hayden Blauzvern
([@&#8203;haydentherapper](https://togithub.com/haydentherapper))
-   Matt Moore ([@&#8203;mattmoor](https://togithub.com/mattmoor))
-   Ross Tannenbaum ([@&#8203;RTann](https://togithub.com/RTann))
-   Ville Aikas ([@&#8203;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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bobcallaway bobcallaway bobcallaway approved these changes

@asraa asraa Awaiting requested review from asraa

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.13.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@haydentherapper @codecov-commenter @bobcallaway @dlorenc