apple / swift-crypto

Open-source implementation of a substantial portion of the API of Apple CryptoKit suitable for use on Linux platforms.
https://apple.github.io/swift-crypto
Apache License 2.0
1.45k stars 157 forks source link

How to set CBC padding behavior? #209

Closed lovetodream closed 10 months ago

lovetodream commented 10 months ago

Question Checklist

Question Subject

I'm currently using CryptoSwift for CBC de-/encryption and I'd like to move that to swift-crypto. For my use-case I have to disable automatic padding, is that possible?

Question Description

Currently I have the following code to decrypt a CBC encrypted payload sent from an Oracle db server during authenticating:

func decryptCBC(_ key: [UInt8], _ encryptedText: [UInt8]) throws -> [UInt8] {
    let iv = [UInt8](repeating: 0, count: 16)
    let aes = try AES(key: key, blockMode: CBC(iv: iv), padding: .noPadding)
    return try aes.decrypt(encryptedText)
}

I tried migrating it to swift-crypto and now it does automatic padding and fails with CryptoKitError.incorrectParameterSize, when trying to trim the padding:

func decryptCBC(_ key: [UInt8], _ encryptedText: [UInt8]) throws -> Data {
    let iv = [UInt8](repeating: 0, count: 16)
    return try AES._CBC.decrypt(encryptedText, using: .init(data: key), iv: AES._CBC.IV(ivBytes: iv)) // noPadding equivalent?
}

I guess I'll have the same problem on encryption.

// current (CryptoSwift):
func encryptCBC(_ key: [UInt8], _ plainText: [UInt8], zeros: Bool = false) throws -> [UInt8] {
    var plainText = plainText
    let blockSize = 16
    let iv = [UInt8](repeating: 0, count: blockSize)
    let n = blockSize - plainText.count % blockSize
    if n != 0, !zeros {
        plainText += Array<UInt8>(repeating: UInt8(n), count: n)
    }
    let aes = try AES(key: key, blockMode: CBC(iv: iv), padding: zeros ? .zeroPadding : .noPadding)
    var encryptor = try aes.makeEncryptor()
    return try encryptor.update(withBytes: plainText, isLast: true) + encryptor.finish()
}

// new (swift-crypto):
func encryptCBC(_ key: [UInt8], _ plainText: [UInt8], zeros: Bool = false) throws -> Data {
    var plainText = plainText
    let blockSize = 16
    let iv = [UInt8](repeating: 0, count: blockSize)
    let n = blockSize - plainText.count % blockSize
    if n != 0, !zeros {
        plainText += Array<UInt8>(repeating: UInt8(n), count: n)
    }
    return try AES._CBC.encrypt(plainText, using: .init(data: key), iv: AES._CBC.IV(ivBytes: iv)) // set padding behavior?
}

This is the related code: https://github.com/lovetodream/oracle-nio/blob/main/Sources/OracleNIO/Helper/Crypto.swift

Lukasa commented 10 months ago

This code is interesting: your padding is almost a perfect match for the PKCS#7 padding we use in Crypto except in the following cases:

  1. With zeros set to true, you pad with zeros. This is almost never a good thing to do, which makes me curious why Oracle does it.
  2. With zeros set to false, you pad using PKCS#8 except if you have a perfect match to the block size when you don't pad at all.

This is a very strange way to use CBC. No padding at all is a roughly acceptable thing to do, I think, but the zero padding mode is deeply confusing. Is it really used?

lovetodream commented 10 months ago

Zero padding is only used once (for debugging purposes), although I didn't implement that yet. It seems like there are other options for that too, but I'd have to ask someone at Oracle. It's not really necessary for the driver.

But enabling no padding would be required in order to do the authentication properly

Lukasa commented 10 months ago

So we can add an overload here that avoids the padding. It should be pretty easy really, it's much like the identical code with a check that the length is exactly a multiple of the block size and then skipping appending the final padding block. The existing implementation and the new one can funnel into the same shared code.

Would you be open to writing that patch?

lovetodream commented 10 months ago

Yeah, sure

lovetodream commented 10 months ago

Do you know of any test files I could include for testing the version without padding @Lukasa?

Lukasa commented 10 months ago

There isn't any good source for this: unpadded CBC is pretty rare. You may be able to find some in some RFCs.