From 14a904873577f03687312e946ac7360be42d26f2 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 3 Jan 2023 14:02:05 -0500 Subject: [PATCH 01/12] _cli: add `sigstore verify identity` This adds `sigstore verify identity`, which is aliased (through some `argparse` hackery) back to `sigstore verify`. This allows us to remain compatible with the existing CLI, while also giving us the flexibility we need to extend verification (e.g. `sigstore verify github`). Signed-off-by: William Woodruff --- sigstore/_cli.py | 41 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/sigstore/_cli.py b/sigstore/_cli.py index cfd89362..4881670c 100644 --- a/sigstore/_cli.py +++ b/sigstore/_cli.py @@ -82,6 +82,30 @@ def _boolify_env(envvar: str) -> bool: raise ValueError(f"can't coerce '{val}' to a boolean") +def _set_default_subparser(parser: argparse.ArgumentParser, name: str) -> None: + """ + A monkeypatch for configuring a default subparser. + + Adapted from + """ + subparser_found = False + for arg in sys.argv[1:]: + if arg in ["-h", "--help"]: # global help if no subparser + break + else: + for x in parser._subparsers._actions: # type: ignore[union-attr] + if not isinstance(x, argparse._SubParsersAction): + continue + for sp_name in x._name_parser_map.keys(): + if sp_name in sys.argv[1:]: + subparser_found = True + if not subparser_found: + # insert default in last position before global positional + # arguments, this implies no global options are specified after + # first positional argument + sys.argv.insert(len(sys.argv), name) + + def _add_shared_instance_options(group: argparse._ArgumentGroup) -> None: group.add_argument( "--staging", @@ -245,8 +269,11 @@ def _parser() -> argparse.ArgumentParser: verify = subcommands.add_parser( "verify", formatter_class=argparse.ArgumentDefaultsHelpFormatter ) + verify_subcommand = verify.add_subparsers(dest="verify_subcommand") - input_options = verify.add_argument_group("Verification inputs") + # `sigstore verify identity` + verify_identity = verify_subcommand.add_parser("identity") + input_options = verify_identity.add_argument_group("Verification inputs") input_options.add_argument( "--certificate", "--cert", @@ -270,7 +297,9 @@ def _parser() -> argparse.ArgumentParser: help="The offline Rekor bundle to verify with; not used with multiple inputs", ) - verification_options = verify.add_argument_group("Extended verification options") + verification_options = verify_identity.add_argument_group( + "Extended verification options" + ) verification_options.add_argument( "--certificate-chain", metavar="FILE", @@ -309,10 +338,10 @@ def _parser() -> argparse.ArgumentParser: help="Require offline Rekor verification with a bundle; implied by --rekor-bundle", ) - instance_options = verify.add_argument_group("Sigstore instance options") + instance_options = verify_identity.add_argument_group("Sigstore instance options") _add_shared_instance_options(instance_options) - verify.add_argument( + verify_identity.add_argument( "files", metavar="FILE", type=Path, @@ -320,6 +349,10 @@ def _parser() -> argparse.ArgumentParser: help="The file to verify", ) + # `sigstore verify` defaults to `sigstore verify identity`, for backwards + # compatibility. + _set_default_subparser(verify, "identity") + # `sigstore get-identity-token` get_identity_token = subcommands.add_parser("get-identity-token") _add_shared_oidc_options(get_identity_token) From 2eaded66cbed6f4501df1e70d1240acb5fe42ec5 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 3 Jan 2023 14:13:26 -0500 Subject: [PATCH 02/12] _cli: help text for `sigstore verify identity` Signed-off-by: William Woodruff --- sigstore/_cli.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sigstore/_cli.py b/sigstore/_cli.py index 4881670c..911db26c 100644 --- a/sigstore/_cli.py +++ b/sigstore/_cli.py @@ -267,12 +267,15 @@ def _parser() -> argparse.ArgumentParser: # `sigstore verify` verify = subcommands.add_parser( - "verify", formatter_class=argparse.ArgumentDefaultsHelpFormatter + "verify", + formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) verify_subcommand = verify.add_subparsers(dest="verify_subcommand") # `sigstore verify identity` - verify_identity = verify_subcommand.add_parser("identity") + verify_identity = verify_subcommand.add_parser( + "identity", help="verify against a known identity and identity provider" + ) input_options = verify_identity.add_argument_group("Verification inputs") input_options.add_argument( "--certificate", From 7ed3aa1db42955a84170011b3a828852ea89d106 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 3 Jan 2023 14:19:19 -0500 Subject: [PATCH 03/12] _cli, Makefile, README: fix `--help` generation Signed-off-by: William Woodruff --- Makefile | 4 ++-- README.md | 26 ++++++++++++++------------ sigstore/_cli.py | 4 +++- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/Makefile b/Makefile index 906a21ec..c812a1d0 100644 --- a/Makefile +++ b/Makefile @@ -117,10 +117,10 @@ check-readme: $(MAKE) -s run ARGS="sign --help" \ ) - # sigstore verify --help + # sigstore verify identity --help @diff \ <( \ - awk '/@begin-sigstore-verify-help@/{f=1;next} /@end-sigstore-verify-help@/{f=0} f' \ + awk '/@begin-sigstore-verify-identity-help@/{f=1;next} /@end-sigstore-verify-identity-help@/{f=0} f' \ < README.md | sed '1d;$$d' \ ) \ <( \ diff --git a/README.md b/README.md index b3301ead..c637441a 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,8 @@ options: ``` -Signing: + +### Signing ``` @@ -150,22 +151,23 @@ Sigstore instance options: ``` -Verifying: +### Verifying - + ``` -usage: sigstore verify [-h] [--certificate FILE] [--signature FILE] - [--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 ...] +usage: sigstore verify identity [-h] [--certificate FILE] [--signature FILE] + [--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: FILE The file to verify -options: +optionals: -h, --help show this help message and exit Verification inputs: @@ -203,7 +205,7 @@ Sigstore instance options: A PEM-encoded root public key for Rekor itself (conflicts with --staging) (default: None) ``` - + ## Example uses diff --git a/sigstore/_cli.py b/sigstore/_cli.py index 911db26c..1f76b82e 100644 --- a/sigstore/_cli.py +++ b/sigstore/_cli.py @@ -274,7 +274,9 @@ def _parser() -> argparse.ArgumentParser: # `sigstore verify identity` verify_identity = verify_subcommand.add_parser( - "identity", help="verify against a known identity and identity provider" + "identity", + help="verify against a known identity and identity provider", + formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) input_options = verify_identity.add_argument_group("Verification inputs") input_options.add_argument( From 76f54a24c829a8b31e82009365e482f5d4802016 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 3 Jan 2023 14:21:33 -0500 Subject: [PATCH 04/12] README: typo Signed-off-by: William Woodruff --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index c637441a..1bb101fb 100644 --- a/README.md +++ b/README.md @@ -167,7 +167,7 @@ usage: sigstore verify identity [-h] [--certificate FILE] [--signature FILE] positional arguments: FILE The file to verify -optionals: +options: -h, --help show this help message and exit Verification inputs: From 7e43fbce22d08883db514f6ecef37d1b90a1e2c8 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 3 Jan 2023 14:26:56 -0500 Subject: [PATCH 05/12] ci: run check-readme against min Python version Signed-off-by: William Woodruff --- .github/workflows/ci.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 490d8e55..cbfb10e4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,11 +57,15 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@755da8c3cf115ac066823e79a1e1788f8940201b # v3.2.0 + + # NOTE: We intentionally lint against our minimum supported Python. - uses: actions/setup-python@5ccb29d8773c3f3f653e1705f474dfaa8a06a912 with: python-version: "3.7" + - name: deps run: make dev SIGSTORE_EXTRA=lint + - name: lint run: make lint @@ -69,10 +73,15 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@755da8c3cf115ac066823e79a1e1788f8940201b # v3.2.0 + + # NOTE: We intentional check `--help` rendering against our minimum Python, + # since it changes slightly between Python versions. - uses: actions/setup-python@5ccb29d8773c3f3f653e1705f474dfaa8a06a912 with: - python-version: "3.x" + python-version: "3.7" + - name: deps run: make dev + - name: check-readme run: make check-readme From 9f95092718ff1acc26c2cd7341043f150d6acbc7 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 3 Jan 2023 14:27:14 -0500 Subject: [PATCH 06/12] Makefile: fix `check-readme` target Signed-off-by: William Woodruff --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index c812a1d0..44f3c4ca 100644 --- a/Makefile +++ b/Makefile @@ -124,7 +124,7 @@ check-readme: < README.md | sed '1d;$$d' \ ) \ <( \ - $(MAKE) -s run ARGS="verify --help" \ + $(MAKE) -s run ARGS="verify identity --help" \ ) From f70a881ae4bf1be856d2c72a0a29949f777feb9e Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 3 Jan 2023 14:27:25 -0500 Subject: [PATCH 07/12] README: render `--help` with min Python version Signed-off-by: William Woodruff --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 1bb101fb..4fbb70a3 100644 --- a/README.md +++ b/README.md @@ -78,7 +78,7 @@ a tool for signing and verifying Python package distributions positional arguments: {sign,verify,get-identity-token} -options: +optional arguments: -h, --help show this help message and exit -V, --version show program's version number and exit -v, --verbose run with additional debug logging; supply multiple @@ -103,7 +103,7 @@ usage: sigstore sign [-h] [--identity-token TOKEN] [--oidc-client-id ID] positional arguments: FILE The file to sign -options: +optional arguments: -h, --help show this help message and exit OpenID Connect options: @@ -167,7 +167,7 @@ usage: sigstore verify identity [-h] [--certificate FILE] [--signature FILE] positional arguments: FILE The file to verify -options: +optional arguments: -h, --help show this help message and exit Verification inputs: From 9d92fbe09d86b992c115d232518a1dd43f8a40ab Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 3 Jan 2023 14:41:22 -0500 Subject: [PATCH 08/12] _cli: fix `sigstore verify` behavior Signed-off-by: William Woodruff --- sigstore/_cli.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/sigstore/_cli.py b/sigstore/_cli.py index 1f76b82e..4acfc56a 100644 --- a/sigstore/_cli.py +++ b/sigstore/_cli.py @@ -84,7 +84,7 @@ def _boolify_env(envvar: str) -> bool: def _set_default_subparser(parser: argparse.ArgumentParser, name: str) -> None: """ - A monkeypatch for configuring a default subparser. + An argparse patch for configuring a default subparser. Adapted from """ @@ -100,10 +100,11 @@ def _set_default_subparser(parser: argparse.ArgumentParser, name: str) -> None: if sp_name in sys.argv[1:]: subparser_found = True if not subparser_found: - # insert default in last position before global positional - # arguments, this implies no global options are specified after - # first positional argument - sys.argv.insert(len(sys.argv), name) + # NOTE: We expect the default subparser to take exactly one + # positional argument (e.g. `sigstore verify identity foo.txt`), so + # we insert the subparser's name right before that positional should + # appear. + sys.argv.insert(len(sys.argv) - 1, name) def _add_shared_instance_options(group: argparse._ArgumentGroup) -> None: From 769bafda7f791e195317726c75aa2c9d31087c57 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 3 Jan 2023 14:50:52 -0500 Subject: [PATCH 09/12] _cli: hackety hack Signed-off-by: William Woodruff --- sigstore/_cli.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/sigstore/_cli.py b/sigstore/_cli.py index 4acfc56a..b9130a98 100644 --- a/sigstore/_cli.py +++ b/sigstore/_cli.py @@ -82,9 +82,9 @@ def _boolify_env(envvar: str) -> bool: raise ValueError(f"can't coerce '{val}' to a boolean") -def _set_default_subparser(parser: argparse.ArgumentParser, name: str) -> None: +def _set_default_verify_subparser(parser: argparse.ArgumentParser, name: str) -> None: """ - An argparse patch for configuring a default subparser. + An argparse patch for configuring a default subparser for `sigstore verify`. Adapted from """ @@ -100,11 +100,12 @@ def _set_default_subparser(parser: argparse.ArgumentParser, name: str) -> None: if sp_name in sys.argv[1:]: subparser_found = True if not subparser_found: - # NOTE: We expect the default subparser to take exactly one - # positional argument (e.g. `sigstore verify identity foo.txt`), so - # we insert the subparser's name right before that positional should - # appear. - sys.argv.insert(len(sys.argv) - 1, name) + # If `sigstore verify identity` wasn't passed explicitly, we need + # to insert the `identity` subcommand into the correct position + # within `sys.argv`. To do that, we get the index of the `verify` + # subcommand, and insert it directly after it. + verify_idx = sys.argv.index("verify") + sys.argv.insert(verify_idx + 1, name) def _add_shared_instance_options(group: argparse._ArgumentGroup) -> None: @@ -357,7 +358,7 @@ def _parser() -> argparse.ArgumentParser: # `sigstore verify` defaults to `sigstore verify identity`, for backwards # compatibility. - _set_default_subparser(verify, "identity") + _set_default_verify_subparser(verify, "identity") # `sigstore get-identity-token` get_identity_token = subcommands.add_parser("get-identity-token") From 3ab05a7586da6a20672bad8b847f221cf54e6231 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 3 Jan 2023 14:58:30 -0500 Subject: [PATCH 10/12] _cli: hackety hack Signed-off-by: William Woodruff --- sigstore/_cli.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/sigstore/_cli.py b/sigstore/_cli.py index b9130a98..747e3078 100644 --- a/sigstore/_cli.py +++ b/sigstore/_cli.py @@ -100,12 +100,17 @@ def _set_default_verify_subparser(parser: argparse.ArgumentParser, name: str) -> if sp_name in sys.argv[1:]: subparser_found = True if not subparser_found: - # If `sigstore verify identity` wasn't passed explicitly, we need - # to insert the `identity` subcommand into the correct position - # within `sys.argv`. To do that, we get the index of the `verify` - # subcommand, and insert it directly after it. - verify_idx = sys.argv.index("verify") - sys.argv.insert(verify_idx + 1, name) + try: + # If `sigstore verify identity` wasn't passed explicitly, we need + # to insert the `identity` subcommand into the correct position + # within `sys.argv`. To do that, we get the index of the `verify` + # subcommand, and insert it directly after it. + verify_idx = sys.argv.index("verify") + sys.argv.insert(verify_idx + 1, name) + except ValueError: + # This happens when we invoke `sigstore sign`, since there's no + # `verify` subcommand to insert under. We do nothing in this case. + pass def _add_shared_instance_options(group: argparse._ArgumentGroup) -> None: From b62e39c052e4d938e73378dd9d03be94da3f6009 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 3 Jan 2023 17:25:13 -0500 Subject: [PATCH 11/12] README: update docs, examples Signed-off-by: William Woodruff --- README.md | 49 ++++++++++++++++++++----------------------------- 1 file changed, 20 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 4fbb70a3..1f510413 100644 --- a/README.md +++ b/README.md @@ -153,6 +153,13 @@ Sigstore instance options: ### Verifying +#### Identities + +This is the most common verification done with `sigstore`, and therefore +the one you probably want: you can use it to verify that a signature was +produced by a particular identity (like `hamilcar@example.com`), as attested +to by a particular OIDC provider (like `https://github.com/login/oauth`). + ``` usage: sigstore verify identity [-h] [--certificate FILE] [--signature FILE] @@ -207,6 +214,10 @@ Sigstore instance options: ``` +For backwards compatibility, `sigstore verify [args ...]` is equivalent to +`sigstore verify identity [args ...]`, but the latter form is **strongly** +preferred. + ## Example uses `sigstore` supports a wide variety of workflows and usages. Some common ones are @@ -272,51 +283,31 @@ same directory as the file being verified: ```console # looks for foo.txt.sig and foo.txt.crt -$ python -m sigstore verify foo.txt +$ python -m sigstore verify identity foo.txt \ + --cert-identity 'hamilcar@example.com' \ + --cert-oidc-issuer 'https://github.com/login/oauth' ``` Multiple files can be verified at once: ```console # looks for {foo,bar}.txt.{sig,crt} -$ python -m sigstore verify foo.txt bar.txt +$ python -m sigstore verify identity foo.txt bar.txt \ + --cert-identity 'hamilcar@example.com' \ + --cert-oidc-issuer 'https://github.com/login/oauth' ``` If your signature and certificate are at different paths, you can specify them explicitly (but only for one file at a time): ```console -$ python -m sigstore verify \ +$ python -m sigstore verify identity foo.txt \ --certificate some/other/path/foo.crt \ --signature some/other/path/foo.sig \ - foo.txt + --cert-identity 'hamilcar@example.com' \ + --cert-oidc-issuer 'https://github.com/login/oauth' ``` -### Extended verification against OpenID Connect claims - -By default, `sigstore verify` only checks the validity of the certificate, -the correctness of the signature, and the consistency of both with the -certificate transparency log. - -To assert further details about the signature (such as *who* or *what* signed for the artifact), -you can test against the OpenID Connect claims embedded within it. - -For example, to accept the signature and certificate only if they correspond to a particular -email identity: - -```console -$ python -m sigstore verify --cert-email developer@example.com foo.txt -``` - -Or to accept only if the OpenID Connect issuer is the expected one: - -```console -$ python -m sigstore verify --cert-oidc-issuer https://github.com/login/oauth foo.txt -``` - -These options can be combined, and further extended validation options (e.g., for -signing results from GitHub Actions) are under development. - ## Licensing `sigstore` is licensed under the Apache 2.0 License. From 088714b32e5d9d08a531d7c8b7ec73b7efd803ba Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 3 Jan 2023 21:32:10 -0500 Subject: [PATCH 12/12] _cli: add warning to bare `sigstore verify` Signed-off-by: William Woodruff --- sigstore/_cli.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sigstore/_cli.py b/sigstore/_cli.py index 747e3078..ed81e3ce 100644 --- a/sigstore/_cli.py +++ b/sigstore/_cli.py @@ -107,6 +107,11 @@ def _set_default_verify_subparser(parser: argparse.ArgumentParser, name: str) -> # subcommand, and insert it directly after it. verify_idx = sys.argv.index("verify") sys.argv.insert(verify_idx + 1, name) + logger.warning( + "`sigstore verify` without a subcommand will be treated as " + "`sigstore verify identity`, but this behavior will be deprecated " + "in a future release" + ) except ValueError: # This happens when we invoke `sigstore sign`, since there's no # `verify` subcommand to insert under. We do nothing in this case.