fix: address PR feedback for read-only users

- Update read-only badge styles and poistion
- Use subtle instead of constant_time_eq dependency
- Add constant-time comparison for encrypted_zeros
- Improve write password entropy
- Optimize user scope updates to avoid double broadcast
- Sort cargo dependencies
- format cli display properly

crates/sshx-core/proto/sshx.proto

@@ -41,15 +41,15 @@ message OpenRequest {
   string origin = 1;         // Web origin of the server.
   bytes encrypted_zeros = 2; // Encrypted zero block, for client verification.
   string name = 3;           // Name of the session (user@hostname).
-  bool enable_readers = 4;  // Enable read-only mode for the session.
+  bool enable_readers = 4;   // Enable read-only mode for the session.
+  optional bytes encrypted_write_zeros = 5;    // Encrypted zero block for the write password (for verification).
 }
 
 // Details of a newly-created sshx session.
 message OpenResponse {
   string name = 1;  // Name of the session.
   string token = 2; // Signed verification token for the client.
   string url = 3;   // Public web URL to view the session.
-  string write_password = 4; // Password for write access (when readers mode enabled)
 }
 
 // Sequence numbers for all active shells, used for synchronization.
@@ -105,7 +105,7 @@ message SerializedSession {
   uint32 next_sid = 3;
   uint32 next_uid = 4;
   string name = 5;
-  string write_password = 6;
+  optional bytes encrypted_write_zeros = 6;
 }
 
 message SerializedShell {

crates/sshx-server/Cargo.toml

@@ -31,6 +31,7 @@ redis = { version = "0.23.3", features = ["tokio-rustls-comp", "tls-rustls-webpk
 serde.workspace = true
 sha2 = "0.10.7"
 sshx-core.workspace = true
+subtle = "2.5.0"
 tokio.workspace = true
 tokio-stream.workspace = true
 tokio-tungstenite = "0.20.0"
@@ -41,7 +42,6 @@ tower-http = { version = "0.4.4", features = ["fs", "redirect", "trace"] }
 tracing.workspace = true
 tracing-subscriber.workspace = true
 zstd = "0.12.4"
-constant_time_eq = "0.3.0"
 
 [dev-dependencies]
 reqwest = { version = "0.11.20", default-features = false, features = ["rustls-tls"] }

crates/sshx-server/src/grpc.rs

@@ -51,19 +51,13 @@ impl SshxService for GrpcServer {
         let name = rand_alphanumeric(10);
         info!(%name, "creating new session");
 
-        let write_password = if request.enable_readers {
-            Some(rand_alphanumeric(8))
-        } else {
-            None
-        };
-
         match self.0.lookup(&name) {
             Some(_) => return Err(Status::already_exists("generated duplicate ID")),
             None => {
                 let metadata = Metadata {
                     encrypted_zeros: request.encrypted_zeros,
                     name: request.name,
-                    write_password: write_password.clone(),
+                    encrypted_write_zeros: request.encrypted_write_zeros,
                 };
                 self.0.insert(&name, Arc::new(Session::new(metadata)));
             }
@@ -74,7 +68,6 @@ impl SshxService for GrpcServer {
             name,
             token: BASE64_STANDARD.encode(token.into_bytes()),
             url,
-            write_password: write_password.unwrap_or_default(),
         }))
     }
 

crates/sshx-server/src/session.rs

@@ -35,7 +35,7 @@ pub struct Metadata {
     pub name: String,
 
     /// Password for write access to the session.
-    pub write_password: Option<String>,
+    pub encrypted_write_zeros: Option<Bytes>,
 }
 
 /// In-memory state for a single sshx session.
@@ -310,7 +310,7 @@ impl Session {
     }
 
     /// Add a new user, and return a guard that removes the user when dropped.
-    pub fn user_scope(&self, id: Uid) -> Result<impl Drop + '_> {
+    pub fn user_scope(&self, id: Uid, can_write: bool) -> Result<impl Drop + '_> {
         use std::collections::hash_map::Entry::*;
 
         #[must_use]
@@ -328,7 +328,7 @@ impl Session {
                     name: format!("User {id}"),
                     cursor: None,
                     focus: None,
-                    can_write: false,
+                    can_write,
                 };
                 v.insert(user.clone());
                 self.broadcast.send(WsServer::UserDiff(id, Some(user))).ok();

crates/sshx-server/src/session/snapshot.rs

@@ -62,7 +62,7 @@ impl Session {
             next_sid: ids.0 .0,
             next_uid: ids.1 .0,
             name: self.metadata().name.clone(),
-            write_password: self.metadata().write_password.clone().unwrap_or_default(),
+            encrypted_write_zeros: self.metadata().encrypted_write_zeros.clone(),
         };
         let data = message.encode_to_vec();
         ensure!(data.len() < MAX_SNAPSHOT_SIZE, "snapshot too large");
@@ -74,16 +74,10 @@ impl Session {
         let data = zstd::bulk::decompress(data, MAX_SNAPSHOT_SIZE)?;
         let message = SerializedSession::decode(&*data)?;
 
-        let write_password = if message.write_password.is_empty() {
-            None
-        } else {
-            Some(message.write_password)
-        };
-
         let metadata = Metadata {
             encrypted_zeros: message.encrypted_zeros,
             name: message.name,
-            write_password,
+            encrypted_write_zeros: message.encrypted_write_zeros,
         };
 
         let session = Self::new(metadata);

crates/sshx-server/src/web/socket.rs

@@ -8,10 +8,10 @@ use axum::extract::{
 };
 use axum::response::IntoResponse;
 use bytes::Bytes;
-use constant_time_eq::constant_time_eq;
 use futures_util::SinkExt;
 use sshx_core::proto::{server_update::ServerMessage, NewShell, TerminalInput, TerminalSize};
 use sshx_core::Sid;
+use subtle::ConstantTimeEq;
 use tokio::sync::mpsc;
 use tokio_stream::StreamExt;
 use tracing::{error, info_span, warn, Instrument};
@@ -98,19 +98,21 @@ async fn handle_socket(socket: &mut WebSocket, session: Arc<Session>) -> Result<
 
     let (user_guard, _) = match recv(socket).await? {
         Some(WsClient::Authenticate(bytes, write_password_bytes)) => {
-            if bytes != metadata.encrypted_zeros {
+            // `ct_eq` returns a `Choice`, and `unwrap_u8()` converts it to 1 (equal) or 0
+            // (not equal).
+            if bytes.ct_eq(metadata.encrypted_zeros.as_ref()).unwrap_u8() != 1 {
                 send(socket, WsServer::InvalidAuth()).await?;
                 return Ok(());
             }
 
-            let can_write = match (write_password_bytes, &metadata.write_password) {
+            let can_write = match (write_password_bytes, &metadata.encrypted_write_zeros) {
                 // No password provided and none stored, it means users can write (Default)
                 (_, None) => true,
 
                 // Both password provided and stored, validate they match using constant-time
                 // comparison.
                 (Some(provided_password), Some(stored_password)) => {
-                    if !constant_time_eq(&provided_password, stored_password.as_bytes()) {
+                    if provided_password.ct_eq(stored_password).unwrap_u8() != 1 {
                         send(socket, WsServer::InvalidAuth()).await?;
                         return Ok(());
                     }
@@ -122,10 +124,7 @@ async fn handle_socket(socket: &mut WebSocket, session: Arc<Session>) -> Result<
             };
 
             // Create user and return both guard and can_write status
-            let user_guard = session.user_scope(user_id)?;
-            session.update_user(user_id, |user| {
-                user.can_write = can_write;
-            })?;
+            let user_guard = session.user_scope(user_id, can_write)?;
 
             (user_guard, can_write)
         }

crates/sshx-server/tests/simple.rs

@@ -15,7 +15,8 @@ async fn test_rpc() -> Result<()> {
         origin: "sshx.io".into(),
         encrypted_zeros: Encrypt::new("").zeros().into(),
         name: String::new(),
-        enable_readers: false,
+        enable_readers: true,
+        encrypted_write_zeros: Some(Encrypt::new("").zeros().into()),
     };
     let resp = client.open(req).await?;
     assert!(!resp.into_inner().name.is_empty());

crates/sshx-server/tests/snapshot.rs

@@ -16,7 +16,7 @@ pub mod common;
 async fn test_basic_restore() -> Result<()> {
     let server = TestServer::new().await;
 
-    let mut controller = Controller::new(&server.endpoint(), "", Runner::Echo, &false).await?;
+    let mut controller = Controller::new(&server.endpoint(), "", Runner::Echo, false).await?;
     let name = controller.name().to_owned();
     let key = controller.encryption_key().to_owned();
     tokio::spawn(async move { controller.run().await });

crates/sshx-server/tests/with_client.rs

@@ -14,7 +14,7 @@ pub mod common;
 #[tokio::test]
 async fn test_handshake() -> Result<()> {
     let server = TestServer::new().await;
-    let controller = Controller::new(&server.endpoint(), "", Runner::Echo, &false).await?;
+    let controller = Controller::new(&server.endpoint(), "", Runner::Echo, false).await?;
     controller.close().await?;
     Ok(())
 }
@@ -23,7 +23,7 @@ async fn test_handshake() -> Result<()> {
 async fn test_command() -> Result<()> {
     let server = TestServer::new().await;
     let runner = Runner::Shell("/bin/bash".into());
-    let mut controller = Controller::new(&server.endpoint(), "", runner, &false).await?;
+    let mut controller = Controller::new(&server.endpoint(), "", runner, false).await?;
 
     let session = server
         .state()
@@ -69,7 +69,7 @@ async fn test_ws_missing() -> Result<()> {
 async fn test_ws_basic() -> Result<()> {
     let server = TestServer::new().await;
 
-    let mut controller = Controller::new(&server.endpoint(), "", Runner::Echo, &false).await?;
+    let mut controller = Controller::new(&server.endpoint(), "", Runner::Echo, false).await?;
     let name = controller.name().to_owned();
     let key = controller.encryption_key().to_owned();
     tokio::spawn(async move { controller.run().await });
@@ -101,7 +101,7 @@ async fn test_ws_basic() -> Result<()> {
 async fn test_ws_resize() -> Result<()> {
     let server = TestServer::new().await;
 
-    let mut controller = Controller::new(&server.endpoint(), "", Runner::Echo, &false).await?;
+    let mut controller = Controller::new(&server.endpoint(), "", Runner::Echo, false).await?;
     let name = controller.name().to_owned();
     let key = controller.encryption_key().to_owned();
     tokio::spawn(async move { controller.run().await });
@@ -145,7 +145,7 @@ async fn test_ws_resize() -> Result<()> {
 async fn test_users_join() -> Result<()> {
     let server = TestServer::new().await;
 
-    let mut controller = Controller::new(&server.endpoint(), "", Runner::Echo, &false).await?;
+    let mut controller = Controller::new(&server.endpoint(), "", Runner::Echo, false).await?;
     let name = controller.name().to_owned();
     let key = controller.encryption_key().to_owned();
     tokio::spawn(async move { controller.run().await });
@@ -174,7 +174,7 @@ async fn test_users_join() -> Result<()> {
 async fn test_users_metadata() -> Result<()> {
     let server = TestServer::new().await;
 
-    let mut controller = Controller::new(&server.endpoint(), "", Runner::Echo, &false).await?;
+    let mut controller = Controller::new(&server.endpoint(), "", Runner::Echo, false).await?;
     let name = controller.name().to_owned();
     let key = controller.encryption_key().to_owned();
     tokio::spawn(async move { controller.run().await });
@@ -199,7 +199,7 @@ async fn test_users_metadata() -> Result<()> {
 async fn test_chat_messages() -> Result<()> {
     let server = TestServer::new().await;
 
-    let mut controller = Controller::new(&server.endpoint(), "", Runner::Echo, &false).await?;
+    let mut controller = Controller::new(&server.endpoint(), "", Runner::Echo, false).await?;
     let name = controller.name().to_owned();
     let key = controller.encryption_key().to_owned();
     tokio::spawn(async move { controller.run().await });

crates/sshx/src/controller.rs

@@ -51,35 +51,47 @@ impl Controller {
         origin: &str,
         name: &str,
         runner: Runner,
-        enable_readers: &bool,
+        enable_readers: bool,
     ) -> Result<Self> {
         debug!(%origin, "connecting to server");
         let encryption_key = rand_alphanumeric(14); // 83.3 bits of entropy
+        let write_password = rand_alphanumeric(14); // 83.3 bits of entropy
 
         let kdf_task = {
             let encryption_key = encryption_key.clone();
             task::spawn_blocking(move || Encrypt::new(&encryption_key))
         };
+
+        let kdf_write_password_task = {
+            let write_password = write_password.clone();
+            task::spawn_blocking(move || Encrypt::new(&write_password))
+        };
+
         let mut client = Self::connect(origin).await?;
         let encrypt = kdf_task.await?;
+        let encrypt_write_password = kdf_write_password_task.await?;
+        let encrypted_write_zeros = if enable_readers {
+            Some(encrypt_write_password.zeros().into())
+        } else {
+            None
+        };
 
         let req = OpenRequest {
             origin: origin.into(),
             encrypted_zeros: encrypt.zeros().into(),
             name: name.into(),
-            enable_readers: *enable_readers,
+            enable_readers,
+            encrypted_write_zeros,
         };
         let mut resp = client.open(req).await?.into_inner();
         resp.url = resp.url + "#" + &encryption_key;
 
-        let write_password = if resp.write_password.is_empty() {
-            None
+        let write_url = if enable_readers {
+            Some(resp.url.clone() + "," + &write_password)
         } else {
-            Some(resp.write_password.clone())
+            None
         };
 
-        let write_url = write_password.map(|wp| resp.url.clone() + "," + &wp);
-
         let (output_tx, output_rx) = mpsc::channel(64);
         Ok(Self {
             origin: origin.into(),

crates/sshx/src/main.rs

@@ -43,9 +43,9 @@ fn print_greeting(shell: &str, controller: &Controller) {
             r#"
       {sshx} {version}
     
-      {arr}  Read-Only Link:  {link_v}
-      {arr}  Writable Link:  {link_e}
-      {arr}  Shell: {shell_v}
+      {arr}  Read-only link:  {link_v}
+      {arr}  Writable  link:  {link_e}
+      {arr}  Shell:           {shell_v}
     "#,
             sshx = Green.bold().paint("sshx"),
             version = Green.paint(&version_str),
@@ -92,7 +92,7 @@ async fn start(args: Args) -> Result<()> {
     });
 
     let runner = Runner::Shell(shell.clone());
-    let mut controller = Controller::new(&args.server, &name, runner, &args.enable_readers).await?;
+    let mut controller = Controller::new(&args.server, &name, runner, args.enable_readers).await?;
     if args.quiet {
         println!("{}", controller.url());
     } else {