Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First few characters in AES decrypted text are strange #70

Closed
chessboy opened this issue Apr 30, 2017 · 8 comments
Closed

First few characters in AES decrypted text are strange #70

chessboy opened this issue Apr 30, 2017 · 8 comments
Assignees
Labels

Comments

@chessboy
Copy link

chessboy commented Apr 30, 2017

Hi IDZ,

I'm getting a few strange characters at the beginning of the decrypted text using the following extensions on String, based on your example code. The [UInt8] data passed to decrypt is coming from a stored file (of the encrypted data). After the first few characters, the rest of the decrypted text is perfect.

Any help would be greatly appreciated! Thank you.

public extension String {

	func encryptAsData(key: [UInt8]) -> [UInt8]? {
				
		let cryptor = Cryptor(operation:.encrypt, algorithm: .aes, options: .PKCS7Padding, key: key, iv: Array<UInt8>())
		
		return cryptor.update(string: self)?.final()
	}
	
	static func decrypt(data: [UInt8], key: [UInt8]) -> String? {
		
		let cryptor = Cryptor(operation: .decrypt, algorithm: .aes, options: .PKCS7Padding, key: key, iv: Array<UInt8>())
		
		if let decryptedData = cryptor.update(byteArray: data)?.final() {
			
			return decryptedData.reduce("") { $0 + String(UnicodeScalar($1)) }
		}
		
		return nil
	}
}
@iosdevzone
Copy link
Owner

@chessboy sorry this issue slipped through the cracks. Did you find the cause of this issue?

@iosdevzone iosdevzone self-assigned this Oct 18, 2017
@iosdevzone
Copy link
Owner

This sounds like a initialization vector problem not a bug in the library. Closing this due to inactivity.

@csotiriou
Copy link

Actually, I am seeing this, too. What do you mean by "initialization vector problem"?

@iosdevzone
Copy link
Owner

I mean interpreting the initialization vector as part of the encrypted data. Usually happens when you are using one API to encrypt and another to decode and don't pass the data as each is expecting. If you send me some examples of what you are seeing and details of how you are encrypting/decrypting I can probably help you out.

@csotiriou
Copy link

csotiriou commented Nov 6, 2017

