Merge pull request #13 from vnidrop/fix/security-hardening

fix(security): harden transfer ACL, tickets, and local secret handling
This commit is contained in:
Hammed Abass
2026-07-13 19:06:49 +02:00
committed by GitHub
37 changed files with 645 additions and 297 deletions

11
.cargo/audit.toml Normal file
View File

@@ -0,0 +1,11 @@
# Known transitive advisories we cannot fully clear without upstream iroh bumps.
# cargo audit in CI fails on new unlisted vulnerabilities.
[advisories]
ignore = [
# rsa: Marvin timing side-channel; no fixed release; pulled by iroh stack.
"RUSTSEC-2023-0071",
# quick-xml / crossbeam-epoch: transitive; track via iroh/dependency updates.
"RUSTSEC-2026-0194",
"RUSTSEC-2026-0195",
"RUSTSEC-2026-0204",
]

View File

@@ -43,3 +43,8 @@ jobs:
env: env:
RUSTDOCFLAGS: -D warnings RUSTDOCFLAGS: -D warnings
run: cargo doc --workspace --no-deps run: cargo doc --workspace --no-deps
- name: Install cargo-audit
run: cargo install cargo-audit --locked
- name: Audit Rust dependencies
# Ignores are listed in .cargo/audit.toml for known transitive issues.
run: cargo audit

View File

@@ -13,6 +13,8 @@
<application <application
android:allowBackup="true" android:allowBackup="true"
android:fullBackupContent="@xml/backup_rules"
android:dataExtractionRules="@xml/data_extraction_rules"
android:icon="@mipmap/ic_launcher" android:icon="@mipmap/ic_launcher"
android:label="@string/app_name" android:label="@string/app_name"
android:roundIcon="@mipmap/ic_launcher_round" android:roundIcon="@mipmap/ic_launcher_round"

View File

@@ -0,0 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- Exclude core identity, tickets, history, and blob store from Auto Backup. -->
<full-backup-content>
<exclude domain="file" path="vnidrop"/>
</full-backup-content>

View File

@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- Exclude core identity, tickets, history, and blob store from cloud backup
and device-to-device transfer. -->
<data-extraction-rules>
<cloud-backup>
<exclude domain="file" path="vnidrop"/>
</cloud-backup>
<device-transfer>
<exclude domain="file" path="vnidrop"/>
</device-transfer>
</data-extraction-rules>

View File

