gsadmin/rustfs
1
0
Fork 0
mirror of https://github.com/rustfs/rustfs.git synced 2026-07-26 08:18:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(security): add shared outbound egress guard (#3567)

Browse Source 
* fix(security): block unsafe outbound webhook targets

* chore: keep issue-3557 plan local only
This commit is contained in:
houseme
2026-06-18 15:30:09 +08:00
committed by GitHub
parent fc93a27974
commit 8cf11af649
10 changed files with 292 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/targets/Cargo.toml
+1
View File
@@ -16,6 +16,7 @@ rustfs-config = { workspace = true, features = ["notify", "constants", "audit",
rustfs-extension-schema = { workspace = true }
rustfs-tls-runtime = { workspace = true }
rustfs-s3-types = { workspace = true }
rustfs-utils = { workspace = true, features = ["egress"] }
async-trait = { workspace = true }
async-nats = { workspace = true }
deadpool-postgres = { workspace = true }
crates/targets/src/config/common.rs
+5
View File
@@ -21,6 +21,7 @@ use rustfs_config::{
NATS_TLS_CLIENT_CERT, NATS_TLS_CLIENT_KEY, NATS_TOKEN, NATS_USERNAME, PULSAR_AUTH_TOKEN, PULSAR_PASSWORD, PULSAR_QUEUE_DIR,
PULSAR_TLS_ALLOW_INSECURE, PULSAR_TLS_CA, PULSAR_TLS_HOSTNAME_VERIFICATION, PULSAR_TOPIC, PULSAR_USERNAME,
};
use rustfs_utils::egress::validate_outbound_url;
use std::collections::HashSet;
use std::path::Path;
use std::str::FromStr;
@@ -181,6 +182,10 @@ pub(super) fn parse_url(value: &str, field_label: &str) -> Result<Url, TargetErr
Url::parse(value).map_err(|e| TargetError::Configuration(format!("Invalid {field_label}: {e} (value: '{value}')")))
}
pub(super) fn validate_outbound_http_url(value: &Url, field_label: &str) -> Result<(), TargetError> {
validate_outbound_url(value).map_err(|e| TargetError::Configuration(format!("{field_label} is not allowed: {e}")))
}
#[cfg(test)]
mod tests {
use super::{validate_nats_server_config, validate_pulsar_broker_config};
crates/targets/src/config/target_args.rs
+35 -5
View File
@@ -12,7 +12,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use super::common::{parse_target_bool, parse_url, validate_nats_server_config, validate_pulsar_broker_config};
use super::common::{
parse_target_bool, parse_url, validate_nats_server_config, validate_outbound_http_url, validate_pulsar_broker_config,
};
use crate::error::TargetError;
use crate::target::{
TargetType,
@@ -137,6 +139,7 @@ pub fn build_webhook_args(config: &KVS, default_queue_dir: &str, target_type: Ta
.ok_or_else(|| TargetError::Configuration("Missing webhook endpoint".to_string()))?;
let parsed_endpoint = endpoint.trim();
let endpoint_url = parse_url(parsed_endpoint, "endpoint URL")?;
validate_outbound_http_url(&endpoint_url, "endpoint URL")?;
Ok(WebhookArgs {
enable: true,
@@ -165,7 +168,8 @@ pub fn validate_webhook_config(config: &KVS, default_queue_dir: &str) -> Result<
.lookup(WEBHOOK_ENDPOINT)
.ok_or_else(|| TargetError::Configuration("Missing webhook endpoint".to_string()))?;
let parsed_endpoint = endpoint.trim();
let _ = parse_url(parsed_endpoint, "endpoint URL")?;
let endpoint_url = parse_url(parsed_endpoint, "endpoint URL")?;
validate_outbound_http_url(&endpoint_url, "endpoint URL")?;
let client_cert = config.lookup(WEBHOOK_CLIENT_CERT).unwrap_or_default();
let client_key = config.lookup(WEBHOOK_CLIENT_KEY).unwrap_or_default();
@@ -594,8 +598,9 @@ pub fn validate_mysql_config(config: &KVS, default_queue_dir: &str) -> Result<()
#[cfg(test)]
mod tests {
use super::{
build_amqp_args, build_kafka_args, build_mysql_args, build_postgres_args, build_redis_args, validate_amqp_config,
validate_kafka_config, validate_mysql_config, validate_postgres_config, validate_redis_config,
build_amqp_args, build_kafka_args, build_mysql_args, build_postgres_args, build_redis_args, build_webhook_args,
validate_amqp_config, validate_kafka_config, validate_mysql_config, validate_postgres_config, validate_redis_config,
validate_webhook_config,
};
use crate::target::{
TargetType,
@@ -610,7 +615,7 @@ mod tests {
MYSQL_QUEUE_DIR, MYSQL_TABLE, MYSQL_TLS_CA, MYSQL_TLS_CLIENT_CERT, MYSQL_TLS_CLIENT_KEY, POSTGRES_DSN_STRING,
POSTGRES_FORMAT, POSTGRES_QUEUE_DIR, POSTGRES_TABLE, POSTGRES_TLS_CA, POSTGRES_TLS_CLIENT_CERT, POSTGRES_TLS_CLIENT_KEY,
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,
REDIS_RECONNECT_RETRY_ATTEMPTS, REDIS_RESPONSE_TIMEOUT, REDIS_TLS_ALLOW_INSECURE, REDIS_URL, WEBHOOK_ENDPOINT,
};
fn absolute_test_path(path: &str) -> String {
@@ -625,6 +630,12 @@ mod tests {
config
}
fn webhook_base_config() -> KVS {
let mut config = KVS::new();
config.insert(WEBHOOK_ENDPOINT.to_string(), "https://example.com/hook".to_string());
config
}
fn kafka_base_config() -> KVS {
let mut config = KVS::new();
config.insert(KAFKA_BROKERS.to_string(), "127.0.0.1:9092".to_string());
@@ -759,6 +770,25 @@ mod tests {
assert!(err.to_string().contains("either in url or username/password"));
}
#[test]
fn build_webhook_args_rejects_loopback_endpoint() {
let mut config = webhook_base_config();
config.insert(WEBHOOK_ENDPOINT.to_string(), "https://127.0.0.1/hook".to_string());
let err = build_webhook_args(&config, "/tmp/webhook-queue", TargetType::NotifyEvent)
.expect_err("loopback endpoint should be rejected");
assert!(err.to_string().contains("not allowed"));
}
#[test]
fn validate_webhook_config_rejects_loopback_endpoint() {
let mut config = webhook_base_config();
config.insert(WEBHOOK_ENDPOINT.to_string(), "https://127.0.0.1/hook".to_string());
let err = validate_webhook_config(&config, "/tmp/webhook-queue").expect_err("loopback endpoint should be rejected");
assert!(err.to_string().contains("not allowed"));
}
#[test]
fn build_kafka_args_accepts_all_ack_alias() {
let mut config = kafka_base_config();
crates/targets/src/target/webhook.rs
+21 -43
View File
@@ -31,6 +31,7 @@ use async_trait::async_trait;
use parking_lot::Mutex;
use reqwest::{Client, StatusCode, Url};
use rustfs_tls_runtime::load_cert_bundle_der_bytes;
use rustfs_utils::egress::validate_outbound_url;
use serde::Serialize;
use serde::de::DeserializeOwned;
use std::{
@@ -102,6 +103,8 @@ impl WebhookArgs {
if self.endpoint.as_str().is_empty() {
return Err(TargetError::Configuration("endpoint empty".to_string()));
}
validate_outbound_url(&self.endpoint)
.map_err(|err| TargetError::Configuration(format!("webhook endpoint is not allowed: {err}")))?;
if !self.queue_dir.is_empty() {
let path = std::path::Path::new(&self.queue_dir);
@@ -679,7 +682,6 @@ where
mod tests {
use super::{WebhookArgs, WebhookTarget};
use crate::target::{REDACTED_SECRET, Target, TargetType, decode_object_name};
use tokio::net::TcpListener;
use url::Url;
use url::form_urlencoded;
@@ -748,6 +750,16 @@ mod tests {
assert!(args.validate().is_ok());
}
#[test]
fn test_validate_rejects_loopback_endpoint() {
let args = WebhookArgs {
endpoint: Url::parse("https://127.0.0.1/hook").expect("loopback endpoint should parse"),
..base_args()
};
let err = args.validate().expect_err("loopback endpoint should be rejected");
assert!(err.to_string().contains("not allowed"));
}
#[test]
fn test_decode_object_name_with_spaces() {
// Test case from the issue: "greeting file (2).csv"
@@ -810,50 +822,16 @@ mod tests {
assert!(!target.is_active().await.unwrap());
}
#[tokio::test]
async fn test_is_active_uses_origin_reachability_for_path_endpoints() {
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let address = listener.local_addr().unwrap();
let server = async move {
use tokio::io::{AsyncReadExt, AsyncWriteExt};
let (mut stream, _) = listener.accept().await.unwrap();
let mut request = Vec::new();
let mut buf = [0u8; 1024];
loop {
let read = stream.read(&mut buf).await.unwrap();
if read == 0 {
break;
}
request.extend_from_slice(&buf[..read]);
if request.windows(4).any(|window| window == b"\r\n\r\n") {
break;
}
}
let request_line = request
.split(|byte| *byte == b'\n')
.next()
.and_then(|line| std::str::from_utf8(line).ok())
.unwrap_or_default()
.trim();
let path = request_line.split_whitespace().nth(1).unwrap_or_default().to_string();
if path == "/" {
let response = b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: close\r\n\r\n";
let _ = stream.write_all(response).await;
}
path
};
#[test]
fn test_origin_reachability_probe_requires_non_local_endpoint() {
let args = WebhookArgs {
endpoint: Url::parse(&format!("http://{address}/hook")).unwrap(),
endpoint: Url::parse("http://127.0.0.1/hook").unwrap(),
..base_args()
};
let target = WebhookTarget::<serde_json::Value>::new("path-probe".to_string(), args).unwrap();
let (is_active, path) = tokio::join!(target.is_active(), server);
assert!(is_active.unwrap());
assert_eq!(path, "/");
let err = match WebhookTarget::<serde_json::Value>::new("path-probe".to_string(), args) {
Ok(_) => panic!("loopback origin probes should now be rejected at construction time"),
Err(err) => err,
};
assert!(err.to_string().contains("not allowed"));
}
}