fix: EIP-191 and JWT panics

relay_rpc/src/auth/cacao.rs

@@ -4,6 +4,7 @@ use {
         payload::Payload,
         signature::{eip1271::get_rpc_url::GetRpcUrl, Signature},
     },
+    alloy_primitives::hex::FromHexError,
     core::fmt::Debug,
     serde::{Deserialize, Serialize},
     serde_json::value::RawValue,
@@ -32,6 +33,9 @@ pub enum CacaoError {
     #[error("Invalid address")]
     AddressInvalid,
 
+    #[error("Address not EIP-191")]
+    AddressNotEip191(FromHexError),
+
     #[error("EIP-1271 signatures not supported")]
     Eip1271NotSupported,
 

relay_rpc/src/auth/cacao/signature/eip1271/mod.rs

@@ -72,14 +72,14 @@ pub async fn verify_eip1271(
 mod test {
     use {
         super::*,
-        crate::auth::cacao::signature::{eip191::eip191_bytes, strip_hex_prefix},
-        alloy_node_bindings::{Anvil, AnvilInstance},
+        crate::auth::cacao::signature::{
+            eip191::eip191_bytes,
+            strip_hex_prefix,
+            test_helpers::{deploy_contract, message_hash, sign_message, spawn_anvil},
+        },
         alloy_primitives::address,
-        k256::{ecdsa::SigningKey, elliptic_curve::SecretKey, Secp256k1},
-        regex::Regex,
+        k256::ecdsa::SigningKey,
         sha3::{Digest, Keccak256},
-        std::process::Stdio,
-        tokio::process::Command,
     };
 
     // Manual test. Paste address, signature, message, and project ID to verify
@@ -105,63 +105,6 @@ mod test {
             .unwrap());
     }
 
-    async fn spawn_anvil() -> (AnvilInstance, Url, SecretKey<Secp256k1>) {
-        let anvil = Anvil::new().spawn();
-        let provider = anvil.endpoint().parse().unwrap();
-        let private_key = anvil.keys().first().unwrap().clone();
-        (anvil, provider, private_key)
-    }
-
-    async fn deploy_contract(rpc_url: &Url, private_key: &SecretKey<Secp256k1>) -> Address {
-        let key_encoded = data_encoding::HEXLOWER_PERMISSIVE.encode(&private_key.to_bytes());
-        let output = Command::new("forge")
-            .args([
-                "create",
-                "--contracts",
-                "relay_rpc/src/auth/cacao/signature/eip1271",
-                "TestContract",
-                "--rpc-url",
-                rpc_url.as_str(),
-                "--private-key",
-                &key_encoded,
-                "--cache-path",
-                "target/.forge/cache",
-                "--out",
-                "target/.forge/out",
-            ])
-            .stdout(Stdio::piped())
-            .stderr(Stdio::piped())
-            .spawn()
-            .unwrap()
-            .wait_with_output()
-            .await
-            .unwrap();
-        let output = String::from_utf8(output.stdout).unwrap();
-        let (_, [contract_address]) = Regex::new("Deployed to: (0x[0-9a-fA-F]+)")
-            .unwrap()
-            .captures(&output)
-            .unwrap()
-            .extract();
-        contract_address.parse().unwrap()
-    }
-
-    fn sign_message(message: &str, private_key: &SecretKey<Secp256k1>) -> Vec<u8> {
-        let (signature, recovery): (k256::ecdsa::Signature, _) =
-            SigningKey::from_bytes(&private_key.to_bytes())
-                .unwrap()
-                .sign_digest_recoverable(Keccak256::new_with_prefix(eip191_bytes(message)))
-                .unwrap();
-        let signature = signature.to_bytes();
-        // need for +27 is mentioned in EIP-1271 reference implementation
-        [&signature[..], &[recovery.to_byte() + 27]].concat()
-    }
-
-    fn message_hash(message: &str) -> [u8; 32] {
-        Keccak256::new_with_prefix(eip191_bytes(message)).finalize()[..]
-            .try_into()
-            .unwrap()
-    }
-
     #[tokio::test]
     async fn test_eip1271_pass() {
         let (_anvil, rpc_url, private_key) = spawn_anvil().await;
@@ -178,13 +121,13 @@ mod test {
     }
 
     #[tokio::test]
-    async fn test_eip1271_fail() {
+    async fn test_eip1271_wrong_signature() {
         let (_anvil, rpc_url, private_key) = spawn_anvil().await;
         let contract_address = deploy_contract(&rpc_url, &private_key).await;
 
         let message = "xxx";
         let mut signature = sign_message(message, &private_key);
-        signature[0] = signature[0].wrapping_add(1);
+        *signature.first_mut().unwrap() = signature.first().unwrap().wrapping_add(1);
 
         assert!(matches!(
             verify_eip1271(signature, contract_address, &message_hash(message), rpc_url).await,
@@ -198,7 +141,10 @@ mod test {
         let contract_address = deploy_contract(&rpc_url, &private_key).await;
 
         let message = "xxx";
-        let signature = sign_message(message, &anvil.keys()[1]);
+        let signature = sign_message(
+            message,
+            &SigningKey::from_bytes(&anvil.keys().get(1).unwrap().to_bytes()).unwrap(),
+        );
 
         assert!(matches!(
             verify_eip1271(signature, contract_address, &message_hash(message), rpc_url).await,
@@ -211,7 +157,8 @@ mod test {
         let (_anvil, rpc_url, private_key) = spawn_anvil().await;
         let mut contract_address = deploy_contract(&rpc_url, &private_key).await;
 
-        contract_address.0[0] = contract_address.0[0].wrapping_add(1);
+        *contract_address.0.first_mut().unwrap() =
+            contract_address.0.first().unwrap().wrapping_add(1);
 
         let message = "xxx";
         let signature = sign_message(message, &private_key);
@@ -221,4 +168,25 @@ mod test {
             Err(CacaoError::Verification)
         ));
     }
+
+    #[tokio::test]
+    async fn test_eip1271_wrong_message() {
+        let (_anvil, rpc_url, private_key) = spawn_anvil().await;
+        let contract_address = deploy_contract(&rpc_url, &private_key).await;
+
+        let message = "xxx";
+        let signature = sign_message(message, &private_key);
+
+        let message2 = "yyy";
+        assert!(matches!(
+            verify_eip1271(
+                signature,
+                contract_address,
+                &message_hash(message2),
+                rpc_url
+            )
+            .await,
+            Err(CacaoError::Verification)
+        ));
+    }
 }

relay_rpc/src/auth/cacao/signature/eip191.rs

@@ -1,6 +1,7 @@
 use {
     super::CacaoError,
     crate::auth::cacao::signature::strip_hex_prefix,
+    alloy_primitives::Address,
     sha3::{Digest, Keccak256},
 };
 
@@ -15,25 +16,88 @@ pub fn eip191_bytes(message: &str) -> Vec<u8> {
     .into()
 }
 
-pub fn verify_eip191(signature: &[u8], address: &str, hash: Keccak256) -> Result<bool, CacaoError> {
+pub fn verify_eip191(
+    signature: &[u8],
+    address: &Address,
+    hash: Keccak256,
+) -> Result<bool, CacaoError> {
     use k256::ecdsa::{RecoveryId, Signature as Sig, VerifyingKey};
 
-    let sig = Sig::try_from(&signature[..64]).map_err(|_| CacaoError::Verification)?;
-    let recovery_id =
-        RecoveryId::try_from(&signature[64] % 27).map_err(|_| CacaoError::Verification)?;
+    let sig = Sig::try_from(signature.get(..64).ok_or(CacaoError::Verification)?)
+        .map_err(|_| CacaoError::Verification)?;
+    let recovery_id = RecoveryId::try_from(signature.get(64).ok_or(CacaoError::Verification)? % 27)
+        .map_err(|_| CacaoError::Verification)?;
 
     let recovered_key = VerifyingKey::recover_from_digest(hash, &sig, recovery_id)
         .map_err(|_| CacaoError::Verification)?;
 
-    let add = &Keccak256::default()
-        .chain_update(&recovered_key.to_encoded_point(false).as_bytes()[1..])
-        .finalize()[12..];
+    let hash = Keccak256::default()
+        .chain_update(
+            recovered_key
+                .to_encoded_point(false)
+                .as_bytes()
+                .get(1..)
+                .ok_or(CacaoError::Verification)?,
+        )
+        .finalize();
+    let add = hash.get(12..).ok_or(CacaoError::Verification)?;
 
     let address_encoded = data_encoding::HEXLOWER_PERMISSIVE.encode(add);
 
-    if address_encoded.to_lowercase() != strip_hex_prefix(address).to_lowercase() {
+    if address_encoded.to_lowercase() != strip_hex_prefix(&address.to_string()).to_lowercase() {
         Err(CacaoError::Verification)
     } else {
         Ok(true)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use {
+        crate::auth::cacao::signature::{
+            eip191::verify_eip191,
+            test_helpers::{message_hash_internal, sign_message},
+        },
+        alloy_primitives::Address,
+        k256::ecdsa::SigningKey,
+    };
+
+    #[test]
+    fn test_eip191() {
+        let private_key = SigningKey::random(&mut rand::thread_rng());
+        let message = "xxx";
+        let signature = sign_message(message, &private_key);
+        let address = Address::from_private_key(&private_key);
+        assert!(verify_eip191(&signature, &address, message_hash_internal(message)).unwrap());
+    }
+
+    #[test]
+    fn test_eip191_wrong_signature() {
+        let private_key = SigningKey::random(&mut rand::thread_rng());
+        let message = "xxx";
+        let mut signature = sign_message(message, &private_key);
+        *signature.first_mut().unwrap() = signature.first().unwrap().wrapping_add(1);
+        let address = Address::from_private_key(&private_key);
+        assert!(verify_eip191(&signature, &address, message_hash_internal(message)).is_err());
+    }
+
+    #[test]
+    fn test_eip191_wrong_address() {
+        let private_key = SigningKey::random(&mut rand::thread_rng());
+        let message = "xxx";
+        let signature = sign_message(message, &private_key);
+        let mut address = Address::from_private_key(&private_key);
+        *address.0.first_mut().unwrap() = address.0.first().unwrap().wrapping_add(1);
+        assert!(verify_eip191(&signature, &address, message_hash_internal(message)).is_err());
+    }
+
+    #[test]
+    fn test_eip191_wrong_message() {
+        let private_key = SigningKey::random(&mut rand::thread_rng());
+        let message = "xxx";
+        let signature = sign_message(message, &private_key);
+        let address = Address::from_private_key(&private_key);
+        let message2 = "yyy";
+        assert!(verify_eip191(&signature, &address, message_hash_internal(message2)).is_err());
+    }
+}

relay_rpc/src/auth/cacao/signature/mod.rs

@@ -13,6 +13,9 @@ use {
 pub mod eip1271;
 pub mod eip191;
 
+#[cfg(test)]
+mod test_helpers;
+
 #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, Hash)]
 pub struct Signature {
     pub t: String,
@@ -34,7 +37,11 @@ impl Signature {
         let hash = Keccak256::new_with_prefix(eip191_bytes(&cacao.siwe_message()?));
 
         match self.t.as_str() {
-            EIP191 => verify_eip191(&signature, &address, hash),
+            EIP191 => verify_eip191(
+                &signature,
+                &address.parse().map_err(CacaoError::AddressNotEip191)?,
+                hash,
+            ),
             EIP1271 => {
                 if let Some(provider) = provider {
                     let chain_id = cacao.p.chain_id_reference()?;

relay_rpc/src/auth/cacao/signature/test_helpers.rs

@@ -0,0 +1,75 @@
+use {
+    super::eip191::eip191_bytes,
+    alloy_node_bindings::{Anvil, AnvilInstance},
+    alloy_primitives::Address,
+    k256::ecdsa::SigningKey,
+    regex::Regex,
+    sha2::Digest,
+    sha3::Keccak256,
+    std::process::Stdio,
+    tokio::process::Command,
+    url::Url,
+};
+
+pub async fn spawn_anvil() -> (AnvilInstance, Url, SigningKey) {
+    let anvil = Anvil::new().spawn();
+    let provider = anvil.endpoint().parse().unwrap();
+    let private_key = anvil.keys().first().unwrap().clone();
+    (
+        anvil,
+        provider,
+        SigningKey::from_bytes(&private_key.to_bytes()).unwrap(),
+    )
+}
+
+pub async fn deploy_contract(rpc_url: &Url, private_key: &SigningKey) -> Address {
+    let key_encoded = data_encoding::HEXLOWER_PERMISSIVE.encode(&private_key.to_bytes());
+    let output = Command::new("forge")
+        .args([
+            "create",
+            "--contracts",
+            "relay_rpc/src/auth/cacao/signature/eip1271",
+            "TestContract",
+            "--rpc-url",
+            rpc_url.as_str(),
+            "--private-key",
+            &key_encoded,
+            "--cache-path",
+            "target/.forge/cache",
+            "--out",
+            "target/.forge/out",
+        ])
+        .stdout(Stdio::piped())
+        .stderr(Stdio::piped())
+        .spawn()
+        .unwrap()
+        .wait_with_output()
+        .await
+        .unwrap();
+    let output = String::from_utf8(output.stdout).unwrap();
+    let (_, [contract_address]) = Regex::new("Deployed to: (0x[0-9a-fA-F]+)")
+        .unwrap()
+        .captures(&output)
+        .unwrap()
+        .extract();
+    contract_address.parse().unwrap()
+}
+
+pub fn sign_message(message: &str, private_key: &SigningKey) -> Vec<u8> {
+    let (signature, recovery): (k256::ecdsa::Signature, _) = private_key
+        .sign_digest_recoverable(message_hash_internal(message))
+        .unwrap();
+    let signature = signature.to_bytes();
+    // need for +27 is mentioned in EIP-1271 reference implementation
+    [&signature[..], &[recovery.to_byte() + 27]].concat()
+}
+
+pub fn message_hash_internal(message: &str) -> Keccak256 {
+    Keccak256::new_with_prefix(eip191_bytes(message))
+}
+
+pub fn message_hash(message: &str) -> [u8; 32] {
+    message_hash_internal(message).finalize()[..]
+        .try_into()
+        .unwrap()
+}