My full goal is to actually be able to encrypt an entire OAuth session (encoded into JSON using Swift 4's Codable) using a pin, hashed with PBKDF2. The result will be stored in the iOS keychain.

This is the class that I wrote in order to handle hashing and encryption:

open class MyCryptor {
    
    open class func encrypt(data : Data, withKey key : String) -> Data? {
        let keyable = arrayFrom(string: key)

        let cryptor = Cryptor(operation:.encrypt, algorithm:.aes, options:.PKCS7Padding, key:keyable, iv: [UInt8]())
        if let updatedData = cryptor.update(data: data), let final = updatedData.final() {
            return Data.init(bytes: final)
        }
        
        return nil
    }
    
    open class func decrypt(data : Data, withKey key: String) -> Data? {
        let keyable = arrayFrom(string: key)

        let cryptor = Cryptor.init(operation: .decrypt, algorithm: .aes, options: .PKCS7Padding, key:keyable, iv: [UInt8]())
        if let uint8Result = cryptor.update(data: data)?.final() {
            return Data.init(bytes: uint8Result)
        }
        return nil
    }
    
    open class func pbkdfDeriveKey(key : String, salt : String, rounds : Int = 1) -> Data {
        let keys6 = PBKDF.deriveKey(password: key, salt: salt, prf: .sha1, rounds: uint(rounds), derivedKeyLength: UInt(16))
        
        return Data.init(bytes: keys6)
    }
    
    open class func pkpdfCheckEquality(hashedData : Data, key : String, salt : String, rounds: Int = 1) -> Bool {
        let derivedKey = pbkdfDeriveKey(key: key, salt: salt, rounds: rounds).hexEncodedString()
        return derivedKey == hashedData.hexEncodedString()
    }
}

This is the session object that is about to be encoded:

public struct SecureSession : Encodable, Decodable {
        
        public var grantType : String?
        public var authenticationUrl : String?
        public var additionalParameters : [String : String] = [:]
        public var additionalHeaders : [String : String] = [:]
        public var authenticationLevel : Int = 0
        public var refreshToken : String?
        public var accessToken : String?
        public var tokenType : String?
        public var expiresIn : Double = 0

        public var isValid : Bool {
            return (
                    self.authenticationUrl != nil
                    && self.grantType != nil
                    && self.refreshToken != nil
                    && self.accessToken != nil
            )
        }
        
        public func encoded() -> Data? {
            if let data = try? JSONEncoder().encode(self) { return data }
            return nil
        }
    }

And this is the code that actually stores the data:

let hashedKey = EUCryptor.pbkdfDeriveKey(key: pin, salt: self.uuid, rounds: 1000)
        guard let encryptedSession = self.encrypted(session: session, usingHashedPin: hashedKey) else { return false }

        self.hashedPin = hashedKey
        self.encryptedSession = encryptedSession
        
        guard let decodedObject = try? JSONDecoder().decode(OAuth2SessionManager.SecureSession.self, from: decryptedSessionData) else {
            log.verbose("unable to decode data")
            return false
        }


func encrypted(session : OAuth2SessionManager.SecureSession, usingHashedPin hashedPin : Data) -> Data? {
        guard let aes256SessionEncodedKey = session.encoded() else { return nil }
        guard let encryptedSession = EUCryptor.encrypt(data: aes256SessionEncodedKey, withKey: hashedPin.hexEncodedString()) else { return nil }
        log.verbose("saved data : \(encryptedSession) using pin: \(hashedPin.hexEncodedString())")
        return encryptedSession
    }

Decryption:

if let hashedPin = self.hashedPin, let encryptedSession = self.encryptedSession {
                
                log.verbose("trying to device using hashed pin: \(hashedPin.hexEncodedString())")
                guard let decryptedSessionData = EUCryptor.decrypt(data: encryptedSession, withKey: hashedPin.hexEncodedString()) else {
                    completionBlock(nil, nil)
                    return
                }

The strange thing that I noticed is that when trying to restore the decrypted data immediately after storing it in the keychain, it works correctly.

However, when the application is closed and reopened, if I try to decrypt the session following the exact same procedure, the first few bytes are not decrypted properly. I initially thought that it had something to do with the way I store things in the keychain, however when I skip the encryption step, everything seems to work correctly...

@iosdevzone
Copy link
Owner

First of all, thanks for the great report. Was very useful to understand what was going on.

So, as I suspected, this is an initialization vector problem, but it also exposes a shortcoming in the error checking in IDZSwiftCommonCrypto; I never check whether the initialization vector is the correct length and this can result in uninitialized bytes being used as part of the IV.

To understand what's going on, take a look at the following image:

Data flow diagram of CBC mode decryption

If the initialization vector is incorrect the first block of plaintext will be corrupted. As long as the key is correct, subsequent blocks will not be affected since neither the IV or any value that depends on it is an input.

You can fix your code to properly us initialization vectors as follows:

open class EUCryptor {
    
    
    private class func generateInitializationVector(for algorithm: Cryptor.Algorithm) -> [UInt8] {
        let byteCount = algorithm.blockSize()
        var iv: [UInt8]
        do {
            iv = try Random.generateBytes(byteCount: byteCount)
        }
        catch {
            // I've never seen the above actually throw, but out of an abundance of
            // caution use `arc4_random` as a backup.
            iv = [UInt8](repeating: 0, count: byteCount)
            arc4random_buf(&iv, byteCount)
        }
        return iv
    }
    
    open class func encrypt(data : Data, withKey key : String) -> Data? {
        let keyable = arrayFrom(string: key)
        let algorithm = Cryptor.Algorithm.aes
        let iv = generateInitializationVector(for: algorithm)
        let cryptor = Cryptor(operation:.encrypt, algorithm: algorithm, options:.PKCS7Padding, key:keyable, iv: iv)
        if let updatedData = cryptor.update(data: data), let final = updatedData.final() {
            // Prepend the initialization vector to the encrypted data
            return Data.init(bytes: iv + final)
        }
        return nil
    }
    
    open class func decrypt(data : Data, withKey key: String) -> Data? {
        let keyable = arrayFrom(string: key)
        let algorithm = Cryptor.Algorithm.aes
        let blockSize = algorithm.blockSize()
        // Split the initialization vector and ciphertext
        let iv = [UInt8](data.prefix(blockSize))
        let ciphertext = data.suffix(from: blockSize)
        let cryptor = Cryptor.init(operation: .decrypt, algorithm: .aes, options: .PKCS7Padding, key:keyable, iv: iv)
        if let uint8Result = cryptor.update(data: ciphertext)?.final() {
            let reducedResult = uint8Result.reduce("") { $0 + String(UnicodeScalar($1)) }
            return reducedResult.data(using: .utf8)
        }
        return nil
    }
    
    open class func pbkdfDeriveKey(key : String, salt : String, rounds : Int = 1) -> Data {
        let keys6 = PBKDF.deriveKey(password: key, salt: salt, prf: .sha1, rounds: uint(rounds), derivedKeyLength: UInt(20))
        
        return Data.init(bytes: keys6)
    }
    
    open class func pkpdfCheckEquality(hashedData : Data, key : String, salt : String, rounds: Int = 1) -> Bool {
        let derivedKey = pbkdfDeriveKey(key: key, salt: salt, rounds: rounds).hexEncodedString()
        return derivedKey == hashedData.hexEncodedString()
    }
}

I'll add an error check to the library so that this cannot happen in future.

@csotiriou
Copy link

Thank you very much for the effort you put into this.

The sample code you provided works like a charm.

@iosdevzone
Copy link
Owner

You're welcome! Your input helped make the library better! I'm going to close this issue out now. Future versions of the library will have more stringent error checking which should ensure this does not catch any one else out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants