Skip to content

Commit 54eab00

Browse files
fix(proxy): add domain validation for proxy requests (#1740)
* fix(proxy): add domain validation for proxy requests * test(proxy): enhance proxy view tests with domain allowlist - Added tests for allowed and forbidden domains, including wildcard and exact matches. - Updated existing tests to use new allowed domains and paths for consistency. * fix(logViewer): handle domain not allowed error
1 parent 5a8b72a commit 54eab00

6 files changed

Lines changed: 176 additions & 10 deletions

File tree

backend/kernelCI_app/constants/localization.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ class ClientStrings:
2424
NO_TREES_FOUND = "No trees were found"
2525
PROXY_FETCH_FAILED = "Failed to fetch resource:"
2626
PROXY_INVALID_URL = "Invalid URL"
27+
PROXY_FORBIDDEN_DOMAIN = "Domain not allowed"
2728
PROXY_ERROR_FETCH = "Error fetching the resource"
2829
NO_ORIGIN_FOUND = "No origins found"
2930
LOG_TABLE_NOT_FOUND = "Table with id 'list' not found"

backend/kernelCI_app/tests/unitTests/views/proxyView_test.py

Lines changed: 91 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,17 @@
88
from kernelCI_app.constants.localization import ClientStrings
99
from kernelCI_app.views.proxyView import TIMEOUT_TIME_IN_SECONDS, ProxyView
1010

11+
MOCK_ALLOWED_DOMAINS = ["*.example.com", "exact.org"]
12+
MOCK_ALLOWED_S3_PATHS = ["/allowed-bucket/"]
13+
1114

1215
class TestProxyView(SimpleTestCase):
1316
def setUp(self):
1417
self.factory = APIRequestFactory()
1518
self.view = ProxyView()
1619
self.url = "/proxy/"
1720

21+
@patch("kernelCI_app.views.proxyView.ALLOWED_DOMAINS", MOCK_ALLOWED_DOMAINS)
1822
@patch("kernelCI_app.views.proxyView.requests.get")
1923
def test_get_proxy_success(self, mock_requests_get):
2024
mock_requests_get.return_value.status_code = HTTPStatus.OK
@@ -25,7 +29,9 @@ def test_get_proxy_success(self, mock_requests_get):
2529
"Content-Disposition": "attachment; filename=file.txt",
2630
}
2731

28-
request = self.factory.get(self.url, {"url": "https://example.com/file.txt"})
32+
request = self.factory.get(
33+
self.url, {"url": "https://files.example.com/file.txt"}
34+
)
2935
response = self.view.get(request)
3036

3137
self.assertEqual(response.status_code, 200)
@@ -36,16 +42,21 @@ def test_get_proxy_success(self, mock_requests_get):
3642
response["Content-Disposition"], "attachment; filename=file.txt"
3743
)
3844
mock_requests_get.assert_called_once_with(
39-
"https://example.com/file.txt", stream=True, timeout=TIMEOUT_TIME_IN_SECONDS
45+
"https://files.example.com/file.txt",
46+
stream=True,
47+
timeout=TIMEOUT_TIME_IN_SECONDS,
4048
)
4149

50+
@patch("kernelCI_app.views.proxyView.ALLOWED_DOMAINS", MOCK_ALLOWED_DOMAINS)
4251
@patch("kernelCI_app.views.proxyView.requests.get")
4352
def test_get_proxy_default_content_type(self, mock_requests_get):
4453
mock_requests_get.return_value.status_code = HTTPStatus.OK
4554
mock_requests_get.return_value.content = b"content"
4655
mock_requests_get.return_value.headers = {}
4756

48-
request = self.factory.get(self.url, {"url": "https://example.com/file.txt"})
57+
request = self.factory.get(
58+
self.url, {"url": "https://files.example.com/file.txt"}
59+
)
4960
response = self.view.get(request)
5061

5162
self.assertEqual(response.status_code, 200)
@@ -65,23 +76,98 @@ def test_get_proxy_missing_url(self):
6576
self.assertEqual(response.status_code, HTTPStatus.BAD_REQUEST)
6677
self.assertEqual(response.data, {"error": ClientStrings.PROXY_INVALID_URL})
6778

