-
Notifications
You must be signed in to change notification settings - Fork 76
Initial Sigstore bundle support #465
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
Changes from all commits
3d9dc54
a0377a4
63d2d87
828c047
f9f711c
56f0636
8651f95
cd7d31c
a25e546
93698cd
ae7cc83
be7bf1b
c2559da
d00d7a8
d045d21
24c2472
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -159,7 +159,7 @@ def _add_shared_instance_options(group: argparse._ArgumentGroup) -> None: | |
| ) | ||
|
|
||
|
|
||
| def _add_shared_input_options(group: argparse._ArgumentGroup) -> None: | ||
| def _add_shared_verify_input_options(group: argparse._ArgumentGroup) -> None: | ||
| """ | ||
| Common input options, shared between all `sigstore verify` subcommands. | ||
| """ | ||
|
|
@@ -185,6 +185,16 @@ def _add_shared_input_options(group: argparse._ArgumentGroup) -> None: | |
| default=os.getenv("SIGSTORE_REKOR_BUNDLE"), | ||
| help="The offline Rekor bundle to verify with; not used with multiple inputs", | ||
| ) | ||
| group.add_argument( | ||
| "--bundle", | ||
| metavar="FILE", | ||
| type=Path, | ||
| default=os.getenv("SIGSTORE_BUNDLE"), | ||
| help=( | ||
| "The Sigstore bundle to verify with; not used with multiple inputs; this option is " | ||
| "experimental and may change between releases until stabilized" | ||
| ), | ||
| ) | ||
| group.add_argument( | ||
| "files", | ||
| metavar="FILE", | ||
|
|
@@ -340,6 +350,25 @@ def _parser() -> argparse.ArgumentParser: | |
| "multiple input files" | ||
| ), | ||
| ) | ||
| output_options.add_argument( | ||
| "--bundle", | ||
| metavar="FILE", | ||
| type=Path, | ||
| default=os.getenv("SIGSTORE_BUNDLE"), | ||
| help=( | ||
| "Write a single Sigstore bundle to the given file; does not work with multiple input " | ||
| "files; this option is experimental and may change between releases until stabilized" | ||
| ), | ||
| ) | ||
| output_options.add_argument( | ||
| "--no-bundle", | ||
| action="store_true", | ||
| default=False, | ||
| help=( | ||
| "Don't emit {input}.sigstore files for each input; this option is experimental " | ||
| "and may change between releases until stabilized" | ||
| ), | ||
| ) | ||
| output_options.add_argument( | ||
| "--overwrite", | ||
| action="store_true", | ||
|
|
@@ -387,7 +416,7 @@ def _parser() -> argparse.ArgumentParser: | |
| formatter_class=argparse.ArgumentDefaultsHelpFormatter, | ||
| ) | ||
| input_options = verify_identity.add_argument_group("Verification inputs") | ||
| _add_shared_input_options(input_options) | ||
| _add_shared_verify_input_options(input_options) | ||
|
|
||
| verification_options = verify_identity.add_argument_group("Verification options") | ||
| _add_shared_verification_options(verification_options) | ||
|
|
@@ -420,7 +449,7 @@ def _parser() -> argparse.ArgumentParser: | |
| ) | ||
|
|
||
| input_options = verify_github.add_argument_group("Verification inputs") | ||
| _add_shared_input_options(input_options) | ||
| _add_shared_verify_input_options(input_options) | ||
|
|
||
| verification_options = verify_github.add_argument_group("Verification options") | ||
| _add_shared_verification_options(verification_options) | ||
|
|
@@ -556,16 +585,37 @@ def _sign(args: argparse.Namespace) -> None: | |
| "upcoming release of sigstore-python in favor of Sigstore-style bundles" | ||
| ) | ||
|
|
||
| # `--no-default-files` has no effect on `--{signature,certificate,rekor-bundle}`, but we | ||
| # forbid it because it indicates user confusion. | ||
| if args.bundle: | ||
| logger.warning( | ||
| "--bundle support is experimental; the behaviour of this flag may change " | ||
| "between releases until stabilized." | ||
| ) | ||
|
|
||
| if args.no_bundle: | ||
| logger.warning( | ||
| "--no-bundle support is experimental; the behaviour of this flag may change " | ||
| "between releases until stabilized." | ||
| ) | ||
|
|
||
| # `--no-default-files` has no effect on `--{signature,certificate,rekor-bundle,bundle}`, | ||
| # but we forbid it because it indicates user confusion. | ||
| if args.no_default_files and ( | ||
| args.signature or args.certificate or args.rekor_bundle | ||
| args.signature or args.certificate or args.rekor_bundle or args.bundle | ||
| ): | ||
| args._parser.error( | ||
| "--no-default-files may not be combined with --signature, " | ||
| "--certificate, or --rekor-bundle", | ||
| "--certificate, --rekor-bundle, or --bundle", | ||
| ) | ||
|
|
||
| # Similarly forbid `--rekor-bundle` with `--bundle`, since it again indicates | ||
| # user confusion around outputs. | ||
| if args.rekor_bundle and args.bundle: | ||
| args._parser.error("--rekor-bundle may not be combined with --bundle") | ||
|
|
||
| # Fail if `--bundle` and `--no-bundle` are both specified. | ||
| if args.bundle and args.no_bundle: | ||
| args._parser.error("--bundle may not be combined with --no-bundle") | ||
|
|
||
| # Fail if `--signature` or `--certificate` is specified *and* we have more | ||
| # than one input. | ||
| if (args.signature or args.certificate or args.rekor_bundle) and len( | ||
|
|
@@ -583,18 +633,33 @@ def _sign(args: argparse.Namespace) -> None: | |
| if not file.is_file(): | ||
| args._parser.error(f"Input must be a file: {file}") | ||
|
|
||
| sig, cert, bundle = args.signature, args.certificate, args.rekor_bundle | ||
| if not sig and not cert and not bundle and not args.no_default_files: | ||
| sig, cert, rekor_bundle, bundle = ( | ||
| args.signature, | ||
| args.certificate, | ||
| args.rekor_bundle, | ||
| args.bundle, | ||
| ) | ||
| if ( | ||
| not sig | ||
| and not cert | ||
| and not rekor_bundle | ||
| and not bundle | ||
| and not args.no_default_files | ||
| ): | ||
| sig = file.parent / f"{file.name}.sig" | ||
| cert = file.parent / f"{file.name}.crt" | ||
| bundle = file.parent / f"{file.name}.rekor" | ||
| rekor_bundle = file.parent / f"{file.name}.rekor" | ||
| if not args.no_bundle: | ||
| bundle = file.parent / f"{file.name}.sigstore" | ||
|
|
||
| if not args.overwrite: | ||
| extants = [] | ||
| if sig and sig.exists(): | ||
| extants.append(str(sig)) | ||
| if cert and cert.exists(): | ||
| extants.append(str(cert)) | ||
| if rekor_bundle and rekor_bundle.exists(): | ||
| extants.append(str(rekor_bundle)) | ||
| if bundle and bundle.exists(): | ||
| extants.append(str(bundle)) | ||
|
|
||
|
|
@@ -604,7 +669,12 @@ def _sign(args: argparse.Namespace) -> None: | |
| f"{', '.join(extants)}" | ||
| ) | ||
|
|
||
| output_map[file] = {"cert": cert, "sig": sig, "bundle": bundle} | ||
| output_map[file] = { | ||
| "cert": cert, | ||
| "sig": sig, | ||
| "rekor_bundle": rekor_bundle, | ||
| "bundle": bundle, | ||
| } | ||
|
|
||
| # Select the signer to use. | ||
| if args.staging: | ||
|
|
@@ -655,7 +725,7 @@ def _sign(args: argparse.Namespace) -> None: | |
| print(f"Transparency log entry created at index: {result.log_entry.log_index}") | ||
|
|
||
| sig_output: TextIO | ||
| if outputs["sig"]: | ||
| if outputs["sig"] is not None: | ||
| sig_output = outputs["sig"].open("w") | ||
| else: | ||
| sig_output = sys.stdout | ||
|
|
@@ -669,11 +739,16 @@ def _sign(args: argparse.Namespace) -> None: | |
| print(result.cert_pem, file=io) | ||
| print(f"Certificate written to {outputs['cert']}") | ||
|
|
||
| if outputs["rekor_bundle"] is not None: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed these back from: if "rekor_bundle" in outputs:This is because when we don't write to an output, the entry still gets added to the map, the file is just set to
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense! I've re-fixed #465 (comment) based on that. |
||
| with outputs["rekor_bundle"].open(mode="w") as io: | ||
| rekor_bundle = RekorBundle.from_entry(result.log_entry) | ||
| print(rekor_bundle.json(by_alias=True), file=io) | ||
| print(f"Rekor bundle written to {outputs['rekor_bundle']}") | ||
|
|
||
| if outputs["bundle"] is not None: | ||
| with outputs["bundle"].open(mode="w") as io: | ||
| bundle = RekorBundle.from_entry(result.log_entry) | ||
| print(bundle.json(by_alias=True), file=io) | ||
| print(f"Rekor bundle written to {outputs['bundle']}") | ||
| print(result._to_bundle().to_json(), file=io) | ||
| print(f"Sigstore bundle written to {outputs['bundle']}") | ||
|
|
||
|
|
||
| def _collect_verification_state( | ||
|
|
@@ -687,6 +762,10 @@ def _collect_verification_state( | |
| purposes) and `materials` is the `VerificationMaterials` to verify with. | ||
| """ | ||
|
|
||
| # TODO: Allow --bundle during verification. Until then, error. | ||
| if args.bundle: | ||
| args._parser.error("--bundle is not supported during verification yet") | ||
|
|
||
| # `--rekor-bundle` is a temporary option, pending stabilization of the | ||
| # Sigstore bundle format. | ||
| if args.rekor_bundle: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,24 @@ | |
| from cryptography.hazmat.primitives.asymmetric.utils import Prehashed | ||
| from cryptography.x509.oid import NameOID | ||
| from pydantic import BaseModel | ||
| from sigstore_protobuf_specs.dev.sigstore.bundle.v1 import ( | ||
| Bundle, | ||
| VerificationMaterial, | ||
| ) | ||
| from sigstore_protobuf_specs.dev.sigstore.common.v1 import ( | ||
| HashAlgorithm, | ||
| HashOutput, | ||
| LogId, | ||
| MessageSignature, | ||
| X509Certificate, | ||
| X509CertificateChain, | ||
| ) | ||
| from sigstore_protobuf_specs.dev.sigstore.rekor.v1 import ( | ||
| InclusionPromise, | ||
| InclusionProof, | ||
| KindVersion, | ||
| TransparencyLogEntry, | ||
| ) | ||
|
|
||
| from sigstore._internal.fulcio import FulcioClient | ||
| from sigstore._internal.oidc import Identity | ||
|
|
@@ -163,6 +181,7 @@ def sign( | |
| logger.debug(f"Transparency log entry created with index: {entry.log_index}") | ||
|
|
||
| return SigningResult( | ||
| input_digest=input_digest.hex(), | ||
| cert_pem=cert.public_bytes(encoding=serialization.Encoding.PEM).decode(), | ||
| b64_signature=b64_artifact_signature, | ||
| log_entry=entry, | ||
|
|
@@ -174,6 +193,11 @@ class SigningResult(BaseModel): | |
| Represents the artifacts of a signing operation. | ||
| """ | ||
|
|
||
| input_digest: str | ||
| """ | ||
| The hex-encoded SHA256 digest of the input that was signed for. | ||
| """ | ||
|
|
||
| cert_pem: str | ||
| """ | ||
| The PEM-encoded public half of the certificate used for signing. | ||
|
|
@@ -188,3 +212,59 @@ class SigningResult(BaseModel): | |
| """ | ||
| A record of the Rekor log entry for the signing operation. | ||
| """ | ||
|
|
||
| def _to_bundle(self) -> Bundle: | ||
di marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| """ | ||
| Creates a Sigstore bundle (as defined by Sigstore's protobuf specs) | ||
| from this `SigningResult`. | ||
| """ | ||
|
|
||
| # TODO: Include the current Fulcio intermediate and root in the | ||
| # chain as well. | ||
| cert = x509.load_pem_x509_certificate(self.cert_pem.encode()) | ||
| cert_der = cert.public_bytes(encoding=serialization.Encoding.DER) | ||
| chain = X509CertificateChain(certificates=[X509Certificate(raw_bytes=cert_der)]) | ||
|
|
||
| inclusion_proof: InclusionProof | None = None | ||
| if self.log_entry.inclusion_proof is not None: | ||
| inclusion_proof = InclusionProof( | ||
| log_index=self.log_entry.inclusion_proof.log_index, | ||
| root_hash=bytes.fromhex(self.log_entry.inclusion_proof.root_hash), | ||
| tree_size=self.log_entry.inclusion_proof.tree_size, | ||
| hashes=[ | ||
| bytes.fromhex(h) for h in self.log_entry.inclusion_proof.hashes | ||
| ], | ||
| ) | ||
|
|
||
| tlog_entry = TransparencyLogEntry( | ||
| log_index=self.log_entry.log_index, | ||
| log_id=LogId(key_id=bytes.fromhex(self.log_entry.log_id)), | ||
| kind_version=KindVersion(kind="hashedrekord", version="0.0.1"), | ||
| integrated_time=self.log_entry.integrated_time, | ||
| inclusion_promise=InclusionPromise( | ||
| signed_entry_timestamp=base64.b64decode( | ||
| self.log_entry.signed_entry_timestamp | ||
| ) | ||
| ), | ||
| inclusion_proof=inclusion_proof, | ||
| canonicalized_body=base64.b64decode(self.log_entry.body), | ||
| ) | ||
|
|
||
| material = VerificationMaterial( | ||
| x509_certificate_chain=chain, | ||
| tlog_entries=[tlog_entry], | ||
| ) | ||
|
|
||
| bundle = Bundle( | ||
| media_type="application/vnd.dev.sigstore.bundle+json;version=0.1", | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd be nice if there was some way to default to this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIUC no :( That said, I think that's sort-of by design? You should only set that
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aha, I missed that detail! That makes sense, then, and it's not a huge deal here.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it would be nice to have const values in the language bindings that would capture the current media type used.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 100% agreed -- I'm not sure how best to accomplish that without hacky patches to the codegen, though...
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, would be messy. I would love for Protobufs to support that natively, so many use-cases I have seen where it would make our lives easier. |
||
| verification_material=material, | ||
| message_signature=MessageSignature( | ||
| message_digest=HashOutput( | ||
| algorithm=HashAlgorithm.SHA2_256, | ||
| digest=bytes.fromhex(self.input_digest), | ||
| ), | ||
| signature=base64.b64decode(self.b64_signature), | ||
| ), | ||
| ) | ||
|
|
||
| return bundle | ||
Uh oh!
There was an error while loading. Please reload this page.