AES-128 Encryption/Decryption of NSData

Question

I recently found an Objective-C extension on NSData that encodes and decodes data with AES-128. I made an attempt to port this code to Swift 2.2:

NSData+AES.swift

extension NSData {

    struct AES {
        static let privateKey = "DefaultPrivateKey"
    }

    func AES128EncryptedDataWithKey(key: String = AES.privateKey) -> NSData? {
        return AES128Operation(CCOperation(kCCEncrypt), key: key)
    }

    func AES128DecryptedDataWithKey(key: String = AES.privateKey) -> NSData? {
        return AES128Operation(CCOperation(kCCDecrypt), key: key)
    }

    private func AES128Operation(operation: CCOperation, key: String) -> NSData? {
        var keyPtr = [CChar](count: kCCKeySizeAES128 + 1, repeatedValue: 0)
        key.getCString(&keyPtr, maxLength: keyPtr.count, encoding: NSUTF8StringEncoding)

        let dataLength = self.length
        let bufferSize = dataLength + kCCBlockSizeAES128
        let buffer = malloc(bufferSize)

        var numBytesEncrypted = 0
        let cryptStatus = CCCrypt(operation, CCAlgorithm(kCCAlgorithmAES128), CCOptions(kCCOptionPKCS7Padding), keyPtr, kCCBlockSizeAES128, nil, self.bytes, dataLength, buffer, bufferSize, &numBytesEncrypted)

        if cryptStatus == CCCryptorStatus(kCCSuccess) {
            // bytesNoCopy will transfer ownership to the NSData object returned, so we don't need to explicitly call free here
            return NSData(bytesNoCopy: buffer, length: numBytesEncrypted)
        }

        // If the operation failed, we never transferred memory ownership, so free the buffer
        free(buffer)

        return nil
    }
}

A few notes:

• I keep a default key as a private struct in the extension so the operation function can be called without arguments for encoding/decoding with a default key.

• No need to call bzero anymore since we init the buffer with a repeatedValue of 0.

• In my first pass of this in Swift, I accidentally was passing in the size of the array (pointer) as the maxLength parameter in getCString (Stack Overflow question here). Is there any way I could make the code clearer so I don't do that in the future?

• Is it necessary to malloc a buffer? Can I use a Swift [Int8] instead?

• According to my boss in a passive comment, AES-128 is "known to be compromised." How complex would it be to refactor this code to use AES-256 or (192) bit encryption?

Answer 1

Disclaimer: I am not an expert on cryptography or security, so please take everything with respect to these topics with a grain (or more) of salt. Note also that there are ready-made crypto libraries (such as RNCryptor) so you might want to use one of those.

I do not see the advantage of a default key – only the risk that you use it for some critical task. I would suggest to remove the default value.

CCCrypt() in encryption mode can only fail if the provided buffer is too small, or if padding is disabled. Both cases cannot occur in your code. Therefore there is no need to make the return value of the encryption method an optional:

func AES128EncryptedDataWithKey(key: String) -> NSData {
    return AES128Operation(CCOperation(kCCEncrypt), key: key)!
}

Your getCString() code is quite clear. There are various methods to convert a C string to a byte buffer. Here are two alternatives which don't require the address-of operator &:

var keyData = Array(key.utf8)
if keyData.count < kCCKeySizeAES128 {
    keyData += Repeat(count: kCCKeySizeAES128 - keyData.count, repeatedValue: 0)
}

or

let keyData = key.dataUsingEncoding(NSUTF8StringEncoding)!.mutableCopy() as! NSMutableData
if keyData.length < kCCKeySizeAES128 {
    keyData.length = kCCKeySizeAES128 // fills with zeros
}
// Use `keyData.bytes` to access the bytes.

(Note that these unwrappings/casts cannot fail.) Any of these methods is fine, use whatever you feel most confident with.

The allocated buffer is "a bit" too large, the required size for PKCS7 padding is the "next multiple" of the block size:

let bufferSize = (self.length/kCCBlockSizeAES128 + 1) * kCCBlockSizeAES128

The most reliable method is to determine the required buffer size by calling CCCrypt() with an empty buffer:

var bufferSize = 0
let status = CCCrypt(operation, CCAlgorithm(kCCAlgorithmAES128), CCOptions(kCCOptionPKCS7Padding), keyData, kCCBlockSizeAES128, nil, self.bytes, self.length, nil, 0, &bufferSize)
assert(status == CCCryptorStatus(kCCBufferTooSmall))

According to the CCCrypt documentation, this requires only a "minimal runtime penalty."

Yes, you can use a Swift array instead of malloc():

var buffer = [UInt8](count:bufferSize, repeatedValue: 0)
// ...
return NSData(bytes: buffer, length: dataOutAvailable)

var numBytesEncrypted is misleading, it can be the number of encrypted or decrypted bytes. CCCrypt() uses dataOutAvailable for that parameter.

As I understand it from Wikipedia: AES, AES 128/192/256 differ only in the size of the key, which means that you have to replace kCCKeySizeAES128 by kCCKeySizeAES192 or kCCKeySizeAES256. (But as I said, I am not an expert on this topic.)

You have chosen CBC (cipher block chaining) mode and not ECB mode (as in the referenced Objective-C code) – which is good. But you should also add an additional parameter to provide an initialization vector., or the possibility to generate a random one. Currently the corresponding parameter is nil so that a block of zeros is used. See – for example – Why use an Initialization Vector (IV)? for more information.