cdricms/vnidrop
1
1
Fork 0
mirror of https://github.com/sudosylabs/vnidrop.git synced 2026-08-05 10:29:58 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(security): harden provider ACL, tickets, and local secrets

Browse Source 
Close high-severity findings from the security review: default-deny blob
gets for unmapped hashes, map collection members for ACL, fail closed on
unknown access modes, require vnd1 tickets only, redact tickets from events,
exclude Android app-data backups, and tighten secret-file creation.
This commit is contained in:
Hammed Abass
2026-07-13 18:41:43 +02:00
parent 98375f479a
commit e7fb0331b5
27 changed files with 461 additions and 283 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
crates/vnidrop/CORE_FLOW.md
View File

@@ -12,24 +12,29 @@ bytes through Kotlin memory.
file into `iroh-blobs`, stores a collection, and returns a VniDrop ticket.
3. New VniDrop shares are `ApprovalRequired` by default. A copied ticket is not
enough to read bytes until the sender approves the receiver endpoint.
4. The sender observes receiver requests through `CoreEvent` entries with
4. The blob provider is **default-deny**: only hashes registered for an active
share (collection root **and** each member blob) may be served, and only when
access policy allows that remote endpoint. Unknown hashes are refused.
5. The sender observes receiver requests through `CoreEvent` entries with
`phase="approval"` and can query them with
`list_receiver_requests(transfer_id)`.
5. `respond_receiver_request(request_id, accepted, reason)` accepts or refuses a
6. `respond_receiver_request(request_id, accepted, reason)` accepts or refuses a
pending request. Accepted requests create a time-limited access session for
the receiver endpoint.
7. Ticket strings are capabilities. Share events emit hash/size metadata only —
never the full ticket payload.
## Receive
1. `receive(ticket, output_dir, receiver_name)` parses and validates the ticket.
2. VniDrop tickets first connect to the handshake ALPN
`/vnidrop/handshake/1` and send `RequestTransfer` metadata to the sender.
`/vnidrop/handshake/2` and send `RequestTransfer` metadata to the sender.
3. If approved, the receiver connects to the blobs ALPN, downloads the
collection, and streams files to `output_dir`.
4. If refused, expired, unknown, or cancelled, the receive transfer is marked
`failed` or `cancelled` and emits an error/lifecycle event.
5. Legacy raw `BlobTicket` values do not carry VniDrop metadata, so they bypass
the app approval handshake and use the underlying blob ticket directly.
5. Only `vnd1:` VniDrop tickets are accepted. Raw iroh `BlobTicket` strings are
rejected at parse time so receive always runs the approval handshake.
## Core States And Events
@@ -77,7 +82,7 @@ bytes through Kotlin memory.
## Blob Retention Policy
Stopping a share immediately removes its provider mapping and approval state,
so neither VniDrop nor legacy blob tickets can read it. Physical blob chunks are
so outstanding VniDrop tickets can no longer download content. Physical blob chunks are
not force-deleted at stop time because content-addressed chunks may be shared by
another active collection. They remain eligible for the blob store's garbage
collection. Restart reconciliation never restores a stopped share.

17
crates/vnidrop/src/access_policy.rs
View File

