[Security] Restrict default CORS origins (#61009)
Signed-off-by: Juan Pérez de Algaba <jperezde@redhat.com> Co-authored-by: Cyrus Leung <tlleungac@connect.ust.hk>
This commit is contained in:
co-authored by
Cyrus Leung
parent
26a488ad93
commit
fcae5c9887
@@ -401,9 +401,9 @@ pub struct SharedRuntimeArgs {
|
||||
#[serde(default)]
|
||||
pub served_model_name: Vec<String>,
|
||||
|
||||
/// CORS allowed origins as a JSON list. `["*"]` allows any origin.
|
||||
#[arg(long, value_parser = parse_json::<JsonStringList>, value_name = "JSON", default_value = r#"["*"]"#)]
|
||||
#[serde(default = "default_cors_wildcard")]
|
||||
/// CORS allowed origins as a JSON list. Defaults to `[]` (no cross-origin access).
|
||||
#[arg(long, value_parser = parse_json::<JsonStringList>, value_name = "JSON", default_value = "[]")]
|
||||
#[serde(default = "default_cors_origins")]
|
||||
pub allowed_origins: JsonStringList,
|
||||
|
||||
/// CORS allowed methods as a JSON list. `["*"]` allows the standard set.
|
||||
@@ -664,6 +664,10 @@ fn default_cors_wildcard() -> JsonStringList {
|
||||
JsonStringList(vec!["*".to_string()])
|
||||
}
|
||||
|
||||
fn default_cors_origins() -> JsonStringList {
|
||||
JsonStringList(Vec::new())
|
||||
}
|
||||
|
||||
fn default_py_bootstrap_parser_selection() -> ParserSelection {
|
||||
ParserSelection::None
|
||||
}
|
||||
|
||||
@@ -205,9 +205,7 @@ fn serve_args_forward_python_flags_with_separator() {
|
||||
disable_log_stats: false,
|
||||
served_model_name: [],
|
||||
allowed_origins: JsonStringList(
|
||||
[
|
||||
"*",
|
||||
],
|
||||
[],
|
||||
),
|
||||
allowed_methods: JsonStringList(
|
||||
[
|
||||
@@ -1075,9 +1073,7 @@ fn frontend_args_accept_json() {
|
||||
disable_log_stats: false,
|
||||
served_model_name: [],
|
||||
allowed_origins: JsonStringList(
|
||||
[
|
||||
"*",
|
||||
],
|
||||
[],
|
||||
),
|
||||
allowed_methods: JsonStringList(
|
||||
[
|
||||
@@ -1382,13 +1378,13 @@ fn serve_args_parse_cors_flags() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn serve_args_cors_defaults_are_permissive() {
|
||||
fn serve_args_cors_defaults_restrict_origins() {
|
||||
let cli = Cli::try_parse_from(["vllm-rs", "serve", "Qwen/Qwen3-0.6B"]).unwrap();
|
||||
|
||||
let Command::Serve(serve) = cli.command else {
|
||||
panic!("expected serve args");
|
||||
};
|
||||
assert_eq!(serve.runtime.allowed_origins.0, ["*"]);
|
||||
assert!(serve.runtime.allowed_origins.0.is_empty());
|
||||
assert_eq!(serve.runtime.allowed_methods.0, ["*"]);
|
||||
assert_eq!(serve.runtime.allowed_headers.0, ["*"]);
|
||||
assert!(!serve.runtime.allow_credentials);
|
||||
@@ -1748,9 +1744,7 @@ fn serve_args_accept_handshake_aliases() {
|
||||
disable_log_stats: false,
|
||||
served_model_name: [],
|
||||
allowed_origins: JsonStringList(
|
||||
[
|
||||
"*",
|
||||
],
|
||||
[],
|
||||
),
|
||||
allowed_methods: JsonStringList(
|
||||
[
|
||||
@@ -1906,9 +1900,7 @@ fn serve_frontend_config_uses_dp_address_as_advertised_host() {
|
||||
sse_keep_alive_interval: None,
|
||||
},
|
||||
cors: CorsConfig {
|
||||
allow_origins: [
|
||||
"*",
|
||||
],
|
||||
allow_origins: [],
|
||||
allow_methods: [
|
||||
"*",
|
||||
],
|
||||
@@ -2002,9 +1994,7 @@ fn serve_frontend_config_keeps_tcp_transport_for_non_local_only_topology() {
|
||||
sse_keep_alive_interval: None,
|
||||
},
|
||||
cors: CorsConfig {
|
||||
allow_origins: [
|
||||
"*",
|
||||
],
|
||||
allow_origins: [],
|
||||
allow_methods: [
|
||||
"*",
|
||||
],
|
||||
@@ -2142,9 +2132,7 @@ fn frontend_config_uses_external_coordinator_when_coordinator_address_is_present
|
||||
sse_keep_alive_interval: None,
|
||||
},
|
||||
cors: CorsConfig {
|
||||
allow_origins: [
|
||||
"*",
|
||||
],
|
||||
allow_origins: [],
|
||||
allow_methods: [
|
||||
"*",
|
||||
],
|
||||
|
||||
@@ -65,7 +65,7 @@ pub struct ApiServerOptions {
|
||||
pub sse_keep_alive_interval: Option<Duration>,
|
||||
}
|
||||
|
||||
/// CORS settings mirroring Python's `CORSMiddleware`; the default is permissive.
|
||||
/// CORS settings mirroring Python's `CORSMiddleware`; origins are denied by default.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
|
||||
pub struct CorsConfig {
|
||||
/// Allowed origins. `["*"]` allows any origin.
|
||||
@@ -81,7 +81,7 @@ pub struct CorsConfig {
|
||||
impl Default for CorsConfig {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
allow_origins: vec!["*".to_string()],
|
||||
allow_origins: Vec::new(),
|
||||
allow_methods: vec!["*".to_string()],
|
||||
allow_headers: vec!["*".to_string()],
|
||||
allow_credentials: false,
|
||||
|
||||
@@ -105,7 +105,7 @@ pub fn cors_layer(cfg: &CorsConfig) -> CorsLayer {
|
||||
|
||||
// Emit `Vary: Origin` only when the allow-origin is dynamic (explicit
|
||||
// origins, or credentials); the wildcard + no-credentials case emits no
|
||||
// `Vary` at all, and an empty list disables the header here.
|
||||
// `Vary` at all. An empty origin list never permits an origin.
|
||||
let vary: Vec<HeaderName> = if !wildcard_origins || cfg.allow_credentials {
|
||||
vec![header::ORIGIN]
|
||||
} else {
|
||||
|
||||
@@ -1784,7 +1784,7 @@ async fn api_key_auth_allows_unguarded_route_without_token() {
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[serial]
|
||||
async fn cors_default_simple_request_allows_any_origin() {
|
||||
async fn cors_default_simple_request_omits_allow_origin() {
|
||||
let (mut app, _engine_task) = test_app_with_cors(CorsConfig::default()).await;
|
||||
let response = app
|
||||
.call(
|
||||
@@ -1799,17 +1799,12 @@ async fn cors_default_simple_request_allows_any_origin() {
|
||||
.expect("call app");
|
||||
|
||||
assert_eq!(response.status(), StatusCode::OK);
|
||||
assert_eq!(
|
||||
header_value(&response, "access-control-allow-origin"),
|
||||
Some("*")
|
||||
);
|
||||
// Wildcard origins without credentials emit no `Vary` (Starlette parity).
|
||||
assert_eq!(header_value(&response, "vary"), None);
|
||||
assert_eq!(header_value(&response, "access-control-allow-origin"), None);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[serial]
|
||||
async fn cors_default_preflight_returns_explicit_methods_and_max_age() {
|
||||
async fn cors_default_preflight_omits_allow_origin() {
|
||||
let (mut app, _engine_task) = test_app_with_cors(CorsConfig::default()).await;
|
||||
let response = app
|
||||
.call(
|
||||
@@ -1825,6 +1820,32 @@ async fn cors_default_preflight_returns_explicit_methods_and_max_age() {
|
||||
.await
|
||||
.expect("call app");
|
||||
|
||||
assert_eq!(response.status(), StatusCode::OK);
|
||||
assert_eq!(header_value(&response, "access-control-allow-origin"), None);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[serial]
|
||||
async fn cors_wildcard_preflight_returns_explicit_methods_and_max_age() {
|
||||
let cors = CorsConfig {
|
||||
allow_origins: vec!["*".to_string()],
|
||||
..CorsConfig::default()
|
||||
};
|
||||
let (mut app, _engine_task) = test_app_with_cors(cors).await;
|
||||
let response = app
|
||||
.call(
|
||||
Request::builder()
|
||||
.method("OPTIONS")
|
||||
.uri("/v1/chat/completions")
|
||||
.header("origin", "http://example.com")
|
||||
.header("access-control-request-method", "POST")
|
||||
.header("access-control-request-headers", "content-type")
|
||||
.body(Body::empty())
|
||||
.expect("build request"),
|
||||
)
|
||||
.await
|
||||
.expect("call app");
|
||||
|
||||
assert_eq!(response.status(), StatusCode::OK);
|
||||
// `*` methods expand to the explicit method list, matching Starlette
|
||||
// (never the literal `*`).
|
||||
@@ -1917,6 +1938,7 @@ async fn cors_explicit_origin_disallowed_omits_allow_origin() {
|
||||
#[serial]
|
||||
async fn cors_wildcard_with_credentials_reflects_origin_without_panic() {
|
||||
let cors = CorsConfig {
|
||||
allow_origins: vec!["*".to_string()],
|
||||
allow_credentials: true,
|
||||
..CorsConfig::default()
|
||||
};
|
||||
@@ -1971,8 +1993,12 @@ async fn cors_unauthorized_response_has_no_cors_headers() {
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[serial]
|
||||
async fn cors_preflight_bypasses_auth_and_returns_cors_headers() {
|
||||
let cors = CorsConfig {
|
||||
allow_origins: vec!["*".to_string()],
|
||||
..CorsConfig::default()
|
||||
};
|
||||
let (mut app, _engine_task) =
|
||||
test_app_with_cors_and_keys(CorsConfig::default(), vec!["secret".to_string()]).await;
|
||||
test_app_with_cors_and_keys(cors, vec!["secret".to_string()]).await;
|
||||
let response = app
|
||||
.call(
|
||||
Request::builder()
|
||||
|
||||
@@ -44,6 +44,13 @@ def serve_parser():
|
||||
return _build_vllm_parsers()["vllm serve"]
|
||||
|
||||
|
||||
def test_cors_origins_require_explicit_opt_in(serve_parser):
|
||||
assert serve_parser.parse_args([]).allowed_origins == []
|
||||
assert serve_parser.parse_args(
|
||||
["--allowed-origins", '["http://trusted.example"]']
|
||||
).allowed_origins == ["http://trusted.example"]
|
||||
|
||||
|
||||
### Test config parsing
|
||||
def test_config_arg_parsing(serve_parser, cli_config_file):
|
||||
args = serve_parser.parse_args([])
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project
|
||||
|
||||
from fastapi import FastAPI
|
||||
from starlette.testclient import TestClient
|
||||
|
||||
from vllm.entrypoints.launchers.cli_args import FrontendArgs
|
||||
from vllm.entrypoints.serve.middleware.register import init_entrypoints_middleware
|
||||
|
||||
|
||||
def _make_client(allowed_origins: list[str] | None = None) -> TestClient:
|
||||
args = FrontendArgs()
|
||||
if allowed_origins is not None:
|
||||
args.allowed_origins = allowed_origins
|
||||
|
||||
app = FastAPI()
|
||||
|
||||
@app.get("/v1/models")
|
||||
async def models():
|
||||
return {"data": [{"id": "private-model"}]}
|
||||
|
||||
@app.post("/v1/chat/completions")
|
||||
async def chat():
|
||||
return {"message": {"content": "ok"}}
|
||||
|
||||
init_entrypoints_middleware(args, app, ())
|
||||
return TestClient(app)
|
||||
|
||||
|
||||
def test_default_origins_prevent_cross_origin_reads_and_json_requests():
|
||||
client = _make_client()
|
||||
response = client.get("/v1/models", headers={"Origin": "http://attacker.example"})
|
||||
assert response.status_code == 200
|
||||
assert "access-control-allow-origin" not in response.headers
|
||||
|
||||
preflight = client.options(
|
||||
"/v1/chat/completions",
|
||||
headers={
|
||||
"Origin": "http://attacker.example",
|
||||
"Access-Control-Request-Method": "POST",
|
||||
"Access-Control-Request-Headers": "content-type",
|
||||
},
|
||||
)
|
||||
assert preflight.status_code == 400
|
||||
assert "access-control-allow-origin" not in preflight.headers
|
||||
|
||||
|
||||
def test_explicit_origin_allows_browser_requests():
|
||||
client = _make_client(["http://trusted.example"])
|
||||
response = client.get("/v1/models", headers={"Origin": "http://trusted.example"})
|
||||
assert response.headers["access-control-allow-origin"] == "http://trusted.example"
|
||||
|
||||
preflight = client.options(
|
||||
"/v1/chat/completions",
|
||||
headers={
|
||||
"Origin": "http://trusted.example",
|
||||
"Access-Control-Request-Method": "POST",
|
||||
"Access-Control-Request-Headers": "content-type",
|
||||
},
|
||||
)
|
||||
assert preflight.status_code == 200
|
||||
assert preflight.headers["access-control-allow-origin"] == "http://trusted.example"
|
||||
@@ -316,8 +316,8 @@ class FrontendArgs(BaseFrontendArgs):
|
||||
while keeping logs for other endpoints."""
|
||||
allow_credentials: bool = False
|
||||
"""Allow credentials."""
|
||||
allowed_origins: list[str] = field(default_factory=lambda: ["*"])
|
||||
"""Allowed origins."""
|
||||
allowed_origins: list[str] = field(default_factory=list)
|
||||
"""Allowed origins. Cross-origin browser access is disabled by default."""
|
||||
allowed_methods: list[str] = field(default_factory=lambda: ["*"])
|
||||
"""Allowed methods."""
|
||||
allowed_headers: list[str] = field(default_factory=lambda: ["*"])
|
||||
|
||||
Reference in New Issue
Block a user