Page MenuHomePhabricator

[lib][native] Add function to encrypt files
ClosedPublic

Authored by bartek on Mar 29 2023, 6:50 AM.
Tags
None
Referenced Files
F3516355: D7233.id24330.diff
Sun, Dec 22, 2:05 PM
F3515734: D7233.diff
Sun, Dec 22, 9:54 AM
Unknown Object (File)
Sat, Dec 14, 4:49 PM
Unknown Object (File)
Sat, Dec 14, 4:49 PM
Unknown Object (File)
Sat, Dec 14, 4:49 PM
Unknown Object (File)
Sat, Dec 14, 4:42 PM
Unknown Object (File)
Thu, Dec 5, 8:35 PM
Unknown Object (File)
Wed, Dec 4, 3:27 AM
Subscribers

Details

Summary

Created a function that opens, encrypts and the saves a single media file.

Depends on D7224, D7222

Test Plan

Tested together with subsequent diffs where it is used

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Mar 29 2023, 7:20 AM

Some questions inline

lib/types/media-types.js
243

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

Are we sure that the exception message here won't inadvertently leak any information about the file?

254

Are we sure that the exception message here won't inadvertently leak any information about the file?

258

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

Are we sure that the exception message here won't inadvertently leak any information about the file?

native/media/encryption-utils.js
37–38

Are we sure we don't want to encrypt or obfuscate the file name? Couldn't it cause information to be leaked?

47–48

My personal preference would be to have separate lines for success and exceptionMessage, but doesn't really matter

50

Was this extension chosen arbitrarily?

This revision is now accepted and ready to land.Mar 29 2023, 12:09 PM

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