Rework signatures

oxidecomputer · Sep 25, 2023 · 7f473e9 · 7f473e9
1 parent a396956
commit 7f473e9
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 13 deletions.
diff --git a/rfd-api/src/authn/key.rs b/rfd-api/src/authn/key.rs
@@ -1,11 +1,11 @@
 use argon2::password_hash::rand_core::{OsRng, RngCore};
 use hex::FromHexError;
-use rsa::pkcs1v15::Signature;
 use thiserror::Error;
 use uuid::Uuid;
 
 use super::{Signer, SigningKeyError};
 
+#[derive(Debug)]
 pub struct RawApiKey {
     clear: Vec<u8>,
 }
@@ -20,6 +20,8 @@ pub enum ApiKeyError {
     MalformedSignature(#[from] rsa::signature::Error),
     #[error("Failed to sign API key: {0}")]
     Signing(SigningKeyError),
+    #[error("Failed to sign API key: {0}")]
+    Verify(SigningKeyError),
 }
 
 impl RawApiKey {
@@ -36,7 +38,7 @@ impl RawApiKey {
     }
 
     pub fn id(&self) -> &[u8] {
-        &self.clear[0..24]
+        &self.clear[0..16]
     }
 
     pub async fn sign(self, signer: &dyn Signer) -> Result<SignedApiKey, ApiKeyError> {
@@ -49,8 +51,12 @@ impl RawApiKey {
         Ok(SignedApiKey::new(hex::encode(self.clear), signature))
     }
 
-    pub fn verify(&self, signer: &dyn Signer, signature: &Signature) -> bool {
-        signer.verify(&self.clear, signature).is_ok()
+    pub fn verify(&self, signer: &dyn Signer, signature: &[u8]) -> Result<bool, ApiKeyError> {
+        let signature = hex::decode(signature)?;
+        Ok(signer
+            .verify(&self.clear, &signature)
+            .map(|_| true)
+            .map_err(ApiKeyError::Verify)?)
     }
 }
 
@@ -60,9 +66,10 @@ impl TryFrom<&str> for RawApiKey {
     fn try_from(value: &str) -> Result<Self, Self::Error> {
         let decoded = hex::decode(value)?;
 
-        if decoded.len() > 24 {
+        if decoded.len() > 16 {
             Ok(RawApiKey { clear: decoded })
         } else {
+            tracing::debug!(len = ?decoded.len(), "API key is too short");
             Err(ApiKeyError::FailedToParse)
         }
     }
@@ -94,6 +101,22 @@ mod tests {
     use super::RawApiKey;
     use crate::util::tests::mock_key;
 
+    #[tokio::test]
+    async fn test_verifies_signature() {
+        let id = Uuid::new_v4();
+        let signer = mock_key().as_signer().await.unwrap();
+
+        let raw = RawApiKey::generate::<8>(&id);
+        println!("{:?}", raw);
+
+        let signed = raw.sign(&*signer).await.unwrap();
+
+        let raw2 = RawApiKey::try_from(signed.key.as_str()).unwrap();
+        println!("{:?}", raw2);
+
+        assert!(raw2.verify(&*signer, signed.signature.as_bytes()).unwrap())
+    }
+
     #[tokio::test]
     async fn test_generates_signatures() {
         let id = Uuid::new_v4();

diff --git a/rfd-api/src/authn/mod.rs b/rfd-api/src/authn/mod.rs
@@ -106,7 +106,7 @@ pub enum SigningKeyError {
 #[async_trait]
 pub trait Signer: Send + Sync {
     async fn sign(&self, message: &[u8]) -> Result<Vec<u8>, SigningKeyError>;
-    fn verify(&self, message: &[u8], signature: &Signature) -> Result<(), SigningKeyError>;
+    fn verify(&self, message: &[u8], signature: &[u8]) -> Result<(), SigningKeyError>;
 }
 
 // A signer that stores a local in memory key for signing new JWTs
@@ -123,13 +123,14 @@ impl Signer for LocalKey {
         let mut rng = rand::thread_rng();
         let signature = self.signing_key.sign_with_rng(&mut rng, message).to_vec();
 
-        self.verify(message, &Signature::try_from(signature.as_ref()).unwrap())
+        self.verify(message, &signature)
             .map_err(|_| SigningKeyError::GeneratedInvalidSignature)?;
 
         Ok(signature)
     }
 
-    fn verify(&self, message: &[u8], signature: &Signature) -> Result<(), SigningKeyError> {
+    fn verify(&self, message: &[u8], signature: &[u8]) -> Result<(), SigningKeyError> {
+        let signature = Signature::try_from(signature)?;
         tracing::trace!("Verifying message");
         Ok(self.verifying_key.verify(message, &signature)?)
     }
@@ -242,7 +243,8 @@ impl Signer for CloudKmsSigner {
         Ok(signature)
     }
 
-    fn verify(&self, message: &[u8], signature: &Signature) -> Result<(), SigningKeyError> {
+    fn verify(&self, message: &[u8], signature: &[u8]) -> Result<(), SigningKeyError> {
+        let signature = Signature::try_from(signature)?;
         Ok(self.verifying_key.verify(message, &signature)?)
     }
 }

diff --git a/rfd-api/src/endpoints/login/oauth/mod.rs b/rfd-api/src/endpoints/login/oauth/mod.rs
@@ -8,7 +8,6 @@ use hyper::{
 };
 use oauth2::{basic::BasicClient, url::ParseError, AuthUrl, ClientId, ClientSecret, TokenUrl};
 use rfd_model::OAuthClient;
-use rsa::pkcs1v15::Signature;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use std::fmt::{Debug, Display};
@@ -173,9 +172,9 @@ pub trait CheckOAuthClient {
 impl CheckOAuthClient for OAuthClient {
     fn is_secret_valid(&self, key: &RawApiKey, signer: &dyn Signer) -> bool {
         for secret in &self.secrets {
-            match Signature::try_from(secret.secret_signature.as_bytes()) {
-                Ok(signature) => {
-                    if key.verify(signer, &signature) {
+            match key.verify(signer, secret.secret_signature.as_bytes()) {
+                Ok(verified) => {
+                    if verified {
                         return true;
                     }
                 }