diff --git a/README.md b/README.md index 2604124..f5560fc 100644 --- a/README.md +++ b/README.md @@ -107,7 +107,17 @@ uploads. The Seafile provider uses account-token auth and the native file download-link API; Nextcloud and generic WebDAV profiles use authenticated `GET` requests against the configured WebDAV endpoint. SMB profiles use `smb://server[:port]/share[/path]` endpoints and deployment-owned or encrypted -stored credentials through `smbprotocol`. +stored credentials through `smbprotocol`. Because that SDK cannot accept a +preconnected socket, private-network deployments currently require an explicit +IP address in SMB endpoints; DNS names fail closed until connection-time DNS +pinning is supported. Public-only deployments resolve and substitute a validated +public IP before invoking the SDK. + +The S3 browse/import implementation and S3 managed-storage backend currently +fail closed before creating a `boto3` client. Botocore does not yet use the +GovOPlaN pinned HTTP transport and may manage redirects itself, so neither an +explicit endpoint nor SDK endpoint discovery is allowed until both peer pinning +and redirect revalidation are enforced. Local connector development assets live in `dev/connectors/`. The compose stack boots Nextcloud, Seafile, WebDAV, and SMB endpoints for provider development and diff --git a/docs/CONNECTOR_BOUNDARY.md b/docs/CONNECTOR_BOUNDARY.md index d2829d3..2b5bafe 100644 --- a/docs/CONNECTOR_BOUNDARY.md +++ b/docs/CONNECTOR_BOUNDARY.md @@ -57,6 +57,13 @@ Every provider must: - preserve source provenance and revision metadata - emit connector audit events for imported or accessed files - treat remote ACLs as upstream checks, not as a replacement for GovOPlaN policy +- use a transport that pins every connection to a policy-validated DNS/IP answer + and revalidates redirects; SDK transports without that guarantee fail closed + +SMB DNS endpoints therefore fail closed in private-network mode until the SDK +supports connection-time pinning; explicit IP endpoints remain available. Live +S3 access is disabled in all modes until `boto3`/botocore can be bound to the +pinned transport, including SDK-managed redirects and endpoint discovery. ## Non-Goals For Files diff --git a/src/govoplan_files/backend/storage/backends.py b/src/govoplan_files/backend/storage/backends.py index 5b137dd..04ef182 100644 --- a/src/govoplan_files/backend/storage/backends.py +++ b/src/govoplan_files/backend/storage/backends.py @@ -98,24 +98,24 @@ class S3StorageBackend: @property def client(self): - try: - import boto3 - except ModuleNotFoundError as exc: - raise StorageBackendError("boto3 is required for the S3 storage backend") from exc try: endpoint_url = validate_unpinned_sdk_http_url( self.endpoint_url, label="File storage S3 endpoint", ) - return boto3.client( - "s3", - endpoint_url=endpoint_url, - region_name=self.region_name, - aws_access_key_id=self.access_key_id, - aws_secret_access_key=self.secret_access_key, - ) except OutboundHttpError as exc: raise StorageBackendError(str(exc)) from exc + try: + import boto3 + except ModuleNotFoundError as exc: + raise StorageBackendError("boto3 is required for the S3 storage backend") from exc + return boto3.client( + "s3", + endpoint_url=endpoint_url, + region_name=self.region_name, + aws_access_key_id=self.access_key_id, + aws_secret_access_key=self.secret_access_key, + ) def put_bytes(self, key: str, data: bytes, *, content_type: str | None = None) -> None: max_bytes = response_limit("file") diff --git a/src/govoplan_files/backend/storage/connector_browse.py b/src/govoplan_files/backend/storage/connector_browse.py index 7001c31..b0a3f3a 100644 --- a/src/govoplan_files/backend/storage/connector_browse.py +++ b/src/govoplan_files/backend/storage/connector_browse.py @@ -15,7 +15,6 @@ from defusedxml import ElementTree as SafeElementTree from govoplan_core.security.outbound_http import ( OutboundHttpError, - outbound_http_policy, pinned_outbound_hostname, validate_unpinned_sdk_http_url, ) @@ -312,24 +311,25 @@ def _browse_s3(profile: ConnectorProfile, *, path: str, library_id: str | None, def _s3_client(profile: ConnectorProfile) -> Any: if profile.secret_ref: raise ConnectorBrowseError("Secret-ref S3 credentials need a runtime secret resolver before live browsing") - try: - boto3 = import_module("boto3") - config_module = import_module("botocore.config") - except ImportError as exc: - raise ConnectorBrowseUnsupported("S3 connector browsing requires the optional boto3 dependency") from exc - kwargs: dict[str, object] = {} if profile.endpoint_url: try: - kwargs["endpoint_url"] = validate_unpinned_sdk_http_url( + endpoint_url = validate_unpinned_sdk_http_url( profile.endpoint_url, label="S3 connector endpoint", ) except OutboundHttpError as exc: raise ConnectorBrowseError(str(exc)) from exc - elif not outbound_http_policy().allow_private_networks: + else: raise ConnectorBrowseError( - "S3 connector endpoint discovery cannot pin the SDK connection address while private-network access is disabled" + "S3 connector endpoint discovery uses an SDK transport that cannot guarantee connection-time DNS/IP " + "pinning; live S3 access is disabled until that transport supports pinning" ) + try: + boto3 = import_module("boto3") + config_module = import_module("botocore.config") + except ImportError as exc: + raise ConnectorBrowseUnsupported("S3 connector browsing requires the optional boto3 dependency") from exc + kwargs: dict[str, object] = {"endpoint_url": endpoint_url} region = _metadata_string(profile, "region") or _metadata_string(profile, "aws_region") if region: kwargs["region_name"] = region diff --git a/src/govoplan_files/backend/storage/connector_providers.py b/src/govoplan_files/backend/storage/connector_providers.py index c7b9ca2..03318f3 100644 --- a/src/govoplan_files/backend/storage/connector_providers.py +++ b/src/govoplan_files/backend/storage/connector_providers.py @@ -110,7 +110,7 @@ def connector_provider_descriptors() -> tuple[ConnectorProviderDescriptor, ...]: conflict_strategy="Managed import/sync uses existing files conflict_strategy handling after download.", preview_strategy="Previews are generated from the frozen managed file after import or sync, not directly from the share.", audit_events=("files.connector.imported", "files.connector.synced", "files.connector.accessed"), - notes="Requires the optional smbprotocol dependency and an smb://server[:port]/share[/path] profile endpoint.", + notes="Requires the optional smbprotocol dependency and an smb://server[:port]/share[/path] profile endpoint. Private-network mode requires an explicit IP endpoint until the transport supports connection-time DNS pinning.", ), ConnectorProviderDescriptor( provider="s3", @@ -125,7 +125,7 @@ def connector_provider_descriptors() -> tuple[ConnectorProviderDescriptor, ...]: conflict_strategy="Managed import/sync uses existing files conflict_strategy handling after object download.", preview_strategy="Previews are generated from the frozen managed file after import or sync, not directly from the bucket.", audit_events=("files.connector.imported", "files.connector.synced", "files.connector.accessed"), - notes="Configure endpoint_url for MinIO or other S3-compatible stores; use profile metadata for bucket, region, addressing_style, verify_tls, and ca_bundle.", + notes="Browse/import logic is implemented, but live boto3 access fails closed until its HTTP transport supports connection-time DNS/IP pinning and redirect revalidation.", ), ConnectorProviderDescriptor( provider="sharepoint", diff --git a/tests/test_connector_providers.py b/tests/test_connector_providers.py index 610c867..90f3e10 100644 --- a/tests/test_connector_providers.py +++ b/tests/test_connector_providers.py @@ -4,8 +4,7 @@ import unittest from datetime import UTC, datetime from unittest.mock import patch -from govoplan_files.backend.storage.connector_browse import _smb_location, browse_connector_profile -from govoplan_files.backend.storage.connector_browse import ConnectorBrowseUnsupported +from govoplan_files.backend.storage.connector_browse import ConnectorBrowseError, _smb_location, browse_connector_profile from govoplan_files.backend.storage.connector_imports import read_connector_file from govoplan_files.backend.storage.connector_profiles import ConnectorProfile, connector_profiles_from_payload from govoplan_files.backend.storage.connector_providers import connector_provider_descriptors @@ -94,6 +93,41 @@ class ConnectorProviderTests(unittest.TestCase): self.assertEqual("93.184.216.34", location.server) + def test_smb_dns_endpoint_fails_closed_when_private_networks_are_enabled(self) -> None: + profile = ConnectorProfile( + id="private-smb", + label="Private SMB", + provider="smb", + endpoint_url="smb://files.internal.example/share", + ) + with patch.dict( + "os.environ", + {"APP_ENV": "production", "GOVOPLAN_CONNECTOR_ALLOW_PRIVATE_NETWORKS": "true"}, + ), patch("govoplan_core.security.outbound_http.socket.getaddrinfo") as resolver, self.assertRaisesRegex( + ConnectorBrowseError, + "cannot pin DNS resolution", + ): + _smb_location(profile) + resolver.assert_not_called() + + def test_smb_explicit_private_ip_is_a_pinned_target(self) -> None: + profile = ConnectorProfile( + id="private-smb", + label="Private SMB", + provider="smb", + endpoint_url="smb://10.0.0.5/share", + ) + with patch.dict( + "os.environ", + {"APP_ENV": "production", "GOVOPLAN_CONNECTOR_ALLOW_PRIVATE_NETWORKS": "true"}, + ), patch( + "govoplan_core.security.outbound_http.socket.getaddrinfo", + return_value=[(2, 1, 6, "", ("10.0.0.5", 445))], + ): + location = _smb_location(profile) + + self.assertEqual("10.0.0.5", location.server) + def test_provider_descriptors_include_s3_and_reserved_microsoft_providers(self) -> None: descriptors = {descriptor.provider: descriptor for descriptor in connector_provider_descriptors()} @@ -150,10 +184,27 @@ class ConnectorProviderTests(unittest.TestCase): self.assertEqual(["archive"], [item.path for item in items]) - def test_s3_browse_reports_missing_optional_dependency(self) -> None: - with patch("govoplan_files.backend.storage.connector_browse.import_module", side_effect=ImportError): - with self.assertRaisesRegex(ConnectorBrowseUnsupported, "boto3"): - browse_connector_profile(s3_profile(), path="") + def test_s3_sdk_transport_fails_closed_before_client_creation_in_private_mode(self) -> None: + with patch.dict( + "os.environ", + {"APP_ENV": "production", "GOVOPLAN_CONNECTOR_ALLOW_PRIVATE_NETWORKS": "true"}, + ), patch( + "govoplan_core.security.outbound_http.socket.getaddrinfo", + return_value=[(2, 1, 6, "", ("127.0.0.1", 9000))], + ), patch("govoplan_files.backend.storage.connector_browse.import_module") as importer, self.assertRaisesRegex( + ConnectorBrowseError, + "until that transport supports.*DNS/IP pinning", + ): + browse_connector_profile(s3_profile(), path="") + importer.assert_not_called() + + def test_s3_sdk_endpoint_discovery_fails_closed(self) -> None: + with patch("govoplan_files.backend.storage.connector_browse.import_module") as importer, self.assertRaisesRegex( + ConnectorBrowseError, + "endpoint discovery.*cannot guarantee.*DNS/IP pinning", + ): + browse_connector_profile(s3_profile(endpoint_url=None), path="") + importer.assert_not_called() def test_s3_import_downloads_object_and_preserves_remote_identity(self) -> None: client = FakeS3Client() diff --git a/tests/test_http_client.py b/tests/test_http_client.py index a417993..fa705bc 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -58,7 +58,10 @@ class ConnectorHttpClientTests(unittest.TestCase): context = MagicMock() context.__enter__.return_value = response - with patch("govoplan_files.backend.storage.http_client._stream_connector_request", return_value=context): + with patch( + "govoplan_core.security.outbound_http.socket.getaddrinfo", + return_value=[(2, 1, 6, "", ("93.184.216.34", 443))], + ), patch("govoplan_files.backend.storage.http_client._stream_connector_request", return_value=context): result = request_connector_bytes("GET", "https://example.test/object", max_bytes=10) self.assertEqual(b"abcdef", result.content) @@ -72,6 +75,9 @@ class ConnectorHttpClientTests(unittest.TestCase): context.__enter__.return_value = response with patch( + "govoplan_core.security.outbound_http.socket.getaddrinfo", + return_value=[(2, 1, 6, "", ("93.184.216.34", 443))], + ), patch( "govoplan_files.backend.storage.http_client._stream_connector_request", return_value=context, ), self.assertRaisesRegex( diff --git a/tests/test_storage_backends.py b/tests/test_storage_backends.py index f0c856a..2a27357 100644 --- a/tests/test_storage_backends.py +++ b/tests/test_storage_backends.py @@ -18,6 +18,20 @@ def _backend() -> S3StorageBackend: class S3StorageBackendTests(unittest.TestCase): + def test_sdk_transport_fails_closed_in_public_and_private_modes(self) -> None: + for allow_private, address in ((False, "93.184.216.34"), (True, "10.0.0.5")): + with self.subTest(allow_private=allow_private), patch.dict( + "os.environ", + { + "APP_ENV": "production", + "GOVOPLAN_CONNECTOR_ALLOW_PRIVATE_NETWORKS": str(allow_private).lower(), + }, + ), patch( + "govoplan_core.security.outbound_http.socket.getaddrinfo", + return_value=[(2, 1, 6, "", (address, 443))], + ), self.assertRaisesRegex(StorageBackendError, "until that transport supports.*DNS/IP pinning"): + _backend().client + def test_get_bytes_rejects_declared_oversize_object_without_reading(self) -> None: body = MagicMock() client = MagicMock()