Skip to content
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

signer: add basic sigstore signer and verifier #522

Merged
merged 10 commits into from
Mar 10, 2023
38 changes: 38 additions & 0 deletions .github/workflows/test-sigstore.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
name: Run Sigstore Signer tests

on:
push:
branches:
- main
pull_request:
Copy link
Collaborator

Choose a reason for hiding this comment

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

should remove this as it will unfortunately keep failing for PRs

We should also maybe file an issue for managing these tests that can be only run on upstream branches... so that an issue would be raised when a test fails

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh, I actually did that in kms-tests: you can probably copy the file-ticket-on-failure step from there

Copy link
Member Author

Choose a reason for hiding this comment

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

should remove this as it will unfortunately keep failing for PRs

Is that the case? Shouldn't it work, once the workflow is merged?

Copy link
Collaborator

@jku jku Mar 9, 2023

Choose a reason for hiding this comment

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

I don't think so: pull requests have read only permissions and you need id-token: write

or in more OIDC terms, pull requests aren't allowed to "authenticate" as the project (even though sometimes it would make sense, since the authentication is not for the whole project but for the specific workflow which could be dedicated for PRs...)

Copy link
Collaborator

Choose a reason for hiding this comment

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

but feel free to test by merging: we can fix with another PR if it does not work

Copy link
Member Author

Choose a reason for hiding this comment

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

@jku: Looks like this actually works:
https://github.com/secure-systems-lab/securesystemslib/actions/runs/4382839569

Build was triggered via pull request and apparently had the required permissions.

Or maybe it only worked because the pr didn't come from a fork. 🤷

Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe it only worked because the pr didn't come from a fork. shrug

yes, correct: I expect branches in upstream repo to work, but branches in forks to fail

workflow_dispatch:

permissions: {}

jobs:
test-sigstore:
runs-on: ubuntu-latest

permissions:
id-token: 'write' # ambient credential is used to sign

steps:
- name: Checkout securesystemslib
uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c

- name: Set up Python
uses: actions/setup-python@d27e3f3d7c64b4bbf8e4abfb9b63b83e846e0435
with:
python-version: '3.x'
cache: 'pip'
cache-dependency-path: 'requirements*.txt'

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install --upgrade tox

- run: |
export CERT_ID=${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/.github/workflows/test-sigstore.yml@${GITHUB_REF}
export CERT_ISSUER=https://token.actions.githubusercontent.com
tox -e sigstore
6 changes: 6 additions & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,9 @@ ignore_missing_imports = True

[mypy-asn1crypto.*]
ignore_missing_imports = True

[mypy-sigstore.*]
ignore_missing_imports = True

[mypy-sigstore_protobuf_specs.*]
ignore_missing_imports = True
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ gcpkms = ["google-cloud-kms", "cryptography>=37.0.0"]
hsm = ["asn1crypto", "cryptography>=37.0.0", "PyKCS11"]
pynacl = ["pynacl>1.2.0"]
PySPX = ["PySPX>=0.5.0"]
sigstore = ["sigstore"]

[tool.hatch.version]
path = "securesystemslib/__init__.py"
Expand Down
1 change: 1 addition & 0 deletions requirements-sigstore.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
sigstore==1.1.1
1 change: 1 addition & 0 deletions securesystemslib/signer/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
Signer,
SSlibSigner,
)
from securesystemslib.signer._sigstore_signer import SigstoreKey, SigstoreSigner

