Skip to content

Commit 554483d

Browse files
woodruffwwoodruffw
authored
workflows: debug staging-tests (#669)
* workflows: debug staging-tests Signed-off-by: William Woodruff <[email protected]> * DEBUG: oidc: disable iat verification Signed-off-by: William Woodruff <[email protected]> * sigstore, test: ratchet subject handling Signed-off-by: William Woodruff <[email protected]> * test: lint Signed-off-by: William Woodruff <[email protected]> * oidc: add a little leeway Signed-off-by: William Woodruff <[email protected]> * test: fixup OIDC test Signed-off-by: William Woodruff <[email protected]> * test: fixup signing tests Signed-off-by: William Woodruff <[email protected]> * oidc: document leeway Signed-off-by: William Woodruff <[email protected]> * workflows/staging-tests: undo debug changes Signed-off-by: William Woodruff <[email protected]> --------- Signed-off-by: William Woodruff <[email protected]>
1 parent 747841e commit 554483d

File tree

4 files changed

+31
-9
lines changed

4 files changed

+31
-9
lines changed

‎.github/workflows/staging-tests.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,11 @@ jobs:
4242
4343
# Our signing target is not important here, so we just sign
4444
# the README in the repository.
45-
./staging-env/bin/python -m sigstore --staging sign README.md
45+
./staging-env/bin/python -m sigstore --verbose --staging sign README.md
4646
4747
# Verification also requires a different Rekor instance, so we
4848
# also test it.
49-
./staging-env/bin/python -m sigstore --staging verify identity \
49+
./staging-env/bin/python -m sigstore --verbose --staging verify identity \
5050
--cert-oidc-issuer https://token.actions.githubusercontent.com \
5151
--cert-identity ${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/.github/workflows/staging-tests.yml@${GITHUB_REF} \
5252
README.md

‎sigstore/oidc.py

+8-1
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,16 @@ def __init__(self, raw_token: str) -> None:
9898
"verify_aud": True,
9999
"verify_iat": True,
100100
"verify_exp": True,
101-
"require": ["aud", "iat", "exp", "iss"],
101+
# These claims are required by OpenID Connect, so
102+
# we can strongly enforce their presence.
103+
# See: https://openid.net/specs/openid-connect-basic-1_0.html#IDToken
104+
"require": ["aud", "sub", "iat", "exp", "iss"],
102105
},
103106
audience=DEFAULT_AUDIENCE,
107+
# NOTE: This leeway shouldn't be strictly necessary, but is
108+
# included to preempt any (small) skew between the host
109+
# and the originating IdP.
110+
leeway=5,
104111
)
105112
except Exception as exc:
106113
raise IdentityError(

‎test/unit/test_oidc.py

+16-2
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ def test_missing_iss(self, dummy_jwt):
3131
jwt = dummy_jwt(
3232
{
3333
"aud": "sigstore",
34+
"sub": "fakesubject",
3435
"iat": now,
3536
"nbf": now,
3637
"exp": now + 600,
@@ -46,6 +47,7 @@ def test_missing_aud(self, dummy_jwt):
4647
now = int(datetime.datetime.now().timestamp())
4748
jwt = dummy_jwt(
4849
{
50+
"sub": "fakesubject",
4951
"iat": now,
5052
"nbf": now,
5153
"exp": now + 600,
@@ -64,6 +66,7 @@ def test_invalid_aud(self, dummy_jwt, aud):
6466
jwt = dummy_jwt(
6567
{
6668
"aud": aud,
69+
"sub": "fakesubject",
6770
"iat": now,
6871
"nbf": now,
6972
"exp": now + 600,
@@ -81,6 +84,7 @@ def test_missing_iat(self, dummy_jwt):
8184
jwt = dummy_jwt(
8285
{
8386
"aud": "sigstore",
87+
"sub": "fakesubject",
8488
"nbf": now,
8589
"exp": now + 600,
8690
"iss": "fake-issuer",
@@ -98,6 +102,7 @@ def test_invalid_iat(self, dummy_jwt, iat):
98102
jwt = dummy_jwt(
99103
{
100104
"aud": "sigstore",
105+
"sub": "fakesubject",
101106
"iat": iat,
102107
"nbf": now,
103108
"exp": now + 600,
@@ -129,6 +134,7 @@ def test_invalid_nbf(self, dummy_jwt):
129134
jwt = dummy_jwt(
130135
{
131136
"aud": "sigstore",
137+
"sub": "fakesubject",
132138
"iat": now,
133139
"nbf": now + 600,
134140
"exp": now + 601,
@@ -147,6 +153,7 @@ def test_missing_exp(self, dummy_jwt):
147153
jwt = dummy_jwt(
148154
{
149155
"aud": "sigstore",
156+
"sub": "fakesubject",
150157
"iat": now,
151158
"nbf": now,
152159
"iss": "fake-issuer",
@@ -163,9 +170,11 @@ def test_invalid_exp(self, dummy_jwt):
163170
jwt = dummy_jwt(
164171
{
165172
"aud": "sigstore",
173+
"sub": "fakesubject",
166174
"iat": now - 600,
167175
"nbf": now - 300,
168-
"exp": now - 1,
176+
# NOTE: 6 seconds due to +/- 5 second flutter.
177+
"exp": now - 6,
169178
"iss": "fake-issuer",
170179
}
171180
)
@@ -175,12 +184,15 @@ def test_invalid_exp(self, dummy_jwt):
175184
):
176185
oidc.IdentityToken(jwt)
177186

178-
@pytest.mark.parametrize("iss", oidc._KNOWN_OIDC_ISSUERS.keys())
187+
@pytest.mark.parametrize(
188+
"iss", [k for k, v in oidc._KNOWN_OIDC_ISSUERS.items() if v != "sub"]
189+
)
179190
def test_missing_identity_claim(self, dummy_jwt, iss):
180191
now = int(datetime.datetime.now().timestamp())
181192
jwt = dummy_jwt(
182193
{
183194
"aud": "sigstore",
195+
"sub": "fakesubject",
184196
"iat": now,
185197
"nbf": now,
186198
"exp": now + 600,
@@ -200,6 +212,7 @@ def test_invalid_federated_claims(self, dummy_jwt, fed):
200212
jwt = dummy_jwt(
201213
{
202214
"aud": "sigstore",
215+
"sub": "fakesubject",
203216
"iat": now,
204217
"nbf": now,
205218
"exp": now + 600,
@@ -240,6 +253,7 @@ def test_ok(self, dummy_jwt, iss, identity_claim, identity_value, fed_iss):
240253
jwt = dummy_jwt(
241254
{
242255
"aud": "sigstore",
256+
"sub": "fakesubject",
243257
"iat": now,
244258
"nbf": now,
245259
"exp": now + 600,

‎test/unit/test_sign.py

+5-4
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,9 @@ def test_sign_rekor_entry_consistent(id_config):
4545

4646
payload = io.BytesIO(secrets.token_bytes(32))
4747
with ctx.signer(identity) as signer:
48-
expected_entry = signer.sign(payload, identity).log_entry
49-
actual_entry = signer._rekor.log.entries.get(log_index=expected_entry.log_index)
48+
expected_entry = signer.sign(payload).log_entry
49+
50+
actual_entry = ctx._rekor.log.entries.get(log_index=expected_entry.log_index)
5051

5152
assert expected_entry.uuid == actual_entry.uuid
5253
assert expected_entry.body == actual_entry.body
@@ -71,7 +72,7 @@ def test_sct_verify_keyring_lookup_error(id_config, monkeypatch):
7172
InvalidSCTError,
7273
) as excinfo:
7374
with ctx.signer(identity) as signer:
74-
signer.sign(payload, identity)
75+
signer.sign(payload)
7576

7677
# The exception subclass is the one we expect.
7778
assert isinstance(excinfo.value, InvalidSCTKeyError)
@@ -109,7 +110,7 @@ def test_identity_proof_claim_lookup(id_config, monkeypatch):
109110

110111
with ctx.signer(identity) as signer:
111112
expected_entry = signer.sign(payload).log_entry
112-
actual_entry = signer._rekor.log.entries.get(log_index=expected_entry.log_index)
113+
actual_entry = ctx._rekor.log.entries.get(log_index=expected_entry.log_index)
113114

114115
assert expected_entry.uuid == actual_entry.uuid
115116
assert expected_entry.body == actual_entry.body

0 commit comments

Comments
 (0)