Fail closed for SMB referral transports

This commit is contained in:
2026-07-21 15:43:56 +02:00
parent d5d0df792b
commit 5248e7de4a
7 changed files with 58 additions and 59 deletions

View File

@@ -108,10 +108,10 @@ download-link API; Nextcloud and generic WebDAV profiles use authenticated `GET`
requests against the configured WebDAV endpoint. SMB profiles use requests against the configured WebDAV endpoint. SMB profiles use
`smb://server[:port]/share[/path]` endpoints and deployment-owned or encrypted `smb://server[:port]/share[/path]` endpoints and deployment-owned or encrypted
stored credentials through `smbprotocol`. Because that SDK cannot accept a stored credentials through `smbprotocol`. Because that SDK cannot accept a
preconnected socket, private-network deployments currently require an explicit preconnected socket and may follow DFS referrals to additional hosts, live SMB
IP address in SMB endpoints; DNS names fail closed until connection-time DNS access fails closed in both public-only and private-network deployments. It will
pinning is supported. Public-only deployments resolve and substitute a validated remain disabled until every initial connection and referral target can be
public IP before invoking the SDK. policy-validated and pinned.
The S3 browse/import implementation and S3 managed-storage backend currently The S3 browse/import implementation and S3 managed-storage backend currently
fail closed before creating a `boto3` client. Botocore does not yet use the fail closed before creating a `boto3` client. Botocore does not yet use the

View File

@@ -138,12 +138,13 @@ cd /mnt/DATA/git/govoplan-files/dev/connectors
/mnt/DATA/git/govoplan-core/.venv/bin/python smoke.py /mnt/DATA/git/govoplan-core/.venv/bin/python smoke.py
``` ```
The script reads `.env` from this directory when present, seeds tiny WebDAV, The script reads `.env` from this directory when present and seeds tiny WebDAV,
Nextcloud, SMB, and MinIO fixtures, then browses and imports them through the Nextcloud, SMB, and MinIO fixtures. WebDAV and Nextcloud are browsed and imported
connector helper layer. Pass `--require-smb` after recreating the SMB container through the pinned connector transport. SMB and S3 product access deliberately
to fail on SMB access errors instead of reporting them as an optional skip. Pass fails closed until their SDK transports support peer pinning, so their fixtures
`--require-s3` after starting MinIO and installing `govoplan-files[s3]` to fail are retained for transport development and reported as expected optional skips.
on S3 access errors instead of reporting them as an optional skip. The `--require-smb` and `--require-s3` switches are useful only while developing
that transport support and currently make the smoke check fail by design.
SMB smoke checks need the optional Python dependency in the environment running SMB smoke checks need the optional Python dependency in the environment running
the script: the script:

View File

@@ -60,9 +60,10 @@ Every provider must:
- use a transport that pins every connection to a policy-validated DNS/IP answer - use a transport that pins every connection to a policy-validated DNS/IP answer
and revalidates redirects; SDK transports without that guarantee fail closed and revalidates redirects; SDK transports without that guarantee fail closed
SMB DNS endpoints therefore fail closed in private-network mode until the SDK Live SMB access is disabled in all modes until `smbprotocol` initial connections
supports connection-time pinning; explicit IP endpoints remain available. Live and DFS referral targets can be pinned and policy-validated. An explicit IP is
S3 access is disabled in all modes until `boto3`/botocore can be bound to the not sufficient because the server may still issue a referral to another peer.
Live S3 access is likewise disabled until `boto3`/botocore can be bound to the
pinned transport, including SDK-managed redirects and endpoint discovery. pinned transport, including SDK-managed redirects and endpoint discovery.
## Non-Goals For Files ## Non-Goals For Files

View File

@@ -15,7 +15,7 @@ from defusedxml import ElementTree as SafeElementTree
from govoplan_core.security.outbound_http import ( from govoplan_core.security.outbound_http import (
OutboundHttpError, OutboundHttpError,
pinned_outbound_hostname, validate_unpinned_sdk_host,
validate_unpinned_sdk_http_url, validate_unpinned_sdk_http_url,
) )
@@ -783,7 +783,7 @@ def _smb_location(profile: ConnectorProfile) -> _SmbLocation:
raise ConnectorBrowseError("SMB connector endpoint_url must include a server") raise ConnectorBrowseError("SMB connector endpoint_url must include a server")
port = parsed.port or _int(profile.metadata.get("port")) or 445 port = parsed.port or _int(profile.metadata.get("port")) or 445
try: try:
server = pinned_outbound_hostname(server, port=port, label="SMB connector endpoint") validate_unpinned_sdk_host(server, port=port, label="SMB connector endpoint")
except OutboundHttpError as exc: except OutboundHttpError as exc:
raise ConnectorBrowseError(str(exc)) from exc raise ConnectorBrowseError(str(exc)) from exc
path_parts = [part for part in unquote(parsed.path or "").strip("/").split("/") if part] path_parts = [part for part in unquote(parsed.path or "").strip("/").split("/") if part]

View File

@@ -194,12 +194,12 @@ def _read_webdav_file(profile: ConnectorProfile, *, path: str, max_bytes: int) -
def _read_smb_file(profile: ConnectorProfile, *, path: str, max_bytes: int) -> ConnectorDownloadedFile: def _read_smb_file(profile: ConnectorProfile, *, path: str, max_bytes: int) -> ConnectorDownloadedFile:
location = _smb_location(profile)
file_path = normalize_connector_browse_path(path) file_path = normalize_connector_browse_path(path)
if not file_path: if not file_path:
raise ConnectorImportError("SMB import requires a file path") raise ConnectorImportError("SMB import requires a file path")
unc_path = _smb_unc_path(location, file_path)
try: try:
location = _smb_location(profile)
unc_path = _smb_unc_path(location, file_path)
smbclient = _smbclient_module() smbclient = _smbclient_module()
kwargs = _smb_client_kwargs(profile, location) kwargs = _smb_client_kwargs(profile, location)
stat_result = smbclient.stat(unc_path, **kwargs) stat_result = smbclient.stat(unc_path, **kwargs)

