Skip to content

Key management improvement #936

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.

Sign up for GitHub

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
woodruffw merged 12 commits into from
Mar 25, 2024
Merged

Key management improvement #936

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e3e89a6
refactor: concentrate keys on trusted root
javanlacerda Mar 13, 2024
486da39
moving keyring logic to trustroot module
javanlacerda Mar 18, 2024
9db2b1e
move ct keyring responsibilities to trustedroot
javanlacerda Mar 18, 2024
2084b6f
adding args checker for ct keys
javanlacerda Mar 18, 2024
e673f9c
removing keys from rekor client
javanlacerda Mar 20, 2024
cf7f6ce
passing args and purpose to trusted root directly
javanlacerda Mar 20, 2024
85e6d6c
removing certificate_chain and rekor-root-pubkey from CLI
javanlacerda Mar 20, 2024
0c559b4
fixing change log
javanlacerda Mar 20, 2024
1290746
Update CHANGELOG.md
javanlacerda Mar 21, 2024
65db74d
removing ctfe from CLI
javanlacerda Mar 21, 2024
0410d79
conform readme to helper
javanlacerda Mar 21, 2024
ea26820
removing comment
javanlacerda Mar 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,10 @@ This is a corrective release for [2.1.1].
replacing the material that was previously baked into `sigstore._store`
([#351](https://github.com/sigstore/sigstore-python/pull/351))

### Removed
* CLI: The `--certificate-chain`, `--rekor-root-pubkey` and `-ctfe` flags have been entirely removed ([#936](https://github.com/sigstore/sigstore-python/pull/936))


<!--Release URLs -->
[Unreleased]: https://github.com/sigstore/sigstore-python/compare/v2.1.2...HEAD
[2.1.2]: https://github.com/sigstore/sigstore-python/compare/v2.1.1...v2.1.2
Expand Down
68 changes: 18 additions & 50 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,33 +96,29 @@ Top-level:

<!-- @begin-sigstore-help@ -->
```
usage: sigstore [-h] [-v] [-V] [--staging] [--rekor-url URL]
[--rekor-root-pubkey FILE]
COMMAND ...
usage: sigstore [-h] [-v] [-V] [--staging] [--rekor-url URL] COMMAND ...

a tool for signing and verifying Python package distributions

positional arguments:
COMMAND the operation to perform
sign sign one or more inputs
verify verify one or more inputs
get-identity-token retrieve and return a Sigstore-compatible OpenID
Connect token
COMMAND the operation to perform
sign sign one or more inputs
verify verify one or more inputs
get-identity-token
retrieve and return a Sigstore-compatible OpenID Connect
token

optional arguments:
-h, --help show this help message and exit
-v, --verbose run with additional debug logging; supply multiple
times to increase verbosity
-V, --version show program's version number and exit
-h, --help show this help message and exit
-v, --verbose run with additional debug logging; supply multiple times
to increase verbosity
-V, --version show program's version number and exit

Sigstore instance options:
--staging Use sigstore's staging instances, instead of the
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: None)
--staging Use sigstore's staging instances, instead of the default
production instances (default: False)
--rekor-url URL The Rekor instance to use (conflicts with --staging)
(default: https://rekor.sigstore.dev)
```
<!-- @end-sigstore-help@ -->

Expand All @@ -137,8 +133,7 @@ usage: sigstore sign [-h] [-v] [--identity-token TOKEN] [--oidc-client-id ID]
[--oauth-force-oob] [--no-default-files]
[--signature FILE] [--certificate FILE] [--bundle FILE]
[--output-directory DIR] [--overwrite] [--staging]
[--rekor-url URL] [--rekor-root-pubkey FILE]
[--fulcio-url URL] [--ctfe FILE]
[--rekor-url URL] [--fulcio-url URL]
FILE [FILE ...]

positional arguments:
Expand Down Expand Up @@ -193,15 +188,8 @@ Sigstore instance options:
This option will be deprecated in favor of the global
`--rekor-url` option in a future release. (default:
None)
--rekor-root-pubkey FILE
A PEM-encoded root public key for Rekor itself
(conflicts with --staging). This option will be
deprecated in favor of the global `--rekor-root-
pubkey` option in a future release. (default: None)
--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: None)
```
<!-- @end-sigstore-sign-help@ -->

Expand All @@ -220,8 +208,7 @@ usage: sigstore verify identity [-h] [-v] [--certificate FILE]
[--signature FILE] [--bundle FILE]
--cert-identity IDENTITY [--offline]
--cert-oidc-issuer URL [--staging]
[--rekor-url URL] [--rekor-root-pubkey FILE]
[--certificate-chain FILE]
[--rekor-url URL]
FILE [FILE ...]

optional arguments:
Expand Down Expand Up @@ -258,15 +245,6 @@ Sigstore instance options:
This option will be deprecated in favor of the global
`--rekor-url` option in a future release. (default:
None)
--rekor-root-pubkey FILE
A PEM-encoded root public key for Rekor itself
(conflicts with --staging). This option will be
deprecated in favor of the global `--rekor-root-
pubkey` option in a future release. (default: None)
--certificate-chain FILE
Path to a list of CA certificates in PEM format which
will be needed when building the certificate chain for
the Fulcio signing certificate (default: None)
```
<!-- @end-sigstore-verify-identity-help@ -->

Expand All @@ -284,8 +262,7 @@ usage: sigstore verify github [-h] [-v] [--certificate FILE]
--cert-identity IDENTITY [--offline]
[--trigger EVENT] [--sha SHA] [--name NAME]
[--repository REPO] [--ref REF] [--staging]
[--rekor-url URL] [--rekor-root-pubkey FILE]
[--certificate-chain FILE]
[--rekor-url URL]
FILE [FILE ...]

optional arguments:
Expand Down Expand Up @@ -329,15 +306,6 @@ Sigstore instance options:
This option will be deprecated in favor of the global
`--rekor-url` option in a future release. (default:
None)
--rekor-root-pubkey FILE
A PEM-encoded root public key for Rekor itself
(conflicts with --staging). This option will be
deprecated in favor of the global `--rekor-root-
pubkey` option in a future release. (default: None)
--certificate-chain FILE
Path to a list of CA certificates in PEM format which
will be needed when building the certificate chain for
the Fulcio signing certificate (default: None)
```
<!-- @end-sigstore-verify-github-help@ -->

Expand Down
97 changes: 6 additions & 91 deletions sigstore/_cli.py
View file
Open in desktop
Copy link
Copy Markdown
Member

@woodruffw woodruffw Mar 20, 2024

Choose a reason for hiding this comment

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

I think this file also needs deletions for the CLI flags themselves: the _parser() function needs to be updated to remove each of the arg definitions that have had their usages removed below.

Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,20 @@
from textwrap import dedent
from typing import NoReturn, Optional, TextIO, Union, cast

from cryptography.x509 import load_pem_x509_certificates
from rich.logging import RichHandler
from sigstore_protobuf_specs.dev.sigstore.bundle.v1 import Bundle

from sigstore import __version__
from sigstore._internal.ctfe import CTKeyring
from sigstore._internal.fulcio.client import (
DEFAULT_FULCIO_URL,
ExpiredCertificate,
FulcioClient,
)
from sigstore._internal.keyring import Keyring
from sigstore._internal.rekor.client import (
DEFAULT_REKOR_URL,
RekorClient,
RekorKeyring,
)
from sigstore._internal.trustroot import TrustedRoot
from sigstore._internal.trustroot import KeyringPurpose, TrustedRoot
from sigstore._utils import PEMCert, cert_der_to_pem, sha256_digest
from sigstore.errors import Error
from sigstore.oidc import (
Expand Down Expand Up @@ -128,18 +124,6 @@ def _add_shared_instance_options(group: argparse._ArgumentGroup) -> None:
"in a future release."
),
)
group.add_argument(
"--rekor-root-pubkey",
dest="__deprecated_rekor_root_pubkey",
metavar="FILE",
type=argparse.FileType("rb"),
default=None,
help=(
"A PEM-encoded root public key for Rekor itself (conflicts with --staging). "
"This option will be deprecated in favor of the global `--rekor-root-pubkey` option "
"in a future release."
),
)


def _add_shared_verify_input_options(group: argparse._ArgumentGroup) -> None:
Expand Down Expand Up @@ -270,13 +254,6 @@ def _parser() -> argparse.ArgumentParser:
default=os.getenv("SIGSTORE_REKOR_URL", DEFAULT_REKOR_URL),
help="The Rekor instance to use (conflicts with --staging)",
)
global_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"),
)

subcommands = parser.add_subparsers(
required=True,
Expand Down Expand Up @@ -366,14 +343,6 @@ def _parser() -> argparse.ArgumentParser:
default=os.getenv("SIGSTORE_FULCIO_URL", DEFAULT_FULCIO_URL),
help="The Fulcio instance to use (conflicts with --staging)",
)
instance_options.add_argument(
"--ctfe",
dest="ctfe_pem",
metavar="FILE",
type=argparse.FileType("rb"),
help="A PEM-encoded public key for the CT log (conflicts with --staging)",
default=os.getenv("SIGSTORE_CTFE"),
)

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

instance_options = verify_identity.add_argument_group("Sigstore instance options")
_add_shared_instance_options(instance_options)
instance_options.add_argument(
"--certificate-chain",
metavar="FILE",
type=argparse.FileType("rb"),
help=(
"Path to a list of CA certificates in PEM format which will be needed when building "
"the certificate chain for the Fulcio signing certificate"
),
)

# `sigstore verify github`
verify_github = verify_subcommand.add_parser(
Expand Down Expand Up @@ -486,15 +446,6 @@ def _parser() -> argparse.ArgumentParser:

instance_options = verify_github.add_argument_group("Sigstore instance options")
_add_shared_instance_options(instance_options)
instance_options.add_argument(
"--certificate-chain",
metavar="FILE",
type=argparse.FileType("rb"),
help=(
"Path to a list of CA certificates in PEM format which will be needed when building "
"the certificate chain for the Fulcio signing certificate"
),
)

# `sigstore get-identity-token`
get_identity_token = subcommands.add_parser(
Expand Down Expand Up @@ -536,13 +487,6 @@ def main() -> None:
"Passing `--rekor-url` as a subcommand option will be deprecated in a future release."
)
args.rekor_url = args.__deprecated_rekor_url
if getattr(args, "__deprecated_rekor_root_pubkey", None):
logger.warning(
"`--rekor-root-pubkey` should be used as a global option, rather than a "
"subcommand option. Passing `--rekor-root-pubkey` as a subcommand option will be "
"deprecated in a future release."
)
args.rekor_root_pubkey = args.__deprecated_rekor_root_pubkey

# Stuff the parser back into our namespace, so that we can use it for
# error handling later.
Expand Down Expand Up @@ -651,22 +595,12 @@ def _sign(args: argparse.Namespace) -> None:
signing_ctx = SigningContext.production()
else:
# Assume "production" trust root if no keys are given as arguments
trusted_root = TrustedRoot.production()
if args.ctfe_pem is not None:
ctfe_keys = [args.ctfe_pem.read()]
else:
ctfe_keys = trusted_root.get_ctfe_keys()
if args.rekor_root_pubkey is not None:
rekor_keys = [args.rekor_root_pubkey.read()]
else:
rekor_keys = trusted_root.get_rekor_keys()

ct_keyring = CTKeyring(Keyring(ctfe_keys))
rekor_keyring = RekorKeyring(Keyring(rekor_keys))
trusted_root = TrustedRoot.production(purpose=KeyringPurpose.SIGN)

signing_ctx = SigningContext(
fulcio=FulcioClient(args.fulcio_url),
rekor=RekorClient(args.rekor_url, rekor_keyring, ct_keyring),
rekor=RekorClient(args.rekor_url),
trusted_root=trusted_root,
)

# The order of precedence for identities is as follows:
Expand Down Expand Up @@ -814,37 +748,18 @@ def _collect_verification_state(
args,
f"Missing verification materials for {(file)}: {', '.join(missing)}",
)

if args.staging:
logger.debug("verify: staging instances requested")
verifier = Verifier.staging()
elif args.rekor_url == DEFAULT_REKOR_URL:
verifier = Verifier.production()
else:
if not args.certificate_chain:
_die(args, "Custom Rekor URL used without specifying --certificate-chain")

try:
certificate_chain = load_pem_x509_certificates(
args.certificate_chain.read()
)
except ValueError as error:
_die(args, f"Invalid certificate chain: {error}")

if args.rekor_root_pubkey is not None:
rekor_keys = [args.rekor_root_pubkey.read()]
else:
trusted_root = TrustedRoot.production()
rekor_keys = trusted_root.get_rekor_keys()
ct_keys = trusted_root.get_ctfe_keys()

trusted_root = TrustedRoot.production(purpose=KeyringPurpose.VERIFY)
verifier = Verifier(
rekor=RekorClient(
url=args.rekor_url,
rekor_keyring=RekorKeyring(Keyring(rekor_keys)),
ct_keyring=CTKeyring(Keyring(ct_keys)),
),
fulcio_certificate_chain=certificate_chain,
trusted_root=trusted_root,
)

all_materials = []
Expand Down
23 changes: 0 additions & 23 deletions sigstore/_internal/ctfe.py
View file
Open in desktop

This file was deleted.

Loading