From a82508b403420bd941c32ddec3ee3e4875f2b8a5 Mon Sep 17 00:00:00 2001
From: Sergio Benitez <sb@sergio.bz>
Date: Wed, 5 Apr 2023 13:04:39 -0700
Subject: [PATCH] Set 'Secure' cookie flag by default under TLS.

If TLS is enabled and active, Rocket will now set the `Secure` cookie
attribute by default.

Resolves #2425.
---
 core/lib/src/cookies.rs   | 25 +++++++++++++++++++------
 examples/tls/Cargo.toml   |  2 +-
 examples/tls/src/tests.rs | 29 +++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/core/lib/src/cookies.rs b/core/lib/src/cookies.rs
index 9e375d2e..032d4eae 100644
--- a/core/lib/src/cookies.rs
+++ b/core/lib/src/cookies.rs
@@ -279,6 +279,8 @@ impl<'a> CookieJar<'a> {
     ///    * `path`: `"/"`
     ///    * `SameSite`: `Strict`
     ///
+    /// Furthermore, if TLS is enabled, the `Secure` cookie flag is set.
+    ///
     /// # Example
     ///
     /// ```rust
@@ -298,7 +300,7 @@ impl<'a> CookieJar<'a> {
     /// }
     /// ```
     pub fn add(&self, mut cookie: Cookie<'static>) {
-        Self::set_defaults(&mut cookie);
+        Self::set_defaults(self.config, &mut cookie);
         self.ops.lock().push(Op::Add(cookie, false));
     }
 
@@ -316,8 +318,9 @@ impl<'a> CookieJar<'a> {
     ///    * `HttpOnly`: `true`
     ///    * `Expires`: 1 week from now
     ///
-    /// These defaults ensure maximum usability and security. For additional
-    /// security, you may wish to set the `secure` flag.
+    /// Furthermore, if TLS is enabled, the `Secure` cookie flag is set. These
+    /// defaults ensure maximum usability and security. For additional security,
+    /// you may wish to set the `secure` flag.
     ///
     /// # Example
     ///
@@ -333,7 +336,7 @@ impl<'a> CookieJar<'a> {
     #[cfg(feature = "secrets")]
     #[cfg_attr(nightly, doc(cfg(feature = "secrets")))]
     pub fn add_private(&self, mut cookie: Cookie<'static>) {
-        Self::set_private_defaults(&mut cookie);
+        Self::set_private_defaults(self.config, &mut cookie);
         self.ops.lock().push(Op::Add(cookie, true));
     }
 
@@ -473,7 +476,8 @@ impl<'a> CookieJar<'a> {
     ///    * `path`: `"/"`
     ///    * `SameSite`: `Strict`
     ///
-    fn set_defaults(cookie: &mut Cookie<'static>) {
+    /// Furthermore, if TLS is enabled, the `Secure` cookie flag is set.
+    fn set_defaults(config: &Config, cookie: &mut Cookie<'static>) {
         if cookie.path().is_none() {
             cookie.set_path("/");
         }
@@ -481,6 +485,10 @@ impl<'a> CookieJar<'a> {
         if cookie.same_site().is_none() {
             cookie.set_same_site(SameSite::Strict);
         }
+
+        if cookie.secure().is_none() && config.tls_enabled() {
+            cookie.set_secure(true);
+        }
     }
 
     /// For each property mentioned below, this method checks if there is a
@@ -492,9 +500,10 @@ impl<'a> CookieJar<'a> {
     ///    * `HttpOnly`: `true`
     ///    * `Expires`: 1 week from now
     ///
+    /// Furthermore, if TLS is enabled, the `Secure` cookie flag is set.
     #[cfg(feature = "secrets")]
     #[cfg_attr(nightly, doc(cfg(feature = "secrets")))]
-    fn set_private_defaults(cookie: &mut Cookie<'static>) {
+    fn set_private_defaults(config: &Config, cookie: &mut Cookie<'static>) {
         if cookie.path().is_none() {
             cookie.set_path("/");
         }
@@ -510,6 +519,10 @@ impl<'a> CookieJar<'a> {
         if cookie.expires().is_none() {
             cookie.set_expires(time::OffsetDateTime::now_utc() + time::Duration::weeks(1));
         }
+
+        if cookie.secure().is_none() && config.tls_enabled() {
+            cookie.set_secure(true);
+        }
     }
 }
 
diff --git a/examples/tls/Cargo.toml b/examples/tls/Cargo.toml
index 50eb6511..11bcc1ff 100644
--- a/examples/tls/Cargo.toml
+++ b/examples/tls/Cargo.toml
@@ -6,4 +6,4 @@ edition = "2021"
 publish = false
 
 [dependencies]
-rocket = { path = "../../core/lib", features = ["tls", "mtls"] }
+rocket = { path = "../../core/lib", features = ["tls", "mtls", "secrets"] }
diff --git a/examples/tls/src/tests.rs b/examples/tls/src/tests.rs
index 171b5b5d..2f48af02 100644
--- a/examples/tls/src/tests.rs
+++ b/examples/tls/src/tests.rs
@@ -21,6 +21,35 @@ fn hello_mutual() {
     }
 }
 
+#[test]
+fn secure_cookies() {
+    use rocket::http::{CookieJar, Cookie};
+
+    #[get("/cookie")]
+    fn cookie(jar: &CookieJar<'_>) {
+        jar.add(Cookie::new("k1", "v1"));
+        jar.add_private(Cookie::new("k2", "v2"));
+
+        jar.add(Cookie::build("k1u", "v1u").secure(false).finish());
+        jar.add_private(Cookie::build("k2u", "v2u").secure(false).finish());
+    }
+
+    let client = Client::tracked(super::rocket().mount("/", routes![cookie])).unwrap();
+    let response = client.get("/cookie").dispatch();
+
+    let c1 = response.cookies().get("k1").unwrap();
+    assert_eq!(c1.secure(), Some(true));
+
+    let c2 = response.cookies().get_private("k2").unwrap();
+    assert_eq!(c2.secure(), Some(true));
+
+    let c1 = response.cookies().get("k1u").unwrap();
+    assert_ne!(c1.secure(), Some(true));
+
+    let c2 = response.cookies().get_private("k2u").unwrap();
+    assert_ne!(c2.secure(), Some(true));
+}
+
 #[test]
 fn hello_world() {
     let profiles = [