AES with CommonCrypto uses too much memory - Objec

2019-03-26 21:06发布

问题:

My goal is to be able to, being given a file/folder and a password, encrypt and decrypt it in AES using Objective-C. I'm no crypto nerd or anything, but I chose AES because I found it was pretty standard and very secure. I am using a NSMutableData category which has methods for encrypting and decrypting it's data. Here it is:

- (NSInteger)AES256EncryptionWithKey: (NSString*)key {
    // The key should be 32 bytes for AES256, will be null-padded otherwise
    char keyPtr[kCCKeySizeAES256 + 1]; // room for terminator (unused)
    bzero(keyPtr, sizeof(keyPtr));     // fill with zeroes (for padding)

    // Fetch key data
    if (![key getCString: keyPtr maxLength: sizeof(keyPtr) encoding: NSUTF8StringEncoding])
    { return 2; } // Length of 'key' is bigger than keyPtr

    NSUInteger dataLength = [self length];

    // See the doc: For block ciphers, the output size will always be less than or 
    // equal to the input size plus the size of one block.
    // That's why we need to add the size of one block here
    size_t bufferSize = dataLength + kCCBlockSizeAES128;
    void* buffer = malloc(bufferSize);

    size_t numBytesEncrypted = 0;
    CCCryptorStatus cryptStatus = CCCrypt(kCCEncrypt, kCCAlgorithmAES128, kCCOptionPKCS7Padding,
                                          keyPtr, kCCKeySizeAES256,
                                          NULL ,                    // initialization vector (optional)
                                          [self bytes], dataLength, // input bytes and it's length
                                          buffer, bufferSize,       // output buffer and it's length
                                          &numBytesEncrypted);      // ??
    if (cryptStatus == kCCSuccess) {
        // The returned NSData takes ownership of the buffer and will free it on deallocation
        [self setData: [NSData dataWithBytesNoCopy: buffer length: numBytesEncrypted]];
        return 0;
    }

    free(buffer); // Free the buffer;
    return 1;
}

- (NSInteger)AES256DecryptionWithKey: (NSString*)key {
    // The key should be 32 bytes for AES256, will be null-padded otherwise
    char keyPtr[kCCKeySizeAES256 + 1]; // room for terminator (unused)
    bzero(keyPtr, sizeof(keyPtr));     // fill with zeroes (for padding)

    // Fetch key data
    if (![key getCString: keyPtr maxLength: sizeof(keyPtr) encoding: NSUTF8StringEncoding])
    { return 2; } // Length of 'key' is bigger than keyPtr

    NSUInteger dataLength = [self length];

    // See the doc: For block ciphers, the output size will always be less than or 
    // equal to the input size plus the size of one block.
    // That's why we need to add the size of one block here
    size_t bufferSize = dataLength + kCCBlockSizeAES128;
    void* buffer = malloc(bufferSize);

    size_t numBytesDecrypted = 0;
    CCCryptorStatus cryptStatus = CCCrypt(kCCDecrypt, kCCAlgorithmAES128, kCCOptionPKCS7Padding,
                                          keyPtr, kCCKeySizeAES256,
                                          NULL, // initialization vector (optional)
                                          [self bytes], dataLength, // input
                                          buffer, bufferSize, // output
                                          &numBytesDecrypted);

    if (cryptStatus == kCCSuccess) {
        // The returned NSData takes ownership of the buffer and will free it on deallocation
        [self setData: [NSData dataWithBytesNoCopy: buffer length: numBytesDecrypted]];
        return 0;
    }

    free(buffer); // Free the buffer;
    return 1;
}

The problem with this code is that it uses about !! 5 !! times in memory the size of the file (opened with NSMutableData) that the user chooses. This is completely unacceptable from the user's perspective (imagine encrypting a file which is 2Gb - 10Gb in memory??), but I am really at a loss here.

Can you suggest any modification that would solve this problem? Probably encrypting one chunk at a time (that way only one chunck or two is in memory at the same time, not the entire file * 5). The big problem with that is that I don't know how to do it. Any ideas?

Thanks

PS: When I use this category, I do it this way:

