From 42bbb81bd2e3919080d3d8689aefb755b7ec8223 Mon Sep 17 00:00:00 2001
From: Jonathan de Jong <jonathan@automatia.nl>
Date: Mon, 24 May 2021 22:31:36 +0200
Subject: [PATCH] Tolerate slightly malformed base64

* add in fixes

* cargo fmt and newline

* sort dependencies
---
 crates/ruma-signatures/Cargo.toml       |  4 ++
 crates/ruma-signatures/src/functions.rs | 77 ++++++++++++++++++++++---
 crates/ruma/Cargo.toml                  |  1 +
 3 files changed, 75 insertions(+), 7 deletions(-)

diff --git a/crates/ruma-signatures/Cargo.toml b/crates/ruma-signatures/Cargo.toml
index 869c0988..3824bad6 100644
--- a/crates/ruma-signatures/Cargo.toml
+++ b/crates/ruma-signatures/Cargo.toml
@@ -17,4 +17,8 @@ ring = "0.16.19"
 ruma-identifiers = { version = "0.19.1", path = "../ruma-identifiers" }
 ruma-serde = { version = "0.4.0", path = "../ruma-serde" }
 serde_json = "1.0.60"
+tracing = { version = "0.1.25", optional = true }
 untrusted = "0.7.1"
+
+[features]
+compat = ["tracing"]
diff --git a/crates/ruma-signatures/src/functions.rs b/crates/ruma-signatures/src/functions.rs
index 1004f08f..27931351 100644
--- a/crates/ruma-signatures/src/functions.rs
+++ b/crates/ruma-signatures/src/functions.rs
@@ -7,7 +7,7 @@ use std::{
     str::FromStr,
 };
 
-use base64::{decode_config, encode_config, STANDARD_NO_PAD, URL_SAFE_NO_PAD};
+use base64::{decode_config, encode_config, Config, STANDARD_NO_PAD, URL_SAFE_NO_PAD};
 use ring::digest::{digest, SHA256};
 use ruma_identifiers::{EventId, RoomVersionId, ServerNameBox, UserId};
 use ruma_serde::{to_canonical_json_string, CanonicalJsonObject, CanonicalJsonValue};
@@ -278,11 +278,18 @@ pub fn verify_json(
                 .get(key_id)
                 .ok_or_else(|| Error::new("no key for signature in public_key_map"))?;
 
-            let signature_bytes = decode_config(signature, STANDARD_NO_PAD)?;
+            let verify = |config: Config| {
+                let signature_bytes = decode_config(signature, config)?;
 
-            let public_key_bytes = decode_config(&public_key, STANDARD_NO_PAD)?;
+                let public_key_bytes = decode_config(&public_key, config)?;
 
-            verify_json_with(&Ed25519Verifier, &public_key_bytes, &signature_bytes, object)?;
+                verify_json_with(&Ed25519Verifier, &public_key_bytes, &signature_bytes, object)
+            };
+
+            #[cfg(feature = "compat")]
+            also_try_forgiving_base64(STANDARD_NO_PAD, verify)?;
+            #[cfg(not(feature = "compat"))]
+            verify(STANDARD_NO_PAD)?;
         }
     }
 
@@ -635,11 +642,18 @@ pub fn verify_event(
             }
         };
 
-        let signature_bytes = decode_config(signature, STANDARD_NO_PAD)?;
+        let verify = |config: Config| {
+            let signature_bytes = decode_config(signature, config)?;
 
-        let public_key_bytes = decode_config(&public_key, STANDARD_NO_PAD)?;
+            let public_key_bytes = decode_config(&public_key, config)?;
 
-        verify_json_with(&Ed25519Verifier, &public_key_bytes, &signature_bytes, &canonical_json)?;
+            verify_json_with(&Ed25519Verifier, &public_key_bytes, &signature_bytes, &canonical_json)
+        };
+
+        #[cfg(feature = "compat")]
+        also_try_forgiving_base64(STANDARD_NO_PAD, verify)?;
+        #[cfg(not(feature = "compat"))]
+        verify(STANDARD_NO_PAD)?;
     }
 
     let calculated_hash = content_hash(object);
@@ -781,6 +795,41 @@ fn is_third_party_invite(object: &CanonicalJsonObject) -> Result<bool, Error> {
     }
 }
 
+#[cfg(feature = "compat")]
+// see https://github.com/ruma/ruma/issues/591
+// synapse allows this, so we must allow it too.
+// shouldn't lose data, but when it does, it'll make verification fail instead.
+pub(crate) fn also_try_forgiving_base64<T, E>(
+    config: Config,
+    twice: impl Fn(Config) -> Result<T, E>,
+) -> Result<T, E>
+where
+    E: std::fmt::Display,
+{
+    use tracing::{debug, warn};
+
+    let first_try = match twice(config) {
+        Ok(t) => return Ok(t),
+        Err(e) => e,
+    };
+
+    let adjusted = config.decode_allow_trailing_bits(true);
+
+    match twice(adjusted) {
+        Ok(t) => {
+            warn!(
+                "Usage of base64 config only worked after allowing trailing bits, first error: {}",
+                first_try
+            );
+            Ok(t)
+        }
+        Err(e) => {
+            debug!("Second error when trying to allow trailing bits: {}", e);
+            Err(first_try)
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use std::{
@@ -864,6 +913,20 @@ mod tests {
         assert!(matches!(verification, Verified::Signatures));
     }
 
+    #[cfg(feature = "compat")]
+    #[test]
+    fn fallback_invalid_base64() {
+        use base64::{decode_config, Config};
+
+        const SLIGHTLY_MALFORMED_BASE64: &str = "3UmJnEIzUr2xWyaUnJg5fXwRybwG5FVC6GqMHverEUn0ztuIsvVxX89JXX2pvdTsOBbLQx+4TVL02l4Cp5wPCm";
+
+        let verify = |config: Config| decode_config(SLIGHTLY_MALFORMED_BASE64, config);
+
+        assert!(verify(STANDARD_NO_PAD).is_err());
+
+        assert!(super::also_try_forgiving_base64(STANDARD_NO_PAD, verify).is_ok());
+    }
+
     #[test]
     fn verify_event_check_signatures_for_both_sender_and_event_id() {
         let key_pair_sender = generate_key_pair();
diff --git a/crates/ruma/Cargo.toml b/crates/ruma/Cargo.toml
index 49381daa..99bbb740 100644
--- a/crates/ruma/Cargo.toml
+++ b/crates/ruma/Cargo.toml
@@ -113,6 +113,7 @@ compat = [
     "ruma-events/compat",
     "ruma-identifiers/compat",
     "ruma-client-api/compat",
+    "ruma-signatures/compat",
 ]
 
 # Helper features that aren't exactly part of the spec but could be helpful