# Register supported private key uri schemes and the Signers implementing them
SIGNER_FOR_URI_SCHEME.update(
Expand Down
175 changes: 175 additions & 0 deletions securesystemslib/signer/_sigstore_signer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
"""Signer implementation for project sigstore.

Example:
```python
from sigstore.oidc import Issuer

from securesystemslib.signer import SigstoreKey, SigstoreSigner

# Create public key
identity = "[email protected]" # change, unless you know my password
issuer = "https://github.com/login/oauth"
public_key = SigstoreKey.from_dict(
"abcdefg",
{
"keytype": "sigstore-oidc",
"scheme": "Fulcio",
"keyval": {
"issuer": issuer,
"identity": identity,
},
},
)

# Create signer
issuer = Issuer.production()
token = issuer.identity_token() # requires sign in with GitHub in a browser
signer = SigstoreSigner(token, public_key)

# Sign
signature = signer.sign(b"data")

# Verify
public_key.verify_signature(signature, b"data")

```

"""

import io
import logging
from typing import Any, Dict, Optional

from securesystemslib.exceptions import (
UnsupportedLibraryError,
UnverifiedSignatureError,
VerificationError,
)
from securesystemslib.signer._signer import (
Key,
SecretsHandler,
Signature,
Signer,
)

IMPORT_ERROR = "sigstore library required to use 'sigstore-oidc' keys"

logger = logging.getLogger(__name__)


class SigstoreKey(Key):
"""Sigstore verifier.

NOTE: unstable API - routines and metadata formats may change!
"""

@classmethod
def from_dict(cls, keyid: str, key_dict: Dict[str, Any]) -> "SigstoreKey":
keytype = key_dict.pop("keytype")
scheme = key_dict.pop("scheme")
keyval = key_dict.pop("keyval")

for content in ["identity", "issuer"]:
if content not in keyval or not isinstance(keyval[content], str):
raise ValueError(
f"{content} string required for scheme {scheme}"
)

return cls(keyid, keytype, scheme, keyval, key_dict)

def to_dict(self) -> Dict:
return {
"keytype": self.keytype,
"scheme": self.scheme,
"keyval": self.keyval,
**self.unrecognized_fields,
}

def verify_signature(self, signature: Signature, data: bytes) -> None:
# pylint: disable=import-outside-toplevel,import-error
result = None
try:
from sigstore.verify import VerificationMaterials, Verifier
from sigstore.verify.policy import Identity
from sigstore_protobuf_specs.dev.sigstore.bundle.v1 import Bundle

verifier = Verifier.production()
Copy link
Collaborator

Choose a reason for hiding this comment

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

seeing this code it seems clear that the content in our key should include everything needed to choose the sigstore "instance".

That said, it's quite unclear to me what that all means (rekor and tuf urls? fulcio?): this is really more a question for the TAP than this implementation which should focus on supporting the default instance for now

So no action requested here.

identity = Identity(
identity=self.keyval["identity"], issuer=self.keyval["issuer"]
)
bundle = Bundle().from_dict(signature.unrecognized_fields["bundle"])
materials = VerificationMaterials.from_bundle(
input_=io.BytesIO(data), bundle=bundle, offline=True
)
result = verifier.verify(materials, identity)

except Exception as e:
logger.info("Key %s failed to verify sig: %s", self.keyid, str(e))
raise VerificationError(
f"Unknown failure to verify signature by {self.keyid}"
) from e

if not result:
logger.info(
"Key %s failed to verify sig: %s",
self.keyid,
getattr(result, "reason", ""),
)
raise UnverifiedSignatureError(
f"Failed to verify signature by {self.keyid}"
)


class SigstoreSigner(Signer):
"""Sigstore signer.

NOTE: unstable API - routines and metadata formats may change!
"""

def __init__(self, token: str, public_key: Key):
lukpueh marked this conversation as resolved.
Show resolved Hide resolved
# TODO: Vet public key
# - signer eligible for keytype/scheme?
# - token matches identity/issuer?
self.public_key = public_key
self._token = token

@classmethod
def from_priv_key_uri(
cls,
priv_key_uri: str,
public_key: Key,
secrets_handler: Optional[SecretsHandler] = None,
) -> "SigstoreSigner":
raise NotImplementedError()

def sign(self, payload: bytes) -> Signature:
"""Signs payload using the OIDC token on the signer instance.

Arguments:
payload: bytes to be signed.

Raises:
Various errors from sigstore-python.

Returns:
Signature.

NOTE: The relevant data is in `unrecognized_fields["bundle"]`.

"""
# pylint: disable=import-outside-toplevel
try:
from sigstore.sign import Signer as _Signer
except ImportError as e:
raise UnsupportedLibraryError(IMPORT_ERROR) from e

signer = _Signer.production()
result = signer.sign(io.BytesIO(payload), self._token)
# TODO: Ask upstream if they can make this public
bundle = result._to_bundle() # pylint: disable=protected-access

return Signature(
self.public_key.keyid,
bundle.message_signature.signature.hex(),
{"bundle": bundle.to_dict()},
)
66 changes: 66 additions & 0 deletions tests/check_sigstore_signer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
"""
Test SigstoreSigner API.

NOTE: The filename prefix is check_ instead of test_ so that tests are
only run when explicitly invoked in a suited environment.

"""
import os
import unittest

from sigstore.oidc import detect_credential # pylint: disable=import-error

from securesystemslib.signer import (
KEY_FOR_TYPE_AND_SCHEME,
Key,
SigstoreKey,
SigstoreSigner,
)

KEY_FOR_TYPE_AND_SCHEME.update(
{
("sigstore-oidc", "Fulcio"): SigstoreKey,
}
)


class TestSigstoreSigner(unittest.TestCase):
"""Test public key parsing, signature creation and verification.

Requires ambient credentials for signing (e.g. from GitHub Action).

See sigstore-python docs for more infos about ambient credentials:
https://github.com/sigstore/sigstore-python#signing-with-ambient-credentials

See securesystemslib SigstoreSigner docs for how to test locally.
"""

def test_sign(self):
token = detect_credential()
self.assertIsNotNone(token, "ambient credentials required")

identity = os.getenv("CERT_ID")
self.assertIsNotNone(token, "certificate identity required")

issuer = os.getenv("CERT_ISSUER")
self.assertIsNotNone(token, "OIDC issuer required")

public_key = Key.from_dict(
"abcdef",
{
"keytype": "sigstore-oidc",
"scheme": "Fulcio",
"keyval": {
"issuer": issuer,
"identity": identity,
},
},
)

signer = SigstoreSigner(token, public_key)
sig = signer.sign(b"data")
public_key.verify_signature(sig, b"data")


if __name__ == "__main__":
unittest.main(verbosity=4, buffer=False)
14 changes: 14 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,20 @@ passenv =
commands =
python -m tests.check_kms_signers

[testenv:sigstore]
deps =
-r{toxinidir}/requirements-pinned.txt
-r{toxinidir}/requirements-sigstore.txt
passenv =
# These are required to detect ambient credentials on GitHub
GITHUB_ACTIONS
ACTIONS_ID_TOKEN_REQUEST_TOKEN
ACTIONS_ID_TOKEN_REQUEST_URL
Comment on lines +54 to +56
Copy link
Collaborator

Choose a reason for hiding this comment

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

these are used by ambient credentials detection?

CERT_ID
CERT_ISSUER
commands =
python -m tests.check_sigstore_signer

# This checks that importing securesystemslib.gpg.constants doesn't shell out on
# import.
[testenv:py311-test-gpg-fails]
Expand Down