Bug 1975760 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

We need to add encryption support to our Bucket File System (OPFS) implementation in order to make it work in Private Browsing Mode (PBM).  While we already have the basic encrypted primitives necessary for this, we only have read OR write encrypted stream implementations not a read-write stream implementation, and honestly they need to be cleaned up a bit.  That is likely to be a lot of the implementation.
We need to add encryption support to our Bucket File System (OPFS) implementation in order to make it work in Private Browsing Mode (PBM).  While we already have the basic encrypted primitives necessary for this, we only have read OR write encrypted stream implementations not a read-write stream implementation, and honestly they need to be cleaned up a bit.  That is likely to be a lot of the implementation.

My specific concerns about the encrypted stream implementations as it relates to cleaning up the logic, likely as part of implementing a read/write stream:
- The current decrypting stream really likes to use Tell/Seek on the underlying stream and itself and this makes the code more complex and harder to reason about than I think is helpful in addition to the potential spurious I/O.  (There are also [3 XXX comments in the code relating to this area from the original author](https://searchfox.org/firefox-main/rev/974b0546f470bf87a34d636a03607d788ba9305f/dom/quota/DecryptingInputStream_impl.h#443,451,466).)  For example, [mozilla::dom::quota::DecryptingInputStream::Available](https://searchfox.org/firefox-main/rev/974b0546f470bf87a34d636a03607d788ba9305f/dom/quota/DecryptingInputStream_impl.h#72) performs 2 self-tells and 2 self-seeks (as in on the decrypting stream, not on the underlying stream), and then [mozilla::dom::quota::DecryptingInputStream::Seek](https://searchfox.org/firefox-main/rev/974b0546f470bf87a34d636a03607d788ba9305f/dom/quota/DecryptingInputStream_impl.h#385) also does a self-tell.
- The current chunked format [stores the actual payload length in the clear](https://searchfox.org/firefox-main/rev/974b0546f470bf87a34d636a03607d788ba9305f/dom/quota/EncryptedBlock.h#20-24) which greatly reduces the privacy characteristics of the encrypted storage since we lose the 4k quantization from our [4k block size](https://searchfox.org/firefox-main/rev/974b0546f470bf87a34d636a03607d788ba9305f/dom/cache/FileUtils.cpp#50-54).  Given that we use a fixed block size there really is no need for this to be stored in the clear.  Storing it as part of the encrypted payload additionally would allow us to potentially engage in adding more than 4k of padding.
  - This is something that must be addressed if we are potentially thinking about making the storage durable for other uses.

There are also some related concerns in this space that would be nice to address:
- Bug 1938479 tracks how our encrypted blob implementation may potentially perform sync I/O from threads that should not be performing sync I/O and are not expecting to perform sync I/O.  With the current implementation, this can and does happen and is partly because the decrypting input stream is so eager to do disk I/O to return answers to Available() (and potentially perturbing the state of the underlying fd).
- Bug 1959527 tracks a systemic problem in how we identify buffered input streams and which will cause our decrypting input stream to perform sync I/O when we're trying to figure out if it's buffering or not (and potentially perturbing the state of the underlying fd).

Much of my experience in this space came from encountering these problems (and a failure of the decrypting input stream to correctly transfer across over IPC, the aforementioned perturbation of the underlying fd) as part of debugging ServiceWorkers in Private Browsing Mode.  https://phabricator.services.mozilla.com/D233624 and https://phabricator.services.mozilla.com/D241157 were targeted fixes I made to the decrypting input stream implementation that provide context and discussion in this space that may be useful for people to check out.

Back to Bug 1975760 Comment 0