Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add read-only mode for terminal sessions #104

Merged
3 changes: 2 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions crates/sshx-core/proto/sshx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ 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.
optional bytes encrypted_write_zeros = 5; // Encrypted zero block for the write password (for verification).
}

// Details of a newly-created sshx session.
Expand Down Expand Up @@ -103,6 +105,7 @@ message SerializedSession {
uint32 next_sid = 3;
uint32 next_uid = 4;
string name = 5;
optional bytes encrypted_write_zeros = 6;
}

message SerializedShell {
Expand Down
1 change: 1 addition & 0 deletions crates/sshx-server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 2 additions & 0 deletions crates/sshx-server/src/grpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,14 @@ impl SshxService for GrpcServer {
}
let name = rand_alphanumeric(10);
info!(%name, "creating new session");

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,
encrypted_write_zeros: request.encrypted_write_zeros,
};
self.0.insert(&name, Arc::new(Session::new(metadata)));
}
Expand Down
16 changes: 15 additions & 1 deletion crates/sshx-server/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ pub struct Metadata {

/// Name of the session (human-readable).
pub name: String,

/// Password for write access to the session.
pub encrypted_write_zeros: Option<Bytes>,
}

/// In-memory state for a single sshx session.
Expand Down Expand Up @@ -307,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]
Expand All @@ -325,6 +328,7 @@ impl Session {
name: format!("User {id}"),
cursor: None,
focus: None,
can_write,
};
v.insert(user.clone());
self.broadcast.send(WsServer::UserDiff(id, Some(user))).ok();
Expand All @@ -341,6 +345,16 @@ impl Session {
self.broadcast.send(WsServer::UserDiff(id, None)).ok();
}

/// Check if a user has write permission in the session.
pub fn check_write_permission(&self, user_id: Uid) -> Result<()> {
let users = self.users.read();
let user = users.get(&user_id).context("user not found")?;
if !user.can_write {
bail!("No write permission");
}
Ok(())
}

/// Send a chat message into the room.
pub fn send_chat(&self, id: Uid, msg: &str) -> Result<()> {
// Populate the message with the current name in case it's not known later.
Expand Down
3 changes: 3 additions & 0 deletions crates/sshx-server/src/session/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ impl Session {
next_sid: ids.0 .0,
next_uid: ids.1 .0,
name: self.metadata().name.clone(),
encrypted_write_zeros: self.metadata().encrypted_write_zeros.clone(),
};
let data = message.encode_to_vec();
ensure!(data.len() < MAX_SNAPSHOT_SIZE, "snapshot too large");
Expand All @@ -72,9 +73,11 @@ impl Session {
pub fn restore(data: &[u8]) -> Result<Self> {
let data = zstd::bulk::decompress(data, MAX_SNAPSHOT_SIZE)?;
let message = SerializedSession::decode(&*data)?;

let metadata = Metadata {
encrypted_zeros: message.encrypted_zeros,
name: message.name,
encrypted_write_zeros: message.encrypted_write_zeros,
};

let session = Self::new(metadata);
Expand Down
7 changes: 5 additions & 2 deletions crates/sshx-server/src/web/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ pub struct WsUser {
pub cursor: Option<(i32, i32)>,
/// Currently focused terminal window ID.
pub focus: Option<Sid>,
/// Whether the user has write permissions in the session.
pub can_write: bool,
}

/// A real-time message sent from the server over WebSocket.
Expand Down Expand Up @@ -71,8 +73,9 @@ pub enum WsServer {
#[derive(Serialize, Deserialize, Debug, Clone)]
#[serde(rename_all = "camelCase")]
pub enum WsClient {
/// Authenticate the user's encryption key by zeros block.
Authenticate(Bytes),
/// Authenticate the user's encryption key by zeros block and write password
/// (if provided).
Authenticate(Bytes, Option<Bytes>),
/// Set the name of the current user.
SetName(String),
/// Send real-time information about the user's cursor.
Expand Down
57 changes: 52 additions & 5 deletions crates/sshx-server/src/web/socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use bytes::Bytes;
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};
Expand Down Expand Up @@ -95,15 +96,45 @@ async fn handle_socket(socket: &mut WebSocket, session: Arc<Session>) -> Result<
session.sync_now();
send(socket, WsServer::Hello(user_id, metadata.name.clone())).await?;

match recv(socket).await? {
Some(WsClient::Authenticate(bytes)) if bytes == metadata.encrypted_zeros => {}
let (user_guard, _) = match recv(socket).await? {
Some(WsClient::Authenticate(bytes, write_password_bytes)) => {
// `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 {
ekzhang marked this conversation as resolved.
Show resolved Hide resolved
send(socket, WsServer::InvalidAuth()).await?;
return Ok(());
}

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 provided_password.ct_eq(stored_password).unwrap_u8() != 1 {
send(socket, WsServer::InvalidAuth()).await?;
ekzhang marked this conversation as resolved.
Show resolved Hide resolved
return Ok(());
}
true
}

// Password stored but not provided, user can't write (Read-Only)
(None, Some(_)) => false,
};

// Create user and return both guard and can_write status
let user_guard = session.user_scope(user_id, can_write)?;

(user_guard, can_write)
}
_ => {
send(socket, WsServer::InvalidAuth()).await?;
return Ok(());
}
}
};

let _user_guard = session.user_scope(user_id)?;
let _user_guard = user_guard;

let update_tx = session.update_tx(); // start listening for updates before any state reads
let mut broadcast_stream = session.subscribe_broadcast();
Expand Down Expand Up @@ -138,7 +169,7 @@ async fn handle_socket(socket: &mut WebSocket, session: Arc<Session>) -> Result<
};

