From 7b642569710acbd561f8d91e9ce4e2a4ab8e4a43 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Wed, 24 May 2023 16:29:50 +0100 Subject: [PATCH 01/17] adds bool option to use ssh via connection secrets --- .../connection_configuration/connection_secrets_postgres.py | 1 + .../connection_configuration/connection_secrets_redshift.py | 1 + 2 files changed, 2 insertions(+) diff --git a/src/fides/api/schemas/connection_configuration/connection_secrets_postgres.py b/src/fides/api/schemas/connection_configuration/connection_secrets_postgres.py index da61afe101..4e0621c047 100644 --- a/src/fides/api/schemas/connection_configuration/connection_secrets_postgres.py +++ b/src/fides/api/schemas/connection_configuration/connection_secrets_postgres.py @@ -17,6 +17,7 @@ class PostgreSQLSchema(ConnectionConfigSecretsSchema): str ] = None # Either the entire "url" *OR* the "host" should be supplied. port: Optional[int] = None + ssh_required: bool = False _required_components: List[str] = ["host"] diff --git a/src/fides/api/schemas/connection_configuration/connection_secrets_redshift.py b/src/fides/api/schemas/connection_configuration/connection_secrets_redshift.py index 1d97d4801e..422cc8ac9a 100644 --- a/src/fides/api/schemas/connection_configuration/connection_secrets_redshift.py +++ b/src/fides/api/schemas/connection_configuration/connection_secrets_redshift.py @@ -15,6 +15,7 @@ class RedshiftSchema(ConnectionConfigSecretsSchema): user: Optional[str] = None password: Optional[str] = None db_schema: Optional[str] = None + ssh_required: bool = False _required_components: List[str] = ["host", "user", "password"] From 52757afa4d8d7372c523410a1a67e553b9e6bf35 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Wed, 24 May 2023 16:30:37 +0100 Subject: [PATCH 02/17] adds security settings for bastion host This is really just an interim state to house a single bastion host to get something out more quickly --- src/fides/core/config/security_settings.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/fides/core/config/security_settings.py b/src/fides/core/config/security_settings.py index 40e0a59c2a..ffb5e35cb9 100644 --- a/src/fides/core/config/security_settings.py +++ b/src/fides/core/config/security_settings.py @@ -123,6 +123,18 @@ class SecuritySettings(FidesSettings): description="Enables or disables the ability to import connector templates with custom functions. When enabled, custom functions which will be loaded in a restricted environment to minimize security risks.", ) + bastion_server_host: Optional[str] = Field( + default=None, description="An optional field to store the bastion server host" + ) + bastion_server_ssh_username: Optional[str] = Field( + default=None, + description="An optional field to store the username used to access the bastion server", + ) + bastion_server_ssh_pkey: Optional[str] = Field( + default=None, + description="An optional field to store the key used to SSH into the bastion server.", + ) + @validator("app_encryption_key") @classmethod def validate_encryption_key_length( From e6b75a3a3a67263f0c368a59df11dc57687ce632 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Wed, 24 May 2023 17:13:26 +0100 Subject: [PATCH 03/17] [skip ci] initial changes to query config --- .../api/service/connectors/sql_connector.py | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/fides/api/service/connectors/sql_connector.py b/src/fides/api/service/connectors/sql_connector.py index 17bff0ff63..48121e5d9a 100644 --- a/src/fides/api/service/connectors/sql_connector.py +++ b/src/fides/api/service/connectors/sql_connector.py @@ -1,6 +1,9 @@ +import io from abc import abstractmethod from typing import Any, Dict, List, Optional, Type +import paramiko +import sshtunnel from loguru import logger from snowflake.sqlalchemy import URL as Snowflake_URL from sqlalchemy import Column, text @@ -46,6 +49,9 @@ SQLQueryConfig, ) from fides.api.util.collection_util import Row +from fides.core.config import get_config + +CONFIG = get_config() class SQLConnector(BaseConnector[Engine]): @@ -197,6 +203,51 @@ def build_uri(self) -> str: dbname = f"/{config.dbname}" if config.dbname else "" return f"postgresql://{user_password}{netloc}{port}{dbname}" + def build_ssh_uri(self, local_port: int) -> str: + """Build URI of format postgresql://[user[:password]@][netloc][:port][/dbname]""" + config = self.secrets_schema(**self.configuration.secrets or {}) + + user_password = "" + if config.username: + user = config.username + password = f":{config.password}" if config.password else "" + user_password = f"{user}{password}@" + + netloc = config.host + port = f":{local_port}" if local_port else "" + dbname = f"/{config.dbname}" if config.dbname else "" + return f"postgresql://{user_password}{netloc}{port}{dbname}" + + 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 {}) + if config.ssh_required: + with io.BytesIO( + CONFIG.security.bastion_server_ssh_pkey.encode("utf8") + ) as binary_file: + with io.TextIOWrapper(binary_file, encoding="utf8") as file_obj: + pkey = paramiko.RSAKey.from_private_key(file_obj=file_obj) + server = sshtunnel.SSHTunnelForwarder( + (CONFIG.security.bastion_server_host), + ssh_username=CONFIG.security.bastion_server_ssh_username, + ssh_pkey=pkey, + remote_bind_address=( + config.host, + config.port, + ), + ) + server.start() + + uri = self.build_ssh_uri(local_port=server.local_bind_port) + else: + uri = config.url or self.build_uri() + return create_engine( + uri, + hide_parameters=self.hide_parameters, + echo=not self.hide_parameters, + ) + def set_schema(self, connection: Connection) -> None: """Sets the schema for a postgres database if applicable""" config = self.secrets_schema(**self.configuration.secrets or {}) From 9472a7ae214341155c945def7eb30113efbcba1b Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Thu, 25 May 2023 23:10:17 +0100 Subject: [PATCH 04/17] include sshtunnel in requirements.txt --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index 5454542029..2aeb47699b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -48,6 +48,7 @@ sqlalchemy-bigquery==1.4.4 sqlalchemy-redshift==0.8.11 sqlalchemy-stubs==0.4 SQLAlchemy-Utils==0.38.3 +sshtunnel==0.4.0 toml>=0.10.1 twilio==7.15.0 typing_extensions==4.5.0 # pinned to work around https://github.com/pydantic/pydantic/issues/5821 From 2d32b896f5e57de5b26bbbb2e6be6b8a7004a185 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Fri, 26 May 2023 13:54:03 +0100 Subject: [PATCH 05/17] mypy and pytest error fixes --- dev-requirements.txt | 1 + src/fides/api/service/connectors/sql_connector.py | 4 ++-- .../ops/api/v1/endpoints/test_connection_config_endpoints.py | 2 ++ .../test_connection_configuration_integration.py | 3 +++ 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index af0d115ef8..166c28d47d 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -14,6 +14,7 @@ pytest==7.2.2 requests-mock==1.10.0 setuptools>=64.0.2 sqlalchemy-stubs +types-paramiko==3.0.0.10 types-PyYAML==6.0.11 types-redis==4.3.4 types-requests diff --git a/src/fides/api/service/connectors/sql_connector.py b/src/fides/api/service/connectors/sql_connector.py index 48121e5d9a..dec14c7d5d 100644 --- a/src/fides/api/service/connectors/sql_connector.py +++ b/src/fides/api/service/connectors/sql_connector.py @@ -3,7 +3,7 @@ from typing import Any, Dict, List, Optional, Type import paramiko -import sshtunnel +import sshtunnel # type: ignore from loguru import logger from snowflake.sqlalchemy import URL as Snowflake_URL from sqlalchemy import Column, text @@ -222,7 +222,7 @@ 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 {}) - if config.ssh_required: + if config.ssh_required and CONFIG.security.bastion_server_ssh_pkey: with io.BytesIO( CONFIG.security.bastion_server_ssh_pkey.encode("utf8") ) as binary_file: diff --git a/tests/ops/api/v1/endpoints/test_connection_config_endpoints.py b/tests/ops/api/v1/endpoints/test_connection_config_endpoints.py index f4b2edfc6a..6df7b05bf8 100644 --- a/tests/ops/api/v1/endpoints/test_connection_config_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_connection_config_endpoints.py @@ -1428,6 +1428,7 @@ def test_put_connection_config_redshift_secrets( "user": "awsuser", "password": "test_password", "db_schema": "test", + "ssh_required": False, } resp = api_client.put( url + "?verify=False", @@ -1448,6 +1449,7 @@ def test_put_connection_config_redshift_secrets( "password": "test_password", "db_schema": "test", "url": None, + "ssh_required": False, } assert redshift_connection_config.last_test_timestamp is None assert redshift_connection_config.last_test_succeeded is None diff --git a/tests/ops/integration_tests/test_connection_configuration_integration.py b/tests/ops/integration_tests/test_connection_configuration_integration.py index 76aaa9851f..ee46fb734c 100644 --- a/tests/ops/integration_tests/test_connection_configuration_integration.py +++ b/tests/ops/integration_tests/test_connection_configuration_integration.py @@ -69,6 +69,7 @@ def test_postgres_db_connection_incorrect_secrets( "password": None, "url": None, "db_schema": None, + "ssh_required": False, } assert connection_config.last_test_timestamp is not None assert connection_config.last_test_succeeded is False @@ -114,6 +115,7 @@ def test_postgres_db_connection_connect_with_components( "password": "postgres", "url": None, "db_schema": None, + "ssh_required": False, } assert connection_config.last_test_timestamp is not None assert connection_config.last_test_succeeded is True @@ -155,6 +157,7 @@ def test_postgres_db_connection_connect_with_url( "password": None, "url": payload["url"], "db_schema": None, + "ssh_required": False, } assert connection_config.last_test_timestamp is not None assert connection_config.last_test_succeeded is True From 11baeda15c82ddce0ee19e494e2b1fe395967a7d Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Tue, 30 May 2023 20:03:33 +0100 Subject: [PATCH 06/17] bug fix: pin paramiko to specific version --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index 2aeb47699b..2393992fe9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -26,6 +26,7 @@ okta==2.7.0 openpyxl==3.0.9 packaging==23.0 pandas==1.4.3 +paramiko==3.1.0 passlib[bcrypt]==1.7.4 plotly==5.13.1 psycopg2-binary==2.9.5 From 10b1b728c10d56fd22caf9514af7a9d438be5c7b Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Tue, 30 May 2023 20:04:06 +0100 Subject: [PATCH 07/17] bug fix: handle M1 specific errors with postgres --- Dockerfile | 7 +++++++ docker-compose.yml | 3 +++ 2 files changed, 10 insertions(+) diff --git a/Dockerfile b/Dockerfile index afeba29ba3..8d737063e8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -64,6 +64,13 @@ RUN if [ "$TARGETPLATFORM" != "linux/arm64" ] ; \ && rm -rf /var/lib/apt/lists/* ; \ fi + +# Required for M1 Macs currently when connecting to Postgres +# https://github.com/psycopg/psycopg2/issues/1360#issuecomment-1455169030 +RUN pip uninstall psycopg2-binary -y +RUN apt update -y && apt install -y build-essential libpq-dev +RUN pip install psycopg2 + # General Application Setup ## COPY . /fides WORKDIR /fides diff --git a/docker-compose.yml b/docker-compose.yml index 94b9134bf6..034f77b05f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -28,6 +28,9 @@ services: FIDES__DEV_MODE: "True" FIDES__USER__ANALYTICS_OPT_OUT: "True" FIDES__SECURITY__ALLOW_CUSTOM_CONNECTOR_FUNCTIONS: "True" + FIDES__SECURITY__BASTION_SERVER_HOST: ${FIDES__SECURITY__BASTION_SERVER_HOST-} + FIDES__SECURITY__BASTION_SERVER_SSH_USERNAME: ${FIDES__SECURITY__BASTION_SERVER_SSH_USERNAME-} + FIDES__SECURITY__BASTION_SERVER_SSH_PKEY: ${FIDES__SECURITY__BASTION_SERVER_SSH_PKEY-} VAULT_ADDR: ${VAULT_ADDR-} VAULT_NAMESPACE: ${VAULT_NAMESPACE-} VAULT_TOKEN: ${VAULT_TOKEN-} From a395d9ea37fe1971cf6eaa275d38d23686ec2cea Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Tue, 30 May 2023 20:05:12 +0100 Subject: [PATCH 08/17] improve: server and attribute handling --- .../api/service/connectors/sql_connector.py | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/fides/api/service/connectors/sql_connector.py b/src/fides/api/service/connectors/sql_connector.py index dec14c7d5d..78bddde489 100644 --- a/src/fides/api/service/connectors/sql_connector.py +++ b/src/fides/api/service/connectors/sql_connector.py @@ -188,6 +188,15 @@ class PostgreSQLConnector(SQLConnector): secrets_schema = PostgreSQLSchema + def __init__(self, configuration: ConnectionConfig): + """Instantiate a SQL-based connector""" + super().__init__(configuration) + if not self.secrets_schema: + raise NotImplementedError( + "SQL Connectors must define their secrets schema class" + ) + self.ssh_server: sshtunnel._ForwardServer = None + def build_uri(self) -> str: """Build URI of format postgresql://[user[:password]@][netloc][:port][/dbname]""" config = self.secrets_schema(**self.configuration.secrets or {}) @@ -203,7 +212,7 @@ def build_uri(self) -> str: dbname = f"/{config.dbname}" if config.dbname else "" return f"postgresql://{user_password}{netloc}{port}{dbname}" - def build_ssh_uri(self, local_port: int) -> str: + def build_ssh_uri(self, local_address: tuple) -> str: """Build URI of format postgresql://[user[:password]@][netloc][:port][/dbname]""" config = self.secrets_schema(**self.configuration.secrets or {}) @@ -213,11 +222,20 @@ def build_ssh_uri(self, local_port: int) -> str: password = f":{config.password}" if config.password else "" user_password = f"{user}{password}@" - netloc = config.host + local_host, local_port = local_address + netloc = local_host port = f":{local_port}" if local_port else "" dbname = f"/{config.dbname}" if config.dbname else "" return f"postgresql://{user_password}{netloc}{port}{dbname}" + def close(self) -> None: + """Close any held resources""" + if self.db_client: + logger.debug(" disposing of {}", self.__class__) + self.db_client.dispose() + if self.ssh_server: + self.ssh_server.stop() + def create_client(self) -> Engine: """Returns a SQLAlchemy Engine that can be used to interact with a database""" @@ -228,7 +246,7 @@ def create_client(self) -> Engine: ) as binary_file: with io.TextIOWrapper(binary_file, encoding="utf8") as file_obj: pkey = paramiko.RSAKey.from_private_key(file_obj=file_obj) - server = sshtunnel.SSHTunnelForwarder( + self.ssh_server = sshtunnel.SSHTunnelForwarder( (CONFIG.security.bastion_server_host), ssh_username=CONFIG.security.bastion_server_ssh_username, ssh_pkey=pkey, @@ -237,9 +255,9 @@ def create_client(self) -> Engine: config.port, ), ) - server.start() + self.ssh_server.start() - uri = self.build_ssh_uri(local_port=server.local_bind_port) + uri = self.build_ssh_uri(local_address=self.ssh_server.local_bind_address) else: uri = config.url or self.build_uri() return create_engine( From d9f146fd8a3f1e63a10f1335a3cbd1312d3c8d28 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Wed, 31 May 2023 00:00:54 +0100 Subject: [PATCH 09/17] fix: further required test update for ssh_required --- .../api/v1/endpoints/test_connection_config_endpoints.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/ops/api/v1/endpoints/test_connection_config_endpoints.py b/tests/ops/api/v1/endpoints/test_connection_config_endpoints.py index 6df7b05bf8..373a09783a 100644 --- a/tests/ops/api/v1/endpoints/test_connection_config_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_connection_config_endpoints.py @@ -1294,7 +1294,11 @@ def test_put_connection_config_secrets( ) -> None: """Note: this test does not attempt to actually connect to the db, via use of verify query param.""" auth_header = generate_auth_header(scopes=[CONNECTION_CREATE_OR_UPDATE]) - payload = {"host": "localhost", "port": "1234", "dbname": "my_test_db"} + payload = { + "host": "localhost", + "port": "1234", + "dbname": "my_test_db", + } resp = api_client.put( url + "?verify=False", headers=auth_header, @@ -1314,6 +1318,7 @@ def test_put_connection_config_secrets( "password": None, "url": None, "db_schema": None, + "ssh_required": False, } payload = {"url": "postgresql://test_user:test_pass@localhost:1234/my_test_db"} @@ -1336,6 +1341,7 @@ def test_put_connection_config_secrets( "password": None, "url": payload["url"], "db_schema": None, + "ssh_required": False, } assert connection_config.last_test_timestamp is None assert connection_config.last_test_succeeded is None From 74d2e38ee7b8acf16408d6a680d62b06607fee98 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Wed, 31 May 2023 16:41:01 +0100 Subject: [PATCH 10/17] [skip ci] remove M1 specific Docker change for tag --- Dockerfile | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Dockerfile b/Dockerfile index 8d737063e8..96d559d0ba 100644 --- a/Dockerfile +++ b/Dockerfile @@ -65,11 +65,6 @@ RUN if [ "$TARGETPLATFORM" != "linux/arm64" ] ; \ fi -# Required for M1 Macs currently when connecting to Postgres -# https://github.com/psycopg/psycopg2/issues/1360#issuecomment-1455169030 -RUN pip uninstall psycopg2-binary -y -RUN apt update -y && apt install -y build-essential libpq-dev -RUN pip install psycopg2 # General Application Setup ## COPY . /fides From 790c224ad2ff5013a54e0cf4a8c78ca2a35d3322 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Fri, 2 Jun 2023 00:14:06 +0100 Subject: [PATCH 11/17] move ssh tunnel creation to SQLConnector --- .../api/service/connectors/sql_connector.py | 53 ++++++++----------- 1 file changed, 21 insertions(+), 32 deletions(-) diff --git a/src/fides/api/service/connectors/sql_connector.py b/src/fides/api/service/connectors/sql_connector.py index 78bddde489..ee99d6dca5 100644 --- a/src/fides/api/service/connectors/sql_connector.py +++ b/src/fides/api/service/connectors/sql_connector.py @@ -67,6 +67,7 @@ def __init__(self, configuration: ConnectionConfig): raise NotImplementedError( "SQL Connectors must define their secrets schema class" ) + self.ssh_server: sshtunnel._ForwardServer = None @staticmethod def cursor_result_to_rows(results: CursorResult) -> List[Row]: @@ -167,6 +168,8 @@ def close(self) -> None: if self.db_client: logger.debug(" disposing of {}", self.__class__) self.db_client.dispose() + if self.ssh_server: + self.ssh_server.stop() def create_client(self) -> Engine: """Returns a SQLAlchemy Engine that can be used to interact with a database""" @@ -182,21 +185,29 @@ def set_schema(self, connection: Connection) -> None: """Optionally override to set the schema for a given database that persists through the entire session""" + def create_ssh_tunnel(self, host: str, port: int) -> None: + """Creates an SSH Tunnel to forward ports as configured.""" + with io.BytesIO( + CONFIG.security.bastion_server_ssh_pkey.encode("utf8") + ) as binary_file: + with io.TextIOWrapper(binary_file, encoding="utf8") as file_obj: + pkey = paramiko.RSAKey.from_private_key(file_obj=file_obj) + self.ssh_server = sshtunnel.SSHTunnelForwarder( + (CONFIG.security.bastion_server_host), + ssh_username=CONFIG.security.bastion_server_ssh_username, + ssh_pkey=pkey, + remote_bind_address=( + host, + port, + ), + ) + class PostgreSQLConnector(SQLConnector): """Connector specific to postgresql""" secrets_schema = PostgreSQLSchema - def __init__(self, configuration: ConnectionConfig): - """Instantiate a SQL-based connector""" - super().__init__(configuration) - if not self.secrets_schema: - raise NotImplementedError( - "SQL Connectors must define their secrets schema class" - ) - self.ssh_server: sshtunnel._ForwardServer = None - def build_uri(self) -> str: """Build URI of format postgresql://[user[:password]@][netloc][:port][/dbname]""" config = self.secrets_schema(**self.configuration.secrets or {}) @@ -228,35 +239,13 @@ def build_ssh_uri(self, local_address: tuple) -> str: dbname = f"/{config.dbname}" if config.dbname else "" return f"postgresql://{user_password}{netloc}{port}{dbname}" - def close(self) -> None: - """Close any held resources""" - if self.db_client: - logger.debug(" disposing of {}", self.__class__) - self.db_client.dispose() - if self.ssh_server: - self.ssh_server.stop() - 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 {}) if config.ssh_required and CONFIG.security.bastion_server_ssh_pkey: - with io.BytesIO( - CONFIG.security.bastion_server_ssh_pkey.encode("utf8") - ) as binary_file: - with io.TextIOWrapper(binary_file, encoding="utf8") as file_obj: - pkey = paramiko.RSAKey.from_private_key(file_obj=file_obj) - self.ssh_server = sshtunnel.SSHTunnelForwarder( - (CONFIG.security.bastion_server_host), - ssh_username=CONFIG.security.bastion_server_ssh_username, - ssh_pkey=pkey, - remote_bind_address=( - config.host, - config.port, - ), - ) + 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) else: uri = config.url or self.build_uri() From 7b65ffb6daf92b184ed4d891e16d84550e2c9c79 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Fri, 2 Jun 2023 09:17:17 +0100 Subject: [PATCH 12/17] include ssh support for RedShift I was originally intending on implementing this and using more of the `SQLConnector` Class but it turned out to be more trouble than it was worth without implementing more of the ssh set variable across other db connectors. As this will likely change again in the near future it made sense to keep this more isolated and leave the duplicate code found in the overridden `create_client` method --- .../api/service/connectors/sql_connector.py | 64 ++++++++++++++----- 1 file changed, 47 insertions(+), 17 deletions(-) diff --git a/src/fides/api/service/connectors/sql_connector.py b/src/fides/api/service/connectors/sql_connector.py index ee99d6dca5..2091da8278 100644 --- a/src/fides/api/service/connectors/sql_connector.py +++ b/src/fides/api/service/connectors/sql_connector.py @@ -185,22 +185,23 @@ def set_schema(self, connection: Connection) -> None: """Optionally override to set the schema for a given database that persists through the entire session""" - def create_ssh_tunnel(self, host: str, port: int) -> None: + def create_ssh_tunnel(self, host: Optional[str], port: Optional[int]) -> None: """Creates an SSH Tunnel to forward ports as configured.""" - with io.BytesIO( - CONFIG.security.bastion_server_ssh_pkey.encode("utf8") - ) as binary_file: - with io.TextIOWrapper(binary_file, encoding="utf8") as file_obj: - pkey = paramiko.RSAKey.from_private_key(file_obj=file_obj) - self.ssh_server = sshtunnel.SSHTunnelForwarder( - (CONFIG.security.bastion_server_host), - ssh_username=CONFIG.security.bastion_server_ssh_username, - ssh_pkey=pkey, - remote_bind_address=( - host, - port, - ), - ) + if CONFIG.security.bastion_server_ssh_pkey: + with io.BytesIO( + CONFIG.security.bastion_server_ssh_pkey.encode("utf8") + ) as binary_file: + with io.TextIOWrapper(binary_file, encoding="utf8") as file_obj: + pkey = paramiko.RSAKey.from_private_key(file_obj=file_obj) + self.ssh_server = sshtunnel.SSHTunnelForwarder( + (CONFIG.security.bastion_server_host), + ssh_username=CONFIG.security.bastion_server_ssh_username, + ssh_pkey=pkey, + remote_bind_address=( + host, + port, + ), + ) class PostgreSQLConnector(SQLConnector): @@ -224,7 +225,7 @@ def build_uri(self) -> str: return f"postgresql://{user_password}{netloc}{port}{dbname}" def build_ssh_uri(self, local_address: tuple) -> str: - """Build URI of format postgresql://[user[:password]@][netloc][:port][/dbname]""" + """Build URI of format postgresql://[user[:password]@][ssh_host][:ssh_port][/dbname]""" config = self.secrets_schema(**self.configuration.secrets or {}) user_password = "" @@ -239,9 +240,9 @@ def build_ssh_uri(self, local_address: tuple) -> str: dbname = f"/{config.dbname}" if config.dbname else "" return f"postgresql://{user_password}{netloc}{port}{dbname}" + # Overrides SQLConnector.create_client 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 {}) if config.ssh_required and CONFIG.security.bastion_server_ssh_pkey: self.create_ssh_tunnel(host=config.host, port=config.port) @@ -328,6 +329,19 @@ class RedshiftConnector(SQLConnector): secrets_schema = RedshiftSchema + def build_ssh_uri(self, local_address: tuple) -> str: + """Build SSH URI of format redshift+psycopg2://[user[:password]@][ssh_host][:ssh_port][/dbname]""" + config = self.secrets_schema(**self.configuration.secrets or {}) + + local_host, local_port = local_address + + config = self.secrets_schema(**self.configuration.secrets or {}) + + port = f":{local_port}" if local_port else "" + database = f"/{config.database}" if config.database else "" + url = f"redshift+psycopg2://{config.user}:{config.password}@{local_host}{port}{database}" + return url + # Overrides BaseConnector.build_uri def build_uri(self) -> str: """Build URI of format redshift+psycopg2://user:password@[host][:port][/database]""" @@ -338,6 +352,22 @@ def build_uri(self) -> str: url = f"redshift+psycopg2://{config.user}:{config.password}@{config.host}{port}{database}" return url + # Overrides SQLConnector.create_client + 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 {}) + if config.ssh_required and CONFIG.security.bastion_server_ssh_pkey: + 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) + else: + uri = config.url or self.build_uri() + return create_engine( + uri, + hide_parameters=self.hide_parameters, + echo=not self.hide_parameters, + ) + def set_schema(self, connection: Connection) -> None: """Sets the search_path for the duration of the session""" config = self.secrets_schema(**self.configuration.secrets or {}) From 181f421e7e1311664de4d9785e7979f4a09799d7 Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Tue, 13 Jun 2023 12:34:43 +0100 Subject: [PATCH 13/17] [skip ci] changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ceed49bddc..a3f1c1069b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ The types of changes are: ## [Unreleased](https://github.com/ethyca/fides/compare/2.15.0...main) +### Added +- Included optional env vars to have postgres or Redshift connected via bastion host [#3374](https://github.com/ethyca/fides/pull/3374/) + ## [2.15.0](https://github.com/ethyca/fides/compare/2.14.1...2.15.0) From 018661655d0149a509df1c07dc822c0778afaf9d Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Tue, 13 Jun 2023 20:50:42 +0100 Subject: [PATCH 14/17] increase timeout to 10 seconds for ssh tunnel --- src/fides/api/service/connectors/sql_connector.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/fides/api/service/connectors/sql_connector.py b/src/fides/api/service/connectors/sql_connector.py index 2091da8278..c3ecd21972 100644 --- a/src/fides/api/service/connectors/sql_connector.py +++ b/src/fides/api/service/connectors/sql_connector.py @@ -53,6 +53,9 @@ CONFIG = get_config() +sshtunnel.SSH_TIMEOUT = 10.0 +sshtunnel.TUNNEL_TIMEOUT = 10.0 + class SQLConnector(BaseConnector[Engine]): """A SQL connector represents an abstract connector to any datastore that can be From aa4fbedf42213abf412e8b406110ba3c60ce7767 Mon Sep 17 00:00:00 2001 From: Sean Preston Date: Tue, 13 Jun 2023 16:54:39 -0400 Subject: [PATCH 15/17] throw exception when bastion is triggered without any config --- src/fides/api/common_exceptions.py | 4 ++ .../api/service/connectors/sql_connector.py | 37 +++++++++++-------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/fides/api/common_exceptions.py b/src/fides/api/common_exceptions.py index c6067b8c08..3f1e1bfff7 100644 --- a/src/fides/api/common_exceptions.py +++ b/src/fides/api/common_exceptions.py @@ -219,6 +219,10 @@ class NoSuchConnectionTypeSecretSchemaError(Exception): """Exception for when a connection type secret schema is not found.""" +class SSHTunnelConfigNotFoundException(Exception): + """Exception for when Fides is configured to use an SSH tunnel without config provided.""" + + class AuthenticationError(HTTPException): """To be raised when attempting to fetch an access token using invalid credentials. diff --git a/src/fides/api/service/connectors/sql_connector.py b/src/fides/api/service/connectors/sql_connector.py index c3ecd21972..42ff3daf71 100644 --- a/src/fides/api/service/connectors/sql_connector.py +++ b/src/fides/api/service/connectors/sql_connector.py @@ -19,7 +19,10 @@ from sqlalchemy.sql import Executable # type: ignore from sqlalchemy.sql.elements import TextClause -from fides.api.common_exceptions import ConnectionException +from fides.api.common_exceptions import ( + ConnectionException, + SSHTunnelConfigNotFoundException, +) from fides.api.graph.traversal import TraversalNode from fides.api.models.connectionconfig import ConnectionConfig, ConnectionTestStatus from fides.api.models.policy import Policy @@ -190,22 +193,26 @@ def set_schema(self, connection: Connection) -> None: def create_ssh_tunnel(self, host: Optional[str], port: Optional[int]) -> None: """Creates an SSH Tunnel to forward ports as configured.""" - if CONFIG.security.bastion_server_ssh_pkey: - with io.BytesIO( - CONFIG.security.bastion_server_ssh_pkey.encode("utf8") - ) as binary_file: - with io.TextIOWrapper(binary_file, encoding="utf8") as file_obj: - pkey = paramiko.RSAKey.from_private_key(file_obj=file_obj) - self.ssh_server = sshtunnel.SSHTunnelForwarder( - (CONFIG.security.bastion_server_host), - ssh_username=CONFIG.security.bastion_server_ssh_username, - ssh_pkey=pkey, - remote_bind_address=( - host, - port, - ), + if not CONFIG.security.bastion_server_ssh_pkey: + raise SSHTunnelConfigNotFoundException( + "Fides is configured to use an SSH tunnel without config provided." ) + with io.BytesIO( + CONFIG.security.bastion_server_ssh_pkey.encode("utf8") + ) as binary_file: + with io.TextIOWrapper(binary_file, encoding="utf8") as file_obj: + pkey = paramiko.RSAKey.from_private_key(file_obj=file_obj) + self.ssh_server = sshtunnel.SSHTunnelForwarder( + (CONFIG.security.bastion_server_host), + ssh_username=CONFIG.security.bastion_server_ssh_username, + ssh_pkey=pkey, + remote_bind_address=( + host, + port, + ), + ) + class PostgreSQLConnector(SQLConnector): """Connector specific to postgresql""" From 3f6497016298d2be708131333d0308304c2c2104 Mon Sep 17 00:00:00 2001 From: Sean Preston Date: Tue, 13 Jun 2023 16:59:50 -0400 Subject: [PATCH 16/17] be explicit about private key being private --- docker-compose.yml | 2 +- src/fides/api/service/connectors/sql_connector.py | 13 +++++++------ src/fides/core/config/security_settings.py | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 034f77b05f..e42200790a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -30,7 +30,7 @@ services: FIDES__SECURITY__ALLOW_CUSTOM_CONNECTOR_FUNCTIONS: "True" FIDES__SECURITY__BASTION_SERVER_HOST: ${FIDES__SECURITY__BASTION_SERVER_HOST-} FIDES__SECURITY__BASTION_SERVER_SSH_USERNAME: ${FIDES__SECURITY__BASTION_SERVER_SSH_USERNAME-} - FIDES__SECURITY__BASTION_SERVER_SSH_PKEY: ${FIDES__SECURITY__BASTION_SERVER_SSH_PKEY-} + FIDES__SECURITY__BASTION_SERVER_SSH_PRIVATE_KEY: ${FIDES__SECURITY__BASTION_SERVER_SSH_PRIVATE_KEY-} VAULT_ADDR: ${VAULT_ADDR-} VAULT_NAMESPACE: ${VAULT_NAMESPACE-} VAULT_TOKEN: ${VAULT_TOKEN-} diff --git a/src/fides/api/service/connectors/sql_connector.py b/src/fides/api/service/connectors/sql_connector.py index 42ff3daf71..9e36fcff2e 100644 --- a/src/fides/api/service/connectors/sql_connector.py +++ b/src/fides/api/service/connectors/sql_connector.py @@ -193,20 +193,21 @@ def set_schema(self, connection: Connection) -> None: def create_ssh_tunnel(self, host: Optional[str], port: Optional[int]) -> None: """Creates an SSH Tunnel to forward ports as configured.""" - if not CONFIG.security.bastion_server_ssh_pkey: + if not CONFIG.security.bastion_server_ssh_private_key: raise SSHTunnelConfigNotFoundException( "Fides is configured to use an SSH tunnel without config provided." ) with io.BytesIO( - CONFIG.security.bastion_server_ssh_pkey.encode("utf8") + CONFIG.security.bastion_server_ssh_private_key.encode("utf8") ) as binary_file: with io.TextIOWrapper(binary_file, encoding="utf8") as file_obj: - pkey = paramiko.RSAKey.from_private_key(file_obj=file_obj) + private_key = paramiko.RSAKey.from_private_key(file_obj=file_obj) + self.ssh_server = sshtunnel.SSHTunnelForwarder( (CONFIG.security.bastion_server_host), ssh_username=CONFIG.security.bastion_server_ssh_username, - ssh_pkey=pkey, + ssh_pkey=private_key, remote_bind_address=( host, port, @@ -254,7 +255,7 @@ def build_ssh_uri(self, local_address: tuple) -> 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 {}) - if config.ssh_required and CONFIG.security.bastion_server_ssh_pkey: + 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) @@ -366,7 +367,7 @@ 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 {}) - if config.ssh_required and CONFIG.security.bastion_server_ssh_pkey: + 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) diff --git a/src/fides/core/config/security_settings.py b/src/fides/core/config/security_settings.py index 69e370ff40..fb9dd6b16c 100644 --- a/src/fides/core/config/security_settings.py +++ b/src/fides/core/config/security_settings.py @@ -134,7 +134,7 @@ class SecuritySettings(FidesSettings): default=None, description="An optional field to store the username used to access the bastion server", ) - bastion_server_ssh_pkey: Optional[str] = Field( + bastion_server_ssh_private_key: Optional[str] = Field( default=None, description="An optional field to store the key used to SSH into the bastion server.", ) From d787c433bd764a2ee07716c2bdb5b16a7d3d2cdb Mon Sep 17 00:00:00 2001 From: SteveDMurphy Date: Tue, 13 Jun 2023 22:44:19 +0100 Subject: [PATCH 17/17] allow timeouts for ssh tunnel to be configurable --- src/fides/api/service/connectors/sql_connector.py | 4 ++-- src/fides/core/config/security_settings.py | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/fides/api/service/connectors/sql_connector.py b/src/fides/api/service/connectors/sql_connector.py index 9e36fcff2e..51c9635b2c 100644 --- a/src/fides/api/service/connectors/sql_connector.py +++ b/src/fides/api/service/connectors/sql_connector.py @@ -56,8 +56,8 @@ CONFIG = get_config() -sshtunnel.SSH_TIMEOUT = 10.0 -sshtunnel.TUNNEL_TIMEOUT = 10.0 +sshtunnel.SSH_TIMEOUT = CONFIG.security.bastion_server_ssh_timeout +sshtunnel.TUNNEL_TIMEOUT = CONFIG.security.bastion_server_ssh_tunnel_timeout class SQLConnector(BaseConnector[Engine]): diff --git a/src/fides/core/config/security_settings.py b/src/fides/core/config/security_settings.py index fb9dd6b16c..dbfceea702 100644 --- a/src/fides/core/config/security_settings.py +++ b/src/fides/core/config/security_settings.py @@ -138,6 +138,14 @@ class SecuritySettings(FidesSettings): default=None, description="An optional field to store the key used to SSH into the bastion server.", ) + bastion_server_ssh_timeout: float = Field( + default=0.1, + description="The timeout in seconds for the transport socket (``socket.settimeout``)", + ) + bastion_server_ssh_tunnel_timeout: float = Field( + default=10, + description="The timeout in seconds for tunnel connection (open_channel timeout)", + ) @validator("app_encryption_key") @classmethod