warehouse, tests: handle malformed JWTs gracefully (#13541)

* warehouse, tests: handle malformed JWTs gracefully

* oidc/services: scope sentry on exception

Signed-off-by: William Woodruff <[email protected]>

* oidc/services: even more scopeage

Signed-off-by: William Woodruff <[email protected]>

* test: fix OIDC service tests

Use a slightly less invasive monkeypatch/stub.

Signed-off-by: William Woodruff <[email protected]>

---------

Signed-off-by: William Woodruff <[email protected]>

Author: woodruffw

tests/unit/oidc/test_services.py

@@ -15,7 +15,7 @@
 import pytest
 
 from cryptography.hazmat.primitives.asymmetric import rsa
-from jwt import PyJWK, PyJWTError, algorithms
+from jwt import DecodeError, PyJWK, PyJWTError, algorithms
 from zope.interface.verify import verifyClass
 
 from tests.common.db.oidc import GitHubPublisherFactory, PendingGitHubPublisherFactory
@@ -104,6 +104,44 @@ def test_verify_jwt_signature(self, monkeypatch):
             )
         ]
 
+    @pytest.mark.parametrize("exc", [DecodeError, TypeError("foo")])
+    def test_verify_jwt_signature_get_key_for_token_fails(self, monkeypatch, exc):
+        service = services.OIDCPublisherService(
+            session=pretend.stub(),
+            publisher="fakepublisher",
+            issuer_url=pretend.stub(),
+            audience="fakeaudience",
+            cache_url=pretend.stub(),
+            metrics=pretend.stub(
+                increment=pretend.call_recorder(lambda *a, **kw: None)
+            ),
+        )
+
+        token = pretend.stub()
+        jwt = pretend.stub(decode=pretend.raiser(exc), PyJWTError=PyJWTError)
+        monkeypatch.setattr(service, "_get_key_for_token", pretend.raiser(exc))
+        monkeypatch.setattr(services, "jwt", jwt)
+        monkeypatch.setattr(
+            services.sentry_sdk,
+            "capture_message",
+            pretend.call_recorder(lambda s: None),
+        )
+
+        assert service.verify_jwt_signature(token) is None
+        assert service.metrics.increment.calls == [
+            pretend.call(
+                "warehouse.oidc.verify_jwt_signature.malformed_jwt",
+                tags=["publisher:fakepublisher"],
+            )
+        ]
+
+        if exc != DecodeError:
+            assert services.sentry_sdk.capture_message.calls == [
+                pretend.call(f"JWT backend raised generic error: {exc}")
+            ]
+        else:
+            assert services.sentry_sdk.capture_message.calls == []
+
     @pytest.mark.parametrize("exc", [PyJWTError, TypeError("foo")])
     def test_verify_jwt_signature_fails(self, monkeypatch, exc):
         service = services.OIDCPublisherService(
@@ -124,9 +162,11 @@ def test_verify_jwt_signature_fails(self, monkeypatch, exc):
             service, "_get_key_for_token", pretend.call_recorder(lambda t: key)
         )
         monkeypatch.setattr(services, "jwt", jwt)
-
-        sentry_sdk = pretend.stub(capture_message=pretend.call_recorder(lambda s: None))
-        monkeypatch.setattr(services, "sentry_sdk", sentry_sdk)
+        monkeypatch.setattr(
+            services.sentry_sdk,
+            "capture_message",
+            pretend.call_recorder(lambda s: None),
+        )
 
         assert service.verify_jwt_signature(token) is None
         assert service.metrics.increment.calls == [
@@ -137,11 +177,11 @@ def test_verify_jwt_signature_fails(self, monkeypatch, exc):
         ]
 
         if exc != PyJWTError:
-            assert sentry_sdk.capture_message.calls == [
-                pretend.call(f"JWT verify raised generic error: {exc}")
+            assert services.sentry_sdk.capture_message.calls == [
+                pretend.call(f"JWT backend raised generic error: {exc}")
             ]
         else:
-            assert sentry_sdk.capture_message.calls == []
+            assert services.sentry_sdk.capture_message.calls == []
 
     def test_find_publisher(self, monkeypatch):
         service = services.OIDCPublisherService(

warehouse/oidc/services.py

@@ -220,7 +220,23 @@ def _get_key_for_token(self, token):
         return self._get_key(unverified_header["kid"])
 
     def verify_jwt_signature(self, unverified_token: str) -> SignedClaims | None:
-        key = self._get_key_for_token(unverified_token)
+        try:
+            key = self._get_key_for_token(unverified_token)
+        except Exception as e:
+            # The user might feed us an entirely nonsense JWT, e.g. one
+            # with missing components.
+            self.metrics.increment(
+                "warehouse.oidc.verify_jwt_signature.malformed_jwt",
+                tags=[f"publisher:{self.publisher}"],
+            )
+
+            if not isinstance(e, jwt.PyJWTError):
+                with sentry_sdk.push_scope() as scope:
+                    scope.fingerprint = e
+                    # Similar to below: Other exceptions indicate an abstraction
+                    # leak, so we log them for upstream reporting.
+                    sentry_sdk.capture_message(f"JWT backend raised generic error: {e}")
+            return None
 
         try:
             # NOTE: Many of the keyword arguments here are defaults, but we
@@ -252,10 +268,12 @@ def verify_jwt_signature(self, unverified_token: str) -> SignedClaims | None:
                 tags=[f"publisher:{self.publisher}"],
             )
             if not isinstance(e, jwt.PyJWTError):
-                # We expect pyjwt to only raise subclasses of PyJWTError, but
-                # we can't enforce this. Other exceptions indicate an abstraction
-                # leak, so we log them for upstream reporting.
-                sentry_sdk.capture_message(f"JWT verify raised generic error: {e}")
+                with sentry_sdk.push_scope() as scope:
+                    scope.fingerprint = e
+                    # We expect pyjwt to only raise subclasses of PyJWTError, but
+                    # we can't enforce this. Other exceptions indicate an abstraction
+                    # leak, so we log them for upstream reporting.
+                    sentry_sdk.capture_message(f"JWT backend raised generic error: {e}")
             return None
 
     def find_publisher(