-
Notifications
You must be signed in to change notification settings - Fork 547
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
Fixing issue 3743 #3744
Fixing issue 3743 #3744
Conversation
Signed-off-by: Meeki1l <meekill2001@gmail.com>
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, thanks for catching this.
cc @ianhundere
pkg/cosign/tsa.go
Outdated
@@ -68,7 +80,7 @@ func GetTSACerts(ctx context.Context, certChainPath string, fn GetTargetStub) (* | |||
|
|||
var raw []byte | |||
var err error | |||
|
|||
var isExist bool |
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.
nit, i'd just call this exists
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.
+1
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.
@haydentherapper Thanks for the comment. Done
@Meeki1l as @haydentherapper mentioned above, thanks for catching this / much appreciated. 🙇🏼 |
Signed-off-by: Meeki1l <meekill2001@gmail.com>
PTAL at the failing test. You can look at https://github.com/sigstore/sigstore/blob/main/pkg/tuf/client_test.go for some examples, or stub out calls to TUF. |
Signed-off-by: Meeki1l <meekill2001@gmail.com>
@haydentherapper disable the falling autotests. I could not find a public TUF mirror with TSA certificates, so it is not possible to write normal autotests. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3744 +/- ##
==========================================
- Coverage 40.10% 36.55% -3.55%
==========================================
Files 155 200 +45
Lines 10044 12232 +2188
==========================================
+ Hits 4028 4472 +444
- Misses 5530 7214 +1684
- Partials 486 546 +60 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Meeki1l <meekill2001@gmail.com>
Summary
Fixes #3743.
When retrieving TSA Certs from the local TUF, an infinite loop occurs, since the GetTargetsByMeta function (used in the GetTufTargets function) returns all certificates of the TSA type.
Also if "tsa_leaf.crt.pem" is missing, panic occurs. This is due to the lack of checking for len(leaves) > 0 in the GetTSACerts function.
Release Note
Documentation