Skip to content

Commit 5d1bfa3

Browse files
Algorithm5838Algorithm5838
committed
fix(cookies): add Secure flag to all Set-Cookie headers
1 parent 07fe98a commit 5d1bfa3

4 files changed

Lines changed: 50 additions & 1 deletion

File tree

‎src/main.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,8 @@ async fn main() {
227227
};
228228

229229
if let Some(expire_time) = hsts {
230+
// Mark cookies as Secure since we are running behind HTTPS.
231+
server::COOKIE_SECURE.store(true, std::sync::atomic::Ordering::Relaxed);
230232
if let Ok(val) = HeaderValue::from_str(&format!("max-age={expire_time}")) {
231233
app.default_headers.insert("Strict-Transport-Security", val);
232234
}

‎src/server.rs‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,20 @@ use std::{
2222
result::Result,
2323
str::{from_utf8, Split},
2424
string::ToString,
25+
sync::atomic::{AtomicBool, Ordering as AtomicOrdering},
2526
sync::Arc,
2627
};
2728
use time::OffsetDateTime;
2829
use tokio::net::TcpListener;
2930

3031
use crate::dbg_msg;
3132

33+
/// Set to `true` at startup (by `main`) when the server is running behind HTTPS
34+
/// (i.e. when `--hsts` is passed). Controls the `Secure` attribute on all
35+
/// `Set-Cookie` headers: cookies with `Secure` are silently dropped by browsers
36+
/// on plain-HTTP origins, so we only set it when we know HTTPS is in use.
37+
pub static COOKIE_SECURE: AtomicBool = AtomicBool::new(false);
38+
3239
/// The unified body type used for all responses.
3340
pub type Body = BoxBody<Bytes, Infallible>;
3441

@@ -275,13 +282,46 @@ impl ResponseExt for Response<Body> {
275282
}
276283

277284
fn remove_cookie(&mut self, name: String) {
278-
let removal_cookie = Cookie::build(name).path("/").http_only(true).expires(OffsetDateTime::now_utc());
285+
let removal_cookie = Cookie::build(name)
286+
.path("/")
287+
.http_only(true)
288+
.secure(COOKIE_SECURE.load(AtomicOrdering::Relaxed))
289+
.same_site(cookie::SameSite::Lax)
290+
.expires(OffsetDateTime::now_utc());
279291
if let Ok(val) = header::HeaderValue::from_str(&removal_cookie.to_string()) {
280292
self.headers_mut().append("Set-Cookie", val);
281293
}
282294
}
283295
}
284296

297+
/// Build a persistent [`Cookie`] with standardised privacy/security attributes.
298+
///
299+
/// Sets `Path=/`, `HttpOnly`, `SameSite=Lax`, and `Secure` only when
300+
/// [`COOKIE_SECURE`] is `true` (i.e. when `--hsts` was passed at startup).
301+
pub fn build_cookie(name: impl Into<String>, value: impl Into<String>, expires: OffsetDateTime) -> Cookie<'static> {
302+
Cookie::build((name.into(), value.into()))
303+
.path("/")
304+
.http_only(true)
305+
.secure(COOKIE_SECURE.load(AtomicOrdering::Relaxed))
306+
.same_site(cookie::SameSite::Lax)
307+
.expires(expires)
308+
.build()
309+
}
310+
311+
/// Build a session-scoped [`Cookie`] with standardised privacy/security attributes.
312+
///
313+
/// Sets `Path=/`, `HttpOnly`, `SameSite=Lax`, and `Secure` only when
314+
/// [`COOKIE_SECURE`] is `true` (i.e. when `--hsts` was passed at startup).
315+
pub fn build_session_cookie(name: impl Into<String>, value: impl Into<String>) -> Cookie<'static> {
316+
Cookie::build((name.into(), value.into()))
317+
.path("/")
318+
.http_only(true)
319+
.secure(COOKIE_SECURE.load(AtomicOrdering::Relaxed))
320+
.same_site(cookie::SameSite::Lax)
321+
.build()
322+
}
323+
324+
285325
impl Route<'_> {
286326
fn method(&mut self, method: &Method, dest: fn(Request<Body>) -> BoxResponse) -> &mut Self {
287327
self.router.add(&format!("/{}{}", method.as_str(), self.path), dest);

‎src/settings.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ pub async fn set(req: Request<Body>) -> Result<Response<Body>, String> {
8484
Cookie::build((name.to_owned(), value.clone()))
8585
.path("/")
8686
.http_only(true)
87+
.secure(true)
8788
.expires(OffsetDateTime::now_utc() + Duration::weeks(52))
8889
.into(),
8990
),
@@ -126,6 +127,7 @@ fn set_cookies_method(req: Request<Body>, remove_cookies: bool) -> Response<Body
126127
Cookie::build((name.to_owned(), value.clone()))
127128
.path("/")
128129
.http_only(true)
130+
.secure(true)
129131
.expires(OffsetDateTime::now_utc() + Duration::weeks(52))
130132
.into(),
131133
),
@@ -167,6 +169,7 @@ fn set_cookies_method(req: Request<Body>, remove_cookies: bool) -> Response<Body
167169
Cookie::build((subscriptions_cookie, list))
168170
.path("/")
169171
.http_only(true)
172+
.secure(true)
170173
.expires(OffsetDateTime::now_utc() + Duration::weeks(52))
171174
.into(),
172175
);
@@ -218,6 +221,7 @@ fn set_cookies_method(req: Request<Body>, remove_cookies: bool) -> Response<Body
218221
Cookie::build((filters_cookie, list))
219222
.path("/")
220223
.http_only(true)
224+
.secure(true)
221225
.expires(OffsetDateTime::now_utc() + Duration::weeks(52))
222226
.into(),
223227
);

‎src/subreddit.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ pub async fn add_quarantine_exception(req: Request<Body>) -> Result<Response<Bod
220220
Cookie::build((&format!("allow_quaran_{}", subreddit.to_lowercase()), "true"))
221221
.path("/")
222222
.http_only(true)
223+
.secure(true)
223224
.expires(cookie::Expiration::Session)
224225
.into(),
225226
);
@@ -390,6 +391,7 @@ pub async fn subscriptions_filters(req: Request<Body>) -> Result<Response<Body>,
390391
Cookie::build((subscriptions_cookie, list))
391392
.path("/")
392393
.http_only(true)
394+
.secure(true)
393395
.expires(OffsetDateTime::now_utc() + Duration::weeks(52))
394396
.into(),
395397
);
@@ -438,6 +440,7 @@ pub async fn subscriptions_filters(req: Request<Body>) -> Result<Response<Body>,
438440
Cookie::build((filters_cookie, list))
439441
.path("/")
440442
.http_only(true)
443+
.secure(true)
441444
.expires(OffsetDateTime::now_utc() + Duration::weeks(52))
442445
.into(),
443446
);

0 commit comments

Comments
 (0)