Remove a potential sidechannel attack on cow names

Author: boreq

moooodotfarm-backend/Cargo.lock

@@ -33,6 +33,6 @@ include_dir = "0.7.4"
 tonic = "0.12.3"
 prost = "0.13.5"
 async-trait = "0.1"
-
+rand = "0.8"
 [build-dependencies]
 tonic-build = "0.12.3"

moooodotfarm-backend/Cargo.toml

@@ -1,6 +1,7 @@
-use crate::app;
 use crate::app::{Herd, Inventory, Metrics};
-use crate::errors::{Error, Result};
+use crate::domain::CensoredHerd;
+use crate::errors::Result;
+use crate::{app, domain};
 use async_trait::async_trait;
 
 #[derive(Clone)]
@@ -19,13 +20,12 @@ where
     }
 
     async fn handle_inner(&self) -> Result<Herd> {
-        let mut statuses = vec![];
-        for cow in self.inventory.list()? {
-            let censored_status = crate::domain::CensoredCow::new(&cow)?;
-            statuses.push(censored_status);
-        }
-        let herd: Herd = statuses.try_into()?;
-        Ok::<Herd, Error>(herd)
+        let cows = self.inventory.list()?;
+        let censored_cows = cows
+            .into_iter()
+            .map(|cow| domain::CensoredCow::new(&cow))
+            .collect::<Result<Vec<domain::CensoredCow>>>()?;
+        CensoredHerd::new(censored_cows).try_into()
     }
 }
 

moooodotfarm-backend/src/app/get_herd.rs

@@ -118,12 +118,16 @@ impl Herd {
     }
 }
 
-impl TryFrom<Vec<domain::CensoredCow>> for Herd {
+impl TryFrom<domain::CensoredHerd> for Herd {
     type Error = Error;
 
-    fn try_from(value: Vec<domain::CensoredCow>) -> Result<Self> {
-        let cows: Result<Vec<_>> = value.iter().map(Cow::try_from).collect();
-        Ok(Self { cows: cows? })
+    fn try_from(value: domain::CensoredHerd) -> Result<Self> {
+        let cows: Vec<Cow> = value
+            .cows()
+            .iter()
+            .map(Cow::try_from)
+            .collect::<Result<Vec<_>>>()?;
+        Ok(Self { cows })
     }
 }
 

moooodotfarm-backend/src/app/mod.rs

@@ -1,4 +1,5 @@
 use crate::app::{CowTxtDownloader, Inventory, Metrics};
+use crate::domain::CensoredHerd;
 use crate::errors::{Error, Result};
 use crate::{app, domain};
 use async_trait::async_trait;
@@ -38,38 +39,42 @@ where
     }
 
     async fn handle_inner(&self) -> Result<()> {
-        let mut censored_statuses = vec![];
+        let mut cows: Vec<domain::Cow> = vec![];
 
-        for cow in self.inventory.list()? {
-            if !cow.should_check() {
+        for peeked_cow in self.inventory.list()? {
+            if !peeked_cow.should_check() {
                 continue;
             }
 
-            let result = self.downloader.download(cow.name()).await;
+            let result = self.downloader.download(peeked_cow.name()).await;
 
-            self.inventory.update(cow.name(), |status| {
-                if let Some(mut status) = status {
+            self.inventory.update(peeked_cow.name(), |cow| {
+                if let Some(mut cow) = cow {
                     match result {
                         Ok(_) => {
-                            status.mark_as_ok();
+                            cow.mark_as_ok();
                         }
                         Err(err) => {
                             log::warn!("cow is missing {}: {}", cow, err);
-                            status.mark_as_missing();
+                            cow.mark_as_missing();
                         }
                     }
 
-                    let censored_status = domain::CensoredCow::new(&cow)?;
-                    censored_statuses.push(censored_status);
+                    cows.push(cow.clone());
 
-                    return Ok(Some(status));
+                    return Ok(Some(cow));
                 }
 
                 Ok(None)
             })?;
         }
 
-        let herd: app::Herd = censored_statuses.try_into()?;
+        let censored_cows: Vec<domain::CensoredCow> =
+            cows.iter()
+                .map(domain::CensoredCow::new)
+                .collect::<Result<Vec<domain::CensoredCow>>>()?;
+        let censored_herd = CensoredHerd::new(censored_cows);
+        let herd: app::Herd = censored_herd.try_into()?;
         self.metrics.update_herd_numbers(&herd);
 
         Ok::<(), Error>(())

moooodotfarm-backend/src/app/update.rs

@@ -4,6 +4,7 @@ use crate::domain::time::{DateTime, Duration};
 use crate::errors::Error;
 use crate::errors::Result;
 use anyhow::anyhow;
+use rand::seq::SliceRandom;
 use std::fmt;
 use std::fmt::{Display, Formatter};
 
@@ -118,7 +119,7 @@ impl fmt::Display for Cow {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Ord, PartialOrd)]
 pub struct VisibleName {
     url: url::Url,
 }
@@ -283,6 +284,40 @@ impl CensoredCow {
     }
 }
 
+impl TryFrom<&Cow> for CensoredCow {
+    type Error = Error;
+
+    fn try_from(value: &Cow) -> Result<Self> {
+        Self::new(value)
+    }
+}
+pub struct CensoredHerd {
+    cows: Vec<CensoredCow>,
+}
+
+impl CensoredHerd {
+    pub fn new(mut cows: Vec<CensoredCow>) -> Self {
+        CensoredHerd::guard_against_side_channel_attacks(&mut cows);
+        Self { cows }
+    }
+
+    fn guard_against_side_channel_attacks(cows: &mut [CensoredCow]) {
+        // we could rely on implementing Ord for VisibleName, but we want to be explicit here.
+        // this type is supposed guard against sidechannel attacks and if someone ever changes
+        // Ord for VisibleName this could lead to accidentally removing this safeguard
+        let mut rng = rand::thread_rng();
+        cows.shuffle(&mut rng);
+        cows.sort_by(|a, b| match (a.name(), b.name()) {
+            (Name::Censored(_), Name::Censored(_)) => std::cmp::Ordering::Equal,
+            (Name::Censored(_), Name::Visible(_)) => std::cmp::Ordering::Greater,
+            (Name::Visible(_), Name::Censored(_)) => std::cmp::Ordering::Less,
+            (Name::Visible(a), Name::Visible(b)) => a.cmp(b),
+        });
+    }
+    pub fn cows(&self) -> &[CensoredCow] {
+        &self.cows
+    }
+}
 pub struct CowTxt<'a> {
     content: std::borrow::Cow<'a, str>,
 }