From 4d0bb0709b168a24ab6a8dbc86da45d7503596ee Mon Sep 17 00:00:00 2001
From: Owen Jacobson <owen@grimoire.ca>
Date: Sat, 28 Sep 2024 01:40:22 -0400
Subject: Wrap credential and credential-holding types to prevent `Debug`
 leaks.

The following values are considered confidential, and should never be logged, even by accident:

* `Password`, which is a durable bearer token for a specific Login;
* `IdentitySecret`, which is an ephemeral but potentially long-lived bearer token for a specific Login; or
* `IdentityToken`, which may hold cookies containing an `IdentitySecret`.

These values are now wrapped in types whose `Debug` impls output opaque values, so that they can be included in structs that `#[derive(Debug)]` without requiring any additional care. The wrappers also avoid implementing `Display`, to prevent inadvertent `to_string()`s.

We don't bother obfuscating `IdentitySecret`s in memory or in the `.hi` database. There's no point: we'd also need to store the information needed to de-obfuscate them, and they can be freely invalidated and replaced by blanking that table and asking everyone to log in again. Passwords _are_ obfuscated for storage, as they're intended to be durable.
---
 src/login/extract.rs | 46 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 5 deletions(-)

(limited to 'src/login/extract.rs')

diff --git a/src/login/extract.rs b/src/login/extract.rs
index 5ef454c..3b31d4c 100644
--- a/src/login/extract.rs
+++ b/src/login/extract.rs
@@ -1,3 +1,5 @@
+use std::fmt;
+
 use axum::{
     extract::FromRequestParts,
     http::request::Parts,
@@ -7,11 +9,22 @@ use axum_extra::extract::cookie::{Cookie, CookieJar};
 
 // The usage pattern here - receive the extractor as an argument, return it in
 // the response - is heavily modelled after CookieJar's own intended usage.
-#[derive(Clone, Debug)]
+#[derive(Clone)]
 pub struct IdentityToken {
     cookies: CookieJar,
 }
 
+impl fmt::Debug for IdentityToken {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("IdentityToken")
+            .field(
+                "identity",
+                &self.cookies.get(IDENTITY_COOKIE).map(|_| "********"),
+            )
+            .finish()
+    }
+}
+
 impl IdentityToken {
     // Creates a new, unpopulated identity token store.
     #[cfg(test)]
@@ -26,14 +39,18 @@ impl IdentityToken {
     // return [None]. If the identity has previously been [set], then this
     // will return that secret, regardless of what the request originally
     // included.
-    pub fn secret(&self) -> Option<&str> {
-        self.cookies.get(IDENTITY_COOKIE).map(Cookie::value)
+    pub fn secret(&self) -> Option<IdentitySecret> {
+        self.cookies
+            .get(IDENTITY_COOKIE)
+            .map(Cookie::value)
+            .map(IdentitySecret::from)
     }
 
     // Positively set the identity secret, and ensure that it will be sent
     // back to the client when this extractor is included in a response.
-    pub fn set(self, secret: &str) -> Self {
-        let identity_cookie = Cookie::build((IDENTITY_COOKIE, String::from(secret)))
+    pub fn set(self, secret: impl Into<IdentitySecret>) -> Self {
+        let IdentitySecret(secret) = secret.into();
+        let identity_cookie = Cookie::build((IDENTITY_COOKIE, secret))
             .http_only(true)
             .path("/api/")
             .permanent()
@@ -76,3 +93,22 @@ impl IntoResponseParts for IdentityToken {
         cookies.into_response_parts(res)
     }
 }
+
+#[derive(sqlx::Type)]
+#[sqlx(transparent)]
+pub struct IdentitySecret(String);
+
+impl fmt::Debug for IdentitySecret {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_tuple("IdentityToken").field(&"********").finish()
+    }
+}
+
+impl<S> From<S> for IdentitySecret
+where
+    S: Into<String>,
+{
+    fn from(value: S) -> Self {
+        Self(value.into())
+    }
+}
-- 
cgit v1.2.3