NSMutableData* data = [NSMutableData dataWithContentsOfFile: @"filepath"];
[data AES256EncryptionWithKey: @"password"];
[data writeToFile: @"newname" atomically: NO];

And just these 3 lines create such a big memory problem.

OH, by the way: do I need an initialization vector? I think it is more secure, or something, but I don't know. If there is really a need, could you tell me how to do it?

EDIT

This is now what I am doing:

NSMutableData* data = [NSMutableData dataWithContentsOfMappedFile: @"filepath"];
[data SafeAES256EncryptionWithKey: @"password"];
[data writeToFile: @"newname" atomically: NO];

And the new method in the category:

- (void)SafeAES256EncryptionWithKey: (NSString*)key {
    // The key should be 32 bytes for AES256, will be null-padded otherwise
    char keyPtr[kCCKeySizeAES256 + 1]; // room for terminator (unused)
    bzero(keyPtr, sizeof(keyPtr));     // fill with zeroes (for padding)

    // Fetch key data
    if (![key getCString: keyPtr maxLength: sizeof(keyPtr) encoding: NSUTF8StringEncoding])
    { return 2; } // Length of 'key' is bigger than keyPtr

    CCCryptorRef cryptor;
    CCCryptorStatus cryptStatus = CCCryptorCreate(kCCEncrypt, kCCAlgorithmAES128, kCCOptionPKCS7Padding,
                                                  keyPtr, kCCKeySizeAES256,
                                                  NULL, // IV - needed?
                                                  &cryptor);

    if (cryptStatus != kCCSuccess) {
        ; // Handle error here
    }

    NSInteger startByte;

    size_t dataOutMoved;
    size_t dataInLength = kChunkSizeBytes; // #define kChunkSizeBytes (16)
    size_t dataOutLength = CCCryptorGetOutputLength(cryptor, dataInLength, FALSE);

    const void* dataIn = malloc(dataInLength);
    void* dataOut = malloc(dataOutLength);
    for (startByte = 0; startByte <= [self length]; startByte += kChunkSizeBytes) {
        if ((startByte + kChunkSizeBytes) > [self length]) { dataInLength = [self length] - startByte; }
        else { dataInLength = kChunkSizeBytes; }

        NSRange bytesRange = NSMakeRange(startByte, (int)dataInLength);

        [self getBytes: dataIn range: bytesRange];
        CCCryptorUpdate(cryptor, dataIn, dataInLength, dataOut, dataOutLength, &dataOutMoved);

        if (dataOutMoved != dataOutLength) {
            NSLog(@"dataOutMoved != dataOutLength");
        }

        [self replaceBytesInRange: bytesRange withBytes: dataOut];

    }

    CCCryptorFinal(cryptor, dataOut, dataOutLength, &dataOutMoved);
    [self appendBytes: dataOut length: dataOutMoved];

    CCCryptorRelease(cryptor);

I can't understand why this sometimes works and other times it doesn't. I am really at a loss here. Could someone please check this code?

In order not to load all the file into memory at once, I use -dataWithContentsOfMappedFile, and then call -getBytes:range:, because I saw here that that way it wouldn't load all the file into real memory at once, only the specified range.

EDIT 2

Please see my answer for what I am doing now.

回答1:

I decided to leave the confortable Objc-C land and rewrote the second NSMutableData category up above with C functions. I did my best, but it would not surprise me if there are flaws in this code, so please make suggestions! I also dropped the category 'scheme' and decided to do a stand-alone method instead. Here:

// What do you think this number should be? 16B, 256B...? 1KB, 1MB? Please tell me
#define kChunkSizeBytes (1024*1024) // 1 MB

- (BOOL)cryptFile: (NSString*)oldFPath
           toFile: (NSString*)newFPath
     withPassword: (NSString*)password
     andOperation: (CCOperation)operation
{
    // READ PASSWORD

    // The key should be 32 bytes for AES256, will be null-padded otherwise
    char keyPtr[kCCKeySizeAES256 + 1]; // room for terminator (unused)
    bzero(keyPtr, sizeof(keyPtr));     // fill with zeroes (for padding)

    // Fetch key data
    if (![password getCString: keyPtr maxLength: sizeof(keyPtr) encoding: NSUTF8StringEncoding])
    { return FALSE; } // Length of 'key' is bigger than keyPtr

    // CREATE CRYPTOR

    CCCryptorRef cryptor;
    CCCryptorStatus cryptStatus = CCCryptorCreate(operation, kCCAlgorithmAES128, kCCOptionPKCS7Padding,
                                                  keyPtr, kCCKeySizeAES256,
                                                  NULL, // IV - needed?
                                                  &cryptor);

    if (cryptStatus != kCCSuccess) {
        return FALSE; // Handle error here
    }

    // OPEN OLD FILE AND READ SIZE

    FILE* oldFile = fopen([oldFPath UTF8String], "rb");
    if(oldFile == NULL) {
        return FALSE; // Could not open old file
    }

    fseek(oldFile, 0, SEEK_END);  
    size_t oldFileSize = ftell(oldFile);
    fseek(oldFile, 0, SEEK_SET);

    // OPEN NEW FILE

    FILE* newFile = fopen([newFPath UTF8String], "ab");
    if(newFile == NULL) {
        return FALSE; // Could not open new file
    }

    // ..CRYPT

    NSInteger byteOffset;

    size_t dataOutMoved;
    size_t dataInLength = kChunkSizeBytes;
    size_t dataOutLength = CCCryptorGetOutputLength(cryptor, dataInLength, FALSE);

    const void* dataIn = malloc(dataInLength);
    void* dataOut = malloc(dataOutLength);

    // ..crypt data one chunk at a time
    for (byteOffset = 0; byteOffset <= oldFileSize; byteOffset += kChunkSizeBytes) {
        if ([[NSThread currentThread] isCancelled]) { break; }

        if ((byteOffset + kChunkSizeBytes) > oldFileSize) { dataInLength = oldFileSize - byteOffset; }
        else { dataInLength = kChunkSizeBytes; }

        fseeko(oldFile, byteOffset, SEEK_SET);
        fread(dataIn, 1, dataInLength, oldFile);

        CCCryptorUpdate(cryptor, dataIn, dataInLength, dataOut, dataOutLength, &dataOutMoved);

        fwrite(dataOut, 1, dataOutMoved, newFile);
    }

    // If thread hasn't been cancelled, finalize
    if (![[NSThread currentThread] isCancelled]) {
        CCCryptorFinal(cryptor, dataOut, dataOutLength, &dataOutMoved);
        fwrite(dataOut, 1, dataOutMoved, newFile);
    }

    // CLOSE AND RELEASE
    free(dataIn);
    free(dataOut);
    fclose(oldFile);
    fclose(newFile);
    CCCryptorRelease(cryptor);

    return TRUE;
}

I know there is no error checking inside the 'for' loop, and that may also be the case elsewhere. Suggestions on that please! There is some code there that checks if a thread has been cancelled. That's because this code is run on a separate thread which my class controls. Whenever the user clicks the "Cancel" button, the thread I created is sent the cancel message. Those if's make sure that the thread actually cancels. Feel free to make suggestions (once again!) and to use this code wherever you feel like it :)

