c/quicproquo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: Sprint 1 — production hardening, TLS lifecycle, CI coverage, lint cleanup

Browse Source 
- Fix 3 client panics: replace .unwrap()/.expect() with proper error
  handling in rpc.rs (AUTH_CONTEXT lock), repl.rs (pending_member),
  and retry.rs (last_err)
- Add --danger-accept-invalid-certs flag with InsecureServerCertVerifier
  for development TLS bypass, plus mdBook TLS documentation
- Add CI coverage job (cargo-tarpaulin) and Docker build validation
  to GitHub Actions workflow, plus README CI badge
- Add [workspace.lints] config, fix 46 clippy warnings across 8 crates,
  zero warnings on all buildable crates
- Update Dockerfile for all 11 workspace members
This commit is contained in:
Chris Nennemann
2026-03-03 23:19:11 +01:00
parent dc4e4e49a0
commit 612b06aa8e
33 changed files with 388 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
crates/quicproquo-client/src/client/commands.rs
View File

@@ -27,8 +27,8 @@ pub fn cmd_whoami(state_path: &Path, password: Option<&str>) -> anyhow::Result<(
let pk_bytes = identity.public_key_bytes();
let fingerprint = sha256(&pk_bytes);
println!("identity_key : {}", hex::encode(&pk_bytes));
println!("fingerprint : {}", hex::encode(&fingerprint));
println!("identity_key : {}", hex::encode(pk_bytes));
println!("fingerprint : {}", hex::encode(fingerprint));
println!(
"hybrid_key : {}",
if state.hybrid_key.is_some() {
@@ -203,6 +203,7 @@ pub async fn cmd_register_user(
}
/// Log in via the OPAQUE protocol and receive a session token.
#[allow(clippy::too_many_arguments)]
pub async fn cmd_login(
server: &str,
ca_cert: &Path,
@@ -522,7 +523,7 @@ async fn do_upload_keypackage(
anyhow::ensure!(server_fp == fingerprint, "fingerprint mismatch");
if let Some(ref hkp) = hybrid_kp {
if let Some(hkp) = &hybrid_kp {
upload_hybrid_key(
&node_client,
&member.identity().public_key_bytes(),
@@ -914,6 +915,7 @@ pub async fn cmd_join(
}
/// Send an application message via DS (single recipient or broadcast to all other members).
#[allow(clippy::too_many_arguments)]
pub async fn cmd_send(
state_path: &Path,
server: &str,
@@ -1115,8 +1117,8 @@ pub fn whoami_json(state_path: &Path, password: Option<&str>) -> anyhow::Result<
let fingerprint = sha256(&pk_bytes);
Ok(format!(
r#"{{"identity_key":"{}", "fingerprint":"{}", "hybrid_key":{}, "group":{}}}"#,
hex::encode(&pk_bytes),
hex::encode(&fingerprint),
hex::encode(pk_bytes),
hex::encode(fingerprint),
state.hybrid_key.is_some(),
state.group.is_some(),
))

4
crates/quicproquo-client/src/client/conversation.rs
View File

@@ -169,7 +169,7 @@ impl ConversationStore {
let salt = get_or_create_salt(&salt_path)?;
let key = derive_convdb_key(password, &salt)?;
let hex_key = hex::encode(&*key);
let hex_key = hex::encode(*key);
let conn = Connection::open(db_path).context("open conversation db")?;
conn.pragma_update(None, "key", format!("x'{hex_key}'"))
@@ -188,7 +188,7 @@ impl ConversationStore {
) -> anyhow::Result<()> {
let salt = get_or_create_salt(salt_path)?;
let key = derive_convdb_key(password, &salt)?;
let hex_key = hex::encode(&*key);
let hex_key = hex::encode(*key);
let enc_path = db_path.with_extension("convdb-enc");

24
crates/quicproquo-client/src/client/repl.rs
View File

@@ -284,6 +284,7 @@ async fn ensure_server(
// ── REPL entry point ─────────────────────────────────────────────────────────
#[allow(clippy::too_many_arguments)]
pub async fn run_repl(
state_path: &Path,
server: &str,
@@ -497,6 +498,7 @@ async fn auto_upload_keys(
}
/// Determine the access token, performing OPAQUE registration/login as needed.
#[allow(clippy::too_many_arguments)]
async fn resolve_access_token(
state_path: &Path,
server: &str,
@@ -715,13 +717,11 @@ fn cmd_list(session: &SessionState) -> anyhow::Result<()> {
fn cmd_switch(session: &mut SessionState, target: &str) -> anyhow::Result<()> {
let target = target.trim();
let conv = if target.starts_with('@') {
let username = &target[1..];
let conv = if let Some(username) = target.strip_prefix('@') {
session.conv_store.list_conversations()?.into_iter().find(|c| {
matches!(&c.kind, ConversationKind::Dm { peer_username: Some(u), .. } if u == username)
})
} else if target.starts_with('#') {
let name = &target[1..];
} else if let Some(name) = target.strip_prefix('#') {
session.conv_store.find_group_by_name(name)?
} else {
// Try as display name
@@ -861,7 +861,7 @@ async fn cmd_dm(
display_name: format!("@{username}"),
mls_group_blob: member
.group_ref()
.map(|g| bincode::serialize(g))
.map(bincode::serialize)
.transpose()
.context("serialize group")?,
keystore_blob: None,
@@ -905,7 +905,7 @@ fn cmd_create_group(session: &mut SessionState, name: &str) -> anyhow::Result<()
display_name: format!("#{name}"),
mls_group_blob: member
.group_ref()
.map(|g| bincode::serialize(g))
.map(bincode::serialize)
.transpose()
.context("serialize group")?,
keystore_blob: None,
@@ -1099,7 +1099,7 @@ async fn cmd_join(
// Try to process with existing groups first
let mut handled = false;
for (_cid, member) in &mut session.members {
for member in session.members.values_mut() {
match member.receive_message(&mls_payload) {
Ok(_) => { handled = true; break; }
Err(_) => continue,
@@ -1147,7 +1147,7 @@ async fn cmd_join(
display_name: format!("#{display}"),
mls_group_blob: new_member
.group_ref()
.map(|g| bincode::serialize(g))
.map(bincode::serialize)
.transpose()
.context("serialize joined group")?,
keystore_blob: None,
@@ -1570,7 +1570,13 @@ async fn try_auto_join(
}
// Take ownership of the pending member.
let member = session.pending_member.take().unwrap();
let member = match session.pending_member.take() {
Some(m) => m,
None => {
tracing::error!("pending_member disappeared after successful join");
return false;
}
};
let member_keys = member.member_identities();
// Figure out the peer (any member that isn't us).

9
crates/quicproquo-client/src/client/retry.rs
View File

@@ -25,15 +25,13 @@ where
Fut: Future<Output = Result<T, E>>,
P: Fn(&E) -> bool,
{
let mut last_err = None;
let mut last_err: Option<E> = None;
for attempt in 0..max_retries {
match op().await {
Ok(t) => return Ok(t),
Err(e) => {
last_err = Some(e);
let err = last_err.as_ref().unwrap();
if !is_retriable(err) || attempt + 1 >= max_retries {
break;
if !is_retriable(&e) || attempt + 1 >= max_retries {
return Err(e);
}
let delay_ms = base_delay_ms * 2u64.saturating_pow(attempt);
let jitter_ms = rand::thread_rng().gen_range(0..=delay_ms / 2);
@@ -44,6 +42,7 @@ where
delay_ms = total_ms,
"RPC failed, retrying after backoff"
);
last_err = Some(e);
tokio::time::sleep(Duration::from_millis(total_ms)).await;
}
}

94
crates/quicproquo-client/src/client/rpc.rs
View File

@@ -5,7 +5,7 @@ use std::sync::Arc;
use anyhow::Context;
use quinn::{ClientConfig, Endpoint};
use quinn_proto::crypto::rustls::QuicClientConfig;
use rustls::pki_types::CertificateDer;
use rustls::pki_types::{CertificateDer, ServerName, UnixTime};
use rustls::{ClientConfig as RustlsClientConfig, RootCertStore};
use tokio_util::compat::{TokioAsyncReadCompatExt, TokioAsyncWriteCompatExt};
use capnp_rpc::{rpc_twoparty_capnp::Side, twoparty, RpcSystem};
@@ -13,34 +13,101 @@ use capnp_rpc::{rpc_twoparty_capnp::Side, twoparty, RpcSystem};
use quicproquo_core::HybridPublicKey;
use quicproquo_proto::node_capnp::{auth, node_service};
use crate::AUTH_CONTEXT;
use crate::{AUTH_CONTEXT, INSECURE_SKIP_VERIFY};
use super::retry::{anyhow_is_retriable, retry_async, DEFAULT_BASE_DELAY_MS, DEFAULT_MAX_RETRIES};
/// Cap'n Proto traversal limit (words). 4 Mi words = 32 MiB; bounds DoS from deeply nested or large messages.
const CAPNP_TRAVERSAL_LIMIT_WORDS: usize = 4 * 1024 * 1024;
/// A [`rustls::client::danger::ServerCertVerifier`] that accepts any certificate.
///
/// **Development only.** Using this in production disables all TLS guarantees.
#[derive(Debug)]
struct InsecureServerCertVerifier;
impl rustls::client::danger::ServerCertVerifier for InsecureServerCertVerifier {
fn verify_server_cert(
&self,
_end_entity: &CertificateDer<'_>,
_intermediates: &[CertificateDer<'_>],
_server_name: &ServerName<'_>,
_ocsp_response: &[u8],
_now: UnixTime,
) -> Result<rustls::client::danger::ServerCertVerified, rustls::Error> {
Ok(rustls::client::danger::ServerCertVerified::assertion())
}
fn verify_tls12_signature(
&self,
_message: &[u8],
_cert: &CertificateDer<'_>,
_dss: &rustls::DigitallySignedStruct,
) -> Result<rustls::client::danger::HandshakeSignatureValid, rustls::Error> {
Ok(rustls::client::danger::HandshakeSignatureValid::assertion())
}
fn verify_tls13_signature(
&self,
_message: &[u8],
_cert: &CertificateDer<'_>,
_dss: &rustls::DigitallySignedStruct,
) -> Result<rustls::client::danger::HandshakeSignatureValid, rustls::Error> {
Ok(rustls::client::danger::HandshakeSignatureValid::assertion())
}
fn supported_verify_schemes(&self) -> Vec<rustls::SignatureScheme> {
rustls::crypto::ring::default_provider()
.signature_verification_algorithms
.supported_schemes()
}
}
/// Establish a QUIC/TLS connection and return a `NodeService` client.
///
/// Must be called from within a `LocalSet` because capnp-rpc is `!Send`.
///
/// Reads [`INSECURE_SKIP_VERIFY`] to decide whether to bypass certificate
/// verification (set once at startup via [`crate::set_insecure_skip_verify`]).
pub async fn connect_node(
server: &str,
ca_cert: &Path,
server_name: &str,
) -> anyhow::Result<node_service::Client> {
let skip = INSECURE_SKIP_VERIFY.load(std::sync::atomic::Ordering::Relaxed);
connect_node_opt(server, ca_cert, server_name, skip).await
}
/// Like [`connect_node`] but with an explicit `insecure_skip_verify` toggle.
///
/// When `insecure_skip_verify` is `true`, certificate verification is disabled entirely.
/// This is intended for development and testing only.
pub async fn connect_node_opt(
server: &str,
ca_cert: &Path,
server_name: &str,
insecure_skip_verify: bool,
) -> anyhow::Result<node_service::Client> {
let addr: SocketAddr = server
.parse()
.with_context(|| format!("server must be host:port, got {server}"))?;
let cert_bytes = std::fs::read(ca_cert).with_context(|| format!("read ca_cert {ca_cert:?}"))?;
let mut roots = RootCertStore::empty();
roots
.add(CertificateDer::from(cert_bytes))
.context("add root cert")?;
let mut tls = RustlsClientConfig::builder()
.with_root_certificates(roots)
.with_no_client_auth();
let mut tls = if insecure_skip_verify {
RustlsClientConfig::builder()
.dangerous()
.with_custom_certificate_verifier(Arc::new(InsecureServerCertVerifier))
.with_no_client_auth()
} else {
let cert_bytes =
std::fs::read(ca_cert).with_context(|| format!("read ca_cert {ca_cert:?}"))?;
let mut roots = RootCertStore::empty();
roots
.add(CertificateDer::from(cert_bytes))
.context("add root cert")?;
RustlsClientConfig::builder()
.with_root_certificates(roots)
.with_no_client_auth()
};
tls.alpn_protocols = vec![b"capnp".to_vec()];
let crypto = QuicClientConfig::try_from(tls)
@@ -76,7 +143,9 @@ pub async fn connect_node(
}
pub fn set_auth(auth: &mut auth::Builder<'_>) -> anyhow::Result<()> {
let guard = AUTH_CONTEXT.read().expect("AUTH_CONTEXT poisoned");
let guard = AUTH_CONTEXT
.read()
.map_err(|e| anyhow::anyhow!("AUTH_CONTEXT lock poisoned: {e}"))?;
let ctx = guard.as_ref().ok_or_else(|| {
anyhow::anyhow!(
"init_auth must be called before RPCs (use a bearer or session token for authenticated commands)"
@@ -257,7 +326,6 @@ pub async fn fetch_wait(
|| {
let client = client.clone();
let recipient_key = recipient_key.clone();
let timeout_ms = timeout_ms;
async move {
let mut req = client.fetch_wait_request();
{

6
crates/quicproquo-client/src/client/session.rs
View File

@@ -51,7 +51,7 @@ impl SessionState {
let hybrid_kp = state
.hybrid_key
.as_ref()
.map(|b| HybridKeypair::from_bytes(b))
.map(HybridKeypair::from_bytes)
.transpose()
.context("decode hybrid key")?;
@@ -109,7 +109,7 @@ impl SessionState {
// Use the first 16 bytes of the group_id as the ConversationId.
let conv_id = if group_id_bytes.len() >= 16 {
ConversationId::from_slice(&group_id_bytes[..16])
.unwrap_or_else(|| ConversationId([0; 16]))
.unwrap_or(ConversationId([0; 16]))
} else {
ConversationId::from_group_name(&hex::encode(&group_id_bytes))
};
@@ -188,7 +188,7 @@ impl SessionState {
let member = self.members.get(conv_id).context("no such conversation")?;
let blob = member
.group_ref()
.map(|g| bincode::serialize(g))
.map(bincode::serialize)
.transpose()
.context("serialize MLS group")?;

15
crates/quicproquo-client/src/lib.rs
View File

@@ -15,6 +15,7 @@
//! docs for details.
use std::sync::RwLock;
use std::sync::atomic::{AtomicBool, Ordering};
pub mod client;
@@ -26,11 +27,23 @@ pub use client::commands::{
};
pub use client::repl::run_repl;
pub use client::rpc::{connect_node, create_channel, enqueue, fetch_wait, resolve_user};
pub use client::rpc::{connect_node, connect_node_opt, create_channel, enqueue, fetch_wait, resolve_user};
// Global auth context — RwLock so the REPL can set it after OPAQUE login.
pub(crate) static AUTH_CONTEXT: RwLock<Option<ClientAuth>> = RwLock::new(None);
/// When `true`, [`connect_node`] skips TLS certificate verification.
/// Set via [`set_insecure_skip_verify`]; read by the RPC layer.
pub(crate) static INSECURE_SKIP_VERIFY: AtomicBool = AtomicBool::new(false);
/// Enable or disable insecure (no-verify) TLS mode globally.
///
/// **Development only.** When enabled, all outgoing connections skip certificate
/// verification, making them vulnerable to MITM attacks.
pub fn set_insecure_skip_verify(enabled: bool) {
INSECURE_SKIP_VERIFY.store(enabled, Ordering::Relaxed);
}
#[derive(Clone, Debug)]
pub struct ClientAuth {
pub(crate) version: u16,

20
crates/quicproquo-client/src/main.rs
View File

@@ -1,6 +1,6 @@
//! quicproquo CLI client.
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use anyhow::Context;
use clap::{Parser, Subcommand};
@@ -9,7 +9,7 @@ use quicproquo_client::{
cmd_chat, cmd_check_key, cmd_create_group, cmd_demo_group, cmd_export, cmd_export_verify,
cmd_fetch_key, cmd_health, cmd_invite, cmd_join, cmd_login, cmd_ping, cmd_recv, cmd_register,
cmd_register_state, cmd_refresh_keypackage, cmd_register_user, cmd_send, cmd_whoami,
init_auth, run_repl, ClientAuth,
init_auth, run_repl, set_insecure_skip_verify, ClientAuth,
};
#[cfg(feature = "tui")]
use quicproquo_client::client::tui::run_tui;
@@ -56,6 +56,15 @@ struct Args {
#[arg(long, global = true, env = "QPQ_STATE_PASSWORD")]
state_password: Option<String>,
/// DANGER: Skip TLS certificate verification. Development only.
/// Disables all certificate checks, making the connection vulnerable to MITM attacks.
#[arg(
long = "danger-accept-invalid-certs",
global = true,
env = "QPQ_DANGER_ACCEPT_INVALID_CERTS"
)]
danger_accept_invalid_certs: bool,
// ── Default-repl args (used when no subcommand is given) ─────────
/// State file path (identity + MLS state). Used when running the default REPL.
#[arg(long, default_value = "qpq-state.bin", env = "QPQ_STATE")]
@@ -393,7 +402,7 @@ enum Command {
/// `state` unchanged. This lets `qpq --username alice` automatically isolate
/// Alice's state without requiring a manual `--state` flag.
fn derive_state_path(state: PathBuf, username: Option<&str>) -> PathBuf {
if state == PathBuf::from("qpq-state.bin") {
if state == Path::new("qpq-state.bin") {
if let Some(uname) = username {
return PathBuf::from(format!("qpq-{uname}.bin"));
}
@@ -417,6 +426,11 @@ async fn main() -> anyhow::Result<()> {
let args = Args::parse();
if args.danger_accept_invalid_certs {
eprintln!("WARNING: TLS verification disabled — insecure mode");
set_insecure_skip_verify(true);
}
// For the REPL and TUI, defer init_auth so they can resolve their own token via OPAQUE.
// For all other subcommands, initialize auth immediately.
#[cfg(not(feature = "tui"))]