security(core): CORS secure-by-default, fix *+credentials, without_hsts
swadhinbiswas committed 3 months ago
bda80da
1 parent 820357d
commit bda80da
1 file changed
+35 -6
Showing 1 changed file+35-6
▼crates/justapi-core/src/middleware.rs
+35-6
| @@ -175,7 +175,11 @@ impl Cors { | ||
175 | 175 | |
176 | 176 | pub fn new() -> Self { |
177 | 177 | Self { |
178 | - allow_origins: vec!["*".to_string()], | |
178 | + // Secure-by-default: no `Access-Control-Allow-Origin` is emitted | |
179 | + // until the caller configures explicit origins via `allow_origin`. | |
180 | + // Browsers then enforce same-origin. Use `permissive()` for the old | |
181 | + // open `*` behavior. | |
182 | + allow_origins: Vec::new(), | |
179 | 183 | allow_methods: "GET, POST, PUT, DELETE, PATCH, OPTIONS".to_string(), |
180 | 184 | allow_headers: "Content-Type, Authorization".to_string(), |
181 | 185 | expose_headers: Vec::new(), |
| @@ -223,7 +227,8 @@ impl Cors { | ||
223 | 227 | |
224 | 228 | impl Default for Cors { |
225 | 229 | fn default() -> Self { |
226 | - Self::permissive() | |
230 | + // Secure-by-default: same-origin only until configured. | |
231 | + Self::new() | |
227 | 232 | } |
228 | 233 | } |
229 | 234 | |
| @@ -238,8 +243,20 @@ impl<B: Send + 'static> Middleware<B> for Cors { | ||
238 | 243 | |
239 | 244 | let allow_all = self.allow_origins.iter().any(|a| a == "*"); |
240 | 245 | |
246 | + // When credentials are allowed, the wildcard "*" must NOT be echoed: | |
247 | + // browsers reject `Access-Control-Allow-Origin: *` together with | |
248 | + // `Access-Control-Allow-Credentials: true`, and echoing "*" with | |
249 | + // credentials would be insecure. Reflect the concrete request origin | |
250 | + // instead (only when one is present). | |
241 | 251 | let matched_origin = if allow_all { |
242 | - "*".to_string() | |
252 | + if self.allow_credentials { | |
253 | + match req_origin { | |
254 | + Some(o) => o.to_string(), | |
255 | + None => return next.run(req).await, | |
256 | + } | |
257 | + } else { | |
258 | + "*".to_string() | |
259 | + } | |
243 | 260 | } else if let Some(origin) = req_origin { |
244 | 261 | if origin_matches(origin, &self.allow_origins) { |
245 | 262 | origin.to_string() |
| @@ -303,6 +320,7 @@ pub struct SecurityHeaders { | ||
303 | 320 | csp_directives: Vec<String>, |
304 | 321 | include_xfo: bool, |
305 | 322 | include_csp: bool, |
323 | + include_hsts: bool, | |
306 | 324 | include_hsts_preload: bool, |
307 | 325 | } |
308 | 326 | |
| @@ -313,6 +331,7 @@ impl Default for SecurityHeaders { | ||
313 | 331 | csp_directives: vec!["default-src 'self'".to_string()], |
314 | 332 | include_xfo: true, |
315 | 333 | include_csp: true, |
334 | + include_hsts: true, | |
316 | 335 | include_hsts_preload: false, |
317 | 336 | } |
318 | 337 | } |
| @@ -339,6 +358,14 @@ impl SecurityHeaders { | ||
339 | 358 | self.include_csp = false; |
340 | 359 | self |
341 | 360 | } |
361 | + | |
362 | + /// Omit `Strict-Transport-Security`. Use for plaintext (non-TLS) deployments: | |
363 | + /// emitting HSTS over HTTP is both useless and can pin a dev environment to | |
364 | + /// HTTPS that it cannot serve. | |
365 | + pub fn without_hsts(mut self) -> Self { | |
366 | + self.include_hsts = false; | |
367 | + self | |
368 | + } | |
342 | 369 | } |
343 | 370 | |
344 | 371 | #[async_trait] |
| @@ -358,7 +385,9 @@ impl<B: Send + 'static> Middleware<B> for SecurityHeaders { | ||
358 | 385 | } else { |
359 | 386 | self.hsts.clone() |
360 | 387 | }; |
361 | - headers.insert("strict-transport-security", hsts_val.parse().unwrap()); | |
388 | + if self.include_hsts { | |
389 | + headers.insert("strict-transport-security", hsts_val.parse().unwrap()); | |
390 | + } | |
362 | 391 | |
363 | 392 | if self.include_csp { |
364 | 393 | let csp = self.csp_directives.join("; "); |
| @@ -1250,7 +1279,7 @@ mod tests { | ||
1250 | 1279 | Box::pin(async { Ok(json_response(StatusCode::OK, "ok")) }) |
1251 | 1280 | }); |
1252 | 1281 | let mut chain = MiddlewareChain::new(handler); |
1253 | - chain.add(Cors::new().allow_credentials()); | |
1282 | + chain.add(Cors::new().allow_origin("https://example.com").allow_credentials()); | |
1254 | 1283 | let req = test_req_with_header(Method::GET, "/hello", "origin", "https://example.com"); |
1255 | 1284 | let resp = chain.run(req).await.unwrap(); |
1256 | 1285 | assert_eq!(resp.headers()["access-control-allow-credentials"], "true"); |
| @@ -1262,7 +1291,7 @@ mod tests { | ||
1262 | 1291 | Box::pin(async { Ok(json_response(StatusCode::OK, "ok")) }) |
1263 | 1292 | }); |
1264 | 1293 | let mut chain = MiddlewareChain::new(handler); |
1265 | - chain.add(Cors::new().expose_headers(&["x-custom-header", "x-trace-id"])); | |
1294 | + chain.add(Cors::new().allow_origin("https://example.com").expose_headers(&["x-custom-header", "x-trace-id"])); | |
1266 | 1295 | let req = test_req_with_header(Method::GET, "/hello", "origin", "https://example.com"); |
1267 | 1296 | let resp = chain.run(req).await.unwrap(); |
1268 | 1297 | assert_eq!(resp.headers()["access-control-expose-headers"], "x-custom-header, x-trace-id"); |
1269 | 1298 | |