PS: I have tested this, both with encryption and decryption and it has worked flawlessly so far. My initial problem (too much memory) seems to be solved as well!



回答2:

Here is an modified implementation of Alex's original code using a category on NSMutableData. It has been thoroughly unit tested with in-memory data, as well as [NSMutableData dataWithContentsOfMappedFile:"filePath"]; with file of varying length, from a few bytes to 50 MB.

NSMutableData+Crypto.h

@interface NSMutableData (Crypto)

- (BOOL)encryptWithKey:(NSString *)key;
- (BOOL)decryptWithKey:(NSString *)key;

@end

And NSMutableData+Crypto.m

#define kChunkSizeBytes (1024 * 1024) // 1 MB

@implementation NSMutableData (Crypto)

/**
* Performs a cipher on an in-place buffer
*/
-(BOOL) doCipher:(NSString *)key operation: (CCOperation) operation
{
    // The key should be 32 bytes for AES256, will be null-padded otherwise
    char keyPtr[kCCKeySizeAES256 + 1]; // room for terminator (unused)
    bzero(keyPtr, sizeof(keyPtr));     // fill with zeroes (for padding)

    // Fetch key data
    if (![key getCString:keyPtr maxLength:sizeof(keyPtr) encoding:NSUTF8StringEncoding]) {return FALSE;} // Length of 'key' is bigger than keyPtr

    CCCryptorRef cryptor;
    CCCryptorStatus cryptStatus = CCCryptorCreate(operation, kCCAlgorithmAES128, kCCOptionPKCS7Padding,
            keyPtr, kCCKeySizeAES256,
            NULL, // IV - needed?
            &cryptor);

    if (cryptStatus != kCCSuccess) { // Handle error here
        return FALSE;
    }

    size_t dataOutMoved;
    size_t dataInLength = kChunkSizeBytes; // #define kChunkSizeBytes (16)
    size_t dataOutLength = CCCryptorGetOutputLength(cryptor, dataInLength, FALSE);
    size_t totalLength = 0; // Keeps track of the total length of the output buffer
    size_t filePtr = 0;   // Maintains the file pointer for the output buffer
    NSInteger startByte; // Maintains the file pointer for the input buffer

    char *dataIn = malloc(dataInLength);
    char *dataOut = malloc(dataOutLength);
    for (startByte = 0; startByte <= [self length]; startByte += kChunkSizeBytes) {
        if ((startByte + kChunkSizeBytes) > [self length]) {
            dataInLength = [self length] - startByte;
        }
        else {
            dataInLength = kChunkSizeBytes;
        }

        // Get the chunk to be ciphered from the input buffer
        NSRange bytesRange = NSMakeRange((NSUInteger) startByte, (NSUInteger) dataInLength);
        [self getBytes:dataIn range:bytesRange];
        cryptStatus = CCCryptorUpdate(cryptor, dataIn, dataInLength, dataOut, dataOutLength, &dataOutMoved);

        if (dataOutMoved != dataOutLength) {
            NSLog(@"dataOutMoved (%d) != dataOutLength (%d)", dataOutMoved, dataOutLength);
        }

        if ( cryptStatus != kCCSuccess)
        {
            NSLog(@"Failed CCCryptorUpdate: %d", cryptStatus);
        }

        // Write the ciphered buffer into the output buffer
        bytesRange = NSMakeRange(filePtr, (NSUInteger) dataOutMoved);
        [self replaceBytesInRange:bytesRange withBytes:dataOut];
        totalLength += dataOutMoved;

        filePtr += dataOutMoved;
    }

    // Finalize encryption/decryption.
    cryptStatus = CCCryptorFinal(cryptor, dataOut, dataOutLength, &dataOutMoved);
    totalLength += dataOutMoved;

    if ( cryptStatus != kCCSuccess)
    {
        NSLog(@"Failed CCCryptorFinal: %d", cryptStatus);
    }

    // In the case of encryption, expand the buffer if it required some padding (an encrypted buffer will always be a multiple of 16).
    // In the case of decryption, truncate our buffer in case the encrypted buffer contained some padding
    [self setLength:totalLength];

    // Finalize the buffer with data from the CCCryptorFinal call
    NSRange bytesRange = NSMakeRange(filePtr, (NSUInteger) dataOutMoved);
    [self replaceBytesInRange:bytesRange withBytes:dataOut];

    CCCryptorRelease(cryptor);

    free(dataIn);
    free(dataOut);

    return 1;
}


- (BOOL)encryptWithKey:(NSString *)key {
    return [self doCipher:key operation:kCCEncrypt];
}

- (BOOL)decryptWithKey:(NSString *)key {
    return [self doCipher:key operation:kCCDecrypt];
}

Note to Alex: your original attempt simply failed to take into account the CCCryptorUpdate may not return the same number of bytes on output as it does input. For example, decrypting 1024 bytes often returns 1008 bytes, as 16 bytes are used for padding. Your example using C file functions took this into account by writing to a new file, rather than replacing memory in-place. I've just implemented a file position pointer similar to your file output method. Thanks for getting me started!