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

Store attestations for PEP740 #16302

Merged
merged 41 commits into from
Aug 21, 2024
Merged

Conversation

DarkaMaul
Copy link
Contributor

@DarkaMaul DarkaMaul commented Jul 18, 2024

Context

This PR follows #16063 and #15952 and build on them.
After verifying attestations, this PR introduces their storage and retrieval when needed.

Main changes

At upload time

  • After attestations have been processed, they are not discarded but stored in using the storage service.
  • A Provenance file is generated and stored with the release file

At download time

  • When requesting a release using the simple API (JSON/HTML), if attestations exist for the release :
    • a new JSON key provenance with the sha256 of the provenance file is set
    • or, a data attribute data-provenance is set with the same value

Implementations choices

  • Attestations are stored based on their hash in buckets:
attestation_sha256_digest = ...
attestation_path =    "/'.join(
    attestation_sha256_digest[:2],
    attestation_sha256_digest[2:4],
    attestation_sha256_digest[4:],
    f"{release_path}.attestation",
)
  • Provenance files are stored along the release file to comply with PEP740

TODOs

  • Improve and fix the tests
  • documentation on the newly created subpackage

/cc @woodruffw @facutuesca

@woodruffw woodruffw mentioned this pull request Jul 18, 2024
25 tasks
warehouse/forklift/legacy.py Outdated Show resolved Hide resolved
warehouse/forklift/legacy.py Outdated Show resolved Hide resolved
warehouse/attestations/_core.py Outdated Show resolved Hide resolved
tests/common/db/attestation.py Outdated Show resolved Hide resolved
warehouse/packaging/models.py Outdated Show resolved Hide resolved
warehouse/attestations/_core.py Outdated Show resolved Hide resolved
warehouse/forklift/legacy.py Outdated Show resolved Hide resolved
@DarkaMaul
Copy link
Contributor Author

Let put this one on hold until trailofbits/pypi-attestations#36 is merged.

@DarkaMaul
Copy link
Contributor Author

This is looking really good, nice work @DarkaMaul!

To confirm my understanding of the flow here:

  1. A package is published using TP and contains one (currently just one) attestation in the attestations POST field;
  2. We parse and verify that attestation against the TP identity that published the package file
  3. We persist the attestation as its own file and content-address it, referencing it from the new Attestation model
  4. We generate the provenance object from the attestation and store it.

Did I get this right?

Exactly

warehouse/forklift/legacy.py Outdated Show resolved Hide resolved
warehouse/attestations/services.py Outdated Show resolved Hide resolved
warehouse/forklift/legacy.py Outdated Show resolved Hide resolved
warehouse/forklift/legacy.py Outdated Show resolved Hide resolved
warehouse/attestations/services.py Outdated Show resolved Hide resolved
warehouse/attestations/services.py Outdated Show resolved Hide resolved
warehouse/forklift/legacy.py Outdated Show resolved Hide resolved
warehouse/attestations/services.py Outdated Show resolved Hide resolved
requirements/main.in Outdated Show resolved Hide resolved
di and others added 2 commits August 21, 2024 18:36
Signed-off-by: William Woodruff <william@trailofbits.com>
@di di merged commit 5545884 into pypi:main Aug 21, 2024
18 checks passed
@di di deleted the dm/store-attestations branch August 21, 2024 18:48
di added a commit to di/warehouse that referenced this pull request Aug 21, 2024
di added a commit that referenced this pull request Aug 21, 2024
di added a commit that referenced this pull request Aug 21, 2024
* Revert "register IntegrityService correctly (#16543)"

This reverts commit 5a39e80.

* Revert "Store attestations for PEP740 (#16302)"

This reverts commit 5545884.

* Fixup migrations
woodruffw added a commit to trail-of-forks/warehouse that referenced this pull request Aug 21, 2024
@DarkaMaul DarkaMaul restored the dm/store-attestations branch August 22, 2024 13:05
ewdurbin pushed a commit that referenced this pull request Sep 3, 2024
* Reapply "Store attestations for PEP740 (#16302)" (#16545)

This reverts commit da7e1ed.

* migrations: re-roll migration history

Signed-off-by: William Woodruff <william@trailofbits.com>

* config: register .attestations for inclusion

Signed-off-by: William Woodruff <william@trailofbits.com>

* attestations: request the appropriate IFileStorage service

IFileStorage requires a name to disambiguate it.

Signed-off-by: William Woodruff <william@trailofbits.com>

* conftest: add archive_files.path to get_app_config

Signed-off-by: William Woodruff <william@trailofbits.com>

* test, warehouse: remove problematic mocks

This removes two mocked `db_request`s from the simple
index tests. These mocks were masking larger architectural
issues with both attestations and our test scaffolding
for attestations.

This isn't quite complete yet, since it does a nasty thing
(uses a file storage with a tmpdir) to get IntegrityService
initialization working.

Signed-off-by: William Woodruff <william@trailofbits.com>

* test_services: rename test class

Signed-off-by: William Woodruff <william@trailofbits.com>

* Try to clean a bit the mess with the migrations.

* begin refactoring IntegrityService

This reduces the overall API surface for IIntegrityService implementers,
and adds an initial NullIntegrityService to make unit-level testing
simpler.

Signed-off-by: William Woodruff <william@trailofbits.com>

* Revert "Try to clean a bit the mess with the migrations."

This reverts commit e19be6c.

* tests, warehouse: more error tests, remove more stubs

Signed-off-by: William Woodruff <william@trailofbits.com>

* test_services: fix match

Signed-off-by: William Woodruff <william@trailofbits.com>

* remove more implicit file service deps

Signed-off-by: William Woodruff <william@trailofbits.com>

* continue to burn down coverage

Remove more ad-hoc stubs as well.

Signed-off-by: William Woodruff <william@trailofbits.com>

* full coverage

Signed-off-by: William Woodruff <william@trailofbits.com>

* test_simple: positive provenance test for /simple

Signed-off-by: William Woodruff <william@trailofbits.com>

* tests: minimize, increase confidence in behavior

Signed-off-by: William Woodruff <william@trailofbits.com>

* Update warehouse/config.py

Co-authored-by: dm <darkamaul@hotmail.fr>

* packaging/test_utils: remove another mock

Signed-off-by: William Woodruff <william@trailofbits.com>

* Remove even more mocks

* Update tests/conftest.py

Co-authored-by: William Woodruff <william@yossarian.net>

* Update test_create_service

* Add a functional test

* Linting

* Fixup migration

* Fix test error

* Revert change

* Apply suggestions from code review

Rename key

Co-authored-by: Dustin Ingram <di@users.noreply.github.com>

* Revert "Apply suggestions from code review
"

This reverts commit 52931a1.

* Give the IntegrityService access to the session

* Add the Attestation object to the session

* Update the functional test with more assertions

* Remove vestigial helper

* Update functional test

---------

Signed-off-by: William Woodruff <william@trailofbits.com>
Co-authored-by: Alexis <alexis.challande@trailofbits.com>
Co-authored-by: dm <darkamaul@hotmail.fr>
Co-authored-by: Dustin Ingram <di@users.noreply.github.com>
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.

4 participants