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.

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

Difficulties with pypi_attestations verify #55

Open
AA-Turner opened this issue Oct 6, 2024 · 6 comments
Open

Difficulties with pypi_attestations verify #55

AA-Turner opened this issue Oct 6, 2024 · 6 comments
Assignees

Comments

@AA-Turner
Copy link

Hi,

I'm looking to use PEP 740 attestations in Sphinx, using the pypi_attestations CLI. The signing and inspection steps seem to work, but I've had some trouble with verify. Hopefully there's an opportunity to improve documenation or guidance:

  1. Running verify dist/*.publish.attestation fails with an unhelpful error. This was my first approach, as I wanted to run sign-inspect-verify consecutively in GHA, and the inspect command runs on .publish.attestation files. Similarly, running verify dist/* fails with the same error -- perhaps the tool could check if the extension is already .publish.attestation, and if so skip that file?

    $ python -m pypi_attestations verify dist/*.publish.attestation --identity ...
    dist/sphinx-8.1.0-py3-none-any.whl.publish.attestation.publish.attestation is not a file.
    $ python -m pypi_attestations verify dist/* --identity ...
    dist/sphinx-8.1.0-py3-none-any.whl.publish.attestation.publish.attestation is not a file.
  2. pypi_attestations inspect doesn't tell me what value I should be using with --identity (I don't know if this is possible, though). The example on README uses an email, so I tried the email I use for commits, which failed.

  3. The failure for an incorrect --identity key is obscure and confusing ("Certificate's SANs do not match"):

    $ python -m pypi_attestations verify dist/*.whl --identity [email protected]
    Verification failed for dist/sphinx-8.1.0-py3-none-any.whl: Verification failed: Certificate's SANs do not match [email protected]; actual SANs: {'https://github.com/AA-Turner/sphinx/.github/workflows/create-release.yml@refs/heads/attestations'}
    Verification failed for dist/sphinx-8.1.0.tar.gz: Verification failed: Certificate's SANs do not match [email protected]; actual SANs: {'https://github.com/AA-Turner/sphinx/.github/workflows/create-release.yml@refs/heads/attestations'}

    At the least expanding the initialism so that I can look up what a SAN is would be helpful. The key for "actual SANs" is a hyperlink, which doesn't look like the example of the verify command on the README.

  4. Failures to verify in CI don't exit with a non-zero status code, so GHA reports all is well -- this seems wrong.

  5. Using the "actual SAN" hyperlink as the value for --identity works, but it is unclear how to auto-generate this for verification purposes. Is it always <address of fork>/<path to workflow>@refs/heads/<branch name>? Is there a difference if annotated or lightweight git tags are used? I can find many of these things by trial-and-error, but the documentation of pypi_attestations verify in README doesn't mention it.

Sorry for the laundry list of problems, but I thought it would be helpful to list my experience here as attestations are still fairly new. If I've missed a very obvious source of documentation please forgive me -- I tried looking in e.g. https://docs.pypi.org/attestations/publish/v1 as listed in pypi_attestations inspect, but every part of that path is a 404.

Thanks,
Adam

P.S. For context, the Sphinx PR that lead to this is at sphinx-doc/sphinx#12981

@woodruffw
Copy link
Member

woodruffw commented Oct 7, 2024

Hi @AA-Turner, no problem at all! I appreciate the detailed issue.

I'll respond to each of your items below, but as a high-level point: the pypi_attestations CLI was built primarily for early experimentation, and we didn't expect anybody to use it directly for CI integrations 😅. Instead, we encourage most people to use the attestations functionality we've built directly into pypa/gh-action-pypi-publish, or to use actions/attest and convert its format (a Sigstore bundle) into a PyPI attestation via this package's Python APIs (not CLI).

With that being said, I agree that all of these are defects.

  1. perhaps the tool could check if the extension is already .publish.attestation, and if so skip that file?

The reason we do this is because we need both files to perform verification (both the dist and the attestation), and so we infer the attestation's name from the file it's attesting for. With that said we could definitely improve the error message here, and emphasize that people should pass in verify dist/*.{whl,tar.gz} or similar.

(We should also make it so that verify dist/* itself just works.)

Update: Done!

2. pypi_attestations inspect doesn't tell me what value I should be using with --identity (I don't know if this is possible, though). The example on README uses an email, so I tried the email I use for commits, which failed.

Yep, that's possible -- the identity to use is embedded as the certificate's SAN (Subject Alternative Name), so we can present that if it isn't already.

Update: Confirmed that this is present already, but will look into improving the presentation.

3. At the least expanding the initialism so that I can look up what a SAN is would be helpful. The key for "actual SANs" is a hyperlink, which doesn't look like the example of the verify command on the README

Agreed, we'll expand it. The reason it's a URL instead of an email like in the README is because it's produced by GitHub Actions, meaning the signing identity is the workflow identity that produced the attestation rather than the user identity that owns the repo. This is intended behavior, and it's what PyPI itself currently expects when uploading with attestations (attestations must currently match the Trusted Publisher workflow identity that's configured on PyPI).

  • Failures to verify in CI don't exit with a non-zero status code, so GHA reports all is well -- this seems wrong.

Yep, that's a bug.

(This goes back to the point about this CLI not being an intended public interface, but it's a bug regardless.)

Update: Fixed on main.

5. Using the "actual SAN" hyperlink as the value for --identity works, but it is unclear how to auto-generate this for verification purposes. Is it always <address of fork>/<path to workflow>@refs/heads/<branch name>?

The identity/SAN will always be the "job workflow ref" for the workflow identity, which will always be https://github.com/<owner>/<repo>/.gjithub/workflows/<workflow>@<ref>, where ref can be either a symbolic ref (branch/tag) or a concrete commit ref. This corresponds with the Trusted Publisher configuration on PyPI, so if you register a Trusted Publisher like this:

Owner: foo
Repo: bar
Workflow: publish.yml

then your attestation identity will look like:

https://github.com/foo/bar/.github/workflows/publish.yml@<ref>

...where ref varies depending on how publish.yml was invoked.

This variance is one of the reasons the pypi_attestations CLI is mostly just intended for experimentation -- the way we handle it during verification on PyPI itself is by using the APIs, which can take a VerificationPolicy that matches individual identity components (e.g. owner/repo) without having to futz around with strings.

If I've missed a very obvious source of documentation please forgive me -- I tried looking in e.g. https://docs.pypi.org/attestations/publish/v1 as listed in pypi_attestations inspect, but every part of that path is a 404.

It's understandable that you missed it, since it's still in a PR! You can find the documentation for much of this in pypi/warehouse#16398; please let me know if you have any questions about it.


As a bottom line: if possible, it'd be ideal for the Sphinx CI/CD to use gh-action-pypi-publish, since it intentionally encapsulates all of these details and represents the intended public interface for the signing/attesting side. If using that action isn't desirable for whatever reason, my follow-on recommendation would be to use this package's Python APIs directly, since those are intended to be the public interface that people rely on.

(The README doesn't say that, which is our fault. I'll also update it to nudge people towards the APIs, with a note that the CLI is principally for experimentation only.)

@woodruffw
Copy link
Member

Yep, that's possible -- the identity to use is embedded as the certificate's SAN (Subject Alternative Name), so we can present that if it isn't already.

I just checked, and it looks like we do indeed print the identity. For example:

$ python -m pypi_attestations inspect test/assets/rfc8785-0.1.2-py3-none-any.whl.publish.attestation 
Warning: The information displayed below are not verified, they are only displayed. Use the verify command to verify them.
File: test/assets/rfc8785-0.1.2-py3-none-any.whl.publish.attestation
Version: 1
Statement:
        Type: https://in-toto.io/Statement/v1
        Subject:
                rfc8785-0.1.2-py3-none-any.whl (digest: c4e92e9ecc828bef2aa7dba1de8ac983511f7532a0df11c770d39099a25cf201)
        Predicate type: https://docs.pypi.org/attestations/publish/v1
        Predicate: None
Certificate:
        Subjects: ['[email protected]']
        Issuer: CN=sigstore-intermediate,O=sigstore.dev
        Validity: 2024-06-06 18:49:05+00:00
Transparency Log (1 entries):
        Log Index: 28175749

The list under Subject: is the set of identities bound to the attestation. We could make that clearer by removing the confusing language around SANs vs. subjects vs. identities, though.

@AA-Turner
Copy link
Author

Thanks Will,

I've switched to the actions/attest approach in the Sphinx PR as that records commit metadata in the attestation (at least in the one uploaded to GitHub), which was requested by a contributor.

I appreciate that you want to keep the CLI as experimental/non-stable, but it is awfully useful. Hyrum's law and all that, I suppose. Nevertheless, the only part of the CLI we now use is inspect, as it's non critical and we don't do anything with the output, so we don't care if it changes.

If your position changes and you become open to making use of the command-line interface more official, it would be very welcome -- I try to keep my use of non-first-party (GitHub) actions to a minimum, even though I do trust the developers of the PyPA action.

One thing I did discover is that the "Transparency Log" entries actually mean something and go somewhere -- I thought originally it was just some internal logging. The GitHub action outputs "Attestation signature uploaded to Rekor transparency log: https://search.sigstore.dev/?logIndex=137921341", which might be a useful thing to point people towards both in documentation and in the tool's output.

Thank you for suffering my ignorance, and for being so prompt in updating things.

Thanks,
Adam

@woodruffw
Copy link
Member

If your position changes and you become open to making use of the command-line interface more official, it would be very welcome -- I try to keep my use of non-first-party (GitHub) actions to a minimum, even though I do trust the developers of the PyPA action.

Makes sense! We'll keep this in mind as we continue to stabilize the package here -- one possibility is that we collapse the CLI here into the sigstore CLI, since the latter is already mature/stable and intended as an integration point.

Thank you for suffering my ignorance, and for being so prompt in updating things.

Thank you for your detailed issue and responses! I greatly appreciate that you've gone and dove straight into the internals of PEP 740.

@AA-Turner
Copy link
Author

As an update, Sphinx 8.1 has been released with PEP 740 attestations.

I tried to validate using the materials from PyPI, but failed. I appreciate that this may be too early, though (I got the provenance URL from the docs PR you linked earlier):

PS> curl https://pypi.io/packages/py3/s/sphinx/sphinx-8.1.0-py3-none-any.whl --remote-name --silent
PS> curl -H"Accept: application/vnd.pypi.integrity.v1+json" https://pypi.org/integrity/Sphinx/8.1.0/sphinx-8.1.0-py3-none-any.whl/provenance -o sphinx-8.1.0-py3-none-any.whl.bundle --silent
PS> sigstore verify github --repository sphinx-doc/sphinx --name create-release.yml --sha 2f1d775dfda9e4f81dfff6cfbe9edf7731e32a97 --bundle sphinx-8.1.0-py3-none-any.whl.bundle sphinx-8.1.0-py3-none-any.whl
[18:08:14] ERROR    An issue occurred while parsing the Sigstore bundle.                                                                                                                                                 errors.py:41
           
                    The provided bundle is malformed and may have been modified maliciously.
           
                    Additional context:
           
                    unsupported bundle format:
           
                    For detailed error information, run sigstore with the `--verbose` flag.

A

xref: GitHub attestations summary

@woodruffw
Copy link
Member

I tried to validate using the materials from PyPI, but failed. I appreciate that this may be too early, though (I got the provenance URL from the docs PR you linked earlier):

You're not too early, but there's unfortunately an extra step that isn't documented yet: that API returns a provenance object (per PEP 740), which contains bundles of (identity, [attestation]). To verify its contents, you need to unpack the attestations, convert them into Sigstore bundles, and verify them against their supplied identities.

The top-level API is Provenance, which you can then pull attestations from. I'm now realizing we don't provide convenience APIs for converting identities into Sigstore verification policies, though, so I'll look at adding those now.

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

No branches or pull requests

2 participants