Skip to content

Conversation

@cmurphy
Copy link
Contributor

@cmurphy cmurphy commented Oct 22, 2025

Fixes #4466

  • Tests

Summary

Release Note

Documentation

@codecov
Copy link

codecov bot commented Oct 22, 2025

Codecov Report

❌ Patch coverage is 45.45455% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 36.72%. Comparing base (2ef6022) to head (8a0638e).
⚠️ Report is 572 commits behind head on main.

Files with missing lines Patch % Lines
cmd/cosign/cli/verify/verify.go 0.00% 4 Missing ⚠️
cmd/cosign/cli/verify/verify_attestation.go 0.00% 4 Missing ⚠️
cmd/cosign/cli/attest/attest.go 0.00% 2 Missing ⚠️
cmd/cosign/cli/sign/sign.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4489      +/-   ##
==========================================
- Coverage   40.10%   36.72%   -3.38%     
==========================================
  Files         155      220      +65     
  Lines       10044    12125    +2081     
==========================================
+ Hits         4028     4453     +425     
- Misses       5530     6984    +1454     
- Partials      486      688     +202     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cmurphy cmurphy force-pushed the insecure-referrers branch from 4b44e41 to 58c3a2f Compare October 23, 2025 00:56
@cmurphy cmurphy marked this pull request as ready for review October 23, 2025 18:03
@cmurphy cmurphy requested review from a team as code owners October 23, 2025 18:03
@steiza
Copy link
Member

steiza commented Oct 23, 2025

I'm not sure if this is working right, or if I'm doing something wrong while testing this.

I'm working off the instructions on https://github.com/tstromberg/sigstore-the-local-way?tab=readme-ov-file#11-starting-a-local-registry.

I think cosign does something special with localhost, because even on main this works:

$ go run cmd/cosign/main.go verify --key cosign.pub --allow-insecure-registry=true --allow-http-registry=true localhost:1338/demo/sigstore-go-verification-ce4f5bf3233ac2b951c3667b2d19da3a:latest 

Verification for localhost:1338/demo/sigstore-go-verification-ce4f5bf3233ac2b951c3667b2d19da3a:latest --
...

But if I set up a DNS alias in /etc/hosts:

$ cat /etc/hosts | grep 'asdf.example.com'
127.0.0.1 asdf.example.com
$ ping asdf.example.com
PING asdf.example.com (127.0.0.1): 56 data bytes

... then verification fails, both on main and with these changes:

$ go run cmd/cosign/main.go verify --key cosign.pub --allow-insecure-registry=true --allow-http-registry=true asdf.example.com:1338/demo/sigstore-go-verification-ce4f5bf3233ac2b951c3667b2d19da3a:latest 
Error: no signatures found

Again, I might be doing something wrong here!

@cmurphy
Copy link
Contributor Author

cmurphy commented Oct 23, 2025

I think cosign does something special with localhost, because even on main this works:

Yep, cosign treats localhost and *.local as special because of google/go-containerregistry#125

then verification fails, both on main and with these changes:

I think no signatures found could be a bundle/no bundle problem, was that image signed with an old version of cosign? Does it work when you add --new-bundle-format=false?

@steiza
Copy link
Member

steiza commented Oct 23, 2025

I think no signatures found could be a bundle/no bundle problem, was that image signed with an old version of cosign? Does it work when you add --new-bundle-format=false?

I don't think this is it, because I'm running off an up-to-date main and your branch. The only thing I'm changing between the verification commands is if the container starts with localhost or asdf.example.com (which resolves to 127.0.0.1).

But just to be sure, I checked again and got the same behavior.

Also updates the registry tests to use TUF so that they can be re-used
for both the legacy format and protobuf bundle format.

Signed-off-by: Colleen Murphy <[email protected]>
@cmurphy cmurphy force-pushed the insecure-referrers branch from 58c3a2f to 8a0638e Compare October 23, 2025 23:04
@cmurphy
Copy link
Contributor Author

cmurphy commented Oct 23, 2025

@steiza you were right, and I fixed the tests to catch that issue.

Copy link
Member

@steiza steiza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor comment, but I'm also in favor of landing this as-is, since it is definitely an improvement!

return err
}
if c.RegistryOptions.AllowHTTPRegistry || c.RegistryOptions.AllowInsecure {
ociremoteOpts = append(ociremoteOpts, ociremote.WithNameOptions(name.Insecure))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: so our RegistryOptions.AllowHTTPRegistry maps to name.Insecure which both mean "it's okay if the registry uses plain HTTP without TLS.

But my interpretation of RegistryOptions.AllowInsecure is that the registry is using TLS, but with a self-signed certificate (or at least a certificate that doesn't verify with the machine's certificate store).

🤷 Are there people actually using that configuration? Or is everyone just using an HTTP registry? I guess we'll find out! I didn't see a "use TLS but allow certificates that don't verify with the machine's certificate store" on https://pkg.go.dev/github.com/google/[email protected]/pkg/name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it didn't seem like go-containerregistry makes a distinction between them.

@cmurphy cmurphy merged commit d470294 into sigstore:main Oct 27, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Image sign with flag --allow-http-registry=true fails

2 participants