Skip to content

Commit 5e77497

Browse files
authored
Merge commit from fork
* Add state validation to OIDC flow to prevent CSRF The OIDC flow in did not verify the `state` parameter returned by the identity provider against the state sent in the request. This could allow tricking the user into using an authorization code obtained by the attacker. All of the response parsing should arguably be moved to _OAuthRedirectServer (to hide the details from `sigstore.oidc`) but I wanted to keep this fix minimal. Test generated by AI. Signed-off-by: Jussi Kukkonen <jkukkonen@google.com> * CHANGELOG: Mention CSRF fix Signed-off-by: Jussi Kukkonen <jkukkonen@google.com> --------- Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
1 parent ae9caa7 commit 5e77497

4 files changed

Lines changed: 69 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ All versions prior to 0.9.0 are untracked.
1010

1111
### Fixed
1212

13+
* Add state validation to OIDC flow to prevent Cross-site request forgery
14+
during OIDC authorization
15+
([GHSA-hm8f-75xx-w2vr](https://github.com/sigstore/sigstore-python/security/advisories/GHSA-hm8f-75xx-w2vr))
1316
* verification now ensures that artifact digest documented in bundle and the real digest match
1417
(this is a bundle consistency check: bundle signature was always verified over real digest)
1518
([#1652](https://github.com/sigstore/sigstore-python/pull/1652))

sigstore/_internal/oidc/oauth.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,10 @@ def __init__(self, client_id: str, client_secret: str, issuer: Issuer):
182182
base64.urlsafe_b64encode(os.urandom(32)).rstrip(b"=").decode()
183183
)
184184

185+
@property
186+
def state(self) -> str:
187+
return self._state
188+
185189
@property
186190
def code_challenge(self) -> str:
187191
return B64Str(

sigstore/oidc.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,10 @@ def identity_token( # nosec: B107
312312
raise IdentityError(
313313
f"Error response from auth endpoint: {auth_error[0]}"
314314
)
315+
316+
if server.auth_response["state"][0] != server.oauth_session.state:
317+
raise IdentityError("OAuth state mismatch")
318+
315319
code = server.auth_response["code"][0]
316320
else:
317321
# In the out-of-band case, we wait until the user provides the code

test/unit/internal/oidc/test_issuer.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
from unittest.mock import MagicMock, patch
16+
1517
import pytest
1618

1719
from sigstore.oidc import IdentityError, Issuer, IssuerError
@@ -34,3 +36,59 @@ def test_get_identity_token_bad_code(monkeypatch):
3436
monkeypatch.setattr("builtins.input", lambda _: "hunter2")
3537
with pytest.raises(IdentityError, match=r"^Token request failed with .+$"):
3638
Issuer("https://oauth2.sigstage.dev/auth").identity_token(force_oob=True)
39+
40+
41+
def test_identity_token_csrf_protection():
42+
"""
43+
Verify that identity_token() raises IdentityError when the returned state
44+
does not match the session state (CSRF protection).
45+
"""
46+
with (
47+
patch("sigstore.oidc.webbrowser.open"),
48+
patch("sigstore._internal.oidc.oauth._OAuthFlow") as MockOAuthFlow,
49+
patch("sigstore.oidc.requests.Session") as MockSession,
50+
patch("sigstore.oidc.IdentityToken"),
51+
):
52+
# Setup the mock server returned by _OAuthFlow context manager
53+
mock_server = MagicMock()
54+
MockOAuthFlow.return_value.__enter__.return_value = mock_server
55+
56+
# Simulate a mismatching state
57+
original_state = "original-secure-state"
58+
malicious_state = "malicious-state"
59+
60+
# The session has the original state (we mock the property access)
61+
# Since we added a property 'state', we need to make sure the mock returns it.
62+
# But here we are mocking the whole server object.
63+
# server.oauth_session.state
64+
mock_server.oauth_session.state = original_state
65+
66+
mock_server.is_oob.return_value = False
67+
mock_server.base_uri = "http://localhost:12345"
68+
mock_server.redirect_uri = "http://localhost:12345/callback"
69+
70+
# The auth response simulates what the redirect handler receives
71+
mock_server.auth_response = {
72+
"code": ["fake-code"],
73+
"state": [malicious_state],
74+
}
75+
76+
# Mock responses for Issuer initialization and token exchange
77+
mock_session_instance = MockSession.return_value
78+
79+
# Mock .well-known/openid-configuration response
80+
mock_config_response = MagicMock()
81+
mock_config_response.json.return_value = {
82+
"authorization_endpoint": "https://auth.example.com",
83+
"token_endpoint": "https://token.example.com",
84+
}
85+
mock_config_response.raise_for_status.return_value = None
86+
87+
mock_session_instance.get.side_effect = [mock_config_response]
88+
89+
# Initialize Issuer
90+
issuer = Issuer("https://issuer.example.com")
91+
92+
# Call identity_token() and expect IdentityError due to state mismatch
93+
with pytest.raises(IdentityError, match="OAuth state mismatch"):
94+
issuer.identity_token()

0 commit comments

Comments
 (0)