264 lines
9.7 KiB
Markdown
264 lines
9.7 KiB
Markdown
# OneCLI Bitwarden Integration Security Analysis
|
|
|
|
## Executive Summary
|
|
|
|
**Verdict: NOT a glaring security hole.** The architecture is sound and follows security best practices. Johan's suspicion that they "download the entire Bitwarden vault into memory" is **incorrect**.
|
|
|
|
---
|
|
|
|
## Architecture Overview
|
|
|
|
OneCLI uses Bitwarden's official **Agent Access SDK** (crates: `ap-client`, `ap-proxy-client`, `ap-proxy-protocol`, `ap-noise` v0.9.0) — an open standard for secure agent credential access announced March 2025.
|
|
|
|
---
|
|
|
|
## Key Technical Findings
|
|
|
|
### 1. Bitwarden Vault Provider Implementation
|
|
|
|
**File:** `apps/gateway/src/vault/bitwarden.rs`
|
|
|
|
The `BitwardenVaultProvider` implements the `VaultProvider` trait:
|
|
|
|
```rust
|
|
#[async_trait]
|
|
pub(crate) trait VaultProvider: Send + Sync {
|
|
fn provider_name(&self) -> &'static str;
|
|
async fn pair(&self, account_id: &str, params: &serde_json::Value) -> Result<PairResult>;
|
|
async fn request_credential(&self, account_id: &str, hostname: &str) -> Option<VaultCredential>;
|
|
async fn status(&self, account_id: &str) -> ProviderStatus;
|
|
async fn disconnect(&self, account_id: &str) -> Result<()>;
|
|
}
|
|
```
|
|
|
|
### 2. On-Demand Credential Fetching (NOT Full Vault Sync)
|
|
|
|
**Line 428-517** in `bitwarden.rs` — the `request_credential` implementation:
|
|
|
|
```rust
|
|
async fn request_credential(
|
|
&self,
|
|
account_id: &str,
|
|
hostname: &str,
|
|
) -> Option<VaultCredential> {
|
|
// Load existing session — returns None if account never paired
|
|
let session = match self.load_session(account_id).await {
|
|
Ok(Some(s)) => s,
|
|
_ => return None,
|
|
};
|
|
|
|
// Check credential cache first — avoids expensive lazy restore if cached
|
|
if let Some(cached) = session.credential_cache.get(hostname) {
|
|
if cached.expires_at > Instant::now() {
|
|
return cached.data.as_ref().map(|c| VaultCredential {
|
|
username: c.username.clone(),
|
|
password: c.password.clone(),
|
|
});
|
|
}
|
|
}
|
|
session.credential_cache.remove(hostname);
|
|
|
|
// ... lazy restore if needed ...
|
|
|
|
let client_guard = session.client.lock().await;
|
|
let client = client_guard.as_ref()?;
|
|
|
|
// REQUEST SINGLE CREDENTIAL BY DOMAIN
|
|
let query = CredentialQuery::Domain(hostname.to_string());
|
|
let result = tokio::time::timeout(REQUEST_TIMEOUT, client.request_credential(&query)).await;
|
|
// ...
|
|
}
|
|
```
|
|
|
|
**Key point:** They send `CredentialQuery::Domain(hostname)` to request **individual credentials by hostname**, not a vault sync. This is a just-in-time fetch pattern.
|
|
|
|
### 3. What Is Stored in the Database?
|
|
|
|
**File:** `apps/gateway/src/db.rs` (lines 60-69)
|
|
|
|
```rust
|
|
pub(crate) struct VaultConnectionRow {
|
|
pub id: String,
|
|
pub provider: String,
|
|
pub name: Option<String>,
|
|
pub status: String,
|
|
pub connection_data: Option<serde_json::Value>, // NOT vault contents
|
|
}
|
|
```
|
|
|
|
**File:** `apps/gateway/src/vault/bitwarden.rs` (lines 43-58)
|
|
|
|
The `connection_data` JSON contains:
|
|
|
|
```rust
|
|
pub(crate) struct BitwardenConnectionData {
|
|
/// Hex-encoded fingerprint of the remote (desktop) device.
|
|
pub fingerprint: Option<String>,
|
|
/// COSE-encoded identity keypair bytes.
|
|
pub key_data: Option<Vec<u8>>,
|
|
/// Noise protocol transport state (CBOR bytes).
|
|
pub transport_state: Option<Vec<u8>>,
|
|
}
|
|
```
|
|
|
|
**What this means:**
|
|
- ✅ Identity keypair (for Noise protocol auth) — stored
|
|
- ✅ Transport state (for session resumption) — stored
|
|
- ❌ **NO master password stored**
|
|
- ❌ **NO vault contents stored**
|
|
- ❌ **NO decrypted credentials stored**
|
|
|
|
### 4. Session Lifecycle and Eviction
|
|
|
|
**Lines 40-44** in `bitwarden.rs` — cache TTLs:
|
|
|
|
```rust
|
|
const CREDENTIAL_CACHE_TTL: Duration = Duration::from_secs(60); // 60 seconds
|
|
const NEGATIVE_CACHE_TTL: Duration = Duration::from_secs(30); // 30 seconds
|
|
const REQUEST_TIMEOUT: Duration = Duration::from_secs(15); // 15 seconds
|
|
const ERROR_COOLDOWN: Duration = Duration::from_secs(60); // 60 seconds after failure
|
|
const EVICTION_INTERVAL: Duration = Duration::from_secs(5 * 60); // 5 minutes
|
|
const SESSION_IDLE_TIMEOUT: Duration = Duration::from_secs(30 * 60); // 30 minutes
|
|
```
|
|
|
|
**Lines 157-191** — Background eviction task:
|
|
|
|
```rust
|
|
fn spawn_eviction_task(sessions: Arc<DashMap<String, Arc<BitwardenUserSession>>>) {
|
|
tokio::spawn(async move {
|
|
let mut interval = tokio::time::interval(EVICTION_INTERVAL);
|
|
loop {
|
|
interval.tick().await;
|
|
|
|
// Collect account_ids to evict
|
|
let to_evict: Vec<String> = sessions
|
|
.iter()
|
|
.filter_map(|entry| {
|
|
let last_used = entry.value().last_used.lock().ok()?;
|
|
if last_used.elapsed() > SESSION_IDLE_TIMEOUT {
|
|
Some(entry.key().clone())
|
|
} else {
|
|
None
|
|
}
|
|
})
|
|
.collect();
|
|
|
|
for account_id in to_evict {
|
|
if let Some((_, session)) = sessions.remove(&account_id) {
|
|
let mut guard = session.client.lock().await;
|
|
guard.take(); // dropping the handle disconnects
|
|
session.credential_cache.clear();
|
|
session.is_ready.store(false, Ordering::Relaxed);
|
|
info!(account_id = %account_id, "bitwarden: evicted idle session");
|
|
}
|
|
}
|
|
}
|
|
});
|
|
}
|
|
```
|
|
|
|
### 5. In-Memory Session Structure
|
|
|
|
**Lines 100-130** in `bitwarden.rs`:
|
|
|
|
```rust
|
|
struct BitwardenUserSession {
|
|
client: Mutex<Option<RemoteClient>>, // Live connection to Bitwarden
|
|
identity: BitwardenIdentityProvider, // Ephemeral identity
|
|
connection_data: Option<BitwardenConnectionData>, // Cached from DB
|
|
credential_cache: DashMap<String, CachedCredential>, // Individual creds (60s TTL)
|
|
last_used: std::sync::Mutex<Instant>, // For eviction tracking
|
|
last_error: Arc<std::sync::Mutex<Option<String>>>,
|
|
error_until: std::sync::Mutex<Option<Instant>>, // Cooldown after failures
|
|
is_ready: Arc<AtomicBool>, // From Bitwarden readiness signal
|
|
}
|
|
|
|
struct CachedCredential {
|
|
data: Option<CredentialData>, // Single credential (username/password)
|
|
expires_at: Instant, // 60s TTL for hits, 30s for misses
|
|
}
|
|
```
|
|
|
|
### 6. Decryption Flow
|
|
|
|
From the documentation and code:
|
|
|
|
1. **Bitwarden Desktop App** holds the encrypted vault
|
|
2. **OneCLI Gateway** establishes Noise protocol connection to the desktop app
|
|
3. **On credential request:** Gateway sends `CredentialQuery::Domain(hostname)`
|
|
4. **Desktop app:**
|
|
- Unlocks vault with master password (entered by user, NOT stored in OneCLI)
|
|
- Finds matching credential
|
|
- Decrypts it locally
|
|
- Sends plaintext back via Noise-encrypted channel
|
|
5. **OneCLI:** Receives already-decrypted credential, caches for 60s, injects into HTTP request
|
|
|
|
**Critical:** Decryption happens in the Bitwarden desktop app, NOT in OneCLI. OneCLI never has the master password or encryption keys.
|
|
|
|
### 7. Request Flow
|
|
|
|
**File:** `apps/gateway/src/gateway.rs` (lines 246-257)
|
|
|
|
```rust
|
|
// Vault fallback: if no DB secrets matched, try vault providers for this user.
|
|
if let Some(cred) = state.vault_service.request_credential(aid, &hostname).await {
|
|
let vault_rules = inject::vault_credential_to_rules(&hostname, &cred);
|
|
if !vault_rules.is_not_empty() {
|
|
injection_rules = vault_rules;
|
|
tracing::info!(
|
|
agent_id = %agent_id,
|
|
hostname = %hostname,
|
|
"using vault credential"
|
|
);
|
|
}
|
|
}
|
|
```
|
|
|
|
---
|
|
|
|
## Security Assessment
|
|
|
|
| Concern | Status | Evidence |
|
|
|---------|--------|----------|
|
|
| Full vault download | ❌ **NO** | Per-credential fetching via `CredentialQuery::Domain(hostname)` |
|
|
| All credentials in memory | ❌ **NO** | Only individual requested credentials cached (60s TTL) |
|
|
| Master password stored | ❌ **NO** | Identity keypair used for Noise auth; decryption in Bitwarden app |
|
|
| Decrypted vault at rest | ❌ **NO** | Only transport state stored in DB |
|
|
| Session persistence | ✅ Bounded | 30-min idle eviction, 60s credential cache |
|
|
| End-to-end encryption | ✅ Yes | Noise protocol over WebSocket |
|
|
| Just-in-time access | ✅ Yes | Credentials fetched on-demand per request |
|
|
|
|
---
|
|
|
|
## Potential Concerns (Minor)
|
|
|
|
1. **Identity keypair in DB:** The COSE-encoded identity keypair is stored in the database. If an attacker gains DB access, they could impersonate the gateway to the Bitwarden desktop app (but the desktop app still controls actual credential decryption).
|
|
|
|
2. **Memory cache window:** Credentials exist in memory for up to 60 seconds after first fetch. This is a reasonable trade-off for performance.
|
|
|
|
3. **Single credential at a time:** The design fetches one credential per request — no bulk operations that could leak multiple secrets.
|
|
|
|
---
|
|
|
|
## Conclusion
|
|
|
|
**Johan's suspicion is incorrect.** OneCLI does NOT download the entire Bitwarden vault. It uses Bitwarden's official Agent Access SDK to:
|
|
|
|
1. Establish an end-to-end encrypted Noise protocol channel to the Bitwarden desktop app
|
|
2. Request **individual credentials by domain** on-demand (just-in-time)
|
|
3. Cache only the specific credential for 60 seconds
|
|
4. Evict idle sessions after 30 minutes
|
|
|
|
This is a **well-architected, security-conscious design** that follows the principle of least privilege and minimizes credential exposure.
|
|
|
|
---
|
|
|
|
## References
|
|
|
|
- `apps/gateway/src/vault/bitwarden.rs` — Provider implementation
|
|
- `apps/gateway/src/vault/bitwarden_db.rs` — Identity and connection storage
|
|
- `apps/gateway/src/vault/mod.rs` — VaultProvider trait
|
|
- `apps/gateway/src/db.rs` — Database queries (vault_connections table)
|
|
- `apps/gateway/Cargo.toml` — Dependencies (`ap-client = "0.9.0"`)
|
|
- Bitwarden Agent Access SDK announcement: https://bitwarden.com/blog/introducing-agent-access-sdk/
|