79+
def test_get_proxy_invalid_scheme(self):
80+
request = self.factory.get(self.url, {"url": "file:///etc/passwd"})
81+
response = self.view.get(request)
82+
83+
self.assertEqual(response.status_code, HTTPStatus.BAD_REQUEST)
84+
self.assertEqual(response.data, {"error": ClientStrings.PROXY_INVALID_URL})
85+
86+
@patch("kernelCI_app.views.proxyView.ALLOWED_DOMAINS", MOCK_ALLOWED_DOMAINS)
6887
@patch("kernelCI_app.views.proxyView.requests.get")
6988
def test_get_proxy_external_error(self, mock_requests_get):
7089
mock_requests_get.return_value.status_code = HTTPStatus.NOT_FOUND
7190
mock_requests_get.return_value.reason = "Not Found"
7291

73-
request = self.factory.get(self.url, {"url": "https://example.com/file.txt"})
92+
request = self.factory.get(
93+
self.url, {"url": "https://files.example.com/file.txt"}
94+
)
7495
response = self.view.get(request)
7596

7697
self.assertEqual(response.status_code, HTTPStatus.NOT_FOUND)
7798
self.assertIn(ClientStrings.PROXY_FETCH_FAILED, response.data["error"])
7899

100+
@patch("kernelCI_app.views.proxyView.ALLOWED_DOMAINS", MOCK_ALLOWED_DOMAINS)
79101
@patch("kernelCI_app.views.proxyView.requests.get")
80102
def test_get_proxy_request_exception(self, mock_requests_get):
81103
mock_requests_get.side_effect = requests.RequestException("Connection error")
82104

83-
request = self.factory.get(self.url, {"url": "https://example.com/file.txt"})
105+
request = self.factory.get(
106+
self.url, {"url": "https://files.example.com/file.txt"}
107+
)
84108
response = self.view.get(request)
85109

86110
self.assertEqual(response.status_code, HTTPStatus.INTERNAL_SERVER_ERROR)
87111
self.assertEqual(response.data, {"error": ClientStrings.PROXY_ERROR_FETCH})
112+
113+
@patch("kernelCI_app.views.proxyView.ALLOWED_DOMAINS", MOCK_ALLOWED_DOMAINS)
114+
def test_get_proxy_forbidden_domain(self):
115+
request = self.factory.get(self.url, {"url": "https://evil.com/file.txt"})
116+
response = self.view.get(request)
117+
118+
self.assertEqual(response.status_code, HTTPStatus.FORBIDDEN)
119+
self.assertEqual(response.data, {"error": ClientStrings.PROXY_FORBIDDEN_DOMAIN})
120+
121+
@patch("kernelCI_app.views.proxyView.ALLOWED_DOMAINS", MOCK_ALLOWED_DOMAINS)
122+
@patch("kernelCI_app.views.proxyView.requests.get")
123+
def test_get_proxy_wildcard_domain_match(self, mock_requests_get):
124+
mock_requests_get.return_value.status_code = HTTPStatus.OK
125+
mock_requests_get.return_value.content = b"content"
126+
mock_requests_get.return_value.headers = {}
127+
128+
request = self.factory.get(
129+
self.url, {"url": "https://any-sub.example.com/file.txt"}
130+
)
131+
response = self.view.get(request)
132+
133+
self.assertEqual(response.status_code, 200)
134+
135+
@patch("kernelCI_app.views.proxyView.ALLOWED_DOMAINS", MOCK_ALLOWED_DOMAINS)
136+
@patch("kernelCI_app.views.proxyView.requests.get")
137+
def test_get_proxy_exact_domain_match(self, mock_requests_get):
138+
mock_requests_get.return_value.status_code = HTTPStatus.OK
139+
mock_requests_get.return_value.content = b"content"
140+
mock_requests_get.return_value.headers = {}
141+
142+
request = self.factory.get(self.url, {"url": "https://exact.org/file.txt"})
143+
response = self.view.get(request)
144+
145+
self.assertEqual(response.status_code, 200)
146+
147+
@patch("kernelCI_app.views.proxyView.ALLOWED_S3_PATHS", MOCK_ALLOWED_S3_PATHS)
148+
@patch("kernelCI_app.views.proxyView.ALLOWED_DOMAINS", ["s3.amazonaws.com"])
149+
@patch("kernelCI_app.views.proxyView.requests.get")
150+
def test_get_proxy_s3_allowed_bucket(self, mock_requests_get):
151+
mock_requests_get.return_value.status_code = HTTPStatus.OK
152+
mock_requests_get.return_value.content = b"log content"
153+
mock_requests_get.return_value.headers = {}
154+
155+
request = self.factory.get(
156+
self.url,
157+
{"url": "https://s3.amazonaws.com/allowed-bucket/file.log"},
158+
)
159+
response = self.view.get(request)
160+
161+
self.assertEqual(response.status_code, 200)
162+
163+
@patch("kernelCI_app.views.proxyView.ALLOWED_S3_PATHS", MOCK_ALLOWED_S3_PATHS)
164+
@patch("kernelCI_app.views.proxyView.ALLOWED_DOMAINS", ["s3.amazonaws.com"])
165+
def test_get_proxy_s3_forbidden_bucket(self):
166+
request = self.factory.get(
167+
self.url,
168+
{"url": "https://s3.amazonaws.com/other-bucket/file.log"},
169+
)
170+
response = self.view.get(request)
171+
172+
self.assertEqual(response.status_code, HTTPStatus.FORBIDDEN)
173+
self.assertEqual(response.data, {"error": ClientStrings.PROXY_FORBIDDEN_DOMAIN})

