diff --git a/README.md b/README.md index 36cbaf8..64ba918 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ The platform exposes four subsystems: - **Actions** — outbound SCM operations initiated by Sentry code. The `SourceCodeManager` class provides 70+ methods covering comments, reactions, pull requests, branches, git objects, reviews, and check runs. With more actions planned to be added as we port more use cases. - **Actions RPC** - outbound SCM commands exposed over the network. The `SourceCodeManager` is fully available over the network. This enables usage of the SCM for services outside the monolith. -- **Git relay** - `git` fetches and pushes for services that hold no provider credentials. Seer signs a grant naming the organization, operation and repositories with `scm.git.grant.sign_grant` and an Ed25519 private key only Seer holds. `scm.git.server.GitServer` verifies the grant against the matching public keys, so whoever verifies grants cannot mint one, and checks each git smart-HTTP request against it before relaying the request to the provider with a token, streaming both bodies. A missing or unusable key on either side raises `scm.errors.GitGrantKeyError`, the server's misconfiguration rather than a refused grant. `require_private_key` and `require_public_keys` check the keys at startup, and `generate_private_key` makes a key for tests and local tools. +- **Git relay** - `git` fetches and pushes for services that hold no provider credentials. Seer signs a grant naming the organization, operation and repositories with `scm.git.grant.sign_grant` and an Ed25519 private key only Seer holds. `scm.git.server.GitServer` verifies the grant against the matching public keys, so whoever verifies grants cannot mint one, and checks each git smart-HTTP request against it before relaying the request to the provider with a token, streaming both bodies. A missing or unusable key on either side raises `scm.errors.GitGrantKeyError`, the server's misconfiguration rather than a refused grant. `require_private_key` and `require_public_keys` check the keys at startup, and `generate_private_key` makes a key for tests and local tools. `key_id` gives a public key's `kid`, the id each grant names its signing key by, so a log can name the key that signed a grant without quoting it. - **Event Stream** — inbound webhook processing. SCM service-providers push events which are deserialized into typed, provider-neutral dataclasses (`CheckRunEvent`, `CommentEvent`, `PullRequestEvent`) which are then dispatched to registered listener functions. ### Why diff --git a/src/scm/git/grant.py b/src/scm/git/grant.py index 400ee2a..7763458 100644 --- a/src/scm/git/grant.py +++ b/src/scm/git/grant.py @@ -10,6 +10,8 @@ Keys are PEM strings, as for objectstore's tokens: the private key as PKCS#8 (``-----BEGIN PRIVATE KEY-----``), public keys as SubjectPublicKeyInfo (``-----BEGIN PUBLIC KEY-----``). ``openssl genpkey -algorithm ed25519`` or ``generate_private_key`` makes a private key, and ``openssl pkey -pubout`` or ``public_key_of`` gives its public key. +``key_id`` gives a public key's ``kid``, so a signer can log which key signed a grant and an operator can tell the keys +in the verifier's list apart. A key that is missing or unusable raises ``GitGrantKeyError`` from every function that takes it, whichever side it is on. ``require_private_key`` and ``require_public_keys`` check the keys on their own, so a signer or a verifier @@ -79,6 +81,16 @@ def public_key_of(private_key: Secret) -> str: return public_key.public_bytes(Encoding.PEM, PublicFormat.SubjectPublicKeyInfo).decode() +def key_id(public_key: str) -> str: + """Return the ``kid`` that grants signed by ``public_key``'s private key carry, for an Ed25519 public key in PEM. + + The id is derived from the key, so it names a key in logs without quoting it. A signer, which holds only its + private key, gets its own with ``key_id(public_key_of(private_key))``. Raises GitGrantKeyError, as + ``verify_grant`` does, if ``public_key`` is not an Ed25519 public key in PEM. + """ + return _key_id(_load_public_key(public_key, "The git grant public key")) + + def generate_private_key() -> Secret: """Return a new Ed25519 private key to sign grants with, in PEM (PKCS#8). @@ -195,28 +207,27 @@ def _public_keys_by_id(public_keys: Sequence[str]) -> dict[str, Ed25519PublicKey raise GitGrantKeyError("There are no git grant public keys, so no grant could verify.") keys: dict[str, Ed25519PublicKey] = {} for index, pem in enumerate(public_keys): - key = _load_public_key(pem, index) + key = _load_public_key(pem, f"public_keys[{index}]") keys[_key_id(key)] = key return keys -def _load_public_key(pem: str, index: int) -> Ed25519PublicKey: - # Each message names the key by its place in the list rather than quoting it. +def _load_public_key(pem: str, name: str) -> Ed25519PublicKey: + # Each message names the key, e.g. by its place in the list, rather than quoting it. if not isinstance(pem, str): - raise GitGrantKeyError(f"public_keys[{index}] must be a PEM string, not {type(pem).__name__}.") + raise GitGrantKeyError(f"{name} must be a PEM string, not {type(pem).__name__}.") # load_pem_public_key refuses a private key too, but saying what it is tells the operator to move it away from # where verifiers read their keys. if "PRIVATE KEY" in pem: raise GitGrantKeyError( - f"public_keys[{index}] is a private key. Verifiers take public keys only; the private key stays with the " - "signer." + f"{name} is a private key. Verifiers take public keys only; the private key stays with the signer." ) try: key = load_pem_public_key(pem.encode()) except (ValueError, UnsupportedAlgorithm) as e: - raise GitGrantKeyError(f"public_keys[{index}] is not a public key in PEM (-----BEGIN PUBLIC KEY-----).") from e + raise GitGrantKeyError(f"{name} is not a public key in PEM (-----BEGIN PUBLIC KEY-----).") from e if not isinstance(key, Ed25519PublicKey): - raise GitGrantKeyError(f"public_keys[{index}] must be an Ed25519 key, not {type(key).__name__}.") + raise GitGrantKeyError(f"{name} must be an Ed25519 key, not {type(key).__name__}.") return key diff --git a/tests/unit/test_git_grant.py b/tests/unit/test_git_grant.py index f939281..a8d07ff 100644 --- a/tests/unit/test_git_grant.py +++ b/tests/unit/test_git_grant.py @@ -24,6 +24,7 @@ from scm.git.grant import ( GRANT_AUDIENCE, generate_private_key, + key_id, public_key_of, require_private_key, require_public_keys, @@ -43,7 +44,8 @@ OTHER_KEY = make_key_pair() -def key_id(key: KeyPair) -> str: +def expected_key_id(key: KeyPair) -> str: + """The kid derived here from the raw key, so the tests check key_id and sign_grant against it.""" raw = load_pem_public_key(key.public.encode()).public_bytes(Encoding.Raw, PublicFormat.Raw) return hashlib.sha256(raw).hexdigest()[:16] @@ -56,7 +58,7 @@ def claims_of(grant: Grant) -> dict: def sign_claims(claims: dict, key: Any = KEY.key, algorithm: str = "EdDSA", kid: str | None = None) -> str: """Sign arbitrary claims the way sign_grant does, to reach the checks that run after the signature.""" - return jwt.encode(claims, key, algorithm=algorithm, headers={"kid": kid or key_id(KEY)}) + return jwt.encode(claims, key, algorithm=algorithm, headers={"kid": kid or expected_key_id(KEY)}) def b64url(data: bytes) -> str: @@ -121,7 +123,7 @@ def test_push_grant(self): def test_token_is_a_jwt_with_the_grants_claims(self): token = sign_grant(KEY.private, make_grant(), now=NOW) - assert jwt.get_unverified_header(token) == {"alg": "EdDSA", "typ": "JWT", "kid": key_id(KEY)} + assert jwt.get_unverified_header(token) == {"alg": "EdDSA", "typ": "JWT", "kid": expected_key_id(KEY)} assert jwt.decode(token, options={"verify_signature": False}) == { "organization_id": 1, "referrer": "seer.coding_job", @@ -136,8 +138,8 @@ def test_token_is_a_jwt_with_the_grants_claims(self): def test_key_id_is_the_same_for_every_grant_a_key_signs(self): # The signer derives it from its private key and the verifier from the public key, with no setting to match. tokens = [sign_grant(KEY.private, make_grant(organization_id=i), now=NOW) for i in (1, 2)] - assert {jwt.get_unverified_header(token)["kid"] for token in tokens} == {key_id(KEY)} - assert key_id(KEY) != key_id(OTHER_KEY) + assert {jwt.get_unverified_header(token)["kid"] for token in tokens} == {expected_key_id(KEY)} + assert expected_key_id(KEY) != expected_key_id(OTHER_KEY) def test_public_key_with_surrounding_whitespace(self): # As a setting read from a file or an environment variable may have. @@ -243,7 +245,7 @@ def test_hs256_keyed_with_the_public_key(self): # The public key is no secret, so a verifier that let a token pick HS256 and used the key it holds as the HMAC # key would accept a grant from anyone. PyJWT refuses to sign with a PEM as an HMAC key, so it is made here. token = handmade_token( - {"alg": "HS256", "typ": "JWT", "kid": key_id(KEY)}, + {"alg": "HS256", "typ": "JWT", "kid": expected_key_id(KEY)}, claims_of(make_grant()), lambda signing_input: hmac.digest(KEY.public.encode(), signing_input, "sha256"), ) @@ -255,7 +257,7 @@ def test_hs256(self): assert_invalid(token, "alg value is not allowed") def test_unsigned_token(self): - token = jwt.encode(claims_of(make_grant()), None, algorithm="none", headers={"kid": key_id(KEY)}) + token = jwt.encode(claims_of(make_grant()), None, algorithm="none", headers={"kid": expected_key_id(KEY)}) assert_invalid(token, "alg value is not allowed") def test_other_asymmetric_algorithm(self): @@ -421,6 +423,51 @@ def test_other_key_type(self, load, key): assert_quotes_no_key(exc_info.value, pem) +class TestKeyId: + def test_kid_of_the_grants_the_key_signs(self): + token = sign_grant(KEY.private, make_grant(), now=NOW) + assert key_id(KEY.public) == jwt.get_unverified_header(token)["kid"] == expected_key_id(KEY) + + def test_signer_gets_its_own_from_its_private_key(self): + private_key = generate_private_key() + token = sign_grant(private_key, make_grant(), now=NOW) + assert key_id(public_key_of(private_key)) == jwt.get_unverified_header(token)["kid"] + + def test_differs_between_keys(self): + assert key_id(KEY.public) != key_id(OTHER_KEY.public) + + def test_public_key_with_surrounding_whitespace(self): + assert key_id(f"\n {KEY.public}\n\n") == key_id(KEY.public) + + def test_private_key(self): + pem = KEY.private.reveal() + with pytest.raises(GitGrantKeyError, match=r"^The git grant public key is a private key\.") as exc_info: + key_id(pem) + assert_quotes_no_key(exc_info.value, pem) + + def test_private_key_as_a_secret(self): + with pytest.raises(GitGrantKeyError, match=r"^The git grant public key must be a PEM string, not Secret\."): + key_id(KEY.private) # type: ignore[arg-type] + + @pytest.mark.parametrize( + "pem", + ["", "not a key", OTHER_KEY.public[:60], pem_body(OTHER_KEY.public)], + ids=["empty", "garbage", "truncated", "no-delimiters"], + ) + def test_not_a_public_key(self, pem): + with pytest.raises(GitGrantKeyError, match=r"^The git grant public key is not a public key in PEM") as exc_info: + key_id(pem) + assert_quotes_no_key(exc_info.value, pem) + + @pytest.mark.parametrize("key", other_key_types(), ids=["rsa", "ec", "ed448"]) + def test_other_key_type(self, key): + pem = public_pem(key) + expected = f"^The git grant public key must be an Ed25519 key, not {type(key.public_key()).__name__}" + with pytest.raises(GitGrantKeyError, match=expected) as exc_info: + key_id(pem) + assert_quotes_no_key(exc_info.value, pem) + + class TestRotatedKeys: @pytest.mark.parametrize("signer", [OLD_KEY, NEW_KEY], ids=["old", "new"]) def test_both_keys_verify_during_rotation(self, signer):