Skip to content

Commit 99abbcc

Browse files
ericbelandericbeland
committed
Fix 5 security issues: path traversal, rate limit bypass, CORS, installer integrity
- Switch wellknown handler from raw HeaderMap to TypedHeader<Host>, add subdomain validation (RFC 1035 labels) to block path traversal and cross-tenant exposure via crafted Host headers - Default rate limiter to PeerIpKeyExtractor; opt into proxy header trust via TRUST_PROXY_HEADERS env var - Restrict CORS to GET method with Accept/Content-Type headers - Add SHA256 checksum verification to install.sh
1 parent 2d35972 commit 99abbcc

6 files changed

Lines changed: 243 additions & 23 deletions

File tree

‎config/deploy.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,4 @@ env:
3333
RUST_LOG: "sorcery_server=info"
3434
TENANTS_DIR: "/app/tenants"
3535
BASE_DOMAIN: "srcuri.com"
36+
TRUST_PROXY_HEADERS: "true"

‎src/main.rs‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@ use axum::{
1010
use axum_extra::TypedHeader;
1111
use headers::Host;
1212
use httpdate::HttpDate;
13-
use std::net::SocketAddr;
13+
use std::net::{IpAddr, SocketAddr};
1414
use std::path::PathBuf;
1515
use std::sync::Arc;
1616
use std::time::{Duration, SystemTime};
1717
use tower_governor::{
18-
governor::GovernorConfigBuilder, key_extractor::SmartIpKeyExtractor, GovernorLayer,
18+
governor::GovernorConfigBuilder,
19+
key_extractor::{KeyExtractor, PeerIpKeyExtractor, SmartIpKeyExtractor},
20+
GovernorError, GovernorLayer,
1921
};
2022
use tower_http::cors::{Any, CorsLayer};
2123
use tower_http::trace::TraceLayer;
@@ -30,6 +32,23 @@ use sorcery_server::{
3032
const ONE_DAY_SECS: u64 = 86_400;
3133
const NINETY_DAYS_SECS: u64 = 7_776_000;
3234

35+
#[derive(Debug, Clone, Copy)]
36+
struct ConfigurableIpExtractor {
37+
trust_proxy: bool,
38+
}
39+
40+
impl KeyExtractor for ConfigurableIpExtractor {
41+
type Key = IpAddr;
42+
43+
fn extract<T>(&self, req: &axum::http::Request<T>) -> Result<Self::Key, GovernorError> {
44+
if self.trust_proxy {
45+
SmartIpKeyExtractor.extract(req)
46+
} else {
47+
PeerIpKeyExtractor.extract(req)
48+
}
49+
}
50+
}
51+
3352
#[tokio::main]
3453
async fn main() {
3554
if let Err(err) = run().await {
@@ -59,12 +78,15 @@ async fn run() -> anyhow::Result<()> {
5978
base_domain,
6079
};
6180

62-
// Rate limiting: 60 requests per minute per IP (1 request per second on average)
81+
let trust_proxy = std::env::var("TRUST_PROXY_HEADERS")
82+
.map(|v| v.eq_ignore_ascii_case("true") || v == "1")
83+
.unwrap_or(false);
84+
6385
let governor_config = Arc::new(
6486
GovernorConfigBuilder::default()
6587
.per_second(1)
6688
.burst_size(60)
67-
.key_extractor(SmartIpKeyExtractor)
89+
.key_extractor(ConfigurableIpExtractor { trust_proxy })
6890
.finish()
6991
.context("build rate limiter config")?,
7092
);
@@ -86,8 +108,8 @@ async fn run() -> anyhow::Result<()> {
86108
.layer(
87109
CorsLayer::new()
88110
.allow_origin(Any)
89-
.allow_methods(Any)
90-
.allow_headers(Any),
111+
.allow_methods([axum::http::Method::GET])
112+
.allow_headers([header::ACCEPT, header::CONTENT_TYPE]),
91113
)
92114
.layer(TraceLayer::new_for_http());
93115

‎src/routes/wellknown.rs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#![allow(clippy::items_after_statements)]
22

3-
use axum::{debug_handler, extract::State, http::HeaderMap, response::Json};
3+
use axum::{debug_handler, extract::State, response::Json};
4+
use axum_extra::TypedHeader;
5+
use headers::Host;
46
use std::sync::Arc;
57

68
use crate::tenant::config::TenantConfig;
@@ -9,14 +11,10 @@ use crate::AppState;
911
#[debug_handler]
1012
pub async fn wellknown_handler(
1113
State(state): State<AppState>,
12-
headers: HeaderMap,
14+
TypedHeader(host): TypedHeader<Host>,
1315
) -> Json<Arc<TenantConfig>> {
14-
let host = headers
15-
.get("host")
16-
.and_then(|h| h.to_str().ok())
17-
.unwrap_or(&state.base_domain);
18-
19-
let subdomain = crate::tenant::TenantManager::extract_subdomain(host);
16+
let hostname = host.hostname();
17+
let subdomain = crate::tenant::TenantManager::extract_subdomain(hostname);
2018
let config = state.tenant_manager.get_config(&subdomain).await;
2119
Json(config)
2220
}

‎src/static/install.sh‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,59 @@ install_linux_appimage() {
246246
success "$APP_NAME installed to $app_path"
247247
}
248248

249+
verify_checksum() {
250+
local artifact_path="$1"
251+
local version="$2"
252+
local artifact_name
253+
artifact_name=$(basename "$artifact_path")
254+
255+
local checksums_url="https://github.com/$REPO/releases/download/$version/SHA256SUMS"
256+
local checksums_file
257+
checksums_file="$(dirname "$artifact_path")/SHA256SUMS"
258+
259+
info "Verifying artifact checksum..."
260+
261+
if command -v curl >/dev/null 2>&1; then
262+
if ! curl -fsSL -o "$checksums_file" "$checksums_url" 2>/dev/null; then
263+
warn "SHA256SUMS not available for this release, skipping verification"
264+
return 0
265+
fi
266+
elif command -v wget >/dev/null 2>&1; then
267+
if ! wget -q -O "$checksums_file" "$checksums_url" 2>/dev/null; then
268+
warn "SHA256SUMS not available for this release, skipping verification"
269+
return 0
270+
fi
271+
fi
272+
273+
local expected_hash
274+
expected_hash=$(grep "$artifact_name" "$checksums_file" | awk '{print $1}')
275+
276+
if [ -z "$expected_hash" ]; then
277+
warn "No checksum entry for $artifact_name, skipping verification"
278+
rm -f "$checksums_file"
279+
return 0
280+
fi
281+
282+
local actual_hash
283+
if command -v sha256sum >/dev/null 2>&1; then
284+
actual_hash=$(sha256sum "$artifact_path" | awk '{print $1}')
285+
elif command -v shasum >/dev/null 2>&1; then
286+
actual_hash=$(shasum -a 256 "$artifact_path" | awk '{print $1}')
287+
else
288+
warn "Neither sha256sum nor shasum found, skipping verification"
289+
rm -f "$checksums_file"
290+
return 0
291+
fi
292+
293+
rm -f "$checksums_file"
294+
295+
if [ "$actual_hash" != "$expected_hash" ]; then
296+
error "Checksum mismatch! Expected $expected_hash, got $actual_hash. The download may be corrupted or tampered with."
297+
fi
298+
299+
success "Checksum verified"
300+
}
301+
249302
launch_app() {
250303
local os="$1"
251304

@@ -306,6 +359,7 @@ main() {
306359
local artifact_path="$temp_dir/$artifact_name"
307360

308361
download_file "$artifact_url" "$artifact_path"
362+
verify_checksum "$artifact_path" "$version"
309363

310364
case "$os" in
311365
macos)

‎src/tenant/mod.rs‎

Lines changed: 111 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,18 @@ pub struct TenantManager {
1313
tenants_dir: PathBuf,
1414
}
1515

16+
fn is_valid_subdomain(s: &str) -> bool {
17+
let len = s.len();
18+
if len == 0 || len > 63 {
19+
return false;
20+
}
21+
if s.starts_with('-') || s.ends_with('-') {
22+
return false;
23+
}
24+
s.bytes()
25+
.all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'-')
26+
}
27+
1628
impl TenantManager {
1729
#[must_use]
1830
pub fn new(tenants_dir: PathBuf) -> Self {
@@ -47,16 +59,106 @@ impl TenantManager {
4759
}
4860

4961
#[must_use]
50-
#[allow(clippy::option_if_let_else)]
5162
pub fn extract_subdomain(host: &str) -> String {
52-
if let Some(subdomain) = host.split('.').next() {
53-
if subdomain == "srcuri" || subdomain.contains(':') {
54-
"default".to_string()
55-
} else {
56-
subdomain.to_string()
57-
}
58-
} else {
59-
"default".to_string()
63+
let host_without_port = host.split(':').next().unwrap_or(host);
64+
65+
match host_without_port.split('.').next() {
66+
Some("srcuri") => "default".to_string(),
67+
Some(sub) if is_valid_subdomain(sub) => sub.to_string(),
68+
_ => "default".to_string(),
6069
}
6170
}
6271
}
72+
73+
#[cfg(test)]
74+
mod tests {
75+
use super::*;
76+
77+
#[test]
78+
fn valid_subdomains() {
79+
assert_eq!(TenantManager::extract_subdomain("acme.srcuri.com"), "acme");
80+
assert_eq!(
81+
TenantManager::extract_subdomain("my-tenant.srcuri.com"),
82+
"my-tenant"
83+
);
84+
assert_eq!(TenantManager::extract_subdomain("a1b2.srcuri.com"), "a1b2");
85+
}
86+
87+
#[test]
88+
fn srcuri_returns_default() {
89+
assert_eq!(TenantManager::extract_subdomain("srcuri.com"), "default");
90+
}
91+
92+
#[test]
93+
fn path_traversal_returns_default() {
94+
assert_eq!(
95+
TenantManager::extract_subdomain("../etc.srcuri.com"),
96+
"default"
97+
);
98+
assert_eq!(
99+
TenantManager::extract_subdomain("foo/bar.srcuri.com"),
100+
"default"
101+
);
102+
assert_eq!(
103+
TenantManager::extract_subdomain("..%2fetc.srcuri.com"),
104+
"default"
105+
);
106+
}
107+
108+
#[test]
109+
fn uppercase_returns_default() {
110+
assert_eq!(
111+
TenantManager::extract_subdomain("ACME.srcuri.com"),
112+
"default"
113+
);
114+
assert_eq!(
115+
TenantManager::extract_subdomain("Acme.srcuri.com"),
116+
"default"
117+
);
118+
}
119+
120+
#[test]
121+
fn empty_returns_default() {
122+
assert_eq!(TenantManager::extract_subdomain(""), "default");
123+
assert_eq!(TenantManager::extract_subdomain(".srcuri.com"), "default");
124+
}
125+
126+
#[test]
127+
fn too_long_returns_default() {
128+
let long = format!("{}.srcuri.com", "a".repeat(64));
129+
assert_eq!(TenantManager::extract_subdomain(&long), "default");
130+
}
131+
132+
#[test]
133+
fn leading_trailing_hyphen_returns_default() {
134+
assert_eq!(
135+
TenantManager::extract_subdomain("-acme.srcuri.com"),
136+
"default"
137+
);
138+
assert_eq!(
139+
TenantManager::extract_subdomain("acme-.srcuri.com"),
140+
"default"
141+
);
142+
}
143+
144+
#[test]
145+
fn port_stripping_works() {
146+
assert_eq!(
147+
TenantManager::extract_subdomain("acme.srcuri.com:8080"),
148+
"acme"
149+
);
150+
assert_eq!(
151+
TenantManager::extract_subdomain("srcuri.com:3000"),
152+
"default"
153+
);
154+
}
155+
156+
#[test]
157+
fn bare_hostname_without_dots() {
158+
assert_eq!(TenantManager::extract_subdomain("localhost"), "localhost");
159+
assert_eq!(
160+
TenantManager::extract_subdomain("localhost:3000"),
161+
"localhost"
162+
);
163+
}
164+
}

‎tests/integration_tests.rs‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ async fn test_wellknown_endpoint() {
2929
.oneshot(
3030
Request::builder()
3131
.uri("/.well-known/srcuri.json")
32+
.header("host", "srcuri.com")
3233
.body(Body::empty())
3334
.unwrap(),
3435
)
@@ -444,6 +445,48 @@ async fn test_valid_branch_accepted() {
444445
assert!(html.contains("srcuri://"));
445446
}
446447

448+
#[tokio::test]
449+
async fn test_wellknown_traversal_host_rejected() {
450+
let app = create_test_app();
451+
let response = app
452+
.oneshot(
453+
Request::builder()
454+
.uri("/.well-known/srcuri.json")
455+
.header("host", "../etc.srcuri.com")
456+
.body(Body::empty())
457+
.unwrap(),
458+
)
459+
.await
460+
.unwrap();
461+
462+
assert_eq!(
463+
response.status(),
464+
StatusCode::BAD_REQUEST,
465+
"Path traversal in Host header should be rejected by typed extractor"
466+
);
467+
}
468+
469+
#[tokio::test]
470+
async fn test_wellknown_valid_tenant_returns_ok() {
471+
use http_body_util::BodyExt;
472+
473+
let app = create_test_app();
474+
let response = app
475+
.oneshot(
476+
Request::builder()
477+
.uri("/.well-known/srcuri.json")
478+
.header("host", "acme.srcuri.com")
479+
.body(Body::empty())
480+
.unwrap(),
481+
)
482+
.await
483+
.unwrap();
484+
485+
assert_eq!(response.status(), StatusCode::OK);
486+
let body = response.into_body().collect().await.unwrap().to_bytes();
487+
let _json: serde_json::Value = serde_json::from_slice(&body).unwrap();
488+
}
489+
447490
fn create_test_app() -> axum::Router {
448491
use axum::routing::get;
449492
use std::path::PathBuf;

0 commit comments

Comments
 (0)