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

Bundle signing using TSA #5

Open
wants to merge 14 commits into
base: dm/tsp
Choose a base branch
from
Open

Bundle signing using TSA #5

wants to merge 14 commits into from

Conversation

DarkaMaul
Copy link

For the internal review round

Based on the verification branch to ease the review.

@@ -58,6 +58,18 @@ jobs:
- name: test
run: make test TEST_ARGS="-vv --showlocals"

- name: test (timestamp-authority)
Copy link
Author

Choose a reason for hiding this comment

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

FLAG: This tests the new Timestamping Functionalities by downloading sigstore/timestamp-authority.

This could either use an external (and reliable) TSA or be more aggressively cached.

pass


class TimestampAuthorityClient:
Copy link
Author

Choose a reason for hiding this comment

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

FLAG: I'm not fond of the name here, but I couldn't come up with a better one.

msg = f"Invalid network: {error}"
raise TimestampError(msg)

# Check that we can parse the response but does not *verify* it
Copy link
Author

Choose a reason for hiding this comment

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

NOTE: We can't verify here because we may not have access to the TSA certificates.

@DarkaMaul
Copy link
Author

The tests are expected to fail because it does not use (yet) the unreleased version of rfc3161-client with the as_bytes method.

Copy link

@facutuesca facutuesca left a comment

Choose a reason for hiding this comment

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

great work! I left a few minor comments

sigstore/_internal/timestamping.py Outdated Show resolved Hide resolved
sigstore/_internal/timestamping.py Outdated Show resolved Hide resolved
sigstore/_internal/timestamping.py Outdated Show resolved Hide resolved
sigstore/_internal/timestamping.py Outdated Show resolved Hide resolved
sigstore/_internal/timestamping.py Outdated Show resolved Hide resolved
sigstore/_internal/timestamping.py Outdated Show resolved Hide resolved
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.

2 participants