Skip to content

Sign Bundle with a Timestamp Authority#1216

Merged
woodruffw merged 26 commits intosigstore:mainfrom
trail-of-forks:dm/timestamp-signing
Nov 25, 2024
Merged

Sign Bundle with a Timestamp Authority#1216
woodruffw merged 26 commits intosigstore:mainfrom
trail-of-forks:dm/timestamp-signing

Conversation

@DarkaMaul
Copy link
Copy Markdown
Collaborator

@DarkaMaul DarkaMaul commented Nov 15, 2024

Final bit of #1182

Summary

This PR introduces the possibility to create a bundle with a timestamp signed by a TimestampAuthority.

Release Note

Added

  • If a Timestamp Authority URL has been provided in the SigningConfig, the bundle are now automatically generated with a signed timestamp.

/cc @woodruffw @facutuesca

DarkaMaul and others added 16 commits November 7, 2024 09:32
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
Co-authored-by: Facundo Tuesca <facu@tuesca.com>
Signed-off-by: dm <darkamaul@hotmail.fr>
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
# Conflicts:
#	CHANGELOG.md
#	sigstore/dsse/__init__.py
#	sigstore/verify/verifier.py
#	test/assets/tsa/bundle.txt.sigstore
#	test/unit/verify/test_verifier.py
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
DarkaMaul and others added 4 commits November 18, 2024 15:27
Co-authored-by: William Woodruff <william@yossarian.net>
Signed-off-by: dm <darkamaul@hotmail.fr>
Co-authored-by: William Woodruff <william@yossarian.net>
Signed-off-by: dm <darkamaul@hotmail.fr>
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
@jku
Copy link
Copy Markdown
Member

jku commented Nov 18, 2024

/gcbrun

@woodruffw
Copy link
Copy Markdown
Member

This looks like something bad on the Windows CI:

    Error configuring OpenSSL build:
        'perl' reported failure with exit code: 2
        Command failed: "perl" "./Configure" "--prefix=C:/Users/runneradmin/AppData/Local/Temp/pip-install-ldkkdr4i/rfc3161-client_a9d25b6f62a14f8aac4a34a83ab37a09/rust/target/release/build/openssl-sys-b380da04be89fe31/out/openssl-build/install" "--openssldir=SYS$MANAGER:[OPENSSL]" "no-shared" "no-ssl3" "no-tests" "no-comp" "no-zlib" "no-zlib-dynamic" "--libdir=lib" "no-md2" "no-rc5" "no-weak-ssl-ciphers" "no-camellia" "no-idea" "no-seed" "no-capieng" "no-asm" "VC-WIN64A"
  

@jku
Copy link
Copy Markdown
Member

jku commented Nov 18, 2024

This looks like something bad on the Windows CI

I think this might require some of the same build config tricks that rfc3161-client CI has... alternatively maybe wait for a rfc3161-client release and avoid installing from git

@woodruffw
Copy link
Copy Markdown
Member

This looks like something bad on the Windows CI

I think this might require some of the same build config tricks that rfc3161-client CI has... alternatively maybe wait for a rfc3161-client release and avoid installing from git

Yeah, I think we can prep another release. CC @DarkaMaul

Signed-off-by: Alexis <alexis.challande@trailofbits.com>
Signed-off-by: Alexis <alexis.challande@trailofbits.com>
@jku
Copy link
Copy Markdown
Member

jku commented Nov 20, 2024

/gcbrun

@woodruffw
Copy link
Copy Markdown
Member

/gcbrun

make test TEST_ARGS="-m timestamp_authority -rs" | tee output
! grep -q "skipping test that requires a Timestamp Authority" output || (echo "ERROR: Found skip message" && exit 1)
env:
SIGSTORE_TIMESTAMP: "v1.2.3"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't love that we pull down a binary here, and hardcode the version. It'd be great if we could get this into Dependabot somehow.

(Not a blocker, flagging as a follow-up.)

fulcio: FulcioClient,
rekor: RekorClient,
trusted_root: TrustedRoot,
tsa_clients: List[TimestampAuthorityClient] | None = None,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Flagging: it's not ideal that this parameter list continues to grow; I think we could probably slice it down substantially by passing the entire ClientTrustConfig and doing the instantiations internally.

That'd be good for a follow-on refactor PR here.

Copy link
Copy Markdown
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

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

LGTM, nice work @DarkaMaul!

I left some non-blocking comments; the one about refactoring SigningContext's ctor in particular would be good for a follow-up PR.

@woodruffw woodruffw added component:signing Core signing functionality component:api Public APIs labels Nov 25, 2024
@woodruffw
Copy link
Copy Markdown
Member

This also needs a CHANGELOG entry, but I'm going to merge as-is and do a follow-up for that.

@woodruffw woodruffw enabled auto-merge (squash) November 25, 2024 17:09
@woodruffw
Copy link
Copy Markdown
Member

/gcbrun

@woodruffw woodruffw merged commit 383797a into sigstore:main Nov 25, 2024
@woodruffw woodruffw deleted the dm/timestamp-signing branch November 25, 2024 17:14
woodruffw added a commit that referenced this pull request Nov 25, 2024
Signed-off-by: William Woodruff <william@trailofbits.com>
woodruffw added a commit that referenced this pull request Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component:api Public APIs component:signing Core signing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants