Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 2.15.1 #3720

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,23 @@ The types of changes are:
- `Fixed` for any bug fixes.
- `Security` in case of vulnerabilities.

## [Unreleased](https://github.com/ethyca/fides/compare/2.15.0...main)
## [Unreleased](https://github.com/ethyca/fides/compare/2.15.1...main)

## [2.15.1](https://github.com/ethyca/fides/compare/2.15.0...2.15.1)

### Added
- Set `sslmode` to `prefer` if connecting to Redshift via ssh [#3685](https://github.com/ethyca/fides/pull/3685)

### Changed
- Privacy center action cards are now able to expand to accommodate longer text [#3669](https://github.com/ethyca/fides/pull/3669)

### Fixed
- Handle names with a double underscore when processing access and erasure requests [#3688](https://github.com/ethyca/fides/pull/3688)
- Allow Privacy Notices banner and modal to scroll as needed [#3713](https://github.com/ethyca/fides/pull/3713)

### Security
- Resolve path traversal vulnerability in webserver API [GHSA-r25m-cr6v-p9hq](https://github.com/ethyca/fides/security/advisories/GHSA-r25m-cr6v-p9hq)


## [2.15.0](https://github.com/ethyca/fides/compare/2.14.1...2.15.0)

Expand Down
3 changes: 3 additions & 0 deletions clients/fides-js/src/components/fides.css
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ div#fides-banner-container {
display: flex;
justify-content: center;
visibility: visible;
max-height: 100%;
}

div#fides-banner {
Expand All @@ -88,6 +89,7 @@ div#fides-banner {
color: var(--fides-overlay-body-font-color);
box-sizing: border-box;
padding: var(--fides-overlay-padding);
overflow-y: scroll;

display: flex;
flex-direction: row;
Expand Down Expand Up @@ -239,6 +241,7 @@ div.fides-modal-content {
border-radius: var(--fides-overlay-component-border-radius);
max-height: 100%;
max-width: 100%;
overflow-y: scroll;

z-index: 2;
position: fixed;
Expand Down
10 changes: 2 additions & 8 deletions clients/privacy-center/components/Card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const Card: React.FC<CardProps> = ({
data-testid="card"
flexDirection="column"
gap="12px"
h="176px"
minH="176px"
key={title}
m={2}
onClick={() => {
Expand Down Expand Up @@ -57,13 +57,7 @@ const Card: React.FC<CardProps> = ({
>
{title}
</Text>
<Text
color="gray.600"
fontSize="xs"
fontWeight="normal"
lineHeight="16px"
noOfLines={3}
>
<Text color="gray.600" fontSize="xs" fontWeight="normal" lineHeight="16px">
{description}
</Text>
</Flex>
Expand Down
4 changes: 4 additions & 0 deletions src/fides/api/common_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,10 @@ class SSHTunnelConfigNotFoundException(Exception):
"""Exception for when Fides is configured to use an SSH tunnel without config provided."""


class MalisciousUrlException(Exception):
"""Fides has detected a potentially maliscious URL."""


class AuthenticationError(HTTPException):
"""To be raised when attempting to fetch an access token using
invalid credentials.
Expand Down
20 changes: 20 additions & 0 deletions src/fides/api/main.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
"""
Contains the code that sets up the API.
"""
import os
import sys
from datetime import datetime, timezone
from logging import WARNING
from typing import Callable, Optional
from urllib.parse import unquote

from fastapi import HTTPException, Request, Response, status
from fastapi.responses import FileResponse
Expand All @@ -21,6 +23,7 @@
log_startup,
run_database_startup,
)
from fides.api.common_exceptions import MalisciousUrlException
from fides.api.middleware import handle_audit_log_resource
from fides.api.schemas.analytics import Event, ExtraData

Expand Down Expand Up @@ -151,13 +154,30 @@ def read_index() -> Response:
return get_admin_index_as_response()


def sanitise_url_path(path: str) -> str:
"""Returns a URL path that does not contain any ../ or //"""
path = unquote(path)
path = os.path.normpath(path)
for token in path.split("/"):
if ".." in token:
logger.warning(f"Potentially dangerous use of URL hierarchy in path: {path}")
raise MalisciousUrlException()
return path


@app.get("/{catchall:path}", response_class=Response, tags=["Default"])
def read_other_paths(request: Request) -> Response:
"""
Return related frontend files. Adapted from https://github.com/tiangolo/fastapi/issues/130
"""
# check first if requested file exists (for frontend assets)
path = request.path_params["catchall"]
logger.debug(f"Catch all path detected: {path}")
try:
path = sanitise_url_path(path)
except MalisciousUrlException:
# if a maliscious URL is detected, route the user to the index
return get_admin_index_as_response()

# search for matching route in package (i.e. /dataset)
ui_file = match_route(get_ui_file_map(), path)
Expand Down
3 changes: 3 additions & 0 deletions src/fides/api/service/connectors/sql_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,16 +367,19 @@ def build_uri(self) -> str:
def create_client(self) -> Engine:
"""Returns a SQLAlchemy Engine that can be used to interact with a database"""
config = self.secrets_schema(**self.configuration.secrets or {})
connect_args = {}
if config.ssh_required and CONFIG.security.bastion_server_ssh_private_key:
self.create_ssh_tunnel(host=config.host, port=config.port)
self.ssh_server.start()
uri = self.build_ssh_uri(local_address=self.ssh_server.local_bind_address)
connect_args["sslmode"] = "prefer"
else:
uri = config.url or self.build_uri()
return create_engine(
uri,
hide_parameters=self.hide_parameters,
echo=not self.hide_parameters,
connect_args=connect_args,
)

def set_schema(self, connection: Connection) -> None:
Expand Down
14 changes: 12 additions & 2 deletions src/fides/api/task/graph_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@
from fides.api.task.refine_target_path import FieldPathNodeInput
from fides.api.task.task_resources import TaskResources
from fides.api.util.cache import get_cache
from fides.api.util.collection_util import NodeInput, Row, append, partition
from fides.api.util.collection_util import (
NodeInput,
Row,
append,
extract_key_for_address,
partition,
)
from fides.api.util.consent_util import add_errored_system_status_for_consent_reporting
from fides.api.util.logger import Pii
from fides.api.util.saas_util import FIDESOPS_GROUPED_INPUTS
Expand Down Expand Up @@ -733,7 +739,11 @@ def get_cached_data_for_erasures(
value_dict = cache.get_encoded_objects_by_prefix(
f"PLACEHOLDER_RESULTS__{privacy_request_id}"
)
return {k.split("__")[-1]: v for k, v in value_dict.items()}
number_of_leading_strings_to_exclude = 3
return {
extract_key_for_address(k, number_of_leading_strings_to_exclude): v
for k, v in value_dict.items()
}


def update_erasure_mapping_from_cache(
Expand Down
11 changes: 8 additions & 3 deletions src/fides/api/task/task_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
)
from fides.api.service.connectors.base_email_connector import BaseEmailConnector
from fides.api.util.cache import get_cache
from fides.api.util.collection_util import Row
from fides.api.util.collection_util import Row, extract_key_for_address


class Connections:
Expand Down Expand Up @@ -147,7 +147,11 @@ def get_all_cached_objects(self) -> Dict[str, Optional[List[Row]]]:
f"{self.request.id}__access_request"
)
# extract request id to return a map of address:value
return {k.split("__")[-1]: v for k, v in value_dict.items()}
number_of_leading_strings_to_exclude = 2
return {
extract_key_for_address(k, number_of_leading_strings_to_exclude): v
for k, v in value_dict.items()
}

def cache_erasure(self, key: str, value: int) -> None:
"""Cache that a node's masking is complete. Object will be stored in redis under
Expand All @@ -163,7 +167,8 @@ def get_all_cached_erasures(self) -> Dict[str, int]:
f"{self.request.id}__erasure_request"
)
# extract request id to return a map of address:value
return {k.split("__")[-1]: v for k, v in value_dict.items()} # type: ignore
number_of_leading_strings_to_exclude = 2
return {extract_key_for_address(k, number_of_leading_strings_to_exclude): v for k, v in value_dict.items()} # type: ignore

def write_execution_log( # pylint: disable=too-many-arguments
self,
Expand Down
18 changes: 18 additions & 0 deletions src/fides/api/util/collection_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,21 @@ def filter_nonempty_values(d: Optional[Dict[Any, Any]]) -> Dict[Any, Any]:
if d:
return {e[0]: e[1] for e in d.items() if e[1]}
return {}


def extract_key_for_address(
full_request_id: str, number_of_leading_strings_to_exclude: int
) -> str:
"""
Handles extracting the correct Dataset:Collection to map to extracted
values.

Due to differences in the number of leading strings based on access or
erasure, a parameter is used to ensure the correct values are returned.

Handles an edge case where double underscores exist in either the fides_key
of the Dataset or the Collection name.
"""
request_id_dataset, collection = full_request_id.split(":")
dataset = request_id_dataset.split("__", number_of_leading_strings_to_exclude)[-1]
return f"{dataset}:{collection}"
10 changes: 10 additions & 0 deletions tests/ops/integration_tests/test_external_database_connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ def snowflake_test_engine() -> Generator:
engine.dispose()


@pytest.mark.integration_external
@pytest.mark.integration_redshift
def test_redshift_sslmode_default(redshift_test_engine):
"""Confirm that sslmode is set to verify-full for non SSH connections"""
_, kwargs = redshift_test_engine.dialect.create_connect_args(
redshift_test_engine.url
)
assert kwargs["sslmode"] == "verify-full"


@pytest.mark.integration_external
@pytest.mark.integration_redshift
def test_redshift_example_data(redshift_test_engine):
Expand Down
7 changes: 7 additions & 0 deletions tests/ops/task/test_task_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ def test_cache_object(self, db, privacy_request, policy, integration_manual_conf
"access_request__postgres_example:payment",
[{"id": 2, "ccn": "111-111-1111-1111", "customer_id": 1}],
)
resources.cache_object(
"access_request__postgres__double__underscore__example:double__underscore__collection",
[{"id": 3, "last_name": "Doe"}],
)
resources.cache_erasure("manual_example:filing-cabinet", 2)

# Only access results from "cache_object" are returned
Expand All @@ -24,6 +28,9 @@ def test_cache_object(self, db, privacy_request, policy, integration_manual_conf
{"id": 2, "ccn": "111-111-1111-1111", "customer_id": 1}
],
"postgres_example:customer": [{"id": 1, "last_name": "Doe"}],
"postgres__double__underscore__example:double__underscore__collection": [
{"id": 3, "last_name": "Doe"}
],
}

def test_cache_erasure(
Expand Down
21 changes: 21 additions & 0 deletions tests/ops/util/test_api_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,24 @@ def test_non_existent_route_404(
f"{V1_URL_PREFIX}/route/does/not/exist/", headers=auth_header
)
assert resp_4.status_code == HTTP_404_NOT_FOUND

def test_malicious_url(
self,
api_client: TestClient,
url,
) -> None:
malicious_paths = [
"../../../../../../../../../etc/passwd",
"..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2fetc/passwd",
"%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/etc/passwd",
"%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2fetc/passwd",
"..%c0%2f..%c0%2f..%c0%2f..%c0%2f..%c0%2f..%c0%2f..%c0%2f..%c0%2f..%c0%2f/etc/passwd",
".../...//.../...//.../...//.../...//.../...//.../...//.../...//.../...//.../...//etc/passwd",
"...%2f...%2f%2f...%2f...%2f%2f...%2f...%2f%2f...%2f...%2f%2f...%2f...%2f%2f...%2f...%2f%2f...%2f...%2f%2f...%2f...%2f%2f...%2f...%2f%2fetc/passwd",
"%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//etc/passwd",
"%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2fetc/passwd",
]
for path in malicious_paths:
resp = api_client.get(f"{url}/{path}")
assert resp.status_code == 200
assert resp.text == "<h1>Privacy is a Human Right!</h1>"