Skip to content

Commit 8d57da1

Browse files
tcconnallyclaude
andauthored
security: never return ciphertext on decryption auth failure (#286)
On an encrypted DB the read path (entity_from_row), FTS reindex, and the history content-change check used decrypt(...).unwrap_or(raw), so a GCM/AAD authentication failure (wrong key or tampered/AAD-mismatched ciphertext) was swallowed and the raw ciphertext returned/indexed as the plaintext body — nullifying the AES-256-GCM integrity guarantee. Add EncryptionManager::decrypt_body which distinguishes decrypted plaintext, legacy plaintext (mixed DBs still work — JSON bodies are never valid base64), and authentication failure; read paths now refuse the bytes on failure (error sentinel + warning for recall; empty FTS entry so ciphertext is never indexed). 5 new tests (roundtrip, legacy passthrough, tamper/wrong-aad/ wrong-key). Full suite 103 passed. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent b4bff56 commit 8d57da1

3 files changed

Lines changed: 179 additions & 7 deletions

File tree

CHANGELOG.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,22 @@ All notable changes to Mimir are documented here. This project adheres to
55

66
## [Unreleased]
77

8+
### Security
9+
- **Decryption failures no longer silently return ciphertext.** On an encrypted DB,
10+
the read path (`entity_from_row`), FTS reindex, and the history content-change
11+
check used `decrypt(...).unwrap_or(raw)`, so any authentication failure — wrong
12+
key, or AAD-mismatched / tampered ciphertext (exactly what AES-256-GCM + AAD exist
13+
to detect) — was swallowed and the raw ciphertext was returned/indexed as if it
14+
were the plaintext body. That nullified the integrity guarantee: an attacker who
15+
could write to the DB file could tamper with a body and have it surface
16+
undetected. New `EncryptionManager::decrypt_body` classifies the input as
17+
decrypted plaintext, a legacy plaintext row (a real JSON body is never valid
18+
base64, so mixed DBs still work), or an authentication failure — and read paths
19+
now refuse to return the bytes on failure (a clear error sentinel + stderr warning
20+
for recall; an empty FTS entry so ciphertext is never indexed). Regression tests
21+
cover roundtrip, legacy-plaintext passthrough, and tamper / wrong-AAD / wrong-key
22+
rejection.
23+
824
## [2.7.0] - 2026-06-28
925

1026
### Distribution

src/db.rs

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -735,9 +735,21 @@ impl Database {
735735
let mut count = 0usize;
736736
for (rowid, category, key, raw_body) in rows {
737737
let aad = format!("{}:{}", category, key);
738-
// Fall back to raw on decrypt failure (e.g. a row written before
739-
// encryption was enabled), mirroring entity_from_row.
740-
let plain = enc.decrypt(&raw_body, aad.as_bytes()).unwrap_or(raw_body);
738+
// Index decrypted text, or a legacy plaintext row. On an
739+
// authentication failure (wrong key / tampered), index an empty
740+
// body rather than the ciphertext: putting ciphertext into the
741+
// plaintext FTS index would both leak it and corrupt search.
742+
let plain = match enc.decrypt_body(&raw_body, aad.as_bytes()) {
743+
crate::encryption::BodyDecrypt::Plaintext(s)
744+
| crate::encryption::BodyDecrypt::LegacyPlaintext(s) => s,
745+
crate::encryption::BodyDecrypt::AuthFailed(e) => {
746+
eprintln!(
747+
"mimir: reindex skipping body text for {}:{} — decryption {}.",
748+
category, key, e
749+
);
750+
"{}".to_string()
751+
}
752+
};
741753
insert.execute(params![rowid, plain])?;
742754
count += 1;
743755
}
@@ -1252,8 +1264,16 @@ impl Database {
12521264
.unwrap_or_default();
12531265
let old_plain_body = if let Some(ref enc) = self.encryption {
12541266
let aad = format!("{}:{}", entity.category, entity.key);
1255-
enc.decrypt(&old_raw_body, aad.as_bytes())
1256-
.unwrap_or_else(|_| old_raw_body.clone())
1267+
match enc.decrypt_body(&old_raw_body, aad.as_bytes()) {
1268+
crate::encryption::BodyDecrypt::Plaintext(s)
1269+
| crate::encryption::BodyDecrypt::LegacyPlaintext(s) => s,
1270+
// Can't authenticate the prior body: do NOT compare against
1271+
// ciphertext. Use a sentinel so content_changed is true and we
1272+
// conservatively snapshot history.
1273+
crate::encryption::BodyDecrypt::AuthFailed(_) => {
1274+
"\u{0}__mimir_undecryptable__".to_string()
1275+
}
1276+
}
12571277
} else {
12581278
old_raw_body.clone()
12591279
};
@@ -4543,8 +4563,24 @@ fn entity_from_row(
45434563
let cat: String = row.get(1)?;
45444564
let k: String = row.get(2)?;
45454565
let aad = format!("{}:{}", cat, k);
4546-
enc.decrypt(&raw_body_json, aad.as_bytes())
4547-
.unwrap_or(raw_body_json) // Fall back to raw if decryption fails (unencrypted DB)
4566+
match enc.decrypt_body(&raw_body_json, aad.as_bytes()) {
4567+
// Decrypted ciphertext, or a legacy plaintext row in a mixed DB.
4568+
crate::encryption::BodyDecrypt::Plaintext(s)
4569+
| crate::encryption::BodyDecrypt::LegacyPlaintext(s) => s,
4570+
// Authentic-looking ciphertext that failed GCM auth (wrong key or
4571+
// tampered). Never return the raw ciphertext — that would silently
4572+
// defeat the AES-256-GCM/AAD integrity guarantee. Surface a sentinel
4573+
// and warn instead.
4574+
crate::encryption::BodyDecrypt::AuthFailed(e) => {
4575+
eprintln!(
4576+
"mimir: refusing to return body for {}:{} — decryption {}. \
4577+
Wrong key or tampered ciphertext.",
4578+
cat, k, e
4579+
);
4580+
"{\"error\":\"mimir: body decryption failed (wrong key or tampered ciphertext)\"}"
4581+
.to_string()
4582+
}
4583+
}
45484584
} else {
45494585
raw_body_json
45504586
};

src/encryption.rs

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,21 @@ pub struct EncryptionManager {
99
cipher: Aes256Gcm,
1010
}
1111

12+
/// Result of attempting to decrypt a stored `body_json` in a possibly-mixed DB
13+
/// (one where encryption was enabled after some rows were already written plain).
14+
pub enum BodyDecrypt {
15+
/// Ciphertext that authenticated and decrypted successfully.
16+
Plaintext(String),
17+
/// The stored value is not Mimir ciphertext at all (a legacy plaintext row);
18+
/// it is safe to use as-is. JSON bodies always start with `{`, which is not in
19+
/// the base64 alphabet, so real plaintext is reliably classified here.
20+
LegacyPlaintext(String),
21+
/// The value WAS well-formed ciphertext but failed authentication — wrong key
22+
/// or tampered / AAD-mismatched data. The raw bytes MUST NOT be returned to the
23+
/// caller; doing so would silently defeat the AES-256-GCM integrity guarantee.
24+
AuthFailed(String),
25+
}
26+
1227
impl EncryptionManager {
1328
/// Load an encryption key from a base64-encoded key file.
1429
/// Supports `~` expansion for home directory paths.
@@ -101,4 +116,109 @@ impl EncryptionManager {
101116
String::from_utf8(plaintext)
102117
.map_err(|e| format!("Decrypted data is not valid UTF-8: {}", e))
103118
}
119+
120+
/// Mixed-DB-aware decrypt for stored bodies. Distinguishes a legacy plaintext
121+
/// row (not ciphertext at all -> safe to pass through) from authentic-looking
122+
/// ciphertext that fails GCM authentication (wrong key or tampering -> the raw
123+
/// value must NOT be used). This is the variant read paths should use: the old
124+
/// `decrypt(...).unwrap_or(raw)` pattern silently returned ciphertext on an
125+
/// auth failure, nullifying the AAD tamper-detection guarantee.
126+
pub fn decrypt_body(&self, encoded: &str, aad: &[u8]) -> BodyDecrypt {
127+
let combined = match B64.decode(encoded) {
128+
Ok(c) => c,
129+
// Not base64 -> cannot be our ciphertext -> legacy plaintext row.
130+
Err(_) => return BodyDecrypt::LegacyPlaintext(encoded.to_string()),
131+
};
132+
// Mimir ciphertext is nonce(12) + GCM tag(16) + body(>=0) = >= 28 bytes.
133+
// Anything shorter is not our ciphertext.
134+
if combined.len() < 12 + 16 {
135+
return BodyDecrypt::LegacyPlaintext(encoded.to_string());
136+
}
137+
let (nonce_bytes, ciphertext) = combined.split_at(12);
138+
let nonce = Nonce::from_slice(nonce_bytes);
139+
let payload = aes_gcm::aead::Payload {
140+
msg: ciphertext,
141+
aad: if aad.is_empty() { b"" } else { aad },
142+
};
143+
match self.cipher.decrypt(nonce, payload) {
144+
Ok(pt) => match String::from_utf8(pt) {
145+
Ok(s) => BodyDecrypt::Plaintext(s),
146+
Err(e) => BodyDecrypt::AuthFailed(format!("decrypted bytes not UTF-8: {}", e)),
147+
},
148+
Err(e) => BodyDecrypt::AuthFailed(format!("authentication failed: {}", e)),
149+
}
150+
}
151+
}
152+
153+
#[cfg(test)]
154+
mod tests {
155+
use super::*;
156+
157+
fn mgr() -> EncryptionManager {
158+
let key = Key::<Aes256Gcm>::from_slice(&[7u8; 32]);
159+
EncryptionManager {
160+
cipher: Aes256Gcm::new(key),
161+
}
162+
}
163+
164+
#[test]
165+
fn decrypt_body_roundtrip_is_plaintext() {
166+
let m = mgr();
167+
let ct = m.encrypt("{\"note\":\"hello\"}", b"cat:key").unwrap();
168+
match m.decrypt_body(&ct, b"cat:key") {
169+
BodyDecrypt::Plaintext(s) => assert_eq!(s, "{\"note\":\"hello\"}"),
170+
_ => panic!("expected Plaintext"),
171+
}
172+
}
173+
174+
#[test]
175+
fn legacy_plaintext_passes_through() {
176+
// A real JSON body starts with '{' (not base64) -> classified legacy plaintext.
177+
let m = mgr();
178+
match m.decrypt_body("{\"note\":\"legacy unencrypted row\"}", b"cat:key") {
179+
BodyDecrypt::LegacyPlaintext(s) => assert!(s.contains("legacy")),
180+
_ => panic!("expected LegacyPlaintext"),
181+
}
182+
}
183+
184+
#[test]
185+
fn tampered_ciphertext_is_authfailed_not_returned() {
186+
let m = mgr();
187+
let ct = m.encrypt("{\"secret\":\"x\"}", b"cat:key").unwrap();
188+
// Flip a byte in the base64 ciphertext body (after the nonce region).
189+
let mut bytes = ct.into_bytes();
190+
let i = bytes.len() - 4;
191+
bytes[i] = if bytes[i] == b'A' { b'B' } else { b'A' };
192+
let tampered = String::from_utf8(bytes).unwrap();
193+
match m.decrypt_body(&tampered, b"cat:key") {
194+
BodyDecrypt::AuthFailed(_) => {}
195+
BodyDecrypt::Plaintext(_) => panic!("tampered ciphertext authenticated (GCM broken?)"),
196+
BodyDecrypt::LegacyPlaintext(s) => {
197+
panic!("tampered ciphertext returned as plaintext: {}", s)
198+
}
199+
}
200+
}
201+
202+
#[test]
203+
fn wrong_aad_is_authfailed() {
204+
let m = mgr();
205+
let ct = m.encrypt("{\"a\":1}", b"cat:key").unwrap();
206+
match m.decrypt_body(&ct, b"different:aad") {
207+
BodyDecrypt::AuthFailed(_) => {}
208+
_ => panic!("AAD mismatch must fail authentication"),
209+
}
210+
}
211+
212+
#[test]
213+
fn wrong_key_is_authfailed() {
214+
let m = mgr();
215+
let ct = m.encrypt("{\"a\":1}", b"cat:key").unwrap();
216+
let other = EncryptionManager {
217+
cipher: Aes256Gcm::new(Key::<Aes256Gcm>::from_slice(&[9u8; 32])),
218+
};
219+
match other.decrypt_body(&ct, b"cat:key") {
220+
BodyDecrypt::AuthFailed(_) => {}
221+
_ => panic!("wrong key must fail authentication"),
222+
}
223+
}
104224
}

0 commit comments

Comments
 (0)