Replies: 3 comments
|
Opt-in plus a consumer-supplied key is the right split. Looking at the current One detail for Decrypt failure as a cache miss matches the existing |
|
Thanks @adamantmm for the review — the size-accounting catch is right, though worth pinning down exactly where it bites. On the decrypt-retry point: already true by construction. A bigger gap: the two named hook points don't cover
|
|
Implemented and merged in #309, following the design revised through this discussion — chunked STREAM-construction AES-GCM for Closing as resolved. |
Uh oh!
There was an error while loading. Please reload this page.
Overview
DataCache's disk tier stores every entry fully in plaintext today — bothresponse.record(JSON metadata, including the original response's headers) anddata.record(the raw body), at a predictable path (<tmp>/com.request-dl-nio.Swift.Cache/<suiteName>), with a merely-obfuscated (reversible base64) key. This proposes optional, consumer-keyed encryption for the disk tier.Does this make sense? Depends what's being cached
A response cache is a copy of something already legitimately received over TLS — for public, non-sensitive content, encrypting it at rest is pure cost with no benefit. For anything carrying PII, session/auth-adjacent data, or regulated content, local-storage compromise (malware, a misconfigured sandbox boundary, physical access to an unlocked device) is a real, distinct threat from transport security. So this is opt-in, not on-by-default — the opposite default from the automatic cookie jar (#251), since there's a genuine performance cost and no universal benefit.
Key management, not encryption, is the hard part — and it's not RequestDL's job to own
AES-GCM via
swift-cryptois a solved problem. Secure key storage is a deeply platform-specific one (Keychain + accessibility classes + biometric gating on Darwin, nothing equivalent throughFoundationEssentialson Linux) that a networking library shouldn't try to own — same reasoning that kept Public Suffix List data and RFC 7616 hashing out of RequestDL's own responsibility in earlier proposals this session. So the key is consumer-supplied: the app decides how to store it (Keychain, a KMS, whatever fits its own threat model), RequestDL just uses it.API
Cache.EncryptionKeywrapsCrypto.SymmetricKeyrather than exposing it directly — matching this codebase's existing convention of never leaking an external type as public API (ResponseHead,HTTPHeadersboth wrap rather than re-export their NIO/AsyncHTTPClient equivalents). Threads through the same path capacity already takes:CacheConfigurationProperty→make.cacheConfiguration.encryptionKey(new field onInternals.CacheConfiguration) →.build(logger:)→DataCache(url:logger:encryptionKey:).This is a new direct dependency, not just a transitive one.
swift-cryptois already inPackage.resolvedtransitively (via the TLS chain), but RequestDL's ownPackage.swiftdoesn't declare it — this adds.product(name: "Crypto", package: "swift-crypto")to the main target, a real manifest change, unlike the PSL/SOCKS proposals which stayed consumer-side or purely transitive.Where it hooks in
DiskStorage.writeAndClose(_:to:)andreadResponseData(at:)(DiskStorage.swift:382/188) are the two places raw bytes cross the filesystem boundary. Encryption wraps right there — encrypt beforehandle.write, decrypt afterhandle.readToEnd— leaving capacity tracking, eviction, and key hashing above it untouched. Bothresponse.recordanddata.recordget encrypted:CachedResponseembedsInternals.ResponseHead, which carries the original response's headers (Set-Cookie, an echoed auth header, anything) — the metadata file is just as sensitive as the body.Scope: whole-blob authenticated encryption, not seekable/chunked
AES-GCM authenticates the entire ciphertext as one unit — no decrypting from an arbitrary offset the way plaintext
pread/pwriterandom access (Internals.FileStreamBuffer) currently allows. Since cached entries aren't served via partial/range reads today (both files are read start-to-finish on a cache hit, never sliced), whole-blob encryption is sufficient for v1. Chunked/independently-decryptable-block encryption would only be worth the real added complexity if partial cached-response reads become a need later.Nonce handling is close to free:
AES.GCM.SealedBox.combinedalready bundles a fresh random nonce with the ciphertext and auth tag in one blob — writesealedBox.combined, reconstruct withAES.GCM.SealedBox(combined:)on read.Failure handling
A decrypt failure (wrong/rotated key, corrupted or tampered file) is a cache miss —
nil, not a crash — consistent with the existingtry?-based fault tolerance already throughoutDiskStorage.Key rotation: freely replaceable, not locked
DataCache.Storageis shared and keyed by directory URL. The key must not be locked at first creation the way capacity-lowering is barred (Internals.Log.loweringCacheCapacityOnInitNotPermitted) — that precedent doesn't transfer, because a key is a rotating secret by design, not a structural resource limit. Supplying a new key just replaces it going forward on the shared storage. Reads under the new key against old-key-encrypted entries fail → become misses (per the failure handling above) → the next write for that entry re-encrypts with the current key. The cache self-heals to the new key organically, one entry at a time, through ordinary use — no special-case code needed.One real nuance: if a rotation happens because the old key may be compromised, lazy replacement alone doesn't fully address that — old entries stay on disk, still decryptable by anyone holding the old key, until naturally evicted. RequestDL doesn't bake automatic clearing into supplying a new key (not every rotation is compromise-driven, and unconditionally wiping the cache on every key change means a thundering-herd refill against the origin every time). Instead:
DataCache.removeAll()already exists as public API today — for a suspected-compromise rotation, the app calls it deliberately alongside supplying the new key. RequestDL provides the primitive; the app makes the call it's actually in a position to make.Out of scope (v1)
MemoryStoragestays unencrypted. Exfiltrating memory-resident data needs a live process dump or swapped pages — a narrower, harder threat than plaintext files sitting indefinitely in a predictable temp directory. Encrypting every in-memory cache hit would be real CPU cost against a threat most apps aren't defending against.Milestone: 4.2.0
All reactions