Implements file operations that work with encryption-decryotion layer defined in previous diffs
Details
Modify NotificationService and AppDelegate so that the first writes to the file upon notification and the second read it on AppLaunch. Examine in the debugger that the content read by AppDelegate is consistent sent notifications.
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
A test plan could contain a description of a code that was written to test this, e.g. a modification to AppDelegate that is run when the app starts, writes something to a file, reads it, decrypt, and checks the result.
As a question:
We have two options to write to the file
- We write from the beginning
- We append to it
At the same time, when decrypting, we do that for the whole content at the same time.
Is the decryption algorithm able to recognize where one encrypted block ends and another begins? Should we read and decrypt block-by-block? Should we separate blocks by some char, e.g. a new line? Have you tested if after writing and appending we are able to decrypt both parts?
I agree it is not a good practice to postpone writing test plan for an API-diff until we have a diff with its usage. I will provide test plans for all "No test plan" I have already submitted. However it is not a first time I submit "Not test plan. Future diffs will provide test plan", but it is the first time I am advised against it. Perhaps it we should persist this rule somewhere so that no one repeats this mistake in the future.
As a question:
We have two options to write to the file
- We write from the beginning
- We append to it
At the same time, when decrypting, we do that for the whole content at the same time.
Is the decryption algorithm able to recognize where one encrypted block ends and another begins? Should we read and decrypt block-by-block? Should we separate blocks by some char, e.g. a new line? Have you tested if after writing and appending we are able to decrypt both parts?
Thanks for pointing it out! I wanted to use "comm.encryptionKey" for encryption, which is 512 bit long. The only algorithm supported by CCCrypt that accepts 512 key is RC4, which happens to be stream-oriented encryption algorithm (https://pl.wikipedia.org/wiki/RC4). This is why I was able encrypt in chunks and receive exactly the same content after decrypting as a whole. This article however raises some concerns whether RC4 is a secure algorithm. Perhaps we will have to use block-oriented encryption and read in chunks as well.
Should we separate blocks by some char, e.g. a new line?
Newline separation takes place in further diffs. Each usage of [EncryptedFileUtils appendData:toFileAtPAth:] passes message with prepended newline separator. But if we decide to use block-oriented encryption we will have to move it to EncryptedFileUtils.
We could use AES-256 with the first 256 bits. Not familiar with RC4 but if it's an efficient/safe symmetric encryption algorithm, we're probably fine
I decided to give up on RC4 since it is not considered secure and most sources I found advise against using it in modern projects. I went with AES working in ECB mode. AES in ECB mode encrypts data in 128 byte blocks treating each block independently. Therefore, by implementing padding with newline characters we can encrypt single messages independently and decrypt entire file content at once, which gives us convenient api to work with file. However AES working in CBC mode is considered more secure but every block is encrypted with previous block, so encryption operation is not additive. That would require to split file content according to some separator and decrypt each message independently. Implementation and API would be less convenient but we would align with top-notch security standards. @ashoat , @karol-bisztyga , @palys-swm what are your opinions on this matter?
According to this response of Apple staff member: https://developer.apple.com/forums/thread/129611, we should be compliant with TLS 1.2 which according to this: https://docs.microsoft.com/en-us/power-platform/admin/server-cipher-tls-requirements does not support AES ECB, but supports AES CBC which is provided by iOS. Therefore I will reimplement file operations to use AES CBC.
Now file operations work as follows:
- Write:
- Message represented as string (NSString) is encrypted with AES CBC to binary (NSData)
- Encrypted binary is encoded to base64.
- Newline character is appended to a file
- Encrypted binary is appended to a file
- Read:
- File content is read into a string
- String is split with spearator being newline character
- Each slice is independently decrypted and decoded from base64 to the original message
Advantages:
- We use AES CBC wich is accepted by TLS 1.2
- No need to worry about messages containing separator characters.
- If one message was written partially (due to error) it does not affect other messages being correctly decrypted and read
- If it appears that reading entire file content into memory is erroneous (too large) this code can be easily refactored to load messages into memory one-by-one as separate lines
native/ios/Comm/TemporaryMessageStorage/EncryptedFileUtils.mm | ||
---|---|---|
19–21 ↗ | (On Diff #14273) | It can be null in two cases:
|
38 ↗ | (On Diff #14273) | closeFile method of NSFileHandle can throw an exception itself - https://developer.apple.com/documentation/foundation/nsfilehandle/1413393-closefile. It can be noted here that I am using deprecated NSFileHandle API, but I do it on purpose since newer methods are not available before iOS 13. Never version of those methods do the same things but the only difference is that they do not throw exception but overwrite NSError variable. |