diff --git a/.github/workflows/staging-tests.yml b/.github/workflows/staging-tests.yml index ab63c073..749f5610 100644 --- a/.github/workflows/staging-tests.yml +++ b/.github/workflows/staging-tests.yml @@ -42,11 +42,11 @@ jobs: # Our signing target is not important here, so we just sign # the README in the repository. - ./staging-env/bin/python -m sigstore --staging sign README.md + ./staging-env/bin/python -m sigstore --verbose --staging sign README.md # Verification also requires a different Rekor instance, so we # also test it. - ./staging-env/bin/python -m sigstore --staging verify identity \ + ./staging-env/bin/python -m sigstore --verbose --staging verify identity \ --cert-oidc-issuer https://token.actions.githubusercontent.com \ --cert-identity ${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/.github/workflows/staging-tests.yml@${GITHUB_REF} \ README.md diff --git a/sigstore/oidc.py b/sigstore/oidc.py index dd8964dc..6c2b53b7 100644 --- a/sigstore/oidc.py +++ b/sigstore/oidc.py @@ -98,9 +98,16 @@ def __init__(self, raw_token: str) -> None: "verify_aud": True, "verify_iat": True, "verify_exp": True, - "require": ["aud", "iat", "exp", "iss"], + # These claims are required by OpenID Connect, so + # we can strongly enforce their presence. + # See: https://openid.net/specs/openid-connect-basic-1_0.html#IDToken + "require": ["aud", "sub", "iat", "exp", "iss"], }, audience=DEFAULT_AUDIENCE, + # NOTE: This leeway shouldn't be strictly necessary, but is + # included to preempt any (small) skew between the host + # and the originating IdP. + leeway=5, ) except Exception as exc: raise IdentityError( diff --git a/test/unit/test_oidc.py b/test/unit/test_oidc.py index f3d9b4b1..4ac7cc3f 100644 --- a/test/unit/test_oidc.py +++ b/test/unit/test_oidc.py @@ -31,6 +31,7 @@ def test_missing_iss(self, dummy_jwt): jwt = dummy_jwt( { "aud": "sigstore", + "sub": "fakesubject", "iat": now, "nbf": now, "exp": now + 600, @@ -46,6 +47,7 @@ def test_missing_aud(self, dummy_jwt): now = int(datetime.datetime.now().timestamp()) jwt = dummy_jwt( { + "sub": "fakesubject", "iat": now, "nbf": now, "exp": now + 600, @@ -64,6 +66,7 @@ def test_invalid_aud(self, dummy_jwt, aud): jwt = dummy_jwt( { "aud": aud, + "sub": "fakesubject", "iat": now, "nbf": now, "exp": now + 600, @@ -81,6 +84,7 @@ def test_missing_iat(self, dummy_jwt): jwt = dummy_jwt( { "aud": "sigstore", + "sub": "fakesubject", "nbf": now, "exp": now + 600, "iss": "fake-issuer", @@ -98,6 +102,7 @@ def test_invalid_iat(self, dummy_jwt, iat): jwt = dummy_jwt( { "aud": "sigstore", + "sub": "fakesubject", "iat": iat, "nbf": now, "exp": now + 600, @@ -129,6 +134,7 @@ def test_invalid_nbf(self, dummy_jwt): jwt = dummy_jwt( { "aud": "sigstore", + "sub": "fakesubject", "iat": now, "nbf": now + 600, "exp": now + 601, @@ -147,6 +153,7 @@ def test_missing_exp(self, dummy_jwt): jwt = dummy_jwt( { "aud": "sigstore", + "sub": "fakesubject", "iat": now, "nbf": now, "iss": "fake-issuer", @@ -163,9 +170,11 @@ def test_invalid_exp(self, dummy_jwt): jwt = dummy_jwt( { "aud": "sigstore", + "sub": "fakesubject", "iat": now - 600, "nbf": now - 300, - "exp": now - 1, + # NOTE: 6 seconds due to +/- 5 second flutter. + "exp": now - 6, "iss": "fake-issuer", } ) @@ -175,12 +184,15 @@ def test_invalid_exp(self, dummy_jwt): ): oidc.IdentityToken(jwt) - @pytest.mark.parametrize("iss", oidc._KNOWN_OIDC_ISSUERS.keys()) + @pytest.mark.parametrize( + "iss", [k for k, v in oidc._KNOWN_OIDC_ISSUERS.items() if v != "sub"] + ) def test_missing_identity_claim(self, dummy_jwt, iss): now = int(datetime.datetime.now().timestamp()) jwt = dummy_jwt( { "aud": "sigstore", + "sub": "fakesubject", "iat": now, "nbf": now, "exp": now + 600, @@ -200,6 +212,7 @@ def test_invalid_federated_claims(self, dummy_jwt, fed): jwt = dummy_jwt( { "aud": "sigstore", + "sub": "fakesubject", "iat": now, "nbf": now, "exp": now + 600, @@ -240,6 +253,7 @@ def test_ok(self, dummy_jwt, iss, identity_claim, identity_value, fed_iss): jwt = dummy_jwt( { "aud": "sigstore", + "sub": "fakesubject", "iat": now, "nbf": now, "exp": now + 600, diff --git a/test/unit/test_sign.py b/test/unit/test_sign.py index a4bea319..a030a495 100644 --- a/test/unit/test_sign.py +++ b/test/unit/test_sign.py @@ -45,8 +45,9 @@ def test_sign_rekor_entry_consistent(id_config): payload = io.BytesIO(secrets.token_bytes(32)) with ctx.signer(identity) as signer: - expected_entry = signer.sign(payload, identity).log_entry - actual_entry = signer._rekor.log.entries.get(log_index=expected_entry.log_index) + expected_entry = signer.sign(payload).log_entry + + actual_entry = ctx._rekor.log.entries.get(log_index=expected_entry.log_index) assert expected_entry.uuid == actual_entry.uuid assert expected_entry.body == actual_entry.body @@ -71,7 +72,7 @@ def test_sct_verify_keyring_lookup_error(id_config, monkeypatch): InvalidSCTError, ) as excinfo: with ctx.signer(identity) as signer: - signer.sign(payload, identity) + signer.sign(payload) # The exception subclass is the one we expect. assert isinstance(excinfo.value, InvalidSCTKeyError) @@ -109,7 +110,7 @@ def test_identity_proof_claim_lookup(id_config, monkeypatch): with ctx.signer(identity) as signer: expected_entry = signer.sign(payload).log_entry - actual_entry = signer._rekor.log.entries.get(log_index=expected_entry.log_index) + actual_entry = ctx._rekor.log.entries.get(log_index=expected_entry.log_index) assert expected_entry.uuid == actual_entry.uuid assert expected_entry.body == actual_entry.body