View File

@@ -110,7 +110,7 @@ def connector_provider_descriptors() -> tuple[ConnectorProviderDescriptor, ...]:
conflict_strategy="Managed import/sync uses existing files conflict_strategy handling after download.", 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.", 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"), 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. Private-network mode requires an explicit IP endpoint until the transport supports connection-time DNS pinning.", notes="Browse/import logic is implemented, but live smbprotocol access fails closed until the SDK supports connection-time DNS/IP pinning for initial connections and DFS referrals.",
), ),
ConnectorProviderDescriptor( ConnectorProviderDescriptor(
provider="s3", provider="s3",

View File

@@ -5,7 +5,7 @@ from datetime import UTC, datetime
from unittest.mock import patch from unittest.mock import patch
from govoplan_files.backend.storage.connector_browse import ConnectorBrowseError, _smb_location, browse_connector_profile 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_imports import ConnectorImportError, read_connector_file
from govoplan_files.backend.storage.connector_profiles import ConnectorProfile, connector_profiles_from_payload from govoplan_files.backend.storage.connector_profiles import ConnectorProfile, connector_profiles_from_payload
from govoplan_files.backend.storage.connector_providers import connector_provider_descriptors from govoplan_files.backend.storage.connector_providers import connector_provider_descriptors
@@ -75,58 +75,55 @@ def s3_profile(**overrides: object) -> ConnectorProfile:
class ConnectorProviderTests(unittest.TestCase): class ConnectorProviderTests(unittest.TestCase):
def test_smb_uses_validated_numeric_target_when_private_networks_are_disabled(self) -> None: def test_smb_sdk_transport_fails_closed_before_client_creation_in_all_modes(self) -> None:
profile = ConnectorProfile( profile = ConnectorProfile(
id="public-smb", id="smb",
label="Public SMB", label="SMB",
provider="smb", provider="smb",
endpoint_url="smb://files.example.test/share", endpoint_url="smb://files.example.test/share",
) )
with patch.dict( for allow_private, address in ((False, "93.184.216.34"), (True, "10.0.0.5")):
"os.environ", with self.subTest(allow_private=allow_private), patch.dict(
{"APP_ENV": "production", "GOVOPLAN_CONNECTOR_ALLOW_PRIVATE_NETWORKS": "false"}, "os.environ",
), patch( {
"govoplan_core.security.outbound_http.socket.getaddrinfo", "APP_ENV": "production",
return_value=[(2, 1, 6, "", ("93.184.216.34", 445))], "GOVOPLAN_CONNECTOR_ALLOW_PRIVATE_NETWORKS": str(allow_private).lower(),
): },
location = _smb_location(profile) ), patch(
"govoplan_core.security.outbound_http.socket.getaddrinfo",
return_value=[(2, 1, 6, "", (address, 445))],
), patch("govoplan_files.backend.storage.connector_browse._smbclient_module") as sdk, self.assertRaisesRegex(
ConnectorBrowseError,
"redirects/referrals.*DNS/IP pinning",
):
browse_connector_profile(profile, path="")
sdk.assert_not_called()
self.assertEqual("93.184.216.34", location.server) def test_smb_explicit_ip_still_fails_closed_because_the_sdk_may_follow_referrals(self) -> None:
profile = ConnectorProfile(id="smb", label="SMB", provider="smb", endpoint_url="smb://10.0.0.5/share")
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( with patch.dict(
"os.environ", "os.environ",
{"APP_ENV": "production", "GOVOPLAN_CONNECTOR_ALLOW_PRIVATE_NETWORKS": "true"}, {"APP_ENV": "production", "GOVOPLAN_CONNECTOR_ALLOW_PRIVATE_NETWORKS": "true"},
), patch( ), patch(
"govoplan_core.security.outbound_http.socket.getaddrinfo", "govoplan_core.security.outbound_http.socket.getaddrinfo",
return_value=[(2, 1, 6, "", ("10.0.0.5", 445))], return_value=[(2, 1, 6, "", ("10.0.0.5", 445))],
): ), self.assertRaisesRegex(ConnectorBrowseError, "redirects/referrals.*DNS/IP pinning"):
location = _smb_location(profile) _smb_location(profile)
self.assertEqual("10.0.0.5", location.server) def test_smb_import_surfaces_fail_closed_policy_as_an_import_error(self) -> None:
profile = ConnectorProfile(id="smb", label="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))],
), patch("govoplan_files.backend.storage.connector_imports._smbclient_module") as sdk, self.assertRaisesRegex(
ConnectorImportError,
"redirects/referrals.*DNS/IP pinning",
):
read_connector_file(profile, library_id="", path="notice.txt", max_bytes=1024)
sdk.assert_not_called()
def test_provider_descriptors_include_s3_and_reserved_microsoft_providers(self) -> None: def test_provider_descriptors_include_s3_and_reserved_microsoft_providers(self) -> None:
descriptors = {descriptor.provider: descriptor for descriptor in connector_provider_descriptors()} descriptors = {descriptor.provider: descriptor for descriptor in connector_provider_descriptors()}