diff --git a/config/config.yaml b/config/config.yaml index 33b46ff2a6..a610259f4d 100644 --- a/config/config.yaml +++ b/config/config.yaml @@ -330,6 +330,13 @@ service: # Prefix for the names of metrics in the /metrics API. # metrics_prefix: qdrant_ + # Allow snapshot recovery from remote HTTP/HTTPS URLs. + # If disabled, snapshot recovery will only work with local files and uploads. + # Disabling this can mitigate SSRF risks in environments where the Qdrant node + # has access to internal resources that should not be reachable by users. + # Default: true + # enable_snapshot_url_recovery: true + cluster: # Use `enabled: true` to run Qdrant in distributed deployment mode enabled: false diff --git a/src/actix/api/snapshot_api.rs b/src/actix/api/snapshot_api.rs index 36c44e3391..e04a1cf0e2 100644 --- a/src/actix/api/snapshot_api.rs +++ b/src/actix/api/snapshot_api.rs @@ -10,7 +10,7 @@ use collection::common::file_utils::move_file; use collection::common::sha_256; use collection::common::snapshot_stream::SnapshotStream; use collection::operations::snapshot_ops::{ - ShardSnapshotRecover, SnapshotPriority, SnapshotRecover, + ShardSnapshotLocation, ShardSnapshotRecover, SnapshotPriority, SnapshotRecover, }; use collection::operations::types::CollectionError; use collection::operations::verification::new_unchecked_verification_pass; @@ -48,6 +48,7 @@ use crate::common::auth::Auth; use crate::common::collections::*; use crate::common::http_client::HttpClient; use crate::common::snapshots::try_take_partial_snapshot_recovery_lock; +use crate::settings::ServiceConfig; #[derive(Deserialize, Serialize, JsonSchema, Validate)] pub struct SnapshotUploadingParam { @@ -249,6 +250,7 @@ async fn upload_snapshot( async fn recover_from_snapshot( dispatcher: web::Data, http_client: web::Data, + service_config: web::Data, collection: valid::Path, request: valid::Json, params: valid::Query, @@ -256,6 +258,15 @@ async fn recover_from_snapshot( ) -> impl Responder { let future = async move { let snapshot_recover = request.into_inner(); + + if !service_config.enable_snapshot_url_recovery + && matches!(snapshot_recover.location.scheme(), "http" | "https") + { + return Err(StorageError::forbidden( + "Snapshot recovery from remote URLs is disabled in the configuration", + )); + } + let http_client = http_client.client(snapshot_recover.api_key.as_deref())?; do_recover_from_snapshot( @@ -442,6 +453,7 @@ async fn stream_shard_snapshot( async fn recover_shard_snapshot( dispatcher: web::Data, http_client: web::Data, + service_config: web::Data, path: valid::Path, query: web::Query, valid::Json(request): valid::Json, @@ -451,6 +463,15 @@ async fn recover_shard_snapshot( let pass = new_unchecked_verification_pass(); let future = async move { + if !service_config.enable_snapshot_url_recovery + && let ShardSnapshotLocation::Url(url) = &request.location + && matches!(url.scheme(), "http" | "https") + { + return Err(StorageError::forbidden( + "Snapshot recovery from remote URLs is disabled in the configuration", + )); + } + let CollectionShardPath { collection_name: collection, shard, @@ -740,6 +761,7 @@ pub struct PartialSnapshotRecoverFrom { async fn recover_partial_snapshot_from( dispatcher: web::Data, http_client: web::Data, + service_config: web::Data, path: valid::Path, query: web::Query, web::Json(request): web::Json, @@ -752,6 +774,17 @@ async fn recover_partial_snapshot_from( let PartialSnapshotRecoverFrom { peer_url, api_key } = request; let SnapshottingParam { wait } = query.into_inner(); + if !service_config.enable_snapshot_url_recovery && matches!(peer_url.scheme(), "http" | "https") + { + return helpers::process_response_error( + StorageError::forbidden( + "Snapshot recovery from remote URLs is disabled in the configuration", + ), + tokio::time::Instant::now(), + None, + ); + } + // nothing to verify let pass = new_unchecked_verification_pass(); diff --git a/src/settings.rs b/src/settings.rs index 7a40a69cfe..8443c75369 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -97,6 +97,13 @@ pub struct ServiceConfig { #[serde(default)] #[validate(custom(function = validate_metrics_prefix))] pub metrics_prefix: Option, + + /// Whether to allow snapshot recovery from remote URLs (http/https). + /// If disabled, snapshot recovery will only work with local files and uploads. + /// Disabling this can mitigate SSRF risks in environments where the Qdrant node + /// has access to internal resources that should not be reachable by users. + #[serde(default = "default_snapshot_url_recovery")] + pub enable_snapshot_url_recovery: bool, } impl ServiceConfig { @@ -425,6 +432,10 @@ const fn default_cors() -> bool { true } +const fn default_snapshot_url_recovery() -> bool { + true +} + const fn default_http_keep_alive_timeout_sec() -> u64 { 5 } diff --git a/tests/consensus_tests/test_disable_url_snapshot_recovery.py b/tests/consensus_tests/test_disable_url_snapshot_recovery.py new file mode 100644 index 0000000000..031275f36c --- /dev/null +++ b/tests/consensus_tests/test_disable_url_snapshot_recovery.py @@ -0,0 +1,73 @@ +import pathlib + +import requests + +from .fixtures import create_collection +from .utils import * + +COLLECTION_NAME = "test_collection" + +# Substring of the error the service returns when URL-based snapshot recovery +# is disabled through the `service.enable_snapshot_url_recovery=false` config. +BLOCKED_ERROR_MESSAGE = "Snapshot recovery from remote URLs is disabled" + + +def assert_url_recovery_blocked(response: requests.Response): + assert response.status_code == 403, ( + f"Expected 403 Forbidden, got {response.status_code}: {response.text}" + ) + error = response.json()["status"]["error"] + assert BLOCKED_ERROR_MESSAGE in error, ( + f"Expected error message to contain {BLOCKED_ERROR_MESSAGE!r}, got: {error!r}" + ) + + +# Assert that disabling URL-based snapshot recovery through the +# `QDRANT__SERVICE__ENABLE_SNAPSHOT_URL_RECOVERY=false` environment variable is +# actually respected by every snapshot recovery endpoint that accepts a remote +# URL: the service must reject `http`/`https` locations with a `403 Forbidden` +# response before attempting any download. +def test_disable_url_snapshot_recovery(tmp_path: pathlib.Path): + assert_project_root() + + peer_urls, _, _ = start_cluster( + tmp_path, + 1, + extra_env={"QDRANT__SERVICE__ENABLE_SNAPSHOT_URL_RECOVERY": "false"}, + ) + peer_url = peer_urls[0] + + create_collection(peer_url, collection=COLLECTION_NAME) + + # Arbitrary remote URLs; the service must reject them up front rather than + # attempting any network request (the `invalid` TLD never resolves). + remote_urls = [ + "http://example.invalid/snapshot.tar", + "https://example.invalid/snapshot.tar", + ] + + shard_id = 0 + + # Full collection snapshot recovery + for location in remote_urls: + resp = requests.put( + f"{peer_url}/collections/{COLLECTION_NAME}/snapshots/recover", + json={"location": location}, + ) + assert_url_recovery_blocked(resp) + + # Shard snapshot recovery + for location in remote_urls: + resp = requests.put( + f"{peer_url}/collections/{COLLECTION_NAME}/shards/{shard_id}/snapshots/recover", + json={"location": location}, + ) + assert_url_recovery_blocked(resp) + + # Partial shard snapshot recovery from a peer URL + for peer_recover_url in remote_urls: + resp = requests.post( + f"{peer_url}/collections/{COLLECTION_NAME}/shards/{shard_id}/snapshot/partial/recover_from", + json={"peer_url": peer_recover_url}, + ) + assert_url_recovery_blocked(resp) diff --git a/tests/consensus_tests/test_shard_snapshot_clocks_consistency.py b/tests/consensus_tests/test_shard_snapshot_clocks_consistency.py index 0e92ee9722..ad0e24e074 100644 --- a/tests/consensus_tests/test_shard_snapshot_clocks_consistency.py +++ b/tests/consensus_tests/test_shard_snapshot_clocks_consistency.py @@ -16,6 +16,8 @@ def test_shard_snapshot_clocks_consistency(tmp_path: pathlib.Path): peer_urls, _, _ = start_cluster(tmp_path, 1, extra_env={ "QDRANT__STAGING__SNAPSHOT_SHARD_CLOCKS_DELAY": "5" }) peer_url = peer_urls[0] + skip_if_no_feature(peer_url, "staging") + # Bootstrap collection create_collection(peer_url) upsert_random_points(peer_url, 10_000, batch_size=100, with_sparse_vector=False)