AzureAD / microsoft-authentication-library-common-for-objc

Common code used by both the Active Directory Authentication Library (ADAL) and the Microsoft Authentication Library (MSAL)
MIT License
34 stars 36 forks source link

Specify an initialization vector in msidAES128DecryptedDataWithKey #990

Closed br-alex-reyes closed 3 years ago

br-alex-reyes commented 3 years ago

In NSData+AES.m line 48, a null input is used for the iv parameter. When using CBC mode, this results in an initialization vector that is all zeroes. This makes the ciphertext more predictable and leaves it susceptible to a dictionary attack. CCCryptorStatus cryptStatus = CCCrypt(kCCDecrypt, kCCAlgorithmAES128, kCCOptionPKCS7Padding, key, keySize, NULL /* initialization vector (optional) */, [self bytes], dataLength, /* input */ buffer, bufferSize, /* output */ &numBytesDecrypted); Using an initialization vector with sufficient length from a random data source would result in a more secure encryption.

ameyapat commented 3 years ago

Hi @br-alex-reyes , Thanks for bringing light to this issue. It is indeed recommended to use a randomized IV for each operation in CBC but in our case the encrypted message communication is client-client. Our message also contains a unique nonce value with each request and hence for the same request, a different cipher will be produced, which is also the point of a randomized IV.

Since the attack would be difficult and the development & backward compatibility effort to use a randomized IV is much greater than what the benefits of doing it are, I'm closing the issue.