Skip to content

_cli: Add --certificate-chain and support --rekor-url for verification #323

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

Merged
merged 11 commits into from
Nov 30, 2022
Merged
28 changes: 19 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ usage: sigstore sign [-h] [--identity-token TOKEN] [--oidc-client-id ID]
[--oidc-disable-ambient-providers] [--oidc-issuer URL]
[--no-default-files] [--signature FILE]
[--certificate FILE] [--rekor-bundle FILE] [--overwrite]
[--staging] [--rekor-url URL] [--fulcio-url URL]
[--ctfe FILE] [--rekor-root-pubkey FILE]
[--staging] [--rekor-url URL] [--rekor-root-pubkey FILE]
[--fulcio-url URL] [--ctfe FILE]
FILE [FILE ...]

positional arguments:
Expand Down Expand Up @@ -136,14 +136,14 @@ Sigstore instance options:
default production instances (default: False)
--rekor-url URL The Rekor instance to use (conflicts with --staging)
(default: https://rekor.sigstore.dev)
--fulcio-url URL The Fulcio instance to use (conflicts with --staging)
(default: https://fulcio.sigstore.dev)
--ctfe FILE A PEM-encoded public key for the CT log (conflicts
with --staging) (default: ctfe.pub (embedded))
--rekor-root-pubkey FILE
A PEM-encoded root public key for Rekor itself
(conflicts with --staging) (default: rekor.pub
(embedded))
--fulcio-url URL The Fulcio instance to use (conflicts with --staging)
(default: https://fulcio.sigstore.dev)
--ctfe FILE A PEM-encoded public key for the CT log (conflicts
with --staging) (default: ctfe.pub (embedded))
```
<!-- @end-sigstore-sign-help@ -->

Expand All @@ -152,9 +152,11 @@ Verifying:
<!-- @begin-sigstore-verify-help@ -->
```
usage: sigstore verify [-h] [--certificate FILE] [--signature FILE]
[--rekor-bundle FILE] [--cert-email EMAIL]
--cert-identity IDENTITY --cert-oidc-issuer URL
[--require-rekor-offline] [--staging] [--rekor-url URL]
[--rekor-bundle FILE] [--certificate-chain FILE]
[--cert-email EMAIL] --cert-identity IDENTITY
--cert-oidc-issuer URL [--require-rekor-offline]
[--staging] [--rekor-url URL]
[--rekor-root-pubkey FILE]
FILE [FILE ...]

positional arguments:
Expand All @@ -173,6 +175,10 @@ Verification inputs:
multiple inputs (default: None)

Extended verification options:
--certificate-chain FILE
Path to a list of CA certificates in PEM format which
will be needed when building the certificate chain for
the signing certificate (default: None)
--cert-email EMAIL Deprecated; causes an error. Use --cert-identity
instead (default: None)
--cert-identity IDENTITY
Expand All @@ -190,6 +196,10 @@ Sigstore instance options:
default production instances (default: False)
--rekor-url URL The Rekor instance to use (conflicts with --staging)
(default: https://rekor.sigstore.dev)
--rekor-root-pubkey FILE
A PEM-encoded root public key for Rekor itself
(conflicts with --staging) (default: rekor.pub
(embedded))
```
<!-- @end-sigstore-verify-help@ -->

Expand Down
51 changes: 39 additions & 12 deletions sigstore/_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@
RekorEntry,
)
from sigstore._sign import Signer
from sigstore._utils import load_pem_public_key
from sigstore._utils import (
SplitCertificateChainError,
load_pem_public_key,
split_certificate_chain,
)
from sigstore._verify import (
CertificateVerificationFailure,
RekorEntryMissing,
Expand Down Expand Up @@ -107,6 +111,13 @@ def _add_shared_instance_options(group: argparse._ArgumentGroup) -> None:
default=os.getenv("SIGSTORE_REKOR_URL", DEFAULT_REKOR_URL),
help="The Rekor instance to use (conflicts with --staging)",
)
group.add_argument(
"--rekor-root-pubkey",
metavar="FILE",
type=argparse.FileType("rb"),
help="A PEM-encoded root public key for Rekor itself (conflicts with --staging)",
default=os.getenv("SIGSTORE_REKOR_ROOT_PUBKEY", _Embedded("rekor.pub")),
)


def _add_shared_oidc_options(
Expand Down Expand Up @@ -229,13 +240,6 @@ def _parser() -> argparse.ArgumentParser:
help="A PEM-encoded public key for the CT log (conflicts with --staging)",
default=os.getenv("SIGSTORE_CTFE", _Embedded("ctfe.pub")),
)
instance_options.add_argument(
"--rekor-root-pubkey",
metavar="FILE",
type=argparse.FileType("rb"),
help="A PEM-encoded root public key for Rekor itself (conflicts with --staging)",
default=os.getenv("SIGSTORE_REKOR_ROOT_PUBKEY", _Embedded("rekor.pub")),
)

sign.add_argument(
"files",
Expand Down Expand Up @@ -275,6 +279,15 @@ def _parser() -> argparse.ArgumentParser:
)

verification_options = verify.add_argument_group("Extended verification options")
verification_options.add_argument(
"--certificate-chain",
metavar="FILE",
type=argparse.FileType("r"),
help=(
"Path to a list of CA certificates in PEM format which will be needed when building "
"the certificate chain for the signing certificate"
),
)
verification_options.add_argument(
"--cert-email",
metavar="EMAIL",
Expand Down Expand Up @@ -536,10 +549,24 @@ def _verify(args: argparse.Namespace) -> None:
elif args.rekor_url == DEFAULT_REKOR_URL:
verifier = Verifier.production()
else:
# TODO: We need CLI flags that allow the user to figure the Fulcio cert chain
# for verification.
args._parser.error(
"Custom Rekor and Fulcio configuration for verification isn't fully supported yet!",
if not args.certificate_chain:
args._parser.error(
"Custom Rekor URL used without specifying --certificate-chain"
)

try:
certificate_chain = split_certificate_chain(args.certificate_chain.read())
except SplitCertificateChainError as error:
args._parser.error(f"Failed to parse certificate chain: {error}")

verifier = Verifier(
rekor=RekorClient(
url=args.rekor_url,
pubkey=args.rekor_root_pubkey.read(),
Copy link
Contributor Author

@tetsuo-cpp tetsuo-cpp Nov 29, 2022

Choose a reason for hiding this comment

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

I wonder whether we should warn if a user supplies --rekor-root-pubkey but don't set the --rekor-url. Same with the new --certificate-chain. At the moment they just get ignored which might be confusing.

Same thing when we set --rekor-url but don't set --rekor-root-pubkey. It defaults to the rekor.pub that comes bundled with sigstore-python (which is almost certainly not what you want when you're using a custom Rekor) instead of warning.

Similar issues also exist in the signing code path.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it would be good to catch this and flag it as a warning at the minimum (or maybe even an error, since we expect it to fail anyways).

Copy link
Member

Choose a reason for hiding this comment

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

Let's track this with a follow-up issue.

# We don't use the CT keyring in verification so we can supply an empty keyring
ct_keyring=CTKeyring(),
),
fulcio_certificate_chain=certificate_chain,
)

for file, inputs in input_map.items():
Expand Down
35 changes: 35 additions & 0 deletions sigstore/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
Shared utilities.
"""

from __future__ import annotations

import base64
import hashlib
from typing import Union
Expand Down Expand Up @@ -69,3 +71,36 @@ def key_id(key: PublicKey) -> bytes:
)

return hashlib.sha256(public_bytes).digest()


class SplitCertificateChainError(Exception):
pass


def split_certificate_chain(chain_pem: str) -> list[bytes]:
"""
Returns a list of PEM bytes for each individual certificate in the chain.
"""
pem_header = "-----BEGIN CERTIFICATE-----"

# Check for no certificates
if not chain_pem:
raise SplitCertificateChainError("empty PEM file")

# Use the "begin certificate" marker as a delimiter to split the chain
certificate_chain = chain_pem.split(pem_header)

# The first entry in the list should be empty since we split by the "begin certificate" marker
# and there should be nothing before the first certificate
if certificate_chain[0]:
raise SplitCertificateChainError(
"encountered unrecognized content before first PEM entry"
)

# Remove the empty entry
certificate_chain = certificate_chain[1:]

# Add the delimiters back into each entry since this is required for valid PEM
certificate_chain = [(pem_header + c).encode() for c in certificate_chain]

return certificate_chain