0

I am trying to use CommonCrypto to encrypt an NSMutableData object in place (copying the resulting bytes to itself, without duplicating it). Previously, I was using CCCrypt() "one-shot" method, mainly because it seemed simple. I noticed that my data object got duplicated in memory. To avoid this, I tried using an NSInputStream object with a buffer size of 2048 bytes. I am reading my NSMutableData object, and continuously call CCCryptorUpdate(), to handle the encryption. The problem is, that it still seems to be duplicated. Here's my current code (please note that it's a category on NSMutableData - mainly because of historical reasons - thus the "self" references):

- (BOOL)encryptWithKey:(NSString *)key
{
    // Key creation - not relevant to the dercribed problem
    char * keyPtr = calloc(1, kCCKeySizeAES256+1); 
    [key getCString: keyPtr maxLength: sizeof(keyPtr) encoding: NSUTF8StringEncoding];

    // Create cryptographic context for encryption
    CCCryptorRef cryptor;
    CCCryptorStatus status = CCCryptorCreate(kCCEncrypt, kCCAlgorithmAES128, kCCOptionECBMode, keyPtr, kCCKeySizeAES256, NULL, &cryptor);
    if (status != kCCSuccess)
    {
        MCLog(@"Failed to create a cryptographic context (%d CCCryptorStatus status).", status);
    }

    // Initialize the input stream
    NSInputStream *inStream = [[NSInputStream alloc] initWithData:self];
    [inStream open];
    NSInteger result;
    // BUFFER_LEN is a define 2048
    uint8_t buffer[BUFFER_LEN];
    size_t bytesWritten;

    while ([inStream hasBytesAvailable])
    {
        result = [inStream read:buffer maxLength:BUFFER_LEN];
        if (result > 0)
        {
            // Encryption goes here
            status = CCCryptorUpdate(
                                     cryptor,               // Previously created cryptographic context
                                     &result,               // Input data
                                     BUFFER_LEN,            // Length of the input data
                                     [self mutableBytes],   // Result is written here
                                     [self length],         // Size of result
                                     &bytesWritten          // Number of bytes written
                                     );

            if (status != kCCSuccess)
            {
                MCLog(@"Error during data encryption (%d CCCryptorStatus status)", status);
            }
        }
        else
        {
            // Error
        }
    }

    // Cleanup
    [inStream close];
    CCCryptorRelease(cryptor);
    free(keyPtr);
    return ( status == kCCSuccess );
}

I am definitely missing something obvious here, encryption, and even using input streams is a bit new to me..

rmaddy
  • 314,917
  • 42
  • 532
  • 579
József Vesza
  • 4,775
  • 3
  • 27
  • 42
  • What is the problem that is being caused by input and output buffers? – zaph May 14 '14 at 15:26
  • The problem is that there are large documents that need to be encrypted, and I don't want to have them duplicated in the memory. – József Vesza May 15 '14 at 08:04
  • So there really isn't a problem, just transient memory usage. Well, that is what the memory is for--to be used. This is called pre-mature optimization, optimizing prior to determining there is a problem. Almost always it is mis-spent time. – zaph May 15 '14 at 11:47
  • While the 60MB of memory probably isn't that big a deal, it is large enough and interesting enough to be a worthwhile learning experience. There's no problem exploring this, particularly as an optimization of already working code. Remember, temporary memory spikes can lead to unnecessary memory warnings on other apps, forcing them to dump caches or possibly even be killed, degrading the user's overall experience. It's worthwhile to try to reduce that (once you have a working solution, and without introducing excessive complexity). – Rob Napier May 19 '14 at 13:17

2 Answers2

2

As long as you only call CCUpdate() one time, you can encrypt into the same buffer you read from without using a stream. See RNCryptManager.m for an example. Study applyOperation:fromStream:toStream:password:error:. I did use streams here, but there's no requirement that you do that if you already have an NSData.

