fixes based on cookie method research

ZcashFoundation · Oct 1, 2024 · 212d517 · 212d517
1 parent 045048e
commit 212d517
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 25 deletions.
diff --git a/zebra-rpc/src/server.rs b/zebra-rpc/src/server.rs
@@ -195,7 +195,7 @@ impl RpcServer {
             }
 
             // generate a cookie
-            let generated_password = cookie::generate().unwrap_or("".to_string());
+            cookie::generate();
 
             // The server is a blocking task, which blocks on executor shutdown.
             // So we need to start it in a std::thread.
@@ -209,7 +209,7 @@ impl RpcServer {
                         .threads(parallel_cpu_threads)
                         // TODO: disable this security check if we see errors from lightwalletd
                         //.allowed_hosts(DomainsValidation::Disabled)
-                        .request_middleware(FixHttpRequestMiddleware::new(generated_password))
+                        .request_middleware(FixHttpRequestMiddleware)
                         .start_http(&listen_addr)
                         .expect("Unable to start RPC server");
 

diff --git a/zebra-rpc/src/server/cookie.rs b/zebra-rpc/src/server/cookie.rs
@@ -1,6 +1,6 @@
 //! Cookie-based authentication for the RPC server.
 
-use base64::Engine;
+use base64::{engine::general_purpose::URL_SAFE, Engine as _};
 use rand::RngCore;
 
 use std::{
@@ -14,18 +14,18 @@ pub const COOKIEAUTH_USER: &str = "__cookie__";
 const COOKIEAUTH_FILE: &str = ".cookie";
 
 /// Generate a new auth cookie and return the encoded password.
-pub fn generate() -> Option<String> {
+pub fn generate() -> Option<()> {
     let mut data = [0u8; 32];
     rand::thread_rng().fill_bytes(&mut data);
-    let encoded_password = base64::prelude::BASE64_STANDARD.encode(data);
+    let encoded_password = URL_SAFE.encode(data);
     let cookie_content = format!("{}:{}", COOKIEAUTH_USER, encoded_password);
 
     let mut file = File::create(COOKIEAUTH_FILE).ok()?;
     file.write_all(cookie_content.as_bytes()).ok()?;
 
     tracing::info!("RPC auth cookie generated successfully");
 
-    Some(encoded_password)
+    Some(())
 }
 
 /// Get the encoded password from the auth cookie.

diff --git a/zebra-rpc/src/server/http_request_compatibility.rs b/zebra-rpc/src/server/http_request_compatibility.rs
@@ -2,6 +2,7 @@
 //!
 //! These fixes are applied at the HTTP level, before the RPC request is parsed.
 
+use base64::{engine::general_purpose::URL_SAFE, Engine as _};
 use futures::TryStreamExt;
 use jsonrpc_http_server::{
     hyper::{body::Bytes, header, Body, Request},
@@ -37,17 +38,22 @@ use crate::server::cookie;
 /// We assume lightwalletd validates data encodings before sending it on to Zebra.
 /// So any fixes Zebra performs won't change user-specified data.
 #[derive(Clone, Debug)]
-pub struct FixHttpRequestMiddleware(String);
+pub struct FixHttpRequestMiddleware;
 
 impl RequestMiddleware for FixHttpRequestMiddleware {
     fn on_request(&self, mut request: Request<Body>) -> RequestMiddlewareAction {
         tracing::trace!(?request, "original HTTP request");
 
+        // Check if the request is authenticated
+        if !FixHttpRequestMiddleware::check_credentials(request.headers_mut()) {
+            request = Self::unauthenticated(request);
+        }
+
         // Fix the request headers if needed and we can do so.
         FixHttpRequestMiddleware::insert_or_replace_content_type_header(request.headers_mut());
 
         // Fix the request body
-        let mut request = request.map(|body| {
+        let request = request.map(|body| {
             let body = body.map_ok(|data| {
                 // To simplify data handling, we assume that any search strings won't be split
                 // across multiple `Bytes` data buffers.
@@ -72,18 +78,6 @@ impl RequestMiddleware for FixHttpRequestMiddleware {
             Body::wrap_stream(body)
         });
 
-        // Check if the request is authenticated
-        match cookie::get() {
-            Some(password) => {
-                if password != self.0 {
-                    request = Self::unauthenticated(request);
-                }
-            }
-            None => {
-                request = Self::unauthenticated(request);
-            }
-        }
-
         tracing::trace!(?request, "modified HTTP request");
 
         RequestMiddlewareAction::Proceed {
@@ -156,11 +150,6 @@ impl FixHttpRequestMiddleware {
         }
     }
 
-    /// Create a new `FixHttpRequestMiddleware`.
-    pub fn new(password: String) -> Self {
-        Self(password)
-    }
-
     /// Change the method name in the JSON request.
     fn change_method_name(data: String) -> String {
         let mut json_data: serde_json::Value = serde_json::from_str(&data).expect("Invalid JSON");
@@ -184,4 +173,27 @@ impl FixHttpRequestMiddleware {
             Body::wrap_stream(body)
         })
     }
+
+    /// Check if the request is authenticated.
+    pub fn check_credentials(headers: &header::HeaderMap) -> bool {
+        headers
+            .get(header::AUTHORIZATION)
+            .and_then(|auth_header| auth_header.to_str().ok())
+            .and_then(|auth| auth.split_whitespace().nth(1))
+            .and_then(|token| URL_SAFE.decode(token).ok())
+            .and_then(|decoded| String::from_utf8(decoded).ok())
+            .and_then(|decoded_str| {
+                decoded_str
+                    .split(':')
+                    .nth(1)
+                    .map(|password| password.to_string())
+            })
+            .map_or(false, |password| {
+                if let Some(cookie_password) = cookie::get() {
+                    cookie_password == password
+                } else {
+                    false
+                }
+            })
+    }
 }