@@ -12,24 +12,29 @@ bytes through Kotlin memory.
file into `iroh-blobs`, stores a collection, and returns a VniDrop ticket. 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 3. New VniDrop shares are `ApprovalRequired` by default. A copied ticket is not
enough to read bytes until the sender approves the receiver endpoint. 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 `phase="approval"` and can query them with
`list_receiver_requests(transfer_id)`. `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 pending request. Accepted requests create a time-limited access session for
the receiver endpoint. the receiver endpoint.
7. Ticket strings are capabilities. Share events emit hash/size metadata only —
never the full ticket payload.
## Receive ## Receive
1. `receive(ticket, output_dir, receiver_name)` parses and validates the ticket. 1. `receive(ticket, output_dir, receiver_name)` parses and validates the ticket.
2. VniDrop tickets first connect to the handshake ALPN 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 3. If approved, the receiver connects to the blobs ALPN, downloads the
collection, and streams files to `output_dir`. collection, and streams files to `output_dir`.
4. If refused, expired, unknown, or cancelled, the receive transfer is marked 4. If refused, expired, unknown, or cancelled, the receive transfer is marked
`failed` or `cancelled` and emits an error/lifecycle event. `failed` or `cancelled` and emits an error/lifecycle event.
5. Legacy raw `BlobTicket` values do not carry VniDrop metadata, so they bypass 5. Only `vnd1:` VniDrop tickets are accepted. Raw iroh `BlobTicket` strings are
the app approval handshake and use the underlying blob ticket directly. rejected at parse time so receive always runs the approval handshake.
## Core States And Events ## Core States And Events
@@ -77,7 +82,7 @@ bytes through Kotlin memory.
## Blob Retention Policy ## Blob Retention Policy
Stopping a share immediately removes its provider mapping and approval state, 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 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 another active collection. They remain eligible for the blob store's garbage
collection. Restart reconciliation never restores a stopped share. collection. Restart reconciliation never restores a stopped share.
@@ -86,7 +91,12 @@ collection. Restart reconciliation never restores a stopped share.
`CoreLimits` controls source count, collection files and bytes, path and ticket `CoreLimits` controls source count, collection files and bytes, path and ticket
sizes, metadata, retained events, pending approvals, concurrent transfers, and sizes, metadata, retained events, pending approvals, concurrent transfers, and
the event persistence queue. `initialize` uses conservative defaults; the event persistence queue. `initialize` uses conservative defaults (including
bounded ticket size, pending approvals, and total collection bytes);
`initialize_with_limits` supports stricter deployments and tests. Cheap limits `initialize_with_limits` supports stricter deployments and tests. Cheap limits
are checked before durable or network work, while remote collection limits are are checked before durable or network work, while remote collection limits are
checked before downloading file content. checked before downloading file content.
Manual `approve_endpoint_for_transfer` only applies to active shares, requires a
non-empty endpoint id, and creates a time-limited session (same TTL as handshake
approval), never a permanent grant.

View File

@@ -5,6 +5,9 @@ use tokio::sync::RwLock;
use crate::api::TransferAccessMode; use crate::api::TransferAccessMode;
use crate::util::now_ms; use crate::util::now_ms;
/// Default lifetime for endpoint approval sessions (matches handshake approval TTL).
pub(crate) const APPROVAL_SESSION_TTL_MS: i64 = 10 * 60 * 1000;
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) enum AccessDecision { pub(crate) enum AccessDecision {
Allow, Allow,
@@ -42,7 +45,12 @@ impl AccessPolicy {
} }
pub(crate) async fn approve_endpoint(&self, transfer_id: u64, endpoint_id: String) { pub(crate) async fn approve_endpoint(&self, transfer_id: u64, endpoint_id: String) {
self.approve_endpoint_until(transfer_id, endpoint_id, None) // Never grant permanent sessions from the public API: always expire.
self.approve_endpoint_until(
transfer_id,
endpoint_id,
Some(now_ms() + APPROVAL_SESSION_TTL_MS),
)
.await; .await;
} }
@@ -63,16 +71,13 @@ impl AccessPolicy {
transfer_id: u64, transfer_id: u64,
endpoint_id: Option<&str>, endpoint_id: Option<&str>,
) -> AccessDecision { ) -> AccessDecision {
match self // Unknown transfers fail closed. Never treat a missing mode as Public.
.modes match self.modes.read().await.get(&transfer_id).cloned() {
.read() None => AccessDecision::Deny {
.await reason: "unknown-transfer",
.get(&transfer_id) },
.cloned() Some(TransferAccessMode::Public) => AccessDecision::Allow,
.unwrap_or(TransferAccessMode::Public) Some(TransferAccessMode::ApprovalRequired) => {
{
TransferAccessMode::Public => AccessDecision::Allow,
TransferAccessMode::ApprovalRequired => {
let Some(endpoint_id) = endpoint_id else { let Some(endpoint_id) = endpoint_id else {
return AccessDecision::Deny { return AccessDecision::Deny {
reason: "missing-endpoint-id", reason: "missing-endpoint-id",

View File

@@ -23,12 +23,15 @@ impl Default for CoreLimits {
Self { Self {
max_sources: 128, max_sources: 128,
max_collection_files: 10_000, max_collection_files: 10_000,
max_total_bytes: 1024 * 1024 * 1024 * 1024, // Cap extreme disk fill while still allowing multi-GB folders.
max_total_bytes: 256 * 1024 * 1024 * 1024,
max_path_bytes: 4_096, max_path_bytes: 4_096,
max_ticket_bytes: 1024 * 1024, // vnd1 tickets are small JSON+base64; 256 KiB is a generous ceiling.
max_ticket_bytes: 256 * 1024,
max_metadata_bytes: 16 * 1024, max_metadata_bytes: 16 * 1024,
max_events: 500, max_events: 500,
max_pending_approvals: 1_024, // Bound handshake spam / notification pressure on the sender.
max_pending_approvals: 64,
max_concurrent_transfers: 8, max_concurrent_transfers: 8,
event_queue_capacity: 1_024, event_queue_capacity: 1_024,
} }
@@ -183,7 +186,6 @@ pub struct StoredTransfer {
pub struct ShareResult { pub struct ShareResult {
pub transfer_id: u64, pub transfer_id: u64,
pub ticket: String, pub ticket: String,
pub blob_ticket: String,
pub hash: String, pub hash: String,
pub transfer_name: String, pub transfer_name: String,
pub file_count: u64, pub file_count: u64,
@@ -227,8 +229,7 @@ impl TransferMetadata {
#[derive(Debug, Clone, Serialize, Deserialize, uniffi::Record)] #[derive(Debug, Clone, Serialize, Deserialize, uniffi::Record)]
pub struct TicketInspection { pub struct TicketInspection {
pub kind: String, pub kind: String,
pub blob_ticket: String, pub metadata: TransferMetadata,
pub metadata: Option<TransferMetadata>,
} }
#[derive(Debug, Clone, Serialize, Deserialize, uniffi::Record)] #[derive(Debug, Clone, Serialize, Deserialize, uniffi::Record)]

View File

@@ -6,7 +6,7 @@ use tokio::sync::{oneshot, Mutex};
use uuid::Uuid; use uuid::Uuid;
use crate::{ use crate::{
access_policy::AccessPolicy, access_policy::{AccessPolicy, APPROVAL_SESSION_TTL_MS},
event_hub::EventHub, event_hub::EventHub,
handshake::{DeliveryReceipt, DeliveryReceiptResponse, HandshakeResponse, RequestTransfer}, handshake::{DeliveryReceipt, DeliveryReceiptResponse, HandshakeResponse, RequestTransfer},
repository::{ReceiverRequestInsert, Repository}, repository::{ReceiverRequestInsert, Repository},
@@ -14,7 +14,7 @@ use crate::{
util::now_ms, util::now_ms,
}; };
const APPROVAL_TTL_MS: i64 = 10 * 60 * 1000; const APPROVAL_TTL_MS: i64 = APPROVAL_SESSION_TTL_MS;
const APPROVAL_WAIT_TIMEOUT: Duration = Duration::from_secs(120); const APPROVAL_WAIT_TIMEOUT: Duration = Duration::from_secs(120);
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone, Serialize, Deserialize)]

View File

@@ -27,6 +27,9 @@ const STALE_PART_AGE: Duration = Duration::from_secs(24 * 60 * 60);
pub(crate) struct TransferImport { pub(crate) struct TransferImport {
pub(crate) tag: TempTag, pub(crate) tag: TempTag,
pub(crate) root_hash: Hash, 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) total_size: u64,
pub(crate) file_count: u64, pub(crate) file_count: u64,
pub(crate) default_name: String, pub(crate) default_name: String,
@@ -579,6 +582,24 @@ fn duplicate_file_descriptor(value: &str) -> Result<OwnedFd> {
if duplicated < 0 { if duplicated < 0 {
return Err(io::Error::last_os_error()).context("failed to duplicate file descriptor"); return Err(io::Error::last_os_error()).context("failed to duplicate file descriptor");
} }
let owned = unsafe { OwnedFd::from_raw_fd(duplicated) };
Ok(owned) // Only regular files are valid share sources. Reject directories, sockets,
// and other fd types that a compromised UI could pass by integer.
let mut stat = std::mem::MaybeUninit::<libc::stat>::uninit();
let rc = unsafe { libc::fstat(duplicated, stat.as_mut_ptr()) };
if rc != 0 {
let error = io::Error::last_os_error();
unsafe {
libc::close(duplicated);
}
return Err(error).context("failed to fstat file descriptor");
}
let mode = unsafe { stat.assume_init() }.st_mode;
if mode & libc::S_IFMT != libc::S_IFREG {
unsafe {
libc::close(duplicated);
}
anyhow::bail!("file descriptor must refer to a regular file");
}
Ok(unsafe { OwnedFd::from_raw_fd(duplicated) })
} }

View File

@@ -23,8 +23,10 @@ pub(crate) fn init_logging(app_data_dir: &Path) -> Result<()> {
fs::create_dir_all(&log_dir)?; fs::create_dir_all(&log_dir)?;
let writer = SizeRotatingWriter::new(log_dir, MAX_LOG_BYTES, MAX_LOG_FILES); let writer = SizeRotatingWriter::new(log_dir, MAX_LOG_BYTES, MAX_LOG_FILES);
let (writer, guard) = tracing_appender::non_blocking(writer); 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() 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() let subscriber = tracing_subscriber::registry()
.with(filter) .with(filter)

View File

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

View File

@@ -51,10 +51,7 @@ impl CoreInner {
.await?; .await?;
active_shares.remove(&transfer_id); active_shares.remove(&transfer_id);
drop(active_shares); drop(active_shares);
self.hash_to_transfer self.unregister_transfer_hashes(transfer_id).await;
.lock()
.await
.retain(|_, id| *id != transfer_id);
self.access_policy.remove_transfer(transfer_id).await; self.access_policy.remove_transfer(transfer_id).await;
self.emit_transfer(transfer_id, "send", "lifecycle", "share-stopped", json!({})); self.emit_transfer(transfer_id, "send", "lifecycle", "share-stopped", json!({}));
return Ok(()); return Ok(());
@@ -106,10 +103,7 @@ impl CoreInner {
} }
self.active_shares.lock().await.remove(&transfer_id); self.active_shares.lock().await.remove(&transfer_id);
self.hash_to_transfer self.unregister_transfer_hashes(transfer_id).await;
.lock()
.await
.retain(|_, id| *id != transfer_id);
self.access_policy.remove_transfer(transfer_id).await; self.access_policy.remove_transfer(transfer_id).await;
// Events are persisted asynchronously. Drain events emitted before this // Events are persisted asynchronously. Drain events emitted before this
// request so none can be written back after the transfer is deleted. // request so none can be written back after the transfer is deleted.
@@ -149,6 +143,21 @@ impl CoreInner {
transfer_id: u64, transfer_id: u64,
endpoint_id: String, endpoint_id: String,
) -> Result<()> { ) -> Result<()> {
let endpoint_id = endpoint_id.trim().to_string();
if endpoint_id.is_empty() {
anyhow::bail!("endpoint id must not be empty");
}
if endpoint_id.len() as u64 > self.limits.max_metadata_bytes {
anyhow::bail!(
"endpoint id is {} bytes, limit is {}",
endpoint_id.len(),
self.limits.max_metadata_bytes
);
}
// Only live shares can gain receiver sessions.
if !self.active_shares.lock().await.contains_key(&transfer_id) {
anyhow::bail!("transfer is not an active share");
}
self.access_policy self.access_policy
.approve_endpoint(transfer_id, endpoint_id.clone()) .approve_endpoint(transfer_id, endpoint_id.clone())
.await; .await;

View File

@@ -16,7 +16,7 @@ mod share;
pub use facade::VnidropCore; pub use facade::VnidropCore;
use std::{ use std::{
collections::HashMap, collections::{HashMap, HashSet},
path::PathBuf, path::PathBuf,
str::FromStr, str::FromStr,
sync::{atomic::AtomicBool, Arc}, 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 // Restored shares have no in-memory tag, but remain tracked so they can be
// counted and explicitly revoked after a restart. // counted and explicitly revoked after a restart.
pub(super) active_shares: TokioMutex<HashMap<u64, Option<TempTag>>>, 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) connection_endpoints: TokioMutex<HashMap<u64, String>>,
pub(super) provider_task: TokioMutex<Option<JoinHandle<()>>>, pub(super) provider_task: TokioMutex<Option<JoinHandle<()>>>,
pub(super) shutdown_started: AtomicBool, pub(super) shutdown_started: AtomicBool,
@@ -130,18 +132,12 @@ impl CoreInner {
let access_policy = AccessPolicy::new(); let access_policy = AccessPolicy::new();
// Restore share ownership and access mode before the router can serve // Restore share ownership and access mode before the router can serve
// any request. Unknown persisted modes fail closed in mode_from_storage. // 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(); let mut restored_active_shares = HashMap::new();
for share in repository.list_active_shares().await? { for share in repository.list_active_shares().await? {
let transfer_id = share.transfer_id; let transfer_id = share.transfer_id;
let valid_root = match Hash::from_str(&share.content_hash) { let Ok(root_hash) = Hash::from_str(&share.content_hash) else {
Ok(hash) => {
store.blobs().has(hash).await.unwrap_or(false)
&& Collection::load(hash, store.as_ref()).await.is_ok()
}
Err(_) => false,
};
if !valid_root {
repository repository
.transition_transfer_status( .transition_transfer_status(
transfer_id, transfer_id,
@@ -157,8 +153,39 @@ impl CoreInner {
json!({ "content_hash": share.content_hash }), json!({ "content_hash": share.content_hash }),
); );
continue; 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); restored_active_shares.insert(transfer_id, None);
access_policy access_policy
.set_mode(transfer_id, mode_from_storage(&share.access_mode)) .set_mode(transfer_id, mode_from_storage(&share.access_mode))
@@ -224,6 +251,25 @@ impl CoreInner {
.emit_transfer(transfer_id, direction, phase, kind, data); .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>> { pub(super) async fn list_events(&self, transfer_id: Option<u64>) -> Result<Vec<CoreEvent>> {
self.event_hub.flush().await; self.event_hub.flush().await;
self.repository self.repository

View File

@@ -71,29 +71,11 @@ impl CoreInner {
); );
} }
ProviderMessage::GetRequestReceived(message) => { ProviderMessage::GetRequestReceived(message) => {
let transfer_id = self.transfer_for_hash(message.inner.request.hash).await; match self
if let Some(transfer_id) = transfer_id { .authorize_hash(message.inner.request.hash, message.inner.connection_id)
let decision = self .await
.access_decision(transfer_id, message.inner.connection_id) {
.await; Ok(transfer_id) => {
if let AccessDecision::Deny { reason } = decision {
self.emit_transfer(
transfer_id,
"send",
"access",
"request-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;
return;
}
self.track_request_updates( self.track_request_updates(
transfer_id, transfer_id,
message.inner.connection_id, message.inner.connection_id,
@@ -101,11 +83,25 @@ impl CoreInner {
message.rx, message.rx,
) )
.await; .await;
}
let _ = message.tx.send(Ok(())).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;
}
}
}
ProviderMessage::GetRequestReceivedNotify(message) => { 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( self.track_request_updates(
transfer_id, transfer_id,
@@ -117,31 +113,11 @@ impl CoreInner {
} }
} }
ProviderMessage::GetManyRequestReceived(message) => { ProviderMessage::GetManyRequestReceived(message) => {
let transfer_id = self match self
.transfer_for_any_hash(&message.inner.request.hashes) .authorize_hashes(&message.inner.request.hashes, message.inner.connection_id)
.await; .await
if let Some(transfer_id) = transfer_id { {
let decision = self Ok(transfer_id) => {
.access_decision(transfer_id, message.inner.connection_id)
.await;
if let AccessDecision::Deny { reason } = decision {
self.emit_transfer(
transfer_id,
"send",
"access",
"request-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;
return;
}
self.track_request_updates( self.track_request_updates(
transfer_id, transfer_id,
message.inner.connection_id, message.inner.connection_id,
@@ -149,12 +125,24 @@ impl CoreInner {
message.rx, message.rx,
) )
.await; .await;
}
let _ = message.tx.send(Ok(())).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;
}
}
}
ProviderMessage::GetManyRequestReceivedNotify(message) => { ProviderMessage::GetManyRequestReceivedNotify(message) => {
if let Some(transfer_id) = self if let Ok(transfer_id) = self
.transfer_for_any_hash(&message.inner.request.hashes) .authorize_hashes(&message.inner.request.hashes, message.inner.connection_id)
.await .await
{ {
self.track_request_updates( self.track_request_updates(
@@ -167,6 +155,12 @@ impl CoreInner {
} }
} }
ProviderMessage::ObserveRequestReceived(message) => { ProviderMessage::ObserveRequestReceived(message) => {
// 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( self.emit_endpoint(
"provider", "provider",
"observe-request", "observe-request",
@@ -177,6 +171,23 @@ impl CoreInner {
); );
let _ = message.tx.send(Ok(())).await; 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) => { ProviderMessage::ObserveRequestReceivedNotify(message) => {
self.emit_endpoint( self.emit_endpoint(
"provider", "provider",
@@ -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 self.hash_to_transfer
.lock() .lock()
.await .await
.get(&hash.to_string()) .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> { async fn allow_any_transfer(
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(
&self, &self,
transfer_id: u64, transfer_ids: &[u64],
connection_id: u64, connection_id: u64,
) -> AccessDecision { ) -> Result<u64, &'static str> {
let endpoint_id = self let endpoint_id = self
.connection_endpoints .connection_endpoints
.lock() .lock()
.await .await
.get(&connection_id) .get(&connection_id)
.cloned(); .cloned();
self.access_policy let mut last_reason = "approval-required";
.decide(transfer_id, endpoint_id.as_deref()) for transfer_id in transfer_ids {
match self
.access_policy
.decide(*transfer_id, endpoint_id.as_deref())
.await .await
{
AccessDecision::Allow => return Ok(*transfer_id),
AccessDecision::Deny { reason } => last_reason = reason,
}
}
Err(last_reason)
} }
pub(super) async fn track_request_updates( pub(super) async fn track_request_updates(

View File

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

View File

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

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)] #[cfg(unix)]
use std::os::unix::fs::PermissionsExt; use std::os::unix::fs::{OpenOptionsExt, PermissionsExt};
use anyhow::{Context, Result}; use anyhow::{Context, Result};
use data_encoding::HEXLOWER; 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 => { Err(error) if error.kind() == io::ErrorKind::NotFound => {
let secret = SecretKey::generate(); 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?; restrict_permissions(&path).await?;
Ok(secret) 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<()> { async fn restrict_permissions(path: &Path) -> Result<()> {
#[cfg(unix)] #[cfg(unix)]
tokio::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)).await?; 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(()) Ok(())
} }

View File

@@ -4,6 +4,23 @@ use crate::{
TransferAccessMode, 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] #[tokio::test]
async fn requires_approved_endpoint_when_locked() { async fn requires_approved_endpoint_when_locked() {
let policy = AccessPolicy::new(); let policy = AccessPolicy::new();
@@ -54,3 +71,27 @@ 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);
}
#[tokio::test]
async fn approve_endpoint_grants_time_limited_session() {
let policy = AccessPolicy::new();
policy
.set_mode(11, TransferAccessMode::ApprovalRequired)
.await;
policy.approve_endpoint(11, "node-b".to_string()).await;
assert_eq!(
policy.decide(11, Some("node-b")).await,
AccessDecision::Allow
);
}

View File

@@ -62,6 +62,26 @@ fn file_descriptor_source_rejects_invalid_values() {
} }
} }
#[cfg(unix)]
#[test]
fn file_descriptor_source_rejects_directory_fds() {
let temp = tempfile::tempdir().unwrap();
let dir = std::fs::File::open(temp.path()).unwrap();
use std::os::fd::AsRawFd;
let error = collect_import_files(vec![ShareSource {
kind: SourceKind::FileDescriptor,
value: dir.as_raw_fd().to_string(),
display_name: Some("folder".to_string()),
is_directory: false,
}])
.unwrap_err()
.to_string();
assert!(
error.contains("regular file"),
"directory fd must be rejected: {error}"
);
}
#[test] #[test]
fn android_content_uri_must_be_opened_by_platform_code() { fn android_content_uri_must_be_opened_by_platform_code() {
let error = collect_import_files(vec![ShareSource { let error = collect_import_files(vec![ShareSource {

View File

@@ -5,6 +5,14 @@ fn default_limits_are_valid() {
CoreLimits::default().validate().unwrap(); CoreLimits::default().validate().unwrap();
} }
#[test]
fn default_limits_bound_ticket_and_approval_pressure() {
let limits = CoreLimits::default();
assert!(limits.max_ticket_bytes <= 256 * 1024);
assert!(limits.max_pending_approvals <= 64);
assert!(limits.max_total_bytes <= 256 * 1024 * 1024 * 1024);
}
#[test] #[test]
fn zero_limit_is_rejected() { fn zero_limit_is_rejected() {
let limits = CoreLimits { let limits = CoreLimits {

View File

@@ -31,10 +31,7 @@ fn metadata_ticket_round_trips() {
let parsed = parse_transfer_ticket(&encoded).unwrap(); let parsed = parse_transfer_ticket(&encoded).unwrap();
assert_eq!(parsed.blob_ticket.hash(), blob_ticket.hash()); assert_eq!(parsed.blob_ticket.hash(), blob_ticket.hash());
assert_eq!( assert_eq!(parsed.metadata.transfer_name, metadata.transfer_name);
parsed.metadata.unwrap().transfer_name,
metadata.transfer_name
);
} }
#[test] #[test]
@@ -60,6 +57,16 @@ fn invalid_ticket_is_rejected() {
assert!(parse_transfer_ticket("not-a-ticket").is_err()); 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] #[test]
fn rejects_unsupported_versions_and_mismatched_hashes() { fn rejects_unsupported_versions_and_mismatched_hashes() {
let blob_ticket = blob_ticket(5); let blob_ticket = blob_ticket(5);

View File

@@ -49,7 +49,7 @@ impl VnidropTicket {
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub(crate) struct ParsedTransferTicket { pub(crate) struct ParsedTransferTicket {
pub(crate) blob_ticket: BlobTicket, pub(crate) blob_ticket: BlobTicket,
pub(crate) metadata: Option<TransferMetadata>, pub(crate) metadata: TransferMetadata,
} }
#[cfg(test)] #[cfg(test)]
@@ -69,7 +69,9 @@ pub(crate) fn parse_transfer_ticket_with_limits(
); );
} }
let normalized = normalize_ticket_input(value); let normalized = normalize_ticket_input(value);
if normalized.starts_with(VNIDROP_TICKET_PREFIX) { if !normalized.starts_with(VNIDROP_TICKET_PREFIX) {
anyhow::bail!("not a VniDrop ticket; expected a vnd1: invitation");
}
let ticket = VnidropTicket::decode(&normalized)?; let ticket = VnidropTicket::decode(&normalized)?;
if ticket.version != VNIDROP_TICKET_VERSION { if ticket.version != VNIDROP_TICKET_VERSION {
anyhow::bail!("unsupported VniDrop ticket version {}", ticket.version); anyhow::bail!("unsupported VniDrop ticket version {}", ticket.version);
@@ -96,22 +98,15 @@ pub(crate) fn parse_transfer_ticket_with_limits(
if ticket.metadata.content_hash != blob_ticket.hash().to_string() { if ticket.metadata.content_hash != blob_ticket.hash().to_string() {
anyhow::bail!("VniDrop ticket metadata hash does not match BlobTicket hash"); anyhow::bail!("VniDrop ticket metadata hash does not match BlobTicket hash");
} }
return Ok(ParsedTransferTicket {
blob_ticket,
metadata: Some(ticket.metadata),
});
}
let blob_ticket = BlobTicket::from_str(&normalized).context("invalid BlobTicket")?;
Ok(ParsedTransferTicket { Ok(ParsedTransferTicket {
blob_ticket, blob_ticket,
metadata: None, metadata: ticket.metadata,
}) })
} }
fn normalize_ticket_input(value: &str) -> String { fn normalize_ticket_input(value: &str) -> String {
// Tickets are commonly copied from text views or chat apps that insert line // Tickets are commonly copied from text views or chat apps that insert line
// breaks. Strip whitespace only; other corrupt characters should still be // 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() value.chars().filter(|char| !char.is_whitespace()).collect()
} }

View File

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

View File

@@ -100,7 +100,7 @@ fn persisted_share_is_recovered_and_can_be_stopped_after_restart() {
} }
#[test] #[test]
fn stopped_share_rejects_direct_legacy_blob_ticket() { fn stopped_share_rejects_receive() {
let source_dir = tempfile::tempdir().unwrap(); let source_dir = tempfile::tempdir().unwrap();
let output_dir = tempfile::tempdir().unwrap(); let output_dir = tempfile::tempdir().unwrap();
let source_path = source_dir.path().join("revoked.txt"); 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(); sender.core.cancel_transfer(share.transfer_id).unwrap();
let result = receiver.core.receive( let result = receiver.core.receive(
share.blob_ticket, share.ticket,
output_dir.path().to_string_lossy().to_string(), output_dir.path().to_string_lossy().to_string(),
Some("receiver".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()); 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] #[test]
fn failed_import_leaves_durable_failed_transfer() { fn failed_import_leaves_durable_failed_transfer() {
let source_dir = tempfile::tempdir().unwrap(); let source_dir = tempfile::tempdir().unwrap();
@@ -205,6 +251,34 @@ fn access_mode_update_requires_active_persisted_share() {
.is_err()); .is_err());
} }
#[test]
fn approve_endpoint_requires_active_share_and_nonempty_id() {
let source_dir = tempfile::tempdir().unwrap();
let source_path = source_dir.path().join("shared.txt");
std::fs::write(&source_path, b"content").unwrap();
let sender = TestNode::new();
let share = share_path(&sender.core, &source_path, 42, "shared.txt", false);
assert!(sender
.core
.approve_endpoint_for_transfer(share.transfer_id, " ".to_string())
.is_err());
assert!(sender
.core
.approve_endpoint_for_transfer(999, "endpoint-a".to_string())
.is_err());
sender
.core
.approve_endpoint_for_transfer(share.transfer_id, "endpoint-a".to_string())
.unwrap();
sender.core.cancel_transfer(share.transfer_id).unwrap();
assert!(sender
.core
.approve_endpoint_for_transfer(share.transfer_id, "endpoint-a".to_string())
.is_err());
}
#[test] #[test]
fn source_limit_rejection_creates_no_transfer_state() { fn source_limit_rejection_creates_no_transfer_state() {
let core_dir = tempfile::tempdir().unwrap(); let core_dir = tempfile::tempdir().unwrap();

View File

@@ -225,8 +225,7 @@ private class AndroidMediaStoreDownloadsSink(
"MediaStore Downloads requires Android 10 or newer" "MediaStore Downloads requires Android 10 or newer"
} }
check(relativePath !in pending) { "Output stream is already open for $relativePath" } check(relativePath !in pending) { "Output stream is already open for $relativePath" }
val parts = relativePath.split('/').filter { it.isNotBlank() } val parts = requireSafeRelativePathParts(relativePath)
require(parts.isNotEmpty()) { "relative path must not be empty" }
val finalName = parts.last() val finalName = parts.last()
val relativeDir = mediaStoreRelativePath(parts.dropLast(1)) val relativeDir = mediaStoreRelativePath(parts.dropLast(1))
check(!mediaStoreItemExists(finalName, relativeDir)) { check(!mediaStoreItemExists(finalName, relativeDir)) {
@@ -346,6 +345,8 @@ private class AndroidTreeReceiveOutputSink(
override fun startFile(relativePath: String) { override fun startFile(relativePath: String) {
check(relativePath !in pending) { "Output stream is already open for $relativePath" } check(relativePath !in pending) { "Output stream is already open for $relativePath" }
// Defense in depth: Rust also validates, but sinks must reject traversal alone.
requireSafeRelativePathParts(relativePath)
val (parent, finalName) = resolveParent(relativePath) val (parent, finalName) = resolveParent(relativePath)
check(findChild(parent, finalName) == null) { "Destination already exists: $relativePath" } check(findChild(parent, finalName) == null) { "Destination already exists: $relativePath" }
val temporaryName = ".$finalName.vnidrop-${UUID.randomUUID()}.part" val temporaryName = ".$finalName.vnidrop-${UUID.randomUUID()}.part"
@@ -391,8 +392,7 @@ private class AndroidTreeReceiveOutputSink(
} }
private fun resolveParent(relativePath: String): Pair<Uri, String> { private fun resolveParent(relativePath: String): Pair<Uri, String> {
val parts = relativePath.split('/').filter { it.isNotBlank() } val parts = requireSafeRelativePathParts(relativePath)
require(parts.isNotEmpty()) { "relative path must not be empty" }
var parent = DocumentsContract.buildDocumentUriUsingTree( var parent = DocumentsContract.buildDocumentUriUsingTree(
treeUri, treeUri,
DocumentsContract.getTreeDocumentId(treeUri), DocumentsContract.getTreeDocumentId(treeUri),
@@ -428,3 +428,18 @@ private class AndroidTreeReceiveOutputSink(
return null return null
} }
} }
/**
* Split a receive relative path and reject traversal / absolute-style components.
* Rust already validates; this keeps Android sinks safe if called incorrectly.
*/
private fun requireSafeRelativePathParts(relativePath: String): List<String> {
val parts = relativePath.split('/').filter { it.isNotBlank() }
require(parts.isNotEmpty()) { "relative path must not be empty" }
require(parts.none { part ->
part == "." || part == ".." || part.contains('\\') || part.contains('\u0000')
}) {
"relative path contains invalid components: $relativePath"
}
return parts
}

View File

@@ -22,6 +22,7 @@
<string name="send_access_approval_description">You approve or refuse every new receiver.</string> <string name="send_access_approval_description">You approve or refuse every new receiver.</string>
<string name="send_access_anyone">Anyone with this transfer</string> <string name="send_access_anyone">Anyone with this transfer</string>
<string name="send_access_anyone_description">No approval is required. Only use this for files you are comfortable sharing.</string> <string name="send_access_anyone_description">No approval is required. Only use this for files you are comfortable sharing.</string>
<string name="send_access_anyone_warning">Anyone who has the ticket can download until you stop the share. Do not use this for private or sensitive files.</string>
<string name="send_file_size_unknown">Size unavailable</string> <string name="send_file_size_unknown">Size unavailable</string>
<string name="send_transfer_created">Transfer created.</string> <string name="send_transfer_created">Transfer created.</string>
<string name="send_transfer_details_title">Transfer details</string> <string name="send_transfer_details_title">Transfer details</string>
@@ -128,7 +129,7 @@
<string name="transfer_event_saving">Saving files</string> <string name="transfer_event_saving">Saving files</string>
<string name="transfer_event_connecting">Connecting to sender</string> <string name="transfer_event_connecting">Connecting to sender</string>
<string name="ticket_details_title">Ticket details</string> <string name="ticket_details_title">Ticket details</string>
<string name="ticket_no_metadata">This ticket does not include VniDrop metadata.</string>
<string name="settings_title">Settings</string> <string name="settings_title">Settings</string>
<string name="settings_subtitle">Configure the local node and app appearance.</string> <string name="settings_subtitle">Configure the local node and app appearance.</string>
<string name="node_title">Node</string> <string name="node_title">Node</string>
@@ -173,6 +174,7 @@
<string name="value_on">On</string> <string name="value_on">On</string>
<string name="value_off">Off</string> <string name="value_off">Off</string>
<string name="approval_connection_request">Connection request</string> <string name="approval_connection_request">Connection request</string>
<string name="approval_endpoint_id">Endpoint ID: %1$s</string>
<string name="approval_pending_count">%1$d requests are waiting</string> <string name="approval_pending_count">%1$d requests are waiting</string>
<string name="core_status_ready">Ready</string> <string name="core_status_ready">Ready</string>
<string name="event_log_title">Event log</string> <string name="event_log_title">Event log</string>

View File

@@ -77,8 +77,7 @@ data class TransferMetadataModel(
data class TicketInspectionModel( data class TicketInspectionModel(
val kind: String, val kind: String,
val blobTicket: String, val metadata: TransferMetadataModel,
val metadata: TransferMetadataModel?,
) )
data class ReceiverRequestModel( data class ReceiverRequestModel(

View File

@@ -335,8 +335,7 @@ private fun ShareResult.toModel(): Share = Share(
private fun TicketInspection.toModel(): TicketInspectionModel = TicketInspectionModel( private fun TicketInspection.toModel(): TicketInspectionModel = TicketInspectionModel(
kind = kind, kind = kind,
blobTicket = blobTicket, metadata = metadata.toModel(),
metadata = metadata?.toModel(),
) )
private fun TransferMetadata.toModel(): TransferMetadataModel = TransferMetadataModel( private fun TransferMetadata.toModel(): TransferMetadataModel = TransferMetadataModel(

View File

@@ -27,6 +27,8 @@ data class PendingApproval(
val transferName: String, val transferName: String,
val receiverName: String?, val receiverName: String?,
val receiverDeviceName: String?, val receiverDeviceName: String?,
/** Cryptographic peer identity from the Iroh connection — not display-name spoofable. */
val remoteEndpointId: String,
val requestedAt: Long, val requestedAt: Long,
) )
@@ -162,6 +164,7 @@ private fun ReceiverRequestModel.toPending(): PendingApproval = PendingApproval(
transferName = transferName, transferName = transferName,
receiverName = receiverName, receiverName = receiverName,
receiverDeviceName = receiverDeviceName, receiverDeviceName = receiverDeviceName,
remoteEndpointId = remoteEndpointId,
requestedAt = requestedAt, requestedAt = requestedAt,
) )

View File

@@ -31,6 +31,7 @@ import com.vnidrop.app.ui.theme.LocalVniDropColors
import org.jetbrains.compose.resources.stringResource import org.jetbrains.compose.resources.stringResource
import vnidrop.shared.generated.resources.Res import vnidrop.shared.generated.resources.Res
import vnidrop.shared.generated.resources.approval_connection_request import vnidrop.shared.generated.resources.approval_connection_request
import vnidrop.shared.generated.resources.approval_endpoint_id
import vnidrop.shared.generated.resources.approval_pending_count import vnidrop.shared.generated.resources.approval_pending_count
import vnidrop.shared.generated.resources.button_approve import vnidrop.shared.generated.resources.button_approve
import vnidrop.shared.generated.resources.button_refuse import vnidrop.shared.generated.resources.button_refuse
@@ -78,6 +79,12 @@ fun ApprovalModalHost(
style = MaterialTheme.typography.bodyLarge, style = MaterialTheme.typography.bodyLarge,
color = colors.foregroundLight, color = colors.foregroundLight,
) )
// Trusted identity is the endpoint id; display names are peer-provided.
Text(
stringResource(Res.string.approval_endpoint_id, request.remoteEndpointId),
style = MaterialTheme.typography.bodySmall,
color = colors.foregroundLighter,
)
if (state.pending.size > 1) { if (state.pending.size > 1) {
Text( Text(
stringResource(Res.string.approval_pending_count, state.pending.size), stringResource(Res.string.approval_pending_count, state.pending.size),

View File

@@ -255,8 +255,8 @@ private fun InvitationReviewPanel(
val metadata = inspection.metadata val metadata = inspection.metadata
Surface(shape = RoundedCornerShape(14.dp), color = LocalVniDropColors.current.backgroundSurface200) { Surface(shape = RoundedCornerShape(14.dp), color = LocalVniDropColors.current.backgroundSurface200) {
Column(Modifier.fillMaxWidth().padding(16.dp), verticalArrangement = Arrangement.spacedBy(8.dp)) { Column(Modifier.fillMaxWidth().padding(16.dp), verticalArrangement = Arrangement.spacedBy(8.dp)) {
Text(metadata?.transferName ?: stringResource(Res.string.receive_unknown_transfer), fontWeight = FontWeight.Bold, maxLines = 2, overflow = TextOverflow.Ellipsis) Text(metadata.transferName, fontWeight = FontWeight.Bold, maxLines = 2, overflow = TextOverflow.Ellipsis)
if (metadata != null) Text("${metadata.fileCount} ${stringResource(Res.string.metadata_files).lowercase()} · ${formatBytes(metadata.totalSize)}", color = LocalVniDropColors.current.foregroundLighter) Text("${metadata.fileCount} ${stringResource(Res.string.metadata_files).lowercase()} · ${formatBytes(metadata.totalSize)}", color = LocalVniDropColors.current.foregroundLighter)
} }
} }
Field(state.receiverName, onReceiverNameChanged, stringResource(Res.string.field_receiver_name)) Field(state.receiverName, onReceiverNameChanged, stringResource(Res.string.field_receiver_name))

View File

@@ -51,6 +51,7 @@ import vnidrop.shared.generated.resources.field_sender_name
import vnidrop.shared.generated.resources.field_transfer_name import vnidrop.shared.generated.resources.field_transfer_name
import vnidrop.shared.generated.resources.send_access_anyone import vnidrop.shared.generated.resources.send_access_anyone
import vnidrop.shared.generated.resources.send_access_anyone_description import vnidrop.shared.generated.resources.send_access_anyone_description
import vnidrop.shared.generated.resources.send_access_anyone_warning
import vnidrop.shared.generated.resources.send_access_approval import vnidrop.shared.generated.resources.send_access_approval
import vnidrop.shared.generated.resources.send_access_approval_description import vnidrop.shared.generated.resources.send_access_approval_description
import vnidrop.shared.generated.resources.send_access_title import vnidrop.shared.generated.resources.send_access_title
@@ -166,6 +167,13 @@ private fun ReviewFileStep(
selected = state.accessPolicy == ShareAccessPolicy.AnyoneWithTransfer, selected = state.accessPolicy == ShareAccessPolicy.AnyoneWithTransfer,
onClick = { onAccessPolicyChanged(ShareAccessPolicy.AnyoneWithTransfer) }, onClick = { onAccessPolicyChanged(ShareAccessPolicy.AnyoneWithTransfer) },
) )
if (state.accessPolicy == ShareAccessPolicy.AnyoneWithTransfer) {
Text(
stringResource(Res.string.send_access_anyone_warning),
color = LocalVniDropColors.current.destructiveDefault,
style = MaterialTheme.typography.bodySmall,
)
}
if (windowClass == WindowClass.Phone) { if (windowClass == WindowClass.Phone) {
Column(verticalArrangement = Arrangement.spacedBy(8.dp)) { Column(verticalArrangement = Arrangement.spacedBy(8.dp)) {
ShareButton(state, coreInitialized, onCreateShare, Modifier.fillMaxWidth()) ShareButton(state, coreInitialized, onCreateShare, Modifier.fillMaxWidth())

View File

@@ -32,7 +32,6 @@ import vnidrop.shared.generated.resources.metadata_size
import vnidrop.shared.generated.resources.metadata_transfer import vnidrop.shared.generated.resources.metadata_transfer
import vnidrop.shared.generated.resources.progress_title import vnidrop.shared.generated.resources.progress_title
import vnidrop.shared.generated.resources.ticket_details_title import vnidrop.shared.generated.resources.ticket_details_title
import vnidrop.shared.generated.resources.ticket_no_metadata
import vnidrop.shared.generated.resources.unknown_sender import vnidrop.shared.generated.resources.unknown_sender
@Composable @Composable
@@ -60,15 +59,14 @@ fun ProgressSection(coreState: CoreState) {
@Composable @Composable
fun TicketInspectionCard(inspection: TicketInspectionModel) { fun TicketInspectionCard(inspection: TicketInspectionModel) {
val metadata = inspection.metadata
AppCard(title = stringResource(Res.string.ticket_details_title)) { AppCard(title = stringResource(Res.string.ticket_details_title)) {
MetadataRow(stringResource(Res.string.metadata_kind), inspection.kind) MetadataRow(stringResource(Res.string.metadata_kind), inspection.kind)
inspection.metadata?.let { metadata ->
MetadataRow(stringResource(Res.string.metadata_transfer), metadata.transferName) MetadataRow(stringResource(Res.string.metadata_transfer), metadata.transferName)
MetadataRow(stringResource(Res.string.metadata_sender), metadata.senderName ?: stringResource(Res.string.unknown_sender)) MetadataRow(stringResource(Res.string.metadata_sender), metadata.senderName ?: stringResource(Res.string.unknown_sender))
MetadataRow(stringResource(Res.string.metadata_files), metadata.fileCount.toString()) MetadataRow(stringResource(Res.string.metadata_files), metadata.fileCount.toString())
MetadataRow(stringResource(Res.string.metadata_size), formatBytes(metadata.totalSize)) MetadataRow(stringResource(Res.string.metadata_size), formatBytes(metadata.totalSize))
MetadataRow(stringResource(Res.string.metadata_hash), metadata.contentHash) MetadataRow(stringResource(Res.string.metadata_hash), metadata.contentHash)
} ?: EmptyText(stringResource(Res.string.ticket_no_metadata))
} }
} }

View File

@@ -299,11 +299,7 @@ class ViewModelsTest {
Dispatchers.setMain(StandardTestDispatcher(testScheduler)) Dispatchers.setMain(StandardTestDispatcher(testScheduler))
val core = FakeCoreGateway().apply { val core = FakeCoreGateway().apply {
mutableState.value = mutableState.value.copy(isInitialized = true) mutableState.value = mutableState.value.copy(isInitialized = true)
inspectionResult = Result.success(com.vnidrop.app.core.TicketInspectionModel( inspectionResult = Result.success(sampleTicketInspection())
kind = "vnidrop",
blobTicket = "blob",
metadata = com.vnidrop.app.core.TransferMetadataModel(1UL, "Photo", null, "hash", 1UL, 42UL),
))
} }
val viewModel = ReceiveViewModel(core, FakeFileSystemService(folder), preferences(), UiMessageController()) val viewModel = ReceiveViewModel(core, FakeFileSystemService(folder), preferences(), UiMessageController())
advanceUntilIdle() advanceUntilIdle()
@@ -379,13 +375,7 @@ class ViewModelsTest {
Dispatchers.setMain(StandardTestDispatcher(testScheduler)) Dispatchers.setMain(StandardTestDispatcher(testScheduler))
val core = FakeCoreGateway().apply { val core = FakeCoreGateway().apply {
mutableState.value = mutableState.value.copy(isInitialized = true) mutableState.value = mutableState.value.copy(isInitialized = true)
inspectionResult = Result.success( inspectionResult = Result.success(sampleTicketInspection())
com.vnidrop.app.core.TicketInspectionModel(
kind = "vnidrop",
blobTicket = "blob",
metadata = com.vnidrop.app.core.TransferMetadataModel(1UL, "Photo", null, "hash", 1UL, 42UL),
),
)
} }
val viewModel = ReceiveViewModel(core, FakeFileSystemService(folder), preferences(), UiMessageController()) val viewModel = ReceiveViewModel(core, FakeFileSystemService(folder), preferences(), UiMessageController())
advanceUntilIdle() advanceUntilIdle()
@@ -408,13 +398,7 @@ class ViewModelsTest {
Dispatchers.setMain(StandardTestDispatcher(testScheduler)) Dispatchers.setMain(StandardTestDispatcher(testScheduler))
val core = FakeCoreGateway().apply { val core = FakeCoreGateway().apply {
mutableState.value = mutableState.value.copy(isInitialized = true) mutableState.value = mutableState.value.copy(isInitialized = true)
inspectionResult = Result.success( inspectionResult = Result.success(sampleTicketInspection())
com.vnidrop.app.core.TicketInspectionModel(
kind = "vnidrop",
blobTicket = "blob",
metadata = com.vnidrop.app.core.TransferMetadataModel(1UL, "Photo", null, "hash", 1UL, 42UL),
),
)
receiveResult = Result.failure(IllegalStateException("sender refused")) receiveResult = Result.failure(IllegalStateException("sender refused"))
} }
val viewModel = ReceiveViewModel(core, FakeFileSystemService(folder), preferences(), UiMessageController()) val viewModel = ReceiveViewModel(core, FakeFileSystemService(folder), preferences(), UiMessageController())
@@ -489,13 +473,7 @@ class ViewModelsTest {
Dispatchers.setMain(StandardTestDispatcher(testScheduler)) Dispatchers.setMain(StandardTestDispatcher(testScheduler))
val core = FakeCoreGateway().apply { val core = FakeCoreGateway().apply {
mutableState.value = mutableState.value.copy(isInitialized = true) mutableState.value = mutableState.value.copy(isInitialized = true)
inspectionResult = Result.success( inspectionResult = Result.success(sampleTicketInspection())
com.vnidrop.app.core.TicketInspectionModel(
kind = "vnidrop",
blobTicket = "blob",
metadata = com.vnidrop.app.core.TransferMetadataModel(1UL, "Photo", null, "hash", 1UL, 42UL),
),
)
// Keep receive suspended so dismiss can be asserted mid-transfer. // Keep receive suspended so dismiss can be asserted mid-transfer.
receiveResult = Result.success(Unit) receiveResult = Result.success(Unit)
receiveSuspend = true receiveSuspend = true
@@ -544,6 +522,11 @@ class ViewModelsTest {
updatedAt = 1L, updatedAt = 1L,
) )
private fun sampleTicketInspection() = com.vnidrop.app.core.TicketInspectionModel(
kind = "vnidrop",
metadata = com.vnidrop.app.core.TransferMetadataModel(1UL, "Photo", null, "hash", 1UL, 42UL),
)
private companion object { private companion object {
val folder = ReceiveFolder(ReceiveFolderKind.FileSystemPath, "/tmp", "Downloads") val folder = ReceiveFolder(ReceiveFolderKind.FileSystemPath, "/tmp", "Downloads")
} }

View File

@@ -61,7 +61,10 @@ class AppUiModelsTest {
fun receiveStateExposesInspectAndReceiveEligibility() { fun receiveStateExposesInspectAndReceiveEligibility() {
val ready = ReceiveState( val ready = ReceiveState(
ticket = "ticket", ticket = "ticket",
inspection = com.vnidrop.app.core.TicketInspectionModel("vnidrop", "blob", null), inspection = com.vnidrop.app.core.TicketInspectionModel(
kind = "vnidrop",
metadata = com.vnidrop.app.core.TransferMetadataModel(1UL, "Photo", null, "hash", 1UL, 42UL),
),
folderAccessStatus = com.vnidrop.app.core.FolderAccessStatus.Writable, folderAccessStatus = com.vnidrop.app.core.FolderAccessStatus.Writable,
) )

View File

@@ -427,6 +427,7 @@ class FoundationComposeTest {
transferName = "Photos", transferName = "Photos",
receiverName = "Alice", receiverName = "Alice",
receiverDeviceName = "Phone", receiverDeviceName = "Phone",
remoteEndpointId = "endpoint-alice",
requestedAt = 1L, requestedAt = 1L,
) )