Details
Tested together with subsequent diffs where it is used
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Some questions inline
| lib/types/media-types.js | ||
|---|---|---|
| 243 ↗ | (On Diff #24330) | Are we going to encrypt or obfuscate the filename here? Are we sure this won't unintentionally leak (or increase the risk of leaking) any information about the encrypted media? |
| 246 ↗ | (On Diff #24330) | Are we sure that the exception message here won't inadvertently leak any information about the file? |
| 254 ↗ | (On Diff #24330) | Are we sure that the exception message here won't inadvertently leak any information about the file? |
| 258 ↗ | (On Diff #24330) | Is this file name going to be the same as the source file name? Or is it going to be encrypted/obfuscated in some way? Thoughts on sourceFile in read_plaintext_file and destinationFile in write_encrypted_file? |
| 261 ↗ | (On Diff #24330) | Are we sure that the exception message here won't inadvertently leak any information about the file? |
| native/media/encryption-utils.js | ||
| 37–38 ↗ | (On Diff #24330) | Are we sure we don't want to encrypt or obfuscate the file name? Couldn't it cause information to be leaked? |
| 47–48 ↗ | (On Diff #24330) | My personal preference would be to have separate lines for success and exceptionMessage, but doesn't really matter |
| 50 ↗ | (On Diff #24330) | Was this extension chosen arbitrarily? |
Pasting my answers from offline chat:
Ok, a few quick notes here:
- These files are temporary removed right after uploading
- But keyserver for some reason stores the filename
- These filenames are only reported in media mission
- The mission report already leaks lots of sensitive information in previous steps, encryption is yet another step on already-displayed filename
- Message recipient device doesn't receive original filename
- No opinion on extension - I just kept original filename + replaced extension to .dat to keep it simple.
- I'm okay with just setting this to random filename, without extension at all
- Native exception messages for encryption related stuff are rather vague: e.g. The operation could not be completed
These are my observations, correct me if I'm wrong