backend/kernelCI_app/views/proxyView.py

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
from urllib.parse import urlparse
1+
from urllib.parse import ParseResult, urlparse
22
import requests
3+
from fnmatch import fnmatch
34
from rest_framework.views import APIView
45
from django.http import HttpResponse
56
from http import HTTPStatus
@@ -11,6 +12,46 @@
1112
# TIMEOUT to avoid people sending very large files through the proxy
1213
TIMEOUT_TIME_IN_SECONDS = 45
1314

15+
ALLOWED_DOMAINS = [
16+
# KernelCI infrastructure
17+
"*.kernelci.org",
18+
# External CI/test infrastructure
19+
"syzkaller.appspot.com",
20+
"*.sirena.org.uk",
21+
"lisalogs6a28e896.blob.core.windows.net",
22+
"storage.tuxsuite.com",
23+
"s3.amazonaws.com",
24+
"*.linaro.org",
25+
]
26+
27+
ALLOWED_S3_PATHS = [
28+
"/arr-cki-prod-trusted-artifacts/",
29+
]
30+
31+
32+
def is_valid_url(parsed_url: ParseResult) -> bool:
33+
if not parsed_url.scheme or not parsed_url.netloc:
34+
return False
35+
36+
if parsed_url.scheme not in ["http", "https"]:
37+
return False
38+
39+
return True
40+
41+
42+
def is_allowed_domain(parsed_url: ParseResult) -> bool:
43+
if not any(fnmatch(parsed_url.hostname, pattern) for pattern in ALLOWED_DOMAINS):
44+
return False
45+
46+
if parsed_url.hostname == "s3.amazonaws.com":
47+
if not any(
48+
parsed_url.path.startswith(allowed_path)
49+
for allowed_path in ALLOWED_S3_PATHS
50+
):
51+
return False
52+
53+
return True
54+
1455