You must ensure that CCUpdate() is only called one time, however. If you call it multiple times it will corrupt its own buffer. This is an open bug in CommonCryptor (radar://9930555).

As a side note: your key generation is extremely insecure, and use of ECB mode for this kind of data barely qualifies as encryption. It leaves patterns in the ciphertext which can be used to decrypt the data, in some cases just by looking at it. I do not recommend this approach if you actually intend to secure this data. If you want to study how to use these tools well, see Properly Encrypting With AES With CommonCrypto. If you want a prepackaged solution, see RNCryptor. (RNCryptor does not currently have a convenient method for encrypting in-place, however.)

Rob Napier
  • 286,113
  • 34
  • 456
  • 610
  • The problem is that the document is large. I would like to have less data in the memory if possible, that's why I figured I would use a stream. (Thanks for the links though, I will definitely study them). – József Vesza May 15 '14 at 07:50
  • While converting in-place is certainly a valid use-case, often it is more convenient to simply decrypt or encrypt as you transfer the data. Look at RNCryptor's async interface for an example of that. For example, it can easily decrypt data as it's being downloaded over `NSURLConnection`. With a little bit of extra code it can decrypt while you're reading a stream or encrypt while you're writing it so you never need an encrypted version in memory. – Rob Napier May 15 '14 at 12:39
  • That is a perfectly valid observation. Unfortunately I am not in the position to change existing (even poor) solutions, so in-place encryption would be my only option. I have also tried solving it without a buffer (either by using the one-shot CCCrypt(), or the "usual" process with CCCreate() and CCCUpdate(), I have not managed to solve it. – József Vesza May 15 '14 at 13:08
  • You've tried making a single call to CCUpdate with the ciphertext and plaintext pointers addressing the same buffer? That should work as long as you only call it once. You may try turning off padding (it's not strictly necessary for ECB). That was another part of the CCUpdater bug. – Rob Napier May 15 '14 at 15:14
  • I have tried [this](https://gist.github.com/jozsef-vesza/fb2c2e75c022a00192a7), unfortunately it did not solve the issue. – József Vesza May 19 '14 at 12:05
  • Just to double-check the actual problem: what it the symptom you're seeing? Are you getting corruption? An error? What? You note "duplicated in memory," but what how do you know that, and what problem is it causing? Are you running getting memory warnings? – Rob Napier May 19 '14 at 12:48
  • No, the encryption itself works well, but I have the data twice in memory (e.g.: I have a 60 mb file, and at the end of the encryption **temporarily** I have a 120 mbs taken). I don't want duplicated memory use. – József Vesza May 19 '14 at 12:50
  • how are you determining that you have 120MBs taken? And what is the nature of the 120MBs? Is it dirty? What objects are in it and where are they allocated? Is your actual high-water mark going up? Do you have an Instruments trace? Determining actual memory usage is notoriously tricky in modern memory management systems. The OS uses lots of tricks. – Rob Napier May 19 '14 at 12:55
  • I have tried monitoring memory usage in both Xcode and Instruments (though I am a bit novice in the latter), and I see a sudden spike in memory usage at the end of my encryption function. Memory usage jumps up by the size of the document I'm trying to encrypt. – József Vesza May 19 '14 at 12:58
  • bbum provides a good introduction to HeapShot analysis in Instruments: http://www.friday.com/bbum/2010/10/17/when-is-a-leak-not-a-leak-using-heapshot-analysis-to-find-undesirable-memory-growth/. Start there. Instruments will tell you where the allocation happens. If it suddenly jumps up at the end, it's very possibly not where you think it is. You may be accidentally copying the `NSData` for instance. This kind of optimization work requires lots of evidence; be careful of assuming beforehand that you know what is happening. – Rob Napier May 19 '14 at 13:12
  • I've checked out heap shot analysis, unfortunately it was inconclusive, as this massive jump does not show up within the allocations (it can be seen in the graph). – József Vesza May 19 '14 at 14:58
1

In the line:

result = [inStream read:buffer maxLength:BUFFER_LEN];

the data is read into buffer and result is set to the outcome of the execution. in lines:

status = CCCryptorUpdate(cryptor, &result, ...

You should be using buffer for the input data, not the status

status = CCCryptorUpdate(cryptor, buffer, ...

Using better names would help eliminate the simple error. If instead of result the variable had been named readStatus the error would most likely not occurred. Likewise instead of naming rthe data variable buffer it had been named streamData things would also have been more clear. Poor naming really can cause errors.

zaph
  • 111,848
  • 21
  • 189
  • 228