mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-29 08:27:06 +00:00
fix(targets): explain webhook outbound allowlist failures (#5616)
This commit is contained in:
@@ -23,7 +23,7 @@ use rustfs_config::{
|
|||||||
NATS_TLS_CLIENT_KEY, NATS_TOKEN, NATS_USERNAME, PULSAR_AUTH_TOKEN, PULSAR_PASSWORD, PULSAR_QUEUE_DIR, PULSAR_TLS_CA,
|
NATS_TLS_CLIENT_KEY, NATS_TOKEN, NATS_USERNAME, PULSAR_AUTH_TOKEN, PULSAR_PASSWORD, PULSAR_QUEUE_DIR, PULSAR_TLS_CA,
|
||||||
PULSAR_TOPIC, PULSAR_USERNAME,
|
PULSAR_TOPIC, PULSAR_USERNAME,
|
||||||
};
|
};
|
||||||
use rustfs_utils::egress::OutboundPolicy;
|
use rustfs_utils::egress::{ENV_OUTBOUND_ALLOW_ORIGINS, OutboundPolicy, OutboundPolicyError};
|
||||||
use std::collections::HashSet;
|
use std::collections::HashSet;
|
||||||
use std::path::Path;
|
use std::path::Path;
|
||||||
use std::str::FromStr;
|
use std::str::FromStr;
|
||||||
@@ -223,7 +223,22 @@ pub(super) fn validate_outbound_http_url(value: &Url, field_label: &str) -> Resu
|
|||||||
OutboundPolicy::from_env_cached().map_err(|err| TargetError::Configuration(format!("invalid outbound policy: {err}")))?;
|
OutboundPolicy::from_env_cached().map_err(|err| TargetError::Configuration(format!("invalid outbound policy: {err}")))?;
|
||||||
policy
|
policy
|
||||||
.validate_url(value)
|
.validate_url(value)
|
||||||
.map_err(|e| TargetError::Configuration(format!("{field_label} is not allowed: {e}")))
|
.map_err(|err| TargetError::Configuration(format_outbound_http_url_error(field_label, value, &err)))
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Formats an outbound-policy rejection and adds an exact-origin hint only when that origin can override the rejection.
|
||||||
|
pub fn format_outbound_http_url_error(field_label: &str, value: &Url, err: &OutboundPolicyError) -> String {
|
||||||
|
let base = format!("{field_label} is not allowed: {err}");
|
||||||
|
let origin = value.origin().ascii_serialization();
|
||||||
|
let can_allow_origin =
|
||||||
|
OutboundPolicy::from_allowed_origins(&origin).is_ok_and(|allowlisted| allowlisted.validate_url(value).is_ok());
|
||||||
|
if can_allow_origin {
|
||||||
|
format!(
|
||||||
|
"{base}; add {origin} to {ENV_OUTBOUND_ALLOW_ORIGINS} (comma-separated) and restart RustFS to allow this operator-owned endpoint (origin only, no path)"
|
||||||
|
)
|
||||||
|
} else {
|
||||||
|
base
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
|
|||||||
@@ -17,6 +17,8 @@ mod instance;
|
|||||||
mod loader;
|
mod loader;
|
||||||
mod target_args;
|
mod target_args;
|
||||||
|
|
||||||
|
pub use common::format_outbound_http_url_error;
|
||||||
|
|
||||||
pub use instance::{
|
pub use instance::{
|
||||||
LegacyTargetInstanceDescriptor, TargetInstanceSourceClass, TargetInstanceSourceHints, TargetPluginInstance,
|
LegacyTargetInstanceDescriptor, TargetInstanceSourceClass, TargetInstanceSourceHints, TargetPluginInstance,
|
||||||
TargetPluginInstanceCompatDescriptor, TargetPluginInstanceRecord, normalize_legacy_target_instances,
|
TargetPluginInstanceCompatDescriptor, TargetPluginInstanceRecord, normalize_legacy_target_instances,
|
||||||
|
|||||||
@@ -624,6 +624,7 @@ mod tests {
|
|||||||
REDIS_CHANNEL, REDIS_CONNECTION_TIMEOUT, REDIS_MAX_RETRY_DELAY, REDIS_MIN_RETRY_DELAY, REDIS_PIPELINE_BUFFER_SIZE,
|
REDIS_CHANNEL, REDIS_CONNECTION_TIMEOUT, REDIS_MAX_RETRY_DELAY, REDIS_MIN_RETRY_DELAY, REDIS_PIPELINE_BUFFER_SIZE,
|
||||||
REDIS_RECONNECT_RETRY_ATTEMPTS, REDIS_RESPONSE_TIMEOUT, REDIS_TLS_ALLOW_INSECURE, REDIS_URL, WEBHOOK_ENDPOINT,
|
REDIS_RECONNECT_RETRY_ATTEMPTS, REDIS_RESPONSE_TIMEOUT, REDIS_TLS_ALLOW_INSECURE, REDIS_URL, WEBHOOK_ENDPOINT,
|
||||||
};
|
};
|
||||||
|
use rustfs_utils::egress::ENV_OUTBOUND_ALLOW_ORIGINS;
|
||||||
|
|
||||||
fn absolute_test_path(path: &str) -> String {
|
fn absolute_test_path(path: &str) -> String {
|
||||||
std::env::temp_dir().join(path).to_string_lossy().into_owned()
|
std::env::temp_dir().join(path).to_string_lossy().into_owned()
|
||||||
@@ -780,11 +781,29 @@ mod tests {
|
|||||||
#[test]
|
#[test]
|
||||||
fn build_webhook_args_rejects_loopback_endpoint() {
|
fn build_webhook_args_rejects_loopback_endpoint() {
|
||||||
let mut config = webhook_base_config();
|
let mut config = webhook_base_config();
|
||||||
config.insert(WEBHOOK_ENDPOINT.to_string(), "https://127.0.0.1/hook".to_string());
|
config.insert(WEBHOOK_ENDPOINT.to_string(), "https://127.0.0.1:8443/hook".to_string());
|
||||||
|
|
||||||
let err = build_webhook_args(&config, "/tmp/webhook-queue", TargetType::NotifyEvent)
|
let err = build_webhook_args(&config, "/tmp/webhook-queue", TargetType::NotifyEvent)
|
||||||
.expect_err("loopback endpoint should be rejected");
|
.expect_err("loopback endpoint should be rejected");
|
||||||
assert!(err.to_string().contains("not allowed"));
|
let message = err.to_string();
|
||||||
|
assert!(message.contains(&format!("add https://127.0.0.1:8443 to {ENV_OUTBOUND_ALLOW_ORIGINS}")));
|
||||||
|
assert!(message.contains("loopback address"));
|
||||||
|
assert!(message.contains("comma-separated"));
|
||||||
|
assert!(message.contains("restart RustFS"));
|
||||||
|
assert!(!message.contains("/hook"));
|
||||||
|
assert!(message.contains("origin only, no path"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn build_webhook_args_does_not_offer_allowlist_for_metadata_endpoint() {
|
||||||
|
let mut config = webhook_base_config();
|
||||||
|
config.insert(WEBHOOK_ENDPOINT.to_string(), "http://169.254.169.254/latest/meta-data".to_string());
|
||||||
|
|
||||||
|
let err = build_webhook_args(&config, "/tmp/webhook-queue", TargetType::NotifyEvent)
|
||||||
|
.expect_err("metadata endpoint should be rejected");
|
||||||
|
let message = err.to_string();
|
||||||
|
assert!(message.contains("not allowed"));
|
||||||
|
assert!(!message.contains(ENV_OUTBOUND_ALLOW_ORIGINS));
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
@@ -28,13 +28,13 @@ use rustfs_targets::{
|
|||||||
check_postgres_server_available, check_pulsar_broker_available, check_redis_server_available,
|
check_postgres_server_available, check_pulsar_broker_available, check_redis_server_available,
|
||||||
config::{
|
config::{
|
||||||
TargetPluginInstanceCompatDescriptor, TargetPluginInstanceRecord, build_amqp_args, build_kafka_args, build_mysql_args,
|
TargetPluginInstanceCompatDescriptor, TargetPluginInstanceRecord, build_amqp_args, build_kafka_args, build_mysql_args,
|
||||||
build_nats_args, build_postgres_args, build_pulsar_args, build_redis_args, try_normalize_target_plugin_instances,
|
build_nats_args, build_postgres_args, build_pulsar_args, build_redis_args, format_outbound_http_url_error,
|
||||||
validate_redis_config,
|
try_normalize_target_plugin_instances, validate_redis_config,
|
||||||
},
|
},
|
||||||
manifest::builtin_target_manifest,
|
manifest::builtin_target_manifest,
|
||||||
target::{TargetType, mqtt::MQTTTlsConfig},
|
target::{TargetType, mqtt::MQTTTlsConfig},
|
||||||
};
|
};
|
||||||
use rustfs_utils::egress::{ENV_OUTBOUND_ALLOW_ORIGINS, OutboundPolicy};
|
use rustfs_utils::egress::OutboundPolicy;
|
||||||
use s3s::{Body, S3Response, S3Result, header::CONTENT_TYPE, s3_error};
|
use s3s::{Body, S3Response, S3Result, header::CONTENT_TYPE, s3_error};
|
||||||
use serde::Serialize;
|
use serde::Serialize;
|
||||||
use std::collections::{HashMap, HashSet};
|
use std::collections::{HashMap, HashSet};
|
||||||
@@ -708,16 +708,9 @@ async fn validate_webhook_request(kv_map: &HashMap<String, String>) -> S3Result<
|
|||||||
}
|
}
|
||||||
let outbound_policy =
|
let outbound_policy =
|
||||||
OutboundPolicy::from_env_cached().map_err(|e| s3_error!(InvalidArgument, "invalid outbound policy: {}", e))?;
|
OutboundPolicy::from_env_cached().map_err(|e| s3_error!(InvalidArgument, "invalid outbound policy: {}", e))?;
|
||||||
outbound_policy.validate_url(&parsed_endpoint).map_err(|e| {
|
outbound_policy
|
||||||
s3_error!(
|
.validate_url(&parsed_endpoint)
|
||||||
InvalidArgument,
|
.map_err(|err| s3_error!(InvalidArgument, "{}", format_outbound_http_url_error("endpoint", &parsed_endpoint, &err)))?;
|
||||||
"endpoint is not allowed by the outbound policy: {}; review {}; private origins must be listed as exact origins such as {}={} and RustFS restarted (metadata and link-local destinations remain blocked)",
|
|
||||||
e,
|
|
||||||
ENV_OUTBOUND_ALLOW_ORIGINS,
|
|
||||||
ENV_OUTBOUND_ALLOW_ORIGINS,
|
|
||||||
parsed_endpoint.origin().ascii_serialization()
|
|
||||||
)
|
|
||||||
})?;
|
|
||||||
if let Some(queue_dir) = kv_map.get("queue_dir") {
|
if let Some(queue_dir) = kv_map.get("queue_dir") {
|
||||||
validate_queue_dir(queue_dir.as_str()).await?;
|
validate_queue_dir(queue_dir.as_str()).await?;
|
||||||
}
|
}
|
||||||
@@ -970,24 +963,40 @@ fn to_kvs(kv_map: &HashMap<String, String>) -> rustfs_config::server_config::KVS
|
|||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod webhook_request_tests {
|
mod webhook_request_tests {
|
||||||
use super::validate_webhook_request;
|
use super::validate_webhook_request;
|
||||||
|
use rustfs_utils::egress::ENV_OUTBOUND_ALLOW_ORIGINS;
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn private_webhook_request_is_rejected_with_operator_action() {
|
async fn private_webhook_request_is_rejected_with_operator_action() {
|
||||||
let config = HashMap::from([("endpoint".to_string(), "http://127.0.0.1:49173/webhook/rustfs".to_string())]);
|
let config = HashMap::from([("endpoint".to_string(), "http://192.168.1.2:1880/webhook/rustfs".to_string())]);
|
||||||
|
|
||||||
let err = validate_webhook_request(&config)
|
let err = validate_webhook_request(&config)
|
||||||
.await
|
.await
|
||||||
.expect_err("a loopback webhook must require an explicit outbound allowlist");
|
.expect_err("a private webhook must require an explicit outbound allowlist");
|
||||||
let message = err
|
let message = err
|
||||||
.message()
|
.message()
|
||||||
.expect("the validation error should explain the operator action");
|
.expect("the validation error should explain the operator action");
|
||||||
|
|
||||||
assert!(message.contains("RUSTFS_OUTBOUND_ALLOW_ORIGINS=http://127.0.0.1:49173"));
|
assert!(message.contains(&format!("add http://192.168.1.2:1880 to {ENV_OUTBOUND_ALLOW_ORIGINS}")));
|
||||||
assert!(message.contains("RustFS restarted"), "unexpected message: {message}");
|
assert!(message.contains("private address"));
|
||||||
|
assert!(message.contains("comma-separated"));
|
||||||
|
assert!(message.contains("restart RustFS"), "unexpected message: {message}");
|
||||||
assert!(!message.contains("/webhook/rustfs"));
|
assert!(!message.contains("/webhook/rustfs"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn metadata_webhook_request_does_not_offer_allowlist_override() {
|
||||||
|
let config = HashMap::from([("endpoint".to_string(), "http://169.254.169.254/latest/meta-data".to_string())]);
|
||||||
|
|
||||||
|
let err = validate_webhook_request(&config)
|
||||||
|
.await
|
||||||
|
.expect_err("the metadata endpoint must remain permanently blocked");
|
||||||
|
let message = err.message().expect("the validation error should explain the rejection");
|
||||||
|
|
||||||
|
assert!(message.contains("metadata endpoint"));
|
||||||
|
assert!(!message.contains(ENV_OUTBOUND_ALLOW_ORIGINS));
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn public_webhook_still_passes_outbound_preflight() {
|
async fn public_webhook_still_passes_outbound_preflight() {
|
||||||
let config = HashMap::from([("endpoint".to_string(), "https://hooks.example/webhook".to_string())]);
|
let config = HashMap::from([("endpoint".to_string(), "https://hooks.example/webhook".to_string())]);
|
||||||
|
|||||||
Reference in New Issue
Block a user