1556
class ProxyView(APIView):
1657
@extend_schema(
@@ -20,11 +61,24 @@ class ProxyView(APIView):
2061
)
2162
def get(self, request):
2263
url = request.GET.get("url")
64+
if not url:
65+
return create_api_error_response(
66+
error_message=ClientStrings.PROXY_INVALID_URL
67+
)
68+
2369
parsed_url = urlparse(url)
24-
if not parsed_url.scheme or not parsed_url.netloc:
70+
71+
if not is_valid_url(parsed_url):
2572
return create_api_error_response(
2673
error_message=ClientStrings.PROXY_INVALID_URL
2774
)
75+
76+
if not is_allowed_domain(parsed_url):
77+
return create_api_error_response(
78+
error_message=ClientStrings.PROXY_FORBIDDEN_DOMAIN,
79+
status_code=HTTPStatus.FORBIDDEN,
80+
)
81+
2882
try:
2983
response = requests.get(url, stream=True, timeout=TIMEOUT_TIME_IN_SECONDS)
3084

dashboard/src/api/logViewer.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { useQuery } from '@tanstack/react-query';
44
import { minutesToMilliseconds } from 'date-fns';
55
import { useIntl } from 'react-intl';
66

7+
import { AxiosError, HttpStatusCode } from 'axios';
8+
79
import { LOG_EXCERPT_ALLOWED_DOMAINS } from '@/utils/constants/log_excerpt_allowed_domain';
810

911
import { RequestData } from './commonRequest';
@@ -43,6 +45,14 @@ async function fetchAndDecompressLog(
4345
}
4446
} catch (error) {
4547
console.error(error);
48+
49+
if (
50+
error instanceof AxiosError &&
51+
error.response?.status === HttpStatusCode.Forbidden
52+
) {
53+
throw new Error('403:Domain not allowed');
54+
}
55+
4656
throw new Error(
4757
`Failed to fetch logs: ${error instanceof Error ? error.message : 'Unknown error'}`,
4858
);

dashboard/src/locales/messages/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,8 @@ export const messages = {
255255
'logSheet.title': 'Log Viewer',
256256
'logViewer.disabledHighlight':
257257
'The text highlight was disabled in this log due to the log size',
258+
'logViewer.domainNotAllowed':
259+
'This log domain is not supported by the proxy. You can still download the log directly using the link above.',
258260
'logViewer.download': 'Download full log',
259261
'logViewer.errorFetchingLogExcerpt':
260262
'Error fetching logExcerpt at: {logExcerpt}',

dashboard/src/pages/LogViewer.tsx

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const Constants = {
66
URL_END_PATH_SIZE: 50,
77
} as const;
88

9-
import { FormattedMessage } from 'react-intl';
9+
import { FormattedMessage, useIntl } from 'react-intl';
1010

1111
import { LogExcerpt } from '@/components/Log/LogExcerpt';
1212
import { truncateUrl } from '@/lib/string';
@@ -57,8 +57,16 @@ export function LogViewer(): JSX.Element {
5757
const { url, type, itemId } = useSearch({ from: '/log-viewer' });
5858
const historyState = useRouterState({ select: s => s.location.state });
5959
const previousSearch = useSearchStore(s => s.previousSearch);
60+
const { formatMessage } = useIntl();
6061

61-
const { data, status } = useLogViewer(url);
62+
const { data, status, error } = useLogViewer(url);
63+
64+
const isDomainNotAllowed = error?.message.startsWith('403');
65+
const domainNotAllowedError = isDomainNotAllowed ? (
66+
<p className="text-weak-gray py-6 text-center text-xl font-semibold">
67+
{formatMessage({ id: 'logViewer.domainNotAllowed' })}
68+
</p>
69+
) : undefined;
6270
const { data: logData, isLoading } = useLogData(itemId ?? '', type);
6371

6472
const { treeName, branch, id } = historyState;
@@ -192,7 +200,12 @@ export function LogViewer(): JSX.Element {
192200
</div>
193201
</div>
194202
<div className="overflow-hidden rounded-md">
195-
<QuerySwitcher data={data} status={status}>
203+
<QuerySwitcher
204+
data={data}
205+
status={status}
206+
error={error}
207+
customError={domainNotAllowedError}
208+
>
196209
<LogExcerpt logExcerpt={data?.content} variant="log-viewer" />
197210
</QuerySwitcher>
198211
</div>

0 commit comments

Comments
 (0)