Skip to content

Commit f355d89

Browse files
ceorourkeceorourke
authored
fix(integrations): Fix security vulnerabilities in Jira (#112409)
1 parent c51f7a0 commit f355d89

File tree

2 files changed

+141
-22
lines changed

2 files changed

+141
-22
lines changed

‎src/sentry/integrations/jira/webhooks/installed.py‎

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import sentry_sdk
22
from django.db import router, transaction
3+
from jwt import DecodeError, ExpiredSignatureError, InvalidKeyError, InvalidSignatureError
34
from rest_framework import status
45
from rest_framework.request import Request
56
from rest_framework.response import Response
@@ -20,9 +21,6 @@
2021
)
2122
from sentry.utils import jwt
2223

23-
# Atlassian sends scanner bots to "test" Atlassian apps and they often hit this endpoint with a bad kid causing errors
24-
INVALID_KEY_IDS = ["fake-kid"]
25-
2624

2725
@control_silo_endpoint
2826
class JiraSentryInstalledWebhook(JiraWebhookBase):
@@ -46,7 +44,14 @@ def post(self, request: Request, *args, **kwargs) -> Response:
4644
lifecycle.record_failure(ProjectManagementFailuresReason.INSTALLATION_STATE_MISSING)
4745
return self.respond(status=status.HTTP_400_BAD_REQUEST)
4846

49-
key_id = jwt.peek_header(token).get("kid")
47+
try:
48+
key_id = jwt.peek_header(token).get("kid")
49+
except DecodeError:
50+
lifecycle.record_halt(halt_reason="Failed to fetch key id")
51+
return self.respond(
52+
{"detail": "Failed to fetch key id"}, status=status.HTTP_400_BAD_REQUEST
53+
)
54+
5055
lifecycle.add_extras(
5156
{
5257
"key_id": key_id,
@@ -56,14 +61,34 @@ def post(self, request: Request, *args, **kwargs) -> Response:
5661
}
5762
)
5863

59-
if key_id:
60-
if key_id in INVALID_KEY_IDS:
61-
lifecycle.record_halt(halt_reason="JWT contained invalid key_id (kid)")
62-
return self.respond(
63-
{"detail": "Invalid key id"}, status=status.HTTP_400_BAD_REQUEST
64-
)
64+
if not key_id:
65+
lifecycle.record_halt(halt_reason="Missing key_id (kid)")
66+
return self.respond(
67+
{"detail": "Missing key id"}, status=status.HTTP_400_BAD_REQUEST
68+
)
69+
try:
6570
decoded_claims = authenticate_asymmetric_jwt(token, key_id)
6671
verify_claims(decoded_claims, request.path, request.GET, method="POST")
72+
except InvalidKeyError:
73+
lifecycle.record_halt(halt_reason="JWT contained invalid key_id (kid)")
74+
return self.respond(
75+
{"detail": "Invalid key id"}, status=status.HTTP_400_BAD_REQUEST
76+
)
77+
except ExpiredSignatureError as e:
78+
lifecycle.record_failure(e)
79+
return self.respond(
80+
{"detail": "Expired signature"}, status=status.HTTP_400_BAD_REQUEST
81+
)
82+
except InvalidSignatureError:
83+
lifecycle.record_halt(halt_reason="JWT contained invalid signature")
84+
return self.respond(
85+
{"detail": "Invalid signature"}, status=status.HTTP_400_BAD_REQUEST
86+
)
87+
except DecodeError:
88+
lifecycle.record_halt(halt_reason="Could not decode JWT token")
89+
return self.respond(
90+
{"detail": "Could not decode JWT token"}, status=status.HTTP_400_BAD_REQUEST
91+
)
6792

6893
data = JiraIntegrationProvider().build_integration(state)
6994
integration = ensure_integration(self.provider, data)

‎tests/sentry/integrations/jira/test_installed.py‎

Lines changed: 106 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import jwt
88
import responses
9+
from jwt import DecodeError, ExpiredSignatureError, InvalidSignatureError
910
from rest_framework import status
1011

1112
from sentry.constants import ObjectStatus
@@ -16,7 +17,11 @@
1617
AtlassianConnectValidationError,
1718
get_query_hash,
1819
)
19-
from sentry.testutils.asserts import assert_count_of_metric, assert_halt_metric
20+
from sentry.testutils.asserts import (
21+
assert_count_of_metric,
22+
assert_failure_metric,
23+
assert_halt_metric,
24+
)
2025
from sentry.testutils.cases import APITestCase
2126
from sentry.testutils.silo import control_silo_test
2227
from sentry.utils.http import absolute_uri
@@ -49,9 +54,6 @@ def _jwt_token(
4954
headers={**(headers or {}), "alg": jira_signing_algorithm},
5055
)
5156

52-
def jwt_token_secret(self):
53-
return self._jwt_token("HS256", self.shared_secret)
54-
5557
def jwt_token_cdn(self):
5658
return self._jwt_token("RS256", RS256_KEY, headers={"kid": self.kid})
5759

@@ -110,18 +112,83 @@ def test_no_claims(self, mock_authenticate_asymmetric_jwt: MagicMock) -> None:
110112
status_code=status.HTTP_409_CONFLICT,
111113
)
112114

