diff --git a/changelog.md b/changelog.md index ff324578e..0cf6b9e9c 100644 --- a/changelog.md +++ b/changelog.md @@ -3,6 +3,14 @@ **Release Date:** Unreleased **Previous Version:** 1.8.0 (Released 2026-10-01) +## Major Features & Enhancements + +- **Outbound Host Validation Hardening**: Enhanced `OutboundHostValidator` with HTTPS-only URL enforcement, DNS resolution validation for public and optionally private network addresses, and an SSRF-aware `aiohttp` connector that validates resolved IPv4 and IPv6 addresses before connecting. + +## Deprecation + +- **Attachment Downloader HTTP Configuration**: Replaced custom `client_factory` injection with `client_session_kwargs`; the deprecated `client_factory` parameter is ignored. This ensures attachment requests always use the validator-managed middleware and connector, preventing custom client sessions from bypassing hostname allowlists, redirect validation, or resolved-address checks while still allowing supported `aiohttp.ClientSession` options to be configured. + ## Bug Fixes - **Connector User-Agent Headers**: Added the SDK `User-Agent` header to `MCSConnectorClient` and ensured `ConnectorClient`, `TeamsConnectorClient`, and `UserTokenClient` apply it to caller-provided `ClientSession` instances. diff --git a/libraries/microsoft-agents-hosting-aiohttp/setup.py b/libraries/microsoft-agents-hosting-aiohttp/setup.py index f7a4e1bbb..88b58e29d 100644 --- a/libraries/microsoft-agents-hosting-aiohttp/setup.py +++ b/libraries/microsoft-agents-hosting-aiohttp/setup.py @@ -13,6 +13,6 @@ version=package_version, install_requires=[ f"microsoft-agents-hosting-core=={package_version}", - "aiohttp>=3.11.11", + "aiohttp>=3.12", ], ) diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/__init__.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/__init__.py index 813f800eb..e1338972e 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/__init__.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/__init__.py @@ -9,7 +9,9 @@ from .middleware_set import Middleware, MiddlewareSet from .rest_channel_service_client_factory import RestChannelServiceClientFactory from .turn_context import TurnContext -from .outbound_host_validator import OutboundHostValidator + +# Security +from .security import OutboundHostValidator # HTTP abstractions from .http import ( diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_http_adapter_base.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_http_adapter_base.py index ec8173431..b4dab00b8 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_http_adapter_base.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_http_adapter_base.py @@ -23,7 +23,7 @@ from .message_factory import MessageFactory from .rest_channel_service_client_factory import RestChannelServiceClientFactory from .turn_context import TurnContext -from .outbound_host_validator import OutboundHostValidator, _try_create_url +from .security import OutboundHostValidator, _try_create_url logger = logging.getLogger(__name__) @@ -80,7 +80,7 @@ async def on_turn_error(context: TurnContext, error: Exception): connection_manager, **(channel_service_client_factory_options or {}), ) - self._host_validator = host_validator or OutboundHostValidator() + self._host_validator = host_validator or OutboundHostValidator(enabled=False) super().__init__(factory) @@ -205,7 +205,7 @@ def _validate_service_url( or not activity_url or claim_url_host.casefold() != activity_url_host.casefold() ): - if self._host_validator and self._host_validator.enabled: + if self._host_validator.enabled: logger.warning( "Service URL host mismatch: %s vs %s", claim_url_host, diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py index 7f6a71965..fa4498c54 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py @@ -2,7 +2,8 @@ # Licensed under the MIT License. import json -from typing import Callable +import logging +from typing import Any, Callable import aiohttp @@ -13,27 +14,40 @@ ) from microsoft_agents.hosting.core.turn_context import TurnContext -from microsoft_agents.hosting.core.outbound_host_validator import OutboundHostValidator +from microsoft_agents.hosting.core.security import ( + OutboundHostValidator, + _SSRFError, +) from .input_file import InputFileDownloader, InputFile from ._utils import _parse_content_type, _basic_url_check +logger = logging.getLogger(__name__) + class AttachmentDownloader(InputFileDownloader): """Downloads attachments from a given turn context.""" def __init__( self, - client_factory: Callable[[], aiohttp.ClientSession] | None = None, host_validator: OutboundHostValidator | None = None, + *, + client_session_kwargs: dict | None = None, + client_factory: Any = None, ): """Constructor for AttachmentDownloader. - :param client_factory: A callable that returns an aiohttp.ClientSession instance. :param host_validator: An optional OutboundHostValidator instance. + :param client_session_kwargs: Optional keyword arguments for the aiohttp.ClientSession. + :param client_factory: (deprecated) A custom client factory, if any. """ - self._client_factory = client_factory or aiohttp.ClientSession + if client_factory is not None: + logger.warning( + "The 'client_factory' parameter is deprecated and will be ignored." + ) + + self._client_session_kwargs = client_session_kwargs or {} self._host_validator = host_validator async def download_files(self, context: TurnContext) -> list[InputFile]: @@ -48,31 +62,36 @@ async def download_files(self, context: TurnContext) -> list[InputFile]: if not context.activity.attachments: return [] - files: list[InputFile] = [] - for attachment in context.activity.attachments: - file = await self._download_file(attachment) - if file: - files.append(file) + client_factory: Callable[[], aiohttp.ClientSession] + if self._host_validator is not None: + host_validator = self._host_validator # to capture in lambda + client_factory = lambda: host_validator.client(self._client_session_kwargs) + else: + client_factory = lambda: aiohttp.ClientSession( + **self._client_session_kwargs + ) + async with client_factory() as client: + files: list[InputFile] = [] + for attachment in context.activity.attachments: + file = await self._download_file(client, attachment) + if file: + files.append(file) return files - async def _download_file(self, attachment: Attachment) -> InputFile | None: + async def _download_file( + self, client: aiohttp.ClientSession, attachment: Attachment + ) -> InputFile | None: """Downloads a single file from the given attachment. + :param client: The aiohttp.ClientSession instance to use for downloading the file. :param attachment: The attachment to download. :return: An InputFile instance if the download is successful, None otherwise. """ if attachment.content_url and _basic_url_check(attachment.content_url): remote_file_url = attachment.content_url - if ( - self._host_validator - and self._host_validator.enabled - and not self._host_validator.is_allowed(remote_file_url) - ): - return None - - async with self._client_factory() as client: + try: async with client.get(remote_file_url) as response: if not (200 <= response.status < 300): @@ -94,6 +113,11 @@ async def _download_file(self, attachment: Attachment) -> InputFile | None: content_url=attachment.content_url, filename=attachment.name, ) + except _SSRFError: + logger.warning( + "Outbound host validation failed for an attachment download URL." + ) + return None else: content = bytes(json.dumps(attachment.content), "utf-8") return InputFile( diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py index 736f4d577..04bd9c2a9 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py @@ -2,6 +2,9 @@ # Licensed under the MIT License. import json +import logging +import warnings + from typing import Callable, cast, Any import aiohttp @@ -9,7 +12,6 @@ from microsoft_agents.activity import ( Attachment, Channels, - ChannelId, ) from microsoft_agents.hosting.core.authorization import ( @@ -17,11 +19,16 @@ Connections, ) from microsoft_agents.hosting.core.turn_context import TurnContext -from microsoft_agents.hosting.core.outbound_host_validator import OutboundHostValidator +from microsoft_agents.hosting.core.security import ( + OutboundHostValidator, + _SSRFError, +) from .input_file import InputFileDownloader, InputFile from ._utils import _parse_content_type, _basic_url_check +logger = logging.getLogger(__name__) + class M365AttachmentDownloader(InputFileDownloader): """Downloads attachments from M365/Teams using the configured Token Provider (from Connections).""" @@ -29,27 +36,35 @@ class M365AttachmentDownloader(InputFileDownloader): def __init__( self, connections: Connections, - client_factory: Callable[[], aiohttp.ClientSession] | None = None, host_validator: OutboundHostValidator | None = None, *, + client_session_kwargs: dict[str, Any] | None = None, token_provider_name: str = "", use_anonymous: bool = False, scopes: list[str] | None = None, + client_factory: Any = None, ): """Constructor for M365AttachmentDownloader. :param connections: A Connections instance. - :param client_factory: A callable that returns an aiohttp.ClientSession instance. :param host_validator: An optional OutboundHostValidator instance. + :param client_session_kwargs: Optional keyword arguments to pass to the aiohttp ClientSession constructor. :param token_provider_name: The name of the token provider. :param use_anonymous: Whether to use anonymous access. :param scopes: A list of scopes for the access token. - :param connections: A Connections instance. + :param client_factory: (deprecated) A custom client factory, if any. """ + if client_factory is not None: + warnings.warn( + "The 'client_factory' parameter is deprecated and will be ignored.", + DeprecationWarning, + stacklevel=2, + ) + self._connections = connections - self._client_factory = client_factory or aiohttp.ClientSession self._host_validator = host_validator + self._client_session_kwargs = client_session_kwargs or {} self._token_provider_name = token_provider_name self._use_anonymous = use_anonymous @@ -109,19 +124,32 @@ async def download_files(self, context: TurnContext) -> list[InputFile]: outgoing_audience_claim, self._scopes ) - files: list[InputFile] = [] - for att in attachments: - file = await self._download_file(att, access_token) - if file: - files.append(file) + client_factory: Callable[[], aiohttp.ClientSession] + if self._host_validator is not None: + host_validator = self._host_validator # to + client_factory = lambda: host_validator.client(self._client_session_kwargs) + else: + client_factory = lambda: aiohttp.ClientSession( + **self._client_session_kwargs + ) + async with client_factory() as client: + files: list[InputFile] = [] + for att in attachments: + file = await self._download_file(client, att, access_token) + if file: + files.append(file) return files async def _download_file( - self, attachment: Attachment, access_token: str + self, + client: aiohttp.ClientSession, + attachment: Attachment, + access_token: str, ) -> InputFile | None: """Download a single file from the given attachment. + :param client: The aiohttp.ClientSession instance to use for downloading the file. :param attachment: The Attachment instance. :param access_token: The access token for authentication. :return: An InputFile instance or None if the download fails. @@ -136,16 +164,10 @@ async def _download_file( download_url = attachment.content_url if download_url and _basic_url_check(download_url): - if ( - self._host_validator is not None - and self._host_validator.enabled - and not self._host_validator.is_allowed(download_url) - ): - return None - - async with self._client_factory() as client: + try: async with client.get( - download_url, headers={"Authorization": f"Bearer {access_token}"} + download_url, + headers={"Authorization": f"Bearer {access_token}"}, ) as response: if not (200 <= response.status < 300): return None @@ -165,6 +187,11 @@ async def _download_file( content_url=attachment.content_url, filename=name, ) + except _SSRFError: + logger.warning( + "Outbound host validation failed for an attachment download URL." + ) + return None else: content = bytes(json.dumps(attachment.content), "utf-8") return InputFile( diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/outbound_host_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/outbound_host_validator.py deleted file mode 100644 index ea58f10e3..000000000 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/outbound_host_validator.py +++ /dev/null @@ -1,117 +0,0 @@ -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. - -from yarl import URL - -_DEFAULT_MICROSOFT_HOSTS = [ - "botframework.com", # Bot Connector / channel services URLs - "smba.trafficmanager.net", # Teams service URLs - "teams.microsoft.com", - "teams.microsoft.us", - "graph.microsoft.com", # Microsoft Graph - "sharepoint.com", # Sharepoint / OneDrive hosted attachments - "svc.ms", # Teams attachment CDN - "blob.core.windows.net", # Azure Blob Storage / Attachment Management Service -] - - -def _try_create_url(url: str | URL) -> URL | None: - """Attempts to create a URL object from the given string or URL. - - :param url: The URL string or URL object to create. - :return: A URL object if successful, None otherwise. - """ - try: - return URL(url) if isinstance(url, str) else url - except (ValueError, TypeError): - return None - - -def _normalize(host: str) -> str | None: - """Normalizes a host string to a suffix for comparison. - - :param host: The host string to normalize. - :return: The normalized host suffix, or None if the input is invalid. - """ - if not host: - return None - - host = host.strip().casefold() - - if host.startswith("*."): - host = host[2:] - - url_obj = _try_create_url(host) - if url_obj and url_obj.host: - host = url_obj.host - else: - slash = host.find("/") - if slash >= 0: - host = host[:slash] - - colon = host.find(":") - if colon >= 0: - host = host[:colon] - - return host if host else None - - -class OutboundHostValidator: - """Validates that an outbound URL targets an allowed host before the SDK makes a - server-side, often token-bearing, request to it (e.g. Activity.service_url callbacks or attachment - downloads). This is the SDK's shared anti-SSRF ("allow_hosts") control.""" - - _enabled: bool - _suffixes: set[str] - - def __init__( - self, - enabled: bool = False, - hosts: list[str] | None = None, - include_default_microsoft_hosts: bool = True, - ): - self._enabled = enabled - suffixes: list[str] = [] - if include_default_microsoft_hosts: - suffixes.extend(_DEFAULT_MICROSOFT_HOSTS) - - if hosts: - for host in hosts: - normalized = _normalize(host) - if normalized is not None: - suffixes.append(normalized) - - self._suffixes = set(suffixes) - - @property - def enabled(self) -> bool: - """Gets whether the validator is enabled. If disabled, all outbound hosts are allowed.""" - return self._enabled - - def is_allowed(self, url: str | URL) -> bool: - """Checks whether the given URL is allowed by the validator. - - :param url: The URL to check. - :return: True if the URL is allowed, False otherwise. - """ - if not self._enabled: - return True - - url_obj = _try_create_url(url) - if not url_obj: - return False - - if not url_obj.absolute: - return False - - host = url_obj.host - if not host: - return False - - host = host.casefold() - - for suffix in self._suffixes: - if host == suffix or host.endswith("." + suffix): - return True - - return False diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py new file mode 100644 index 000000000..4c99637e8 --- /dev/null +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py @@ -0,0 +1,14 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +from ._connector import _SSRFError, _SSRFConnector +from ._utils import _try_create_url, _normalize_host +from .outbound_host_validator import OutboundHostValidator + +__all__ = [ + "OutboundHostValidator", + "_SSRFError", + "_SSRFConnector", + "_try_create_url", + "_normalize_host", +] diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py new file mode 100644 index 000000000..17ef00675 --- /dev/null +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py @@ -0,0 +1,58 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +from collections.abc import Sequence +from typing import Protocol + +from aiohttp import TCPConnector +from aiohttp.abc import ResolveResult +from aiohttp.tracing import Trace + + +class _ResolveResultValidator(Protocol): + """Protocol for validating resolved results to prevent SSRF attacks.""" + + def is_allowed(self, resolved: ResolveResult) -> bool: + """Check if the resolved result is valid to prevent SSRF attacks. + + :param resolved: The resolved result to validate. + :return: True if the resolved result is valid, False otherwise. + """ + ... + + +class _SSRFError(ValueError): + """Exception raised when an SSRF attack is detected.""" + + +class _SSRFConnector(TCPConnector): + """TCPConnector subclass that validates resolved host results to prevent SSRF attacks.""" + + def __init__( + self, resolved_result_validator: _ResolveResultValidator, *args, **kwargs + ): + """Initialize the SSRFConnector with a resolved result validator. + + :param resolved_result_validator: The validator used to check resolved host results. + """ + super().__init__(*args, **kwargs) + self._resolved_result_validator = resolved_result_validator + + async def _resolve_host( + self, host: str, port: int, traces: Sequence[Trace] | None = None + ) -> list[ResolveResult]: + """Resolve the host and validate the resolved results to prevent SSRF attacks. + + :param host: The hostname to resolve. + :param port: The port number to resolve. + :param traces: Optional sequence of Trace objects for tracing the resolution process. + :return: A list of validated ResolveResult objects. + :raises _SSRFError: If any resolved result is invalid. + """ + + res = await super()._resolve_host(host, port, traces) + + for r in res: + if not self._resolved_result_validator.is_allowed(r): + raise _SSRFError(f"Invalid resolved result: {r}") + return res diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py new file mode 100644 index 000000000..1c14c8912 --- /dev/null +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py @@ -0,0 +1,31 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +from typing import Callable, Awaitable + +from aiohttp import ClientRequest, ClientHandlerType, ClientResponse + +from ._connector import _SSRFError +from ._validator import _URLValidator + + +def _validator_middleware( + validator: _URLValidator, +) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: + """Creates a middleware that validates outbound URLs using the given validator. + + :param validator: The outbound host validator to use. + :return: A middleware function that validates outbound URLs. + """ + + async def _middleware( + req: ClientRequest, handler: ClientHandlerType + ) -> ClientResponse: + """Middleware that validates outbound URLs using the given host validator.""" + + if not validator.is_allowed(req.url): + raise _SSRFError(f"URL is not allowed by the outbound host validator.") + + return await handler(req) + + return _middleware diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py new file mode 100644 index 000000000..271a22f7a --- /dev/null +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py @@ -0,0 +1,76 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +from __future__ import annotations + +import ipaddress + +from yarl import URL + + +def _try_create_url(url: str | URL) -> URL | None: + """Attempts to create a URL object from the given string or URL. + + :param url: The URL string or URL object to create. + :return: A URL object if successful, None otherwise. + """ + try: + return URL(url) if isinstance(url, str) else url + except (ValueError, TypeError): + return None + + +def _normalize_ip_address(address: str) -> str | None: + """Check for potential ip address and normalize + + :param address: The IP address string to normalize. + :return: The normalized IP address string if valid, None otherwise. + """ + + right_bracket = -1 + if address.startswith("["): + for i, c in enumerate(address[::-1]): + if c == "]": + right_bracket = len(address) - i - 1 + break + + if right_bracket >= 0: + address = address[1:right_bracket] + + try: + return str(ipaddress.ip_address(address)) + except ValueError: + return None + + +def _normalize_host(host: str) -> str | None: + """Normalizes the given host string. + + :param host: The host string to normalize. + :return: The normalized host string. + """ + if not host: + return None + + host = host.strip().casefold() + + ip_address = _normalize_ip_address(host) + if ip_address is not None: + return ip_address + + if host.startswith("*."): + host = host[2:] + + url_obj = _try_create_url(host) + if url_obj and url_obj.host: + host = url_obj.host + else: + slash = host.find("/") + if slash >= 0: + host = host[:slash] + + colon = host.find(":") + if colon >= 0: + host = host[:colon] + + return host if host else None diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_validator.py new file mode 100644 index 000000000..cebf2a69a --- /dev/null +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_validator.py @@ -0,0 +1,35 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +import logging + +from typing import Protocol +from yarl import URL + +from aiohttp import ClientSession + +logger = logging.getLogger(__name__) + + +class _URLValidator(Protocol): + """Protocol for URL validators.""" + + def is_allowed(self, url: str | URL) -> bool: + """Checks if the given URL is allowed. + + :param url: The URL to check. + :return: True if the URL is allowed, False otherwise. + """ + ... + + +class _OutboundHostValidator(_URLValidator, Protocol): + """Protocol for outbound host validators.""" + + def client(self, client_session_kwargs: dict | None = None) -> ClientSession: + """Returns a client session for making outbound requests. + + :param client_session_kwargs: Optional keyword arguments to pass to the ClientSession constructor. + :return: A ClientSession instance. + """ + ... diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py new file mode 100644 index 000000000..2187cbac7 --- /dev/null +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py @@ -0,0 +1,242 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +import ipaddress +import logging + +from functools import lru_cache + +from aiohttp import ClientSession +from aiohttp.abc import ResolveResult + +from yarl import URL + +from ._connector import _ResolveResultValidator, _SSRFConnector, _SSRFError +from ._validator import _OutboundHostValidator +from ._middleware import _validator_middleware +from ._utils import _normalize_host, _try_create_url + +logger = logging.getLogger(__name__) + +_DEFAULT_MICROSOFT_HOSTS = [ + "botframework.com", # Bot Connector / channel services URLs + "smba.trafficmanager.net", # Teams service URLs + "teams.microsoft.com", + "teams.microsoft.us", + "graph.microsoft.com", # Microsoft Graph + "sharepoint.com", # Sharepoint / OneDrive hosted attachments + "svc.ms", # Teams attachment CDN + "blob.core.windows.net", # Azure Blob Storage / Attachment Management Service +] + +_ALLOWED_CLIENT_SESSION_KWARGS = frozenset( + { + "auth", + "auto_decompress", + "base_url", + "cookie_jar", + "cookies", + "fallback_charset_resolver", + "headers", + "json_serialize", + "max_field_size", + "max_line_size", + "raise_for_status", + "read_bufsize", + "requote_redirect_url", + "response_class", + "skip_auto_headers", + "timeout", + "trace_configs", + "version", + "ws_response_class", + } +) + + +def _allow_addr( + address: ipaddress.IPv4Address | ipaddress.IPv6Address, + allow_private_network_addresses: bool, +) -> bool: + """Allows or denies an IP address based on network address rules. + + :param address: The IP address to check. + :param allow_private_network_addresses: Whether to allow private network addresses. + :return: True if the address is allowed, False otherwise. + """ + if ( + address.is_multicast + or address.is_loopback + or address.is_link_local + or address.is_unspecified + or address.is_reserved + ): + return False + + if address.is_global: + return True + if address.is_private and allow_private_network_addresses: + return True + return False + + +class _BasicResolveResultValidator(_ResolveResultValidator): + """Validates resolved network addresses used by the SSRF-safe connector.""" + + def __init__(self, *, allow_private_network_addresses: bool = False): + self._allow_private_network_addresses = allow_private_network_addresses + + def is_allowed(self, resolved: ResolveResult) -> bool: + """Checks if the resolved network address is allowed. + + :param resolved: The resolved network address result. + :return: True if the address is allowed, False otherwise. + """ + + host = resolved.get("host") + + if not host: + return False + + try: + address = ipaddress.ip_address(host) + if not _allow_addr(address, self._allow_private_network_addresses): + logger.warning("Private or loopback network address not allowed") + return False + + except ValueError: + logger.warning("Invalid IP address") + return False + + return True + + +class OutboundHostValidator(_OutboundHostValidator): + """Outbound host validator that checks if URLs are allowed based on configured hosts and network address rules.""" + + def __init__( + self, + hosts: list[str] | None = None, + *, + enabled: bool | None = None, + include_default_microsoft_hosts: bool = True, + allow_private_network_addresses: bool = False, + ): + """ + Initializes the outbound host validator. + + :param hosts: A list of allowed hosts. If None, only the default Microsoft hosts will be considered (if included). + :param enabled: Whether the validator is enabled. If None, it will be automatically enabled if hosts are provided. + :param include_default_microsoft_hosts: Whether to include the default Microsoft hosts in the allowed list. + :param allow_private_network_addresses: Whether to allow private network addresses. + """ + self._enabled = bool(hosts) if enabled is None else enabled + + self._hosts: set[str] = set() + + for host in hosts or []: + normalized_host = _normalize_host(host) + if normalized_host: + self._hosts.add(normalized_host) + else: + logger.warning("Failed to normalize host: %s", host) + + self._allow_private_network_addresses = allow_private_network_addresses + + if include_default_microsoft_hosts: + for host in _DEFAULT_MICROSOFT_HOSTS: + normalized_host = _normalize_host(host) + if normalized_host: + self._hosts.add(normalized_host) + else: + logger.warning("Failed to normalize host: %s", host) + + self._resolved_validator = _BasicResolveResultValidator( + allow_private_network_addresses=self._allow_private_network_addresses + ) + + @property + def enabled(self) -> bool: + """Indicates whether the outbound host validator is enabled.""" + return self._enabled + + def client(self, client_session_kwargs: dict | None = None) -> ClientSession: + """Creates a new client session with the outbound host validator applied. + + :param client_session_kwargs: Optional allowlisted keyword arguments to pass + to the ClientSession constructor. + :return: A new client session with the outbound host validator applied. + """ + + kwargs = client_session_kwargs or {} + unsupported_options = set(kwargs) - _ALLOWED_CLIENT_SESSION_KWARGS + if unsupported_options: + options = ", ".join(sorted(unsupported_options)) + raise ValueError(f"Unsupported ClientSession options: {options}.") + + middleware = [_validator_middleware(self)] + + if not self._enabled: + return ClientSession(**kwargs) + + return ClientSession( + connector=_SSRFConnector(self._resolved_validator), + connector_owner=True, + middlewares=middleware, + **kwargs, + ) + + def is_allowed(self, url: str | URL) -> bool: + """ + Checks if the given URL is allowed based on the configured hosts and network address rules. + + :param url: The URL string or URL object to check. + :return: True if the URL is allowed, False otherwise. + """ + try: + if not self._enabled: + return True + return self._is_allowed(url) + except _SSRFError: + return False + + @lru_cache + def _is_allowed(self, _url: str | URL) -> bool: + """ + Determines if the given URL is allowed based on the configured hosts and network address rules. + + :param _url: The URL string or URL object to check. + :return: True if the URL is allowed, False otherwise. + """ + url = _try_create_url(_url) + if not url: + logger.warning("Invalid URL: %s", _url) + return False + + if url.host is None: + return False + + if url.scheme != "https": + return False + + # disallow user info in the URL (scheme://user:password@localhost) + if url.user or url.password: + return False + + if not url.host: + return False + + try: + address = ipaddress.ip_address(url.host) + if not _allow_addr(address, self._allow_private_network_addresses): + return False + + except ValueError: + pass + + url_host = url.host.casefold() + + for valid_host in self._hosts: + if url_host == valid_host or url_host.endswith("." + valid_host): + return True + return False diff --git a/libraries/microsoft-agents-hosting-core/setup.py b/libraries/microsoft-agents-hosting-core/setup.py index 56079d3ee..fa8450d45 100644 --- a/libraries/microsoft-agents-hosting-core/setup.py +++ b/libraries/microsoft-agents-hosting-core/setup.py @@ -18,7 +18,7 @@ "python-dotenv>=1.1.1", "opentelemetry-api>=1.27.0", "opentelemetry-sdk>=1.27.0", - "aiohttp>=3.11.11", + "aiohttp>=3.12", "yarl>=1.17.0,<2.0", "azure-core", ], diff --git a/libraries/microsoft-agents-hosting-msteams/setup.py b/libraries/microsoft-agents-hosting-msteams/setup.py index c3cb4c4fa..668348f94 100644 --- a/libraries/microsoft-agents-hosting-msteams/setup.py +++ b/libraries/microsoft-agents-hosting-msteams/setup.py @@ -13,7 +13,7 @@ version=package_version, install_requires=[ f"microsoft-agents-hosting-core=={package_version}", - "aiohttp>=3.11.11", + "aiohttp>=3.12", "microsoft-teams-api==2.1.0", "msgraph-sdk>=1.58.0", ], diff --git a/libraries/microsoft-agents-hosting-slack/setup.py b/libraries/microsoft-agents-hosting-slack/setup.py index f7a4e1bbb..88b58e29d 100644 --- a/libraries/microsoft-agents-hosting-slack/setup.py +++ b/libraries/microsoft-agents-hosting-slack/setup.py @@ -13,6 +13,6 @@ version=package_version, install_requires=[ f"microsoft-agents-hosting-core=={package_version}", - "aiohttp>=3.11.11", + "aiohttp>=3.12", ], ) diff --git a/libraries/microsoft-agents-hosting-teams/setup.py b/libraries/microsoft-agents-hosting-teams/setup.py index 62611d341..ce37c4139 100644 --- a/libraries/microsoft-agents-hosting-teams/setup.py +++ b/libraries/microsoft-agents-hosting-teams/setup.py @@ -13,7 +13,7 @@ version=package_version, install_requires=[ f"microsoft-agents-hosting-core=={package_version}", - "aiohttp>=3.11.11", + "aiohttp>=3.12", "microsoft-teams-api>=2.0.0,<3", ], ) diff --git a/tests/hosting_core/app/test_attachment_downloader.py b/tests/hosting_core/app/test_attachment_downloader.py index 799b3478f..d9ccdcc8a 100644 --- a/tests/hosting_core/app/test_attachment_downloader.py +++ b/tests/hosting_core/app/test_attachment_downloader.py @@ -14,7 +14,7 @@ ) from microsoft_agents.hosting.core import TurnContext from microsoft_agents.hosting.core.app.attachment_downloader import AttachmentDownloader -from microsoft_agents.hosting.core.outbound_host_validator import OutboundHostValidator +from microsoft_agents.hosting.core.security import OutboundHostValidator, _SSRFError def _make_context( @@ -53,11 +53,18 @@ async def __aexit__(self, *args) -> bool: class _FakeSession: - def __init__(self, response: _FakeResponse): + def __init__( + self, + response: _FakeResponse, + validator: OutboundHostValidator | None = None, + ): self._response = response + self._validator = validator self.requested_urls: list[str] = [] def get(self, url: str, **kwargs) -> _FakeResponse: + if self._validator and not self._validator.is_allowed(url): + raise _SSRFError(f"URL '{url}' is not allowed") self.requested_urls.append(url) return self._response @@ -68,6 +75,13 @@ async def __aexit__(self, *args) -> bool: return False +def _patch_client_session(monkeypatch, session: _FakeSession) -> None: + monkeypatch.setattr( + "microsoft_agents.hosting.core.app.attachment_downloader.aiohttp.ClientSession", + lambda **kwargs: session, + ) + + class TestAttachmentDownloaderChannelAndEmptyCases: @pytest.mark.asyncio async def test_returns_empty_list_for_teams_channel(self): @@ -112,12 +126,13 @@ async def test_downloads_inline_content_as_json_bytes(self): class TestAttachmentDownloaderRemoteContent: @pytest.mark.asyncio - async def test_downloads_remote_file_and_returns_input_file(self): + async def test_downloads_remote_file_and_returns_input_file(self, monkeypatch): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) session = _FakeSession(response) - downloader = AttachmentDownloader(client_factory=lambda: session) + _patch_client_session(monkeypatch, session) + downloader = AttachmentDownloader() attachment = Attachment( content_type="text/plain", content_url="https://example.org/file.txt", @@ -135,12 +150,13 @@ async def test_downloads_remote_file_and_returns_input_file(self): assert session.requested_urls == ["https://example.org/file.txt"] @pytest.mark.asyncio - async def test_allows_http_localhost_urls(self): + async def test_allows_http_localhost_urls_without_a_validator(self, monkeypatch): response = _FakeResponse( status=200, content=b"local-bytes", content_type="text/plain" ) session = _FakeSession(response) - downloader = AttachmentDownloader(client_factory=lambda: session) + _patch_client_session(monkeypatch, session) + downloader = AttachmentDownloader() attachment = Attachment( content_type="text/plain", content_url="http://localhost:3000/file.txt" ) @@ -149,36 +165,74 @@ async def test_allows_http_localhost_urls(self): files = await downloader.download_files(context) assert len(files) == 1 + assert session.requested_urls == ["http://localhost:3000/file.txt"] @pytest.mark.asyncio @pytest.mark.parametrize( "content_url", [ - "http://localhost.evil.example/file.txt", - "http://localhost@evil.example/file.txt", + "http://example.org/file.txt", + "ftp://example.org/file.txt", + "/relative/file.txt", ], ) - async def test_does_not_request_spoofed_localhost_urls(self, content_url): + async def test_does_not_request_urls_that_fail_basic_validation( + self, monkeypatch, content_url + ): + response = _FakeResponse( + status=200, content=b"remote-bytes", content_type="text/plain" + ) + session = _FakeSession(response) + _patch_client_session(monkeypatch, session) + downloader = AttachmentDownloader() + attachment = Attachment( + content_type="application/vnd.custom", + content={"source": "inline"}, + content_url=content_url, + name="data.json", + ) + context = _make_context(attachments=[attachment]) + + files = await downloader.download_files(context) + + assert len(files) == 1 + assert files[0].content == bytes(json.dumps({"source": "inline"}), "utf-8") + assert session.requested_urls == [] + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "content_url", + [ + "https://localhost.evil.example/file.txt", + "https://localhost@evil.example/file.txt", + ], + ) + async def test_forwards_urls_without_a_host_validator( + self, monkeypatch, content_url + ): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) session = _FakeSession(response) - downloader = AttachmentDownloader(client_factory=lambda: session) + _patch_client_session(monkeypatch, session) + downloader = AttachmentDownloader() context = _make_context( attachments=[Attachment(content_type="text/plain", content_url=content_url)] ) - await downloader.download_files(context) + files = await downloader.download_files(context) - assert session.requested_urls == [] + assert len(files) == 1 + assert session.requested_urls == [content_url] @pytest.mark.asyncio - async def test_accepts_partial_content_response(self): + async def test_accepts_partial_content_response(self, monkeypatch): response = _FakeResponse( status=206, content=b"partial-bytes", content_type="text/plain" ) session = _FakeSession(response) - downloader = AttachmentDownloader(client_factory=lambda: session) + _patch_client_session(monkeypatch, session) + downloader = AttachmentDownloader() context = _make_context( attachments=[ Attachment( @@ -194,12 +248,13 @@ async def test_accepts_partial_content_response(self): assert files[0].content == b"partial-bytes" @pytest.mark.asyncio - async def test_normalizes_image_content_type_to_png(self): + async def test_normalizes_image_content_type_to_png(self, monkeypatch): response = _FakeResponse( status=200, content=b"\x89PNG", content_type="image/jpeg" ) session = _FakeSession(response) - downloader = AttachmentDownloader(client_factory=lambda: session) + _patch_client_session(monkeypatch, session) + downloader = AttachmentDownloader() attachment = Attachment( content_type="image/jpeg", content_url="https://example.org/pic.jpg" ) @@ -210,10 +265,11 @@ async def test_normalizes_image_content_type_to_png(self): assert files[0].content_type == "image/png" @pytest.mark.asyncio - async def test_returns_none_for_non_success_status(self): + async def test_returns_none_for_non_success_status(self, monkeypatch): response = _FakeResponse(status=404, content=b"", content_type="text/plain") session = _FakeSession(response) - downloader = AttachmentDownloader(client_factory=lambda: session) + _patch_client_session(monkeypatch, session) + downloader = AttachmentDownloader() attachment = Attachment( content_type="text/plain", content_url="https://example.org/missing.txt" ) @@ -222,15 +278,16 @@ async def test_returns_none_for_non_success_status(self): assert await downloader.download_files(context) == [] @pytest.mark.asyncio - async def test_skips_disallowed_hosts_when_host_validator_enabled(self): + async def test_skips_disallowed_hosts_when_host_validator_enabled( + self, monkeypatch + ): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) - session = _FakeSession(response) host_validator = OutboundHostValidator(enabled=True, hosts=["contoso.com"]) - downloader = AttachmentDownloader( - client_factory=lambda: session, host_validator=host_validator - ) + session = _FakeSession(response, validator=host_validator) + monkeypatch.setattr(host_validator, "client", lambda kwargs: session) + downloader = AttachmentDownloader(host_validator=host_validator) attachment = Attachment( content_type="text/plain", content_url="https://evil.example.com/file.txt" ) @@ -240,15 +297,16 @@ async def test_skips_disallowed_hosts_when_host_validator_enabled(self): assert session.requested_urls == [] @pytest.mark.asyncio - async def test_allows_permitted_hosts_when_host_validator_enabled(self): + async def test_allows_permitted_hosts_when_host_validator_enabled( + self, monkeypatch + ): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) - session = _FakeSession(response) host_validator = OutboundHostValidator(enabled=True, hosts=["contoso.com"]) - downloader = AttachmentDownloader( - client_factory=lambda: session, host_validator=host_validator - ) + session = _FakeSession(response, validator=host_validator) + monkeypatch.setattr(host_validator, "client", lambda kwargs: session) + downloader = AttachmentDownloader(host_validator=host_validator) attachment = Attachment( content_type="text/plain", content_url="https://contoso.com/file.txt" ) diff --git a/tests/hosting_core/app/test_m365_attachment_downloader.py b/tests/hosting_core/app/test_m365_attachment_downloader.py index f7abc6d78..32c1d43d9 100644 --- a/tests/hosting_core/app/test_m365_attachment_downloader.py +++ b/tests/hosting_core/app/test_m365_attachment_downloader.py @@ -21,7 +21,7 @@ from microsoft_agents.hosting.core.app.m365_attachment_downloader import ( M365AttachmentDownloader, ) -from microsoft_agents.hosting.core.outbound_host_validator import OutboundHostValidator +from microsoft_agents.hosting.core.security import OutboundHostValidator, _SSRFError class _FakeTokenProvider(AccessTokenProviderBase): @@ -121,11 +121,18 @@ async def __aexit__(self, *args) -> bool: class _FakeSession: - def __init__(self, response: _FakeResponse): + def __init__( + self, + response: _FakeResponse, + validator: OutboundHostValidator | None = None, + ): self._response = response + self._validator = validator self.requested_urls: list[str] = [] def get(self, url: str, headers=None, **kwargs) -> _FakeResponse: + if self._validator and not self._validator.is_allowed(url): + raise _SSRFError(f"URL '{url}' is not allowed") self.requested_urls.append(url) self._response.request_headers = headers return self._response @@ -137,6 +144,13 @@ async def __aexit__(self, *args) -> bool: return False +def _patch_client_session(monkeypatch, session: _FakeSession) -> None: + monkeypatch.setattr( + "microsoft_agents.hosting.core.app.m365_attachment_downloader.aiohttp.ClientSession", + lambda **kwargs: session, + ) + + class TestM365AttachmentDownloaderChannelAndValidation: @pytest.mark.asyncio async def test_raises_when_context_has_no_identity(self): @@ -203,14 +217,14 @@ async def test_returns_empty_list_for_non_teams_channel(self): assert await downloader.download_files(context) == [] @pytest.mark.asyncio - async def test_allows_m365_copilot_channel(self): + async def test_allows_m365_copilot_channel(self, monkeypatch): response = _FakeResponse( status=200, content=b"bytes", content_type="text/plain" ) session = _FakeSession(response) + _patch_client_session(monkeypatch, session) downloader = M365AttachmentDownloader( connections=_FakeConnections(_FakeTokenProvider("p")), - client_factory=lambda: session, ) context = _make_context( channel_id="msteams:COPILOT", @@ -268,14 +282,17 @@ async def test_downloads_inline_content_as_json_bytes(self): class TestM365AttachmentDownloaderRemoteContent: @pytest.mark.asyncio - async def test_downloads_remote_file_using_download_url_from_content(self): + async def test_downloads_remote_file_using_download_url_from_content( + self, monkeypatch + ): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) session = _FakeSession(response) + _patch_client_session(monkeypatch, session) token_provider = _FakeTokenProvider("p") downloader = M365AttachmentDownloader( - connections=_FakeConnections(token_provider), client_factory=lambda: session + connections=_FakeConnections(token_provider) ) attachment = Attachment( content_type="text/plain", @@ -293,14 +310,16 @@ async def test_downloads_remote_file_using_download_url_from_content(self): assert session.requested_urls == ["https://example.org/real-download"] @pytest.mark.asyncio - async def test_falls_back_to_content_url_when_download_url_missing(self): + async def test_falls_back_to_content_url_when_download_url_missing( + self, monkeypatch + ): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) session = _FakeSession(response) + _patch_client_session(monkeypatch, session) downloader = M365AttachmentDownloader( connections=_FakeConnections(_FakeTokenProvider("p")), - client_factory=lambda: session, ) attachment = Attachment( content_type="text/plain", @@ -314,22 +333,81 @@ async def test_falls_back_to_content_url_when_download_url_missing(self): assert len(files) == 1 assert session.requested_urls == ["https://example.org/file.txt"] + @pytest.mark.asyncio + async def test_allows_http_localhost_download_urls(self, monkeypatch): + response = _FakeResponse( + status=200, content=b"local-bytes", content_type="text/plain" + ) + session = _FakeSession(response) + _patch_client_session(monkeypatch, session) + downloader = M365AttachmentDownloader( + connections=_FakeConnections(_FakeTokenProvider("p")), + ) + attachment = Attachment( + content_type="text/plain", + content_url="https://example.org/file.txt", + content={"downloadUrl": "http://localhost:3000/file.txt"}, + ) + context = _make_context(attachments=[attachment]) + + files = await downloader.download_files(context) + + assert len(files) == 1 + assert session.requested_urls == ["http://localhost:3000/file.txt"] + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "download_url", + [ + "http://example.org/file.txt", + "ftp://example.org/file.txt", + "/relative/file.txt", + ], + ) + async def test_does_not_request_urls_that_fail_basic_validation( + self, monkeypatch, download_url + ): + response = _FakeResponse( + status=200, content=b"remote-bytes", content_type="text/plain" + ) + session = _FakeSession(response) + _patch_client_session(monkeypatch, session) + downloader = M365AttachmentDownloader( + connections=_FakeConnections(_FakeTokenProvider("p")), + ) + content = {"downloadUrl": download_url, "source": "inline"} + attachment = Attachment( + content_type="application/vnd.custom", + content_url="https://example.org/file.txt", + content=content, + name="data.json", + ) + context = _make_context(attachments=[attachment]) + + files = await downloader.download_files(context) + + assert len(files) == 1 + assert files[0].content == bytes(json.dumps(content), "utf-8") + assert session.requested_urls == [] + @pytest.mark.asyncio @pytest.mark.parametrize( "download_url", [ - "http://localhost.evil.example/file.txt", - "http://localhost@evil.example/file.txt", + "https://localhost.evil.example/file.txt", + "https://localhost@evil.example/file.txt", ], ) - async def test_does_not_request_spoofed_localhost_download_urls(self, download_url): + async def test_forwards_download_urls_without_a_host_validator( + self, monkeypatch, download_url + ): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) session = _FakeSession(response) + _patch_client_session(monkeypatch, session) downloader = M365AttachmentDownloader( connections=_FakeConnections(_FakeTokenProvider("p")), - client_factory=lambda: session, ) attachment = Attachment( content_type="text/plain", @@ -338,19 +416,20 @@ async def test_does_not_request_spoofed_localhost_download_urls(self, download_u ) context = _make_context(attachments=[attachment]) - await downloader.download_files(context) + files = await downloader.download_files(context) - assert session.requested_urls == [] + assert len(files) == 1 + assert session.requested_urls == [download_url] @pytest.mark.asyncio - async def test_accepts_partial_content_response(self): + async def test_accepts_partial_content_response(self, monkeypatch): response = _FakeResponse( status=206, content=b"partial-bytes", content_type="text/plain" ) session = _FakeSession(response) + _patch_client_session(monkeypatch, session) downloader = M365AttachmentDownloader( connections=_FakeConnections(_FakeTokenProvider("p")), - client_factory=lambda: session, ) context = _make_context( attachments=[ @@ -367,14 +446,14 @@ async def test_accepts_partial_content_response(self): assert files[0].content == b"partial-bytes" @pytest.mark.asyncio - async def test_normalizes_image_content_type_to_png(self): + async def test_normalizes_image_content_type_to_png(self, monkeypatch): response = _FakeResponse( status=200, content=b"\x89PNG", content_type="image/jpeg" ) session = _FakeSession(response) + _patch_client_session(monkeypatch, session) downloader = M365AttachmentDownloader( connections=_FakeConnections(_FakeTokenProvider("p")), - client_factory=lambda: session, ) attachment = Attachment( content_type="image/jpeg", content_url="https://example.org/pic.jpg" @@ -386,12 +465,12 @@ async def test_normalizes_image_content_type_to_png(self): assert files[0].content_type == "image/png" @pytest.mark.asyncio - async def test_returns_none_entry_for_failed_status(self): + async def test_returns_none_entry_for_failed_status(self, monkeypatch): response = _FakeResponse(status=404, content=b"", content_type="text/plain") session = _FakeSession(response) + _patch_client_session(monkeypatch, session) downloader = M365AttachmentDownloader( connections=_FakeConnections(_FakeTokenProvider("p")), - client_factory=lambda: session, ) attachment = Attachment( content_type="text/plain", content_url="https://example.org/missing.txt" @@ -401,15 +480,17 @@ async def test_returns_none_entry_for_failed_status(self): assert await downloader.download_files(context) == [] @pytest.mark.asyncio - async def test_skips_disallowed_hosts_when_host_validator_enabled(self): + async def test_skips_disallowed_hosts_when_host_validator_enabled( + self, monkeypatch + ): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) - session = _FakeSession(response) host_validator = OutboundHostValidator(enabled=True, hosts=["contoso.com"]) + session = _FakeSession(response, validator=host_validator) + monkeypatch.setattr(host_validator, "client", lambda kwargs: session) downloader = M365AttachmentDownloader( connections=_FakeConnections(_FakeTokenProvider("p")), - client_factory=lambda: session, host_validator=host_validator, ) attachment = Attachment( @@ -423,15 +504,15 @@ async def test_skips_disallowed_hosts_when_host_validator_enabled(self): assert session.requested_urls == [] @pytest.mark.asyncio - async def test_uses_anonymous_mode_without_requesting_token(self): + async def test_uses_anonymous_mode_without_requesting_token(self, monkeypatch): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) session = _FakeSession(response) + _patch_client_session(monkeypatch, session) token_provider = _FakeTokenProvider("p") downloader = M365AttachmentDownloader( connections=_FakeConnections(token_provider), - client_factory=lambda: session, use_anonymous=True, ) attachment = Attachment( @@ -445,13 +526,14 @@ async def test_uses_anonymous_mode_without_requesting_token(self): assert token_provider.requested is None @pytest.mark.asyncio - async def test_uses_named_token_provider_when_configured(self): + async def test_uses_named_token_provider_when_configured(self, monkeypatch): named_provider = _FakeTokenProvider("named") + session = _FakeSession( + _FakeResponse(status=200, content=b"bytes", content_type="text/plain") + ) + _patch_client_session(monkeypatch, session) downloader = M365AttachmentDownloader( connections=_FakeConnections(named_provider), - client_factory=lambda: _FakeSession( - _FakeResponse(status=200, content=b"bytes", content_type="text/plain") - ), token_provider_name="named-connection", ) attachment = Attachment( diff --git a/tests/hosting_core/test_outbound_host_validator.py b/tests/hosting_core/test_outbound_host_validator.py index a0fc66988..1577c55f7 100644 --- a/tests/hosting_core/test_outbound_host_validator.py +++ b/tests/hosting_core/test_outbound_host_validator.py @@ -1,14 +1,47 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +import socket +from types import SimpleNamespace +from unittest.mock import AsyncMock, Mock + import pytest +from aiohttp import ClientTimeout, TCPConnector +from aiohttp.abc import ResolveResult from yarl import URL -from microsoft_agents.hosting.core.outbound_host_validator import ( - OutboundHostValidator, - _normalize, +from microsoft_agents.hosting.core.security._connector import ( + _SSRFConnector, + _SSRFError, +) +from microsoft_agents.hosting.core.security._middleware import ( + _validator_middleware, +) +from microsoft_agents.hosting.core.security._utils import ( + _normalize_host, _try_create_url, ) +from microsoft_agents.hosting.core.security.outbound_host_validator import ( + OutboundHostValidator, + _BasicResolveResultValidator, +) + + +def _resolve_result( + host: str, + *, + hostname: str = "example.com", + family: int = socket.AF_INET, + port: int = 443, +) -> ResolveResult: + return { + "hostname": hostname, + "host": host, + "port": port, + "family": family, + "proto": 0, + "flags": 0, + } class TestTryCreateUrl: @@ -22,12 +55,19 @@ def test_returns_existing_url(self): assert _try_create_url(url) is url - @pytest.mark.parametrize("url", ["http://[invalid", "https://example.com:invalid"]) + @pytest.mark.parametrize( + "url", + [ + "http://[invalid", + "https://example.com:invalid", + None, + ], + ) def test_returns_none_for_invalid_url(self, url): assert _try_create_url(url) is None -class TestNormalize: +class TestNormalizeHost: @pytest.mark.parametrize( ("host", "expected"), [ @@ -37,14 +77,92 @@ class TestNormalize: ("example.com:443", "example.com"), ("example.com/path", "example.com"), ("https://Example.COM:443/path", "example.com"), + ("2606:4700:4700::1111", "2606:4700:4700::1111"), + (" 2606:4700:4700::1111 ", "2606:4700:4700::1111"), + ("[2606:4700:4700::1111]", "2606:4700:4700::1111"), + ("[2606:4700:4700::1111]:443", "2606:4700:4700::1111"), + ( + "https://[2606:4700:4700::1111]:443/path", + "2606:4700:4700::1111", + ), ], ) def test_normalizes_host(self, host, expected): - assert _normalize(host) == expected + assert _normalize_host(host) == expected @pytest.mark.parametrize("host", ["", " "]) def test_returns_none_for_empty_normalized_host(self, host): - assert _normalize(host) is None + assert _normalize_host(host) is None + + +class TestBasicResolveResultValidator: + @pytest.mark.parametrize( + "host", + [ + "8.8.8.8", + "2606:4700:4700::1111", + ], + ) + def test_allows_global_addresses(self, host): + validator = _BasicResolveResultValidator() + + assert validator.is_allowed(_resolve_result(host)) is True + + @pytest.mark.parametrize( + "host", + [ + "10.0.0.1", + "127.0.0.1", + "169.254.169.254", + "::1", + "fe80::1", + ], + ) + def test_denies_non_global_addresses_by_default(self, host): + validator = _BasicResolveResultValidator() + + assert validator.is_allowed(_resolve_result(host)) is False + + @pytest.mark.parametrize("host", ["10.0.0.1", "fd00::1"]) + def test_allows_private_addresses_when_configured(self, host): + validator = _BasicResolveResultValidator(allow_private_network_addresses=True) + + assert validator.is_allowed(_resolve_result(host)) is True + + @pytest.mark.parametrize( + "host", + [ + "127.0.0.1", + "::1", + "169.254.169.254", + "fe80::1", + "0.0.0.0", + "240.0.0.1", + ], + ) + def test_denies_special_addresses_when_private_addresses_are_configured(self, host): + validator = _BasicResolveResultValidator(allow_private_network_addresses=True) + + assert validator.is_allowed(_resolve_result(host)) is False + + @pytest.mark.parametrize( + "resolved", + [ + _resolve_result("not-an-ip"), + { + "hostname": "example.com", + "host": "", + "port": 443, + "family": socket.AF_INET, + "proto": 0, + "flags": 0, + }, + ], + ) + def test_denies_invalid_resolved_results(self, resolved): + validator = _BasicResolveResultValidator() + + assert validator.is_allowed(resolved) is False class TestOutboundHostValidator: @@ -70,6 +188,26 @@ def test_default_options_disable_enforcement(self): assert validator.enabled is False assert validator.is_allowed("https://evil.example.com/relay") is True + def test_explicitly_disabled_with_hosts_remains_disabled(self): + validator = OutboundHostValidator( + hosts=["contoso.com"], + enabled=False, + include_default_microsoft_hosts=False, + ) + + assert validator.enabled is False + assert validator.is_allowed("https://evil.example.com/relay") is True + + def test_hosts_enable_enforcement_when_enabled_is_unspecified(self): + validator = OutboundHostValidator( + hosts=["contoso.com"], + include_default_microsoft_hosts=False, + ) + + assert validator.enabled is True + assert validator.is_allowed("https://contoso.com/api") is True + assert validator.is_allowed("https://evil.example.com/api") is False + @pytest.mark.parametrize( "url", [ @@ -84,7 +222,6 @@ def test_default_options_disable_enforcement(self): def test_enabled_allows_first_party_microsoft_hosts(self, url): validator = OutboundHostValidator(enabled=True) - assert validator.enabled is True assert validator.is_allowed(url) is True @pytest.mark.parametrize( @@ -94,11 +231,14 @@ def test_enabled_allows_first_party_microsoft_hosts(self, url): "https://169.254.169.254/latest/meta-data", "https://internal-test.local:8443/secret", "http://localhost/admin", + "http://graph.microsoft.com/v1.0/me", "https://localhost/admin", "https://evil.trafficmanager.net/relay", + "ftp://graph.microsoft.com/file", + None, ], ) - def test_enabled_denies_unknown_hosts(self, url): + def test_enabled_denies_unknown_or_invalid_urls(self, url): validator = OutboundHostValidator(enabled=True) assert validator.is_allowed(url) is False @@ -110,31 +250,31 @@ def test_enabled_denies_unknown_hosts(self, url): "https://contoso.com/some/path", "contoso.com:8443", "contoso.com/path", + "*.contoso.com", ], ) - def test_enabled_normalizes_configured_host(self, configured_host): - validator = OutboundHostValidator(enabled=True, hosts=[configured_host]) + def test_normalizes_configured_host(self, configured_host): + validator = OutboundHostValidator( + hosts=[configured_host], + include_default_microsoft_hosts=False, + ) assert validator.is_allowed("https://contoso.com/api") is True assert validator.is_allowed("https://files.contoso.com/api") is True - def test_enabled_allows_configured_host_exact_and_subdomain(self): - validator = OutboundHostValidator(enabled=True, hosts=["contoso.com"]) + def test_allows_configured_host_exact_and_subdomain(self): + validator = OutboundHostValidator( + hosts=["contoso.com"], + include_default_microsoft_hosts=False, + ) assert validator.is_allowed("https://contoso.com/api") is True assert validator.is_allowed("https://files.contoso.com/api") is True assert validator.is_allowed("https://notcontoso.com/api") is False assert validator.is_allowed("https://contoso.com.evil.com/api") is False - def test_enabled_accepts_wildcard_prefix_in_configured_host(self): - validator = OutboundHostValidator(enabled=True, hosts=["*.fabrikam.com"]) - - assert validator.is_allowed("https://api.fabrikam.com/x") is True - assert validator.is_allowed("https://fabrikam.com/x") is True - - def test_enabled_without_defaults_denies_microsoft_hosts(self): + def test_can_exclude_default_microsoft_hosts(self): validator = OutboundHostValidator( - enabled=True, hosts=["contoso.com"], include_default_microsoft_hosts=False, ) @@ -142,27 +282,13 @@ def test_enabled_without_defaults_denies_microsoft_hosts(self): assert validator.is_allowed("https://graph.microsoft.com/v1.0/me") is False assert validator.is_allowed("https://contoso.com/x") is True - def test_enabled_host_match_is_case_insensitive(self): + def test_host_match_is_case_insensitive(self): validator = OutboundHostValidator(enabled=True) assert validator.is_allowed("https://GRAPH.MICROSOFT.COM/v1.0/me") is True - @pytest.mark.parametrize( - "url", - [ - "not-a-uri", - "/relative/path", - None, - ], - ) - def test_enabled_denies_non_absolute_or_invalid_urls(self, url): - validator = OutboundHostValidator(enabled=True) - - assert validator.is_allowed(url) is False - def test_accepts_url_object(self): validator = OutboundHostValidator( - enabled=True, hosts=["example.com"], include_default_microsoft_hosts=False, ) @@ -171,9 +297,186 @@ def test_accepts_url_object(self): def test_rejects_userinfo_host_confusion(self): validator = OutboundHostValidator( - enabled=True, hosts=["example.com"], include_default_microsoft_hosts=False, ) assert validator.is_allowed("https://example.com@evil.example/path") is False + + def test_private_ip_literal_requires_both_host_and_private_network_opt_in(self): + validator = OutboundHostValidator( + hosts=["10.0.0.1"], + include_default_microsoft_hosts=False, + allow_private_network_addresses=True, + ) + + assert validator.is_allowed("https://10.0.0.1/file") is True + assert validator.is_allowed("https://10.0.0.2/file") is False + + @pytest.mark.asyncio + async def test_client_uses_ssrf_connector_when_enabled(self): + validator = OutboundHostValidator( + hosts=["example.com"], + include_default_microsoft_hosts=False, + ) + + timeout = ClientTimeout(total=30) + client = validator.client( + { + "headers": {"X-Test": "value"}, + "raise_for_status": True, + "timeout": timeout, + } + ) + try: + assert isinstance(client.connector, _SSRFConnector) + assert client.headers["X-Test"] == "value" + assert client.raise_for_status is True + assert client.timeout is timeout + assert len(client._middlewares) == 1 + finally: + await client.close() + + @pytest.mark.asyncio + async def test_client_uses_standard_connector_when_disabled(self): + validator = OutboundHostValidator(enabled=False) + + client = validator.client() + try: + assert isinstance(client.connector, TCPConnector) + assert not isinstance(client.connector, _SSRFConnector) + assert client._middlewares == () + finally: + await client.close() + + @pytest.mark.parametrize( + "option", + [ + {"connector": Mock()}, + {"connector_owner": False}, + {"middlewares": []}, + {"proxy": "http://proxy.example"}, + {"proxy_auth": Mock()}, + {"trust_env": True}, + {"request_class": Mock()}, + {"loop": Mock()}, + {"unknown_option": True}, + ], + ) + def test_client_rejects_non_allowlisted_session_options(self, option): + validator = OutboundHostValidator(enabled=True) + + option_name = next(iter(option)) + with pytest.raises( + ValueError, + match=f"Unsupported ClientSession options: {option_name}", + ): + validator.client(option) + + def test_client_reports_all_non_allowlisted_session_options(self): + validator = OutboundHostValidator(enabled=True) + + with pytest.raises( + ValueError, + match="Unsupported ClientSession options: proxy, trust_env", + ): + validator.client({"trust_env": True, "proxy": "http://proxy.example"}) + + +class TestValidatorMiddleware: + @pytest.mark.asyncio + async def test_calls_handler_when_url_is_allowed(self): + validator = Mock() + validator.is_allowed.return_value = True + request = SimpleNamespace(url=URL("https://example.com/file")) + response = object() + handler = AsyncMock(return_value=response) + + result = await _validator_middleware(validator)(request, handler) + + assert result is response + validator.is_allowed.assert_called_once_with(request.url) + handler.assert_awaited_once_with(request) + + @pytest.mark.asyncio + async def test_raises_ssrf_error_without_calling_handler_when_url_is_denied(self): + validator = Mock() + validator.is_allowed.return_value = False + request = SimpleNamespace(url=URL("https://evil.example/file")) + handler = AsyncMock() + + with pytest.raises( + _SSRFError, match="URL is not allowed by the outbound host validator" + ): + await _validator_middleware(validator)(request, handler) + + handler.assert_not_awaited() + + +class TestSSRFConnector: + @pytest.mark.asyncio + async def test_returns_all_results_when_validator_allows_them(self, monkeypatch): + resolved = [ + _resolve_result("8.8.8.8"), + _resolve_result( + "2606:4700:4700::1111", + family=socket.AF_INET6, + ), + ] + traces = [Mock()] + resolve_host = AsyncMock(return_value=resolved) + monkeypatch.setattr(TCPConnector, "_resolve_host", resolve_host) + validator = Mock() + validator.is_allowed.return_value = True + connector = _SSRFConnector(validator) + + try: + result = await connector._resolve_host("example.com", 443, traces) + finally: + await connector.close() + + assert result is resolved + resolve_host.assert_awaited_once_with("example.com", 443, traces) + assert validator.is_allowed.call_count == 2 + validator.is_allowed.assert_any_call(resolved[0]) + validator.is_allowed.assert_any_call(resolved[1]) + + @pytest.mark.asyncio + async def test_raises_when_any_resolved_result_is_denied(self, monkeypatch): + resolved = [ + _resolve_result("8.8.8.8"), + _resolve_result("127.0.0.1"), + ] + monkeypatch.setattr( + TCPConnector, + "_resolve_host", + AsyncMock(return_value=resolved), + ) + validator = Mock() + validator.is_allowed.side_effect = [True, False] + connector = _SSRFConnector(validator) + + try: + with pytest.raises(_SSRFError, match="127.0.0.1"): + await connector._resolve_host("example.com", 443) + finally: + await connector.close() + + assert validator.is_allowed.call_count == 2 + + @pytest.mark.asyncio + async def test_returns_empty_result_without_validation(self, monkeypatch): + monkeypatch.setattr( + TCPConnector, + "_resolve_host", + AsyncMock(return_value=[]), + ) + validator = Mock() + connector = _SSRFConnector(validator) + + try: + assert await connector._resolve_host("example.com", 443) == [] + finally: + await connector.close() + + validator.is_allowed.assert_not_called()