Both versions have the same problem: you tell CommonCrypto to write past the end of your buffer, and then ignore the result.
First version:
[self mutableBytes], [self length] + kCCBlockSizeAES128,
Second version:
// Calculate byte block alignment for all calls through to and including final. bufferPtrSize = CCCryptorGetOutputLength(thisEncipher, plainTextBufferSize, true); // Allocate buffer. bufferPtr = [self mutableBytes];
This is not true. You are not highlighting anything. You say write bufferPtrSize bytes to a buffer of size [self length] !
You want to do something more similar (if you really want to encrypt in place):
// Calculate byte block alignment for all calls through to and including final. bufferPtrSize = CCCryptorGetOutputLength(thisEncipher, plainTextBufferSize, true); // Increase my size if necessary: if (bufferPtrSize > self.length) { self.length = bufferPtrSize; }
I am also not sure why encryption is in place, while decryption is not; the latter is easier to do if you like.
Your second version has additional problems:
- You release what you do not allocate:
if(bufferPtr) free(bufferPtr); - You may have read the end of the line:
(const void *)[key UTF8String], kCCKeySizeAES128
Additional cryptological issues:
- Keys are assumed to be fixed and have a decent amount of entropy. Converting a string to bytes naively is not a good key (for example, keys longer than 16 bytes are effectively truncated). The least you could do is a hash. You can also iterate the hash or just use PBKDF2 (though I did not find the spec / test vectors for PBKDF2 ...)
- You almost certainly want to use random IV too (see SecRandomCopyBytes).
Addendum:
The reason you see the truncated result is because you are returning a truncated response (with the addition of PKCS7, the encrypted result is always larger than the original data). The odds (around 255/256) are that the last block of ciphertext was incorrectly padded (because you gave truncated CCryptor data), so ccStatus says that an error occurred, but you ignored this and returned the result anyway. This is an incredibly bad practice. (Also, you really want to use the MAC address with CBC to avoid the oracle security cold .)
EDIT:
Some code that seems to work looks something like this (complete with test cases):
Notes:
- In fact, I have not tested it on iOS (although the iOS code should not work on iOS, namely that SecRandomCopyBytes is a slightly more convenient interface, but not available in OS X).
- The read () loop may be right, but not fully verified.
- The ciphertext has the prefix IV. This is a textbook method, but makes ciphertexts large.
- There is no authentication, so this code can act as a profile.
- There is no support for AES-192 or AES-256. It is not difficult to add (you just need to include the key length and choose the right algorithm).
- The key is listed as NSData, so you need to do something like
[string dataUsingEncoding:NSUTF8StringEncoding] . For bonus points, run it through CC_SHA256 and take the first 16 output bytes. - There is no operation in place. I didn’t think it was worth it.
.
#include <Foundation/Foundation.h> #include <CommonCrypto/CommonCryptor.h> #if TARGET_OS_IPHONE #include <Security/SecRandom.h> #else #include <fcntl.h> #include <unistd.h> #endif @interface NSData(AES) - (NSData*) encryptedDataUsingAESKey: (NSData *) key; - (NSData*) decryptedDataUsingAESKey: (NSData *) key; @end @implementation NSData(AES) - (NSData*) encryptedDataUsingAESKey: (NSData *) key { uint8_t iv[kCCBlockSizeAES128]; #if TARGET_OS_IPHONE if (0 != SecRandomCopyBytes(kSecRandomDefault, sizeof(iv), iv)) { return nil; } #else { int fd = open("/dev/urandom", O_RDONLY); if (fd < 0) { return nil; } ssize_t bytesRead; for (uint8_t * p = iv; (bytesRead = read(fd,p,iv+sizeof(iv)-p)); p += (size_t)bytesRead) { // 0 means EOF. if (bytesRead == 0) { close(fd); return nil; } // -1, EINTR means we got a system call before any data could be read. // Pretend we read 0 bytes (since we already handled EOF). if (bytesRead < 0 && errno == EINTR) { bytesRead = 0; } // Other errors are real errors. if (bytesRead < 0) { close(fd); return nil; } } close(fd); } #endif size_t retSize = 0; CCCryptorStatus result = CCCrypt(kCCEncrypt, kCCAlgorithmAES128, kCCOptionPKCS7Padding, [key bytes], [key length], iv, [self bytes], [self length], NULL, 0, &retSize); if (result != kCCBufferTooSmall) { return nil; } // Prefix the data with the IV (the textbook method). // This requires adding sizeof(iv) in a few places later; oh well. void * retPtr = malloc(retSize+sizeof(iv)); if (!retPtr) { return nil; } // Copy the IV. memcpy(retPtr, iv, sizeof(iv)); result = CCCrypt(kCCEncrypt, kCCAlgorithmAES128, kCCOptionPKCS7Padding, [key bytes], [key length], iv, [self bytes], [self length], retPtr+sizeof(iv),retSize, &retSize); if (result != kCCSuccess) { free(retPtr); return nil; } NSData * ret = [NSData dataWithBytesNoCopy:retPtr length:retSize+sizeof(iv)]; // Does +[NSData dataWithBytesNoCopy:length:] free if allocation of the NSData fails? // Assume it does. if (!ret) { free(retPtr); return nil; } return ret; } - (NSData*) decryptedDataUsingAESKey: (NSData *) key { const uint8_t * p = [self bytes]; size_t length = [self length]; if (length < kCCBlockSizeAES128) { return nil; } size_t retSize = 0; CCCryptorStatus result = CCCrypt(kCCDecrypt, kCCAlgorithmAES128, kCCOptionPKCS7Padding, [key bytes], [key length], p, p+kCCBlockSizeAES128, length-kCCBlockSizeAES128, NULL, 0, &retSize); if (result != kCCBufferTooSmall) { return nil; } void * retPtr = malloc(retSize); if (!retPtr) { return nil; } result = CCCrypt(kCCDecrypt, kCCAlgorithmAES128, kCCOptionPKCS7Padding, [key bytes], [key length], p, p+kCCBlockSizeAES128, length-kCCBlockSizeAES128, retPtr, retSize, &retSize); if (result != kCCSuccess) { free(retPtr); return nil; } NSData * ret = [NSData dataWithBytesNoCopy:retPtr length:retSize]; // Does +[NSData dataWithBytesNoCopy:length:] free if allocation of the NSData fails? // Assume it does. if (!ret) { free(retPtr); return nil; } return ret; } @end void test(NSData * data, NSData * key) { NSLog(@"%@, %@", data, key); NSData * enc = [data encryptedDataUsingAESKey:key]; NSLog(@"%@", enc); NSData * dec = [enc decryptedDataUsingAESKey:key]; NSLog(@"%@", dec); NSLog((data == dec || [data isEqual:dec]) ? @"pass" : @"FAIL"); } int main() { #define d(x) [NSData dataWithBytesNoCopy:("" x) length:sizeof("" x)-1 freeWhenDone:0] [NSAutoreleasePool new]; NSData * key = d("0123456789abcdef"); test([NSData data], key); test(d(""), key); test(d("a"), key); test(d("0123456789abcde"), key); test(d("0123456789abcdef"), key); test(d("0123456789abcdef0"), key); test(d("0123456789abcdef0123456789abcde"), key); test(d("0123456789abcdef0123456789abcdef"), key); test(d("0123456789abcdef0123456789abcdef0"), key); }