115+
@patch(
116+
"sentry.integrations.jira.webhooks.installed.authenticate_asymmetric_jwt",
117+
side_effect=ExpiredSignatureError(),
118+
)
113119
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
114-
@patch("sentry_sdk.set_tag")
115-
def test_with_shared_secret(self, mock_set_tag: MagicMock, mock_record_event) -> None:
116-
self.get_success_response(
120+
@responses.activate
121+
def test_expired_signature(
122+
self, mock_record_event: MagicMock, mock_authenticate_asymmetric_jwt: MagicMock
123+
) -> None:
124+
self.add_response()
125+
126+
self.get_error_response(
117127
**self.body(),
118-
extra_headers=dict(HTTP_AUTHORIZATION="JWT " + self.jwt_token_secret()),
128+
extra_headers=dict(HTTP_AUTHORIZATION="JWT " + self.jwt_token_cdn()),
129+
status_code=status.HTTP_400_BAD_REQUEST,
130+
)
131+
# SLO metric asserts
132+
# ENSURE_CONTROL_SILO (success) -> VERIFY_INSTALLATION (failure) -> GET_CONTROL_RESPONSE (success)
133+
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3)
134+
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.FAILURE, 1)
135+
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2)
136+
assert_failure_metric(
137+
mock_record_event,
138+
ExpiredSignatureError(),
119139
)
120-
integration = Integration.objects.get(provider="jira", external_id=self.external_id)
121140

122-
mock_set_tag.assert_any_call("integration_id", integration.id)
123-
assert integration.status == ObjectStatus.ACTIVE
124-
mock_record_event.assert_called_with(EventLifecycleOutcome.SUCCESS, None, False, None)
141+
@patch(
142+
"sentry.integrations.jira.webhooks.installed.authenticate_asymmetric_jwt",
143+
side_effect=InvalidSignatureError(),
144+
)
145+
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
146+
@responses.activate
147+
def test_invalid_signature(
148+
self, mock_record_event: MagicMock, mock_authenticate_asymmetric_jwt: MagicMock
149+
) -> None:
150+
self.add_response()
151+
152+
self.get_error_response(
153+
**self.body(),
154+
extra_headers=dict(HTTP_AUTHORIZATION="JWT " + self.jwt_token_cdn()),
155+
status_code=status.HTTP_400_BAD_REQUEST,
156+
)
157+
# SLO metric asserts
158+
# ENSURE_CONTROL_SILO (success) -> VERIFY_INSTALLATION (halt) -> GET_CONTROL_RESPONSE (success)
159+
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3)
160+
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1)
161+
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2)
162+
assert_halt_metric(
163+
mock_record_event,
164+
"JWT contained invalid signature",
165+
)
166+
167+
@patch(
168+
"sentry.integrations.jira.webhooks.installed.authenticate_asymmetric_jwt",
169+
side_effect=DecodeError(),
170+
)
171+
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
172+
@responses.activate
173+
def test_decode_error(
174+
self, mock_record_event: MagicMock, mock_authenticate_asymmetric_jwt: MagicMock
175+
) -> None:
176+
self.add_response()
177+
178+
self.get_error_response(
179+
**self.body(),
180+
extra_headers=dict(HTTP_AUTHORIZATION="JWT " + self.jwt_token_cdn()),
181+
status_code=status.HTTP_400_BAD_REQUEST,
182+
)
183+
# SLO metric asserts
184+
# ENSURE_CONTROL_SILO (success) -> VERIFY_INSTALLATION (halt) -> GET_CONTROL_RESPONSE (success)
185+
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3)
186+
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1)
187+
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2)
188+
assert_halt_metric(
189+
mock_record_event,
190+
"Could not decode JWT token",
191+
)
125192

126193
@patch("sentry_sdk.set_tag")
127194
@responses.activate
@@ -138,7 +205,34 @@ def test_with_key_id(self, mock_set_tag: MagicMock) -> None:
138205
assert integration.status == ObjectStatus.ACTIVE
139206

140207
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
208+
def test_without_key_id(self, mock_record_event: MagicMock) -> None:
209+
self.get_error_response(
210+
**self.body(),
211+
extra_headers=dict(
212+
HTTP_AUTHORIZATION="JWT " + self._jwt_token("RS256", RS256_KEY, headers={})
213+
),
214+
status_code=status.HTTP_400_BAD_REQUEST,
215+
)
216+
# SLO metric asserts
217+
# ENSURE_CONTROL_SILO (success) -> VERIFY_INSTALLATION (halt) -> GET_CONTROL_RESPONSE (success)
218+
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3)
219+
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1)
220+
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2)
221+
assert_halt_metric(
222+
mock_record_event,
223+
"Missing key_id (kid)",
224+
)
225+
226+
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
227+
@responses.activate
141228
def test_with_invalid_key_id(self, mock_record_event: MagicMock) -> None:
229+
responses.add(
230+
responses.GET,
231+
"https://connect-install-keys.atlassian.com/fake-kid",
232+
body=b"Not Found",
233+
status=404,
234+
)
235+
142236
self.get_error_response(
143237
**self.body(),
144238
extra_headers=dict(

0 commit comments

Comments
 (0)