@@ -63,16 +63,13 @@ impl AccessPolicy {
transfer_id: u64,
endpoint_id: Option<&str>,
) -> AccessDecision {
match self
.modes
.read()
.await
.get(&transfer_id)
.cloned()
.unwrap_or(TransferAccessMode::Public)
{
TransferAccessMode::Public => AccessDecision::Allow,
TransferAccessMode::ApprovalRequired => {
// Unknown transfers fail closed. Never treat a missing mode as Public.
match self.modes.read().await.get(&transfer_id).cloned() {
None => AccessDecision::Deny {
reason: "unknown-transfer",
},
Some(TransferAccessMode::Public) => AccessDecision::Allow,
Some(TransferAccessMode::ApprovalRequired) => {
let Some(endpoint_id) = endpoint_id else {
return AccessDecision::Deny {
reason: "missing-endpoint-id",

4
crates/vnidrop/src/api.rs
View File

@@ -183,7 +183,6 @@ pub struct StoredTransfer {
pub struct ShareResult {
pub transfer_id: u64,
pub ticket: String,
pub blob_ticket: String,
pub hash: String,
pub transfer_name: String,
pub file_count: u64,
@@ -227,8 +226,7 @@ impl TransferMetadata {
#[derive(Debug, Clone, Serialize, Deserialize, uniffi::Record)]
pub struct TicketInspection {
pub kind: String,
pub blob_ticket: String,
pub metadata: Option<TransferMetadata>,
pub metadata: TransferMetadata,
}
#[derive(Debug, Clone, Serialize, Deserialize, uniffi::Record)]

3
crates/vnidrop/src/filesystem.rs
View File

@@ -27,6 +27,9 @@ const STALE_PART_AGE: Duration = Duration::from_secs(24 * 60 * 60);
pub(crate) struct TransferImport {
pub(crate) tag: TempTag,
pub(crate) root_hash: Hash,
/// Per-file content hashes in the collection. Provider ACL maps these too
/// so child blob gets are not fail-open when only the root is tracked.
pub(crate) member_hashes: Vec<Hash>,
pub(crate) total_size: u64,
pub(crate) file_count: u64,
pub(crate) default_name: String,

4
crates/vnidrop/src/logging.rs
View File

@@ -23,8 +23,10 @@ pub(crate) fn init_logging(app_data_dir: &Path) -> Result<()> {
fs::create_dir_all(&log_dir)?;
let writer = SizeRotatingWriter::new(log_dir, MAX_LOG_BYTES, MAX_LOG_FILES);
let (writer, guard) = tracing_appender::non_blocking(writer);
// Default to info so ticket-adjacent and endpoint noise is not retained at
// debug volume in app logs. Operators can raise with RUST_LOG.
let filter = EnvFilter::try_from_default_env()
.unwrap_or_else(|_| EnvFilter::new("vnidrop=debug,iroh=info,iroh_blobs=info,warn"));
.unwrap_or_else(|_| EnvFilter::new("vnidrop=info,iroh=info,iroh_blobs=info,warn"));
let subscriber = tracing_subscriber::registry()
.with(filter)

7
crates/vnidrop/src/runtime/facade.rs
View File

@@ -224,12 +224,7 @@ impl VnidropCore {
.context("failed to parse transfer ticket")
.map_err(VnidropError::ticket)?;
Ok(TicketInspection {
kind: if parsed.metadata.is_some() {
"vnidrop".to_string()
} else {
"legacy".to_string()
},
blob_ticket: parsed.blob_ticket.to_string(),
kind: "vnidrop".to_string(),
metadata: parsed.metadata,
})
}

10
crates/vnidrop/src/runtime/lifecycle.rs
View File

@@ -51,10 +51,7 @@ impl CoreInner {
.await?;
active_shares.remove(&transfer_id);
drop(active_shares);
self.hash_to_transfer
.lock()
.await
.retain(|_, id| *id != transfer_id);
self.unregister_transfer_hashes(transfer_id).await;
self.access_policy.remove_transfer(transfer_id).await;
self.emit_transfer(transfer_id, "send", "lifecycle", "share-stopped", json!({}));
return Ok(());
@@ -106,10 +103,7 @@ impl CoreInner {
}
self.active_shares.lock().await.remove(&transfer_id);
self.hash_to_transfer
.lock()
.await
.retain(|_, id| *id != transfer_id);
self.unregister_transfer_hashes(transfer_id).await;
self.access_policy.remove_transfer(transfer_id).await;
// Events are persisted asynchronously. Drain events emitted before this
// request so none can be written back after the transfer is deleted.

70
crates/vnidrop/src/runtime/mod.rs
View File

@@ -16,7 +16,7 @@ mod share;
pub use facade::VnidropCore;
use std::{
collections::HashMap,
collections::{HashMap, HashSet},
path::PathBuf,
str::FromStr,
sync::{atomic::AtomicBool, Arc},
@@ -68,7 +68,9 @@ pub(super) struct CoreInner {
// Restored shares have no in-memory tag, but remain tracked so they can be
// counted and explicitly revoked after a restart.
pub(super) active_shares: TokioMutex<HashMap<u64, Option<TempTag>>>,
pub(super) hash_to_transfer: TokioMutex<HashMap<String, u64>>,
/// Content hash → active share transfer ids (root and collection members).
/// Multiple transfers can share the same content-addressed hash.
pub(super) hash_to_transfer: TokioMutex<HashMap<String, HashSet<u64>>>,
pub(super) connection_endpoints: TokioMutex<HashMap<u64, String>>,
pub(super) provider_task: TokioMutex<Option<JoinHandle<()>>>,
pub(super) shutdown_started: AtomicBool,
@@ -130,18 +132,12 @@ impl CoreInner {
let access_policy = AccessPolicy::new();
// Restore share ownership and access mode before the router can serve
// any request. Unknown persisted modes fail closed in mode_from_storage.
let mut restored_hashes = HashMap::new();
// Register root + every collection member so child gets stay under ACL.
let mut restored_hashes: HashMap<String, HashSet<u64>> = HashMap::new();
let mut restored_active_shares = HashMap::new();
for share in repository.list_active_shares().await? {
let transfer_id = share.transfer_id;
let valid_root = match Hash::from_str(&share.content_hash) {
Ok(hash) => {
store.blobs().has(hash).await.unwrap_or(false)
&& Collection::load(hash, store.as_ref()).await.is_ok()
}
Err(_) => false,
};
if !valid_root {
let Ok(root_hash) = Hash::from_str(&share.content_hash) else {
repository
.transition_transfer_status(
transfer_id,
@@ -157,8 +153,39 @@ impl CoreInner {
json!({ "content_hash": share.content_hash }),
);
continue;
};
let collection = if store.blobs().has(root_hash).await.unwrap_or(false) {
Collection::load(root_hash, store.as_ref()).await.ok()
} else {
None
};
let Some(collection) = collection else {
repository
.transition_transfer_status(
transfer_id,
TransferStatus::Sharing,
TransferStatus::Failed,
)
.await?;
event_hub.emit_transfer(
transfer_id,
TransferDirection::Send.as_str(),
"recovery",
"share-root-missing-or-corrupt",
json!({ "content_hash": share.content_hash }),
);
continue;
};
restored_hashes
.entry(root_hash.to_string())
.or_default()
.insert(transfer_id);
for (_, member_hash) in collection.iter() {
restored_hashes
.entry(member_hash.to_string())
.or_default()
.insert(transfer_id);
}
restored_hashes.insert(share.content_hash, transfer_id);
restored_active_shares.insert(transfer_id, None);
access_policy
.set_mode(transfer_id, mode_from_storage(&share.access_mode))
@@ -224,6 +251,25 @@ impl CoreInner {
.emit_transfer(transfer_id, direction, phase, kind, data);
}
pub(super) async fn register_share_hashes(
&self,
transfer_id: u64,
hashes: impl IntoIterator<Item = Hash>,
) {
let mut map = self.hash_to_transfer.lock().await;
for hash in hashes {
map.entry(hash.to_string()).or_default().insert(transfer_id);
}
}
pub(super) async fn unregister_transfer_hashes(&self, transfer_id: u64) {
let mut map = self.hash_to_transfer.lock().await;
map.retain(|_, transfers| {
transfers.remove(&transfer_id);
!transfers.is_empty()
});
}
pub(super) async fn list_events(&self, transfer_id: Option<u64>) -> Result<Vec<CoreEvent>> {
self.event_hub.flush().await;
self.repository

211
crates/vnidrop/src/runtime/provider.rs
View File

@@ -71,41 +71,37 @@ impl CoreInner {
);
}
ProviderMessage::GetRequestReceived(message) => {
let transfer_id = self.transfer_for_hash(message.inner.request.hash).await;
if let Some(transfer_id) = transfer_id {
let decision = self
.access_decision(transfer_id, message.inner.connection_id)
.await;
if let AccessDecision::Deny { reason } = decision {
self.emit_transfer(
match self
.authorize_hash(message.inner.request.hash, message.inner.connection_id)
.await
{
Ok(transfer_id) => {
self.track_request_updates(
transfer_id,
"send",
"access",
"request-denied",
json!({
"connection_id": message.inner.connection_id,
"request_id": message.inner.request_id,
"reason": reason,
}),
message.inner.connection_id,
message.inner.request_id,
message.rx,
)
.await;
let _ = message.tx.send(Ok(())).await;
}
Err(reason) => {
self.emit_denied_request(
message.inner.connection_id,
message.inner.request_id,
reason,
);
let _ = message
.tx
.send(Err(iroh_blobs::provider::events::AbortReason::Permission))
.await;
return;
}
self.track_request_updates(
transfer_id,
message.inner.connection_id,
message.inner.request_id,
message.rx,
)
.await;
}
let _ = message.tx.send(Ok(())).await;
}
ProviderMessage::GetRequestReceivedNotify(message) => {
if let Some(transfer_id) = self.transfer_for_hash(message.inner.request.hash).await
if let Ok(transfer_id) = self
.authorize_hash(message.inner.request.hash, message.inner.connection_id)
.await
{
self.track_request_updates(
transfer_id,
@@ -117,44 +113,36 @@ impl CoreInner {
}
}
ProviderMessage::GetManyRequestReceived(message) => {
let transfer_id = self
.transfer_for_any_hash(&message.inner.request.hashes)
.await;
if let Some(transfer_id) = transfer_id {
let decision = self
.access_decision(transfer_id, message.inner.connection_id)
.await;
if let AccessDecision::Deny { reason } = decision {
self.emit_transfer(
match self
.authorize_hashes(&message.inner.request.hashes, message.inner.connection_id)
.await
{
Ok(transfer_id) => {
self.track_request_updates(
transfer_id,
"send",
"access",
"request-denied",
json!({
"connection_id": message.inner.connection_id,
"request_id": message.inner.request_id,
"reason": reason,
}),
message.inner.connection_id,
message.inner.request_id,
message.rx,
)
.await;
let _ = message.tx.send(Ok(())).await;
}
Err(reason) => {
self.emit_denied_request(
message.inner.connection_id,
message.inner.request_id,
reason,
);
let _ = message
.tx
.send(Err(iroh_blobs::provider::events::AbortReason::Permission))
.await;
return;
}
self.track_request_updates(
transfer_id,
message.inner.connection_id,
message.inner.request_id,
message.rx,
)
.await;
}
let _ = message.tx.send(Ok(())).await;
}
ProviderMessage::GetManyRequestReceivedNotify(message) => {
if let Some(transfer_id) = self
.transfer_for_any_hash(&message.inner.request.hashes)
if let Ok(transfer_id) = self
.authorize_hashes(&message.inner.request.hashes, message.inner.connection_id)
.await
{
self.track_request_updates(
@@ -167,15 +155,38 @@ impl CoreInner {
}
}
ProviderMessage::ObserveRequestReceived(message) => {
self.emit_endpoint(
"provider",
"observe-request",
json!({
"connection_id": message.inner.connection_id,
"request_id": message.inner.request_id,
}),
);
let _ = message.tx.send(Ok(())).await;
// Observe can leak presence of content; use the same ACL as get.
match self
.authorize_hash(message.inner.request.hash, message.inner.connection_id)
.await
{
Ok(_) => {
self.emit_endpoint(
"provider",
"observe-request",
json!({
"connection_id": message.inner.connection_id,
"request_id": message.inner.request_id,
}),
);
let _ = message.tx.send(Ok(())).await;
}
Err(reason) => {
self.emit_endpoint(
"provider",
"observe-denied",
json!({
"connection_id": message.inner.connection_id,
"request_id": message.inner.request_id,
"reason": reason,
}),
);
let _ = message
.tx
.send(Err(iroh_blobs::provider::events::AbortReason::Permission))
.await;
}
}
}
ProviderMessage::ObserveRequestReceivedNotify(message) => {
self.emit_endpoint(
@@ -209,35 +220,81 @@ impl CoreInner {
}
}
pub(super) async fn transfer_for_hash(&self, hash: Hash) -> Option<u64> {
fn emit_denied_request(&self, connection_id: u64, request_id: u64, reason: &'static str) {
self.emit_endpoint(
"provider",
"request-denied",
json!({
"connection_id": connection_id,
"request_id": request_id,
"reason": reason,
}),
);
}
/// Default-deny: hash must belong to an active share the peer may read.
pub(super) async fn authorize_hash(
&self,
hash: Hash,
connection_id: u64,
) -> Result<u64, &'static str> {
let transfer_ids = self.transfer_ids_for_hash(hash).await;
if transfer_ids.is_empty() {
return Err("unknown-hash");
}
self.allow_any_transfer(&transfer_ids, connection_id).await
}
/// Every hash in a multi-get must be authorized; progress is attributed to
/// the first allowing transfer id.
pub(super) async fn authorize_hashes(
&self,
hashes: &[Hash],
connection_id: u64,
) -> Result<u64, &'static str> {
if hashes.is_empty() {
return Err("empty-request");
}
let mut attributed = None;
for hash in hashes {
let transfer_id = self.authorize_hash(*hash, connection_id).await?;
attributed.get_or_insert(transfer_id);
}
attributed.ok_or("empty-request")
}
pub(super) async fn transfer_ids_for_hash(&self, hash: Hash) -> Vec<u64> {
self.hash_to_transfer
.lock()
.await
.get(&hash.to_string())
.copied()
.map(|set| set.iter().copied().collect())
.unwrap_or_default()
}
pub(super) async fn transfer_for_any_hash(&self, hashes: &[Hash]) -> Option<u64> {
let map = self.hash_to_transfer.lock().await;
hashes
.iter()
.find_map(|hash| map.get(&hash.to_string()).copied())
}
pub(super) async fn access_decision(
async fn allow_any_transfer(
&self,
transfer_id: u64,
transfer_ids: &[u64],
connection_id: u64,
) -> AccessDecision {
) -> Result<u64, &'static str> {
let endpoint_id = self
.connection_endpoints
.lock()
.await
.get(&connection_id)
.cloned();
self.access_policy
.decide(transfer_id, endpoint_id.as_deref())
.await
let mut last_reason = "approval-required";
for transfer_id in transfer_ids {
match self
.access_policy
.decide(*transfer_id, endpoint_id.as_deref())
.await
{
AccessDecision::Allow => return Ok(*transfer_id),
AccessDecision::Deny { reason } => last_reason = reason,
}
}
Err(last_reason)
}
pub(super) async fn track_request_updates(

99
crates/vnidrop/src/runtime/receive.rs
View File

@@ -26,7 +26,6 @@ use crate::{
repository::TransferUpsert,
ticket::{parse_transfer_ticket_with_limits, ParsedTransferTicket},
transfer_state::{TransferDirection, TransferStatus},
util::unique_transfer_id,
};
pub(super) enum ReceiveTarget {
@@ -135,11 +134,7 @@ impl CoreInner {
return Err(error);
}
};
let transfer_id = parsed
.metadata
.as_ref()
.map(|metadata| metadata.transfer_id)
.unwrap_or_else(unique_transfer_id);
let transfer_id = parsed.metadata.transfer_id;
self.persist_receive_start(transfer_id, &parsed, receiver_name.as_deref())
.await?;
// Cancellation is cooperative: it stops our receive future and marks
@@ -202,19 +197,15 @@ impl CoreInner {
let sender_addr = parsed.blob_ticket.addr().clone();
self.emit_transfer(transfer_id, "receive", "network", "connecting", json!({}));
let delivery_receipt = if let Some(metadata) = &parsed.metadata {
Some(
self.request_transfer_approval(
transfer_id,
sender_addr.clone(),
metadata,
receiver_name.as_deref(),
)
.await?,
// Every VniDrop ticket carries metadata and must complete the handshake.
let delivery_receipt = self
.request_transfer_approval(
transfer_id,
sender_addr.clone(),
&parsed.metadata,
receiver_name.as_deref(),
)
} else {
None
};
.await?;
let connection = self
.endpoint
.connect(sender_addr.clone(), iroh_blobs::ALPN)
@@ -280,32 +271,30 @@ impl CoreInner {
)
.await?;
self.emit_transfer(transfer_id, "receive", "lifecycle", "done", json!({}));
if let Some(receipt) = delivery_receipt {
let sender_transfer_id = receipt.transfer_id;
let client = HandshakeService::client(self.endpoint.clone(), sender_addr);
match client.report_delivery(receipt).await {
Ok(DeliveryReceiptResponse::Recorded) => self.emit_transfer(
transfer_id,
"receive",
"delivery",
"receipt-recorded",
json!({ "sender_transfer_id": sender_transfer_id }),
),
Ok(DeliveryReceiptResponse::Rejected { reason }) => self.emit_transfer(
transfer_id,
"receive",
"delivery",
"receipt-rejected",
json!({ "reason": reason }),
),
Err(error) => self.emit_transfer(
transfer_id,
"receive",
"delivery",
"receipt-failed",
json!({ "reason": error.to_string() }),
),
}
let sender_transfer_id = delivery_receipt.transfer_id;
let client = HandshakeService::client(self.endpoint.clone(), sender_addr);
match client.report_delivery(delivery_receipt).await {
Ok(DeliveryReceiptResponse::Recorded) => self.emit_transfer(
transfer_id,
"receive",
"delivery",
"receipt-recorded",
json!({ "sender_transfer_id": sender_transfer_id }),
),
Ok(DeliveryReceiptResponse::Rejected { reason }) => self.emit_transfer(
transfer_id,
"receive",
"delivery",
"receipt-rejected",
json!({ "reason": reason }),
),
Err(error) => self.emit_transfer(
transfer_id,
"receive",
"delivery",
"receipt-failed",
json!({ "reason": error.to_string() }),
),
}
Ok(())
}
@@ -323,25 +312,11 @@ impl CoreInner {
peer_id: Some(&peer_id),
direction: TransferDirection::Receive,
status: TransferStatus::Receiving,
transfer_name: parsed
.metadata
.as_ref()
.map(|metadata| metadata.transfer_name.as_str()),
content_hash: parsed
.metadata
.as_ref()
.map(|metadata| metadata.content_hash.as_str()),
transfer_name: Some(parsed.metadata.transfer_name.as_str()),
content_hash: Some(parsed.metadata.content_hash.as_str()),
ticket: None,
file_count: parsed
.metadata
.as_ref()
.map(|metadata| metadata.file_count)
.unwrap_or_default(),
total_size: parsed
.metadata
.as_ref()
.map(|metadata| metadata.total_size)
.unwrap_or_default(),
file_count: parsed.metadata.file_count,
total_size: parsed.metadata.total_size,
access_mode: mode_to_storage(&TransferAccessMode::ApprovalRequired),
})
.await?;

18
crates/vnidrop/src/runtime/share.rs
View File

@@ -138,7 +138,7 @@ impl CoreInner {
import.file_count,
import.total_size,
);
let ticket = VnidropTicket::new(blob_ticket.clone(), ticket_metadata)
let ticket = VnidropTicket::new(blob_ticket, ticket_metadata)
.encode()
.context("failed to encode VniDrop transfer ticket")?;
let content_hash = import.root_hash.to_string();
@@ -160,10 +160,13 @@ impl CoreInner {
access_mode: mode_to_storage(&access_mode),
})
.await?;
self.hash_to_transfer
.lock()
.await
.insert(content_hash, metadata.transfer_id);
// Map root + every collection member so provider ACL cannot fail-open
// on child blob hashes that are not the collection root.
self.register_share_hashes(
metadata.transfer_id,
std::iter::once(import.root_hash).chain(import.member_hashes.iter().copied()),
)
.await;
self.access_policy
.set_mode(metadata.transfer_id, access_mode)
.await;
@@ -172,13 +175,13 @@ impl CoreInner {
.await
.insert(metadata.transfer_id, Some(import.tag));
// Tickets are capabilities: never persist the full string in events.
self.emit_transfer(
metadata.transfer_id,
"send",
"ticket",
"created",
json!({
"ticket": ticket,
"hash": import.root_hash.to_string(),
"total_size": import.total_size,
"file_count": import.file_count,
@@ -188,7 +191,6 @@ impl CoreInner {
Ok(ShareResult {
transfer_id: metadata.transfer_id,
ticket,
blob_ticket: blob_ticket.to_string(),
hash: import.root_hash.to_string(),
transfer_name,
file_count: import.file_count,
@@ -242,6 +244,7 @@ impl CoreInner {
.into_iter()
.map(|(name, tag, _)| ((name, tag.hash()), tag))
.unzip::<_, _, Collection, Vec<_>>();
let member_hashes = collection.iter().map(|(_, hash)| *hash).collect::<Vec<_>>();
let collection_tag = collection.clone().store(&self.store).await?;
let root_hash = collection_tag.hash();
let file_count = tags.len() as u64;
@@ -258,6 +261,7 @@ impl CoreInner {
Ok(TransferImport {
tag: collection_tag,
root_hash,
member_hashes,
total_size,
file_count,
default_name,

36
crates/vnidrop/src/secret.rs
View File

@@ -1,7 +1,12 @@
use std::{io, path::Path, str::FromStr};
use std::{
fs::OpenOptions,
io::{self, Write},
path::Path,
str::FromStr,
};
#[cfg(unix)]
use std::os::unix::fs::PermissionsExt;
use std::os::unix::fs::{OpenOptionsExt, PermissionsExt};
use anyhow::{Context, Result};
use data_encoding::HEXLOWER;
@@ -26,7 +31,9 @@ pub(crate) async fn load_or_create_secret(app_data_dir: &Path) -> Result<SecretK
}
Err(error) if error.kind() == io::ErrorKind::NotFound => {
let secret = SecretKey::generate();
tokio::fs::write(&path, HEXLOWER.encode(&secret.to_bytes())).await?;
let encoded = HEXLOWER.encode(&secret.to_bytes());
// Create with owner-only mode on Unix so the key is never briefly 0644.
write_secret_file(&path, encoded.as_bytes()).await?;
restrict_permissions(&path).await?;
Ok(secret)
}
@@ -34,8 +41,31 @@ pub(crate) async fn load_or_create_secret(app_data_dir: &Path) -> Result<SecretK
}
}
async fn write_secret_file(path: &Path, bytes: &[u8]) -> Result<()> {
let path = path.to_path_buf();
let bytes = bytes.to_vec();
tokio::task::spawn_blocking(move || {
let mut options = OpenOptions::new();
options.write(true).create_new(true);
#[cfg(unix)]
options.mode(0o600);
let mut file = options
.open(&path)
.with_context(|| format!("failed to create {}", path.display()))?;
file.write_all(&bytes)
.with_context(|| format!("failed to write {}", path.display()))?;
file.sync_all()
.with_context(|| format!("failed to sync {}", path.display()))?;
Ok::<(), anyhow::Error>(())
})
.await?
}
async fn restrict_permissions(path: &Path) -> Result<()> {
#[cfg(unix)]
tokio::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)).await?;
// Windows: file lives under the user profile app-data dir with default ACLs
// limited to the current user. No portable owner-only API in std.
let _ = path;
Ok(())
}

28
crates/vnidrop/src/tests/access_policy.rs
View File

@@ -4,6 +4,23 @@ use crate::{
TransferAccessMode,
};
#[tokio::test]
async fn unknown_transfer_fails_closed() {
let policy = AccessPolicy::new();
assert_eq!(
policy.decide(1, Some("node-a")).await,
AccessDecision::Deny {
reason: "unknown-transfer"
}
);
assert_eq!(
policy.decide(1, None).await,
AccessDecision::Deny {
reason: "unknown-transfer"
}
);
}
#[tokio::test]
async fn requires_approved_endpoint_when_locked() {
let policy = AccessPolicy::new();
@@ -54,3 +71,14 @@ async fn rejects_expired_approval_sessions() {
}
);
}
#[tokio::test]
async fn public_mode_allows_without_session() {
let policy = AccessPolicy::new();
policy.set_mode(7, TransferAccessMode::Public).await;
assert_eq!(
policy.decide(7, Some("node-a")).await,
AccessDecision::Allow
);
assert_eq!(policy.decide(7, None).await, AccessDecision::Allow);
}

15
crates/vnidrop/src/tests/ticket.rs
View File

@@ -31,10 +31,7 @@ fn metadata_ticket_round_trips() {
let parsed = parse_transfer_ticket(&encoded).unwrap();
assert_eq!(parsed.blob_ticket.hash(), blob_ticket.hash());
assert_eq!(
parsed.metadata.unwrap().transfer_name,
metadata.transfer_name
);
assert_eq!(parsed.metadata.transfer_name, metadata.transfer_name);
}
#[test]
@@ -60,6 +57,16 @@ fn invalid_ticket_is_rejected() {
assert!(parse_transfer_ticket("not-a-ticket").is_err());
}
#[test]
fn rejects_raw_blob_tickets() {
let raw = blob_ticket(3).to_string();
let error = parse_transfer_ticket(&raw).unwrap_err().to_string();
assert!(
error.contains("not a VniDrop ticket"),
"raw BlobTicket must not be accepted as a transfer invitation: {error}"
);
}
#[test]
fn rejects_unsupported_versions_and_mismatched_hashes() {
let blob_ticket = blob_ticket(5);

67
crates/vnidrop/src/ticket.rs
View File

@@ -49,7 +49,7 @@ impl VnidropTicket {
#[derive(Debug, Clone)]
pub(crate) struct ParsedTransferTicket {
pub(crate) blob_ticket: BlobTicket,
pub(crate) metadata: Option<TransferMetadata>,
pub(crate) metadata: TransferMetadata,
}
#[cfg(test)]
@@ -69,49 +69,44 @@ pub(crate) fn parse_transfer_ticket_with_limits(
);
}
let normalized = normalize_ticket_input(value);
if normalized.starts_with(VNIDROP_TICKET_PREFIX) {
let ticket = VnidropTicket::decode(&normalized)?;
if ticket.version != VNIDROP_TICKET_VERSION {
anyhow::bail!("unsupported VniDrop ticket version {}", ticket.version);
}
if ticket.metadata.version != VNIDROP_TICKET_VERSION {
anyhow::bail!(
"unsupported VniDrop metadata version {}",
ticket.metadata.version
);
}
if ticket.metadata.transfer_id == 0 {
anyhow::bail!("VniDrop ticket metadata is missing a valid transfer id");
}
if ticket.metadata.transfer_name.trim().is_empty() {
anyhow::bail!("VniDrop ticket metadata is missing a transfer name");
}
limits.validate_metadata_text(
"transfer name",
Some(ticket.metadata.transfer_name.as_str()),
)?;
limits.validate_metadata_text("sender name", ticket.metadata.sender_name.as_deref())?;
let blob_ticket = BlobTicket::from_str(&ticket.blob_ticket)
.context("invalid BlobTicket inside VniDrop ticket")?;
if ticket.metadata.content_hash != blob_ticket.hash().to_string() {
anyhow::bail!("VniDrop ticket metadata hash does not match BlobTicket hash");
}
return Ok(ParsedTransferTicket {
blob_ticket,
metadata: Some(ticket.metadata),
});
if !normalized.starts_with(VNIDROP_TICKET_PREFIX) {
anyhow::bail!("not a VniDrop ticket; expected a vnd1: invitation");
}
let ticket = VnidropTicket::decode(&normalized)?;
if ticket.version != VNIDROP_TICKET_VERSION {
anyhow::bail!("unsupported VniDrop ticket version {}", ticket.version);
}
if ticket.metadata.version != VNIDROP_TICKET_VERSION {
anyhow::bail!(
"unsupported VniDrop metadata version {}",
ticket.metadata.version
);
}
if ticket.metadata.transfer_id == 0 {
anyhow::bail!("VniDrop ticket metadata is missing a valid transfer id");
}
if ticket.metadata.transfer_name.trim().is_empty() {
anyhow::bail!("VniDrop ticket metadata is missing a transfer name");
}
limits.validate_metadata_text(
"transfer name",
Some(ticket.metadata.transfer_name.as_str()),
)?;
limits.validate_metadata_text("sender name", ticket.metadata.sender_name.as_deref())?;
let blob_ticket = BlobTicket::from_str(&ticket.blob_ticket)
.context("invalid BlobTicket inside VniDrop ticket")?;
if ticket.metadata.content_hash != blob_ticket.hash().to_string() {
anyhow::bail!("VniDrop ticket metadata hash does not match BlobTicket hash");
}
let blob_ticket = BlobTicket::from_str(&normalized).context("invalid BlobTicket")?;
Ok(ParsedTransferTicket {
blob_ticket,
metadata: None,
metadata: ticket.metadata,
})
}
fn normalize_ticket_input(value: &str) -> String {
// Tickets are commonly copied from text views or chat apps that insert line
// breaks. Strip whitespace only; other corrupt characters should still be
// rejected by the base64 or BlobTicket decoders.
// rejected by the base64 decoder.
value.chars().filter(|char| !char.is_whitespace()).collect()
}

4
crates/vnidrop/src/util.rs
View File

@@ -11,7 +11,3 @@ pub(crate) fn now_ms() -> i64 {
.map(|duration| duration.as_millis() as i64)
.unwrap_or_default()
}
pub(crate) fn unique_transfer_id() -> u64 {
now_ms() as u64
}

50
crates/vnidrop/tests/lifecycle.rs
View File

@@ -100,7 +100,7 @@ fn persisted_share_is_recovered_and_can_be_stopped_after_restart() {
}
#[test]
fn stopped_share_rejects_direct_legacy_blob_ticket() {
fn stopped_share_rejects_receive() {
let source_dir = tempfile::tempdir().unwrap();
let output_dir = tempfile::tempdir().unwrap();
let source_path = source_dir.path().join("revoked.txt");
@@ -111,7 +111,7 @@ fn stopped_share_rejects_direct_legacy_blob_ticket() {
sender.core.cancel_transfer(share.transfer_id).unwrap();
let result = receiver.core.receive(
share.blob_ticket,
share.ticket,
output_dir.path().to_string_lossy().to_string(),
Some("receiver".to_string()),
);
@@ -120,6 +120,52 @@ fn stopped_share_rejects_direct_legacy_blob_ticket() {
assert!(!output_dir.path().join("revoked.txt").exists());
}
#[test]
fn ticket_created_event_does_not_include_full_ticket() {
let source_dir = tempfile::tempdir().unwrap();
let source_path = source_dir.path().join("secret.txt");
std::fs::write(&source_path, b"capability material").unwrap();
let sender = TestNode::new();
let share = share_path(&sender.core, &source_path, 40, "secret.txt", false);
let ticket_events: Vec<_> = sender
.sink
.events()
.into_iter()
.filter(|event| event.phase == "ticket" && event.kind == "created")
.collect();
assert_eq!(ticket_events.len(), 1);
let data = &ticket_events[0].data_json;
assert!(
!data.contains(&share.ticket),
"events must not retain the full vnd1 ticket capability"
);
assert!(
!data.contains("vnd1:"),
"events must not embed ticket prefixes"
);
assert!(
data.contains(&share.hash),
"events should still record the content hash for diagnostics"
);
}
#[test]
fn receive_rejects_non_vnidrop_ticket_input() {
let output_dir = tempfile::tempdir().unwrap();
let receiver = TestNode::new();
let result = receiver.core.receive(
"blobaaabcdefghijklmnopqrstuvwxyz0123456789".to_string(),
output_dir.path().to_string_lossy().to_string(),
Some("receiver".to_string()),
);
assert!(
result.is_err(),
"non-vnd1 tickets must be rejected before network work"
);
}
#[test]
fn failed_import_leaves_durable_failed_transfer() {
let source_dir = tempfile::tempdir().unwrap();