match msg {
WsClient::Authenticate(_) => {}
WsClient::Authenticate(_, _) => {}
WsClient::SetName(name) => {
if !name.is_empty() {
session.update_user(user_id, |user| user.name = name)?;
Expand All @@ -151,6 +182,10 @@ async fn handle_socket(socket: &mut WebSocket, session: Arc<Session>) -> Result<
session.update_user(user_id, |user| user.focus = id)?;
}
WsClient::Create(x, y) => {
if let Err(e) = session.check_write_permission(user_id) {
send(socket, WsServer::Error(e.to_string())).await?;
continue;
}
let id = session.counter().next_sid();
session.sync_now();
let new_shell = NewShell { id: id.0, x, y };
Expand All @@ -159,9 +194,17 @@ async fn handle_socket(socket: &mut WebSocket, session: Arc<Session>) -> Result<
.await?;
}
WsClient::Close(id) => {
if let Err(e) = session.check_write_permission(user_id) {
send(socket, WsServer::Error(e.to_string())).await?;
continue;
}
update_tx.send(ServerMessage::CloseShell(id.0)).await?;
}
WsClient::Move(id, winsize) => {
if let Err(e) = session.check_write_permission(user_id) {
send(socket, WsServer::Error(e.to_string())).await?;
continue;
}
if let Err(err) = session.move_shell(id, winsize) {
send(socket, WsServer::Error(err.to_string())).await?;
continue;
Expand All @@ -176,6 +219,10 @@ async fn handle_socket(socket: &mut WebSocket, session: Arc<Session>) -> Result<
}
}
WsClient::Data(id, data, offset) => {
if let Err(e) = session.check_write_permission(user_id) {
send(socket, WsServer::Error(e.to_string())).await?;
continue;
}
let input = TerminalInput {
id: id.0,
data,
Expand Down
3 changes: 2 additions & 1 deletion crates/sshx-server/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ impl ClientSocket {

async fn authenticate(&mut self) {
let encrypted_zeros = self.encrypt.zeros().into();
self.send(WsClient::Authenticate(encrypted_zeros)).await;
self.send(WsClient::Authenticate(encrypted_zeros, None))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think we should write at least one test for this read/write functionality so we're sure that it's working according to spec and continues working as such, is that something you'd be able to take on?

.await;
}

pub async fn send(&mut self, msg: WsClient) {
Expand Down
2 changes: 2 additions & 0 deletions crates/sshx-server/tests/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ async fn test_rpc() -> Result<()> {
origin: "sshx.io".into(),
encrypted_zeros: Encrypt::new("").zeros().into(),
name: String::new(),
enable_readers: true,
encrypted_write_zeros: Some(Encrypt::new("").zeros().into()),
};
let resp = client.open(req).await?;
assert!(!resp.into_inner().name.is_empty());
Expand Down
2 changes: 1 addition & 1 deletion crates/sshx-server/tests/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).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 });
Expand Down
14 changes: 7 additions & 7 deletions crates/sshx-server/tests/with_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).await?;
let controller = Controller::new(&server.endpoint(), "", Runner::Echo, false).await?;
controller.close().await?;
Ok(())
}
Expand All @@ -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).await?;
let mut controller = Controller::new(&server.endpoint(), "", runner, false).await?;

let session = server
.state()
Expand Down Expand Up @@ -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).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 });
Expand Down Expand Up @@ -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).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 });
Expand Down Expand Up @@ -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).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 });
Expand Down Expand Up @@ -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).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 });
Expand All @@ -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).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 });
Expand Down
Loading
Loading