Project

General

Profile

Bug #12582

Updated by patrick.oscity (Patrick Oscity) over 7 years ago

The current API for using ciphers with Authenticated Encryption (currently only AES-GCM) is rather misleading and quickly leads to subtle bugs related to the length of `auth_tag`. 

 In particular, the current implementation will _not_ check for the length of the `auth_tag`. Because GCM mode allows arbitrary sizes of the `auth_tag` up to 128 bytes, only a single byte needs to be supplied to make the authentication pass. This means that an attacker needs at most 256 attempts in order to forge a valid `auth_tag`. 

 ~~~ 
 data = 'secret' 

 cipher = OpenSSL::Cipher.new('aes-128-gcm') 
 cipher.encrypt 
 key = cipher.random_key 
 iv = cipher.random_iv 
 cipher.auth_data = 'auth_data' 
 ciphertext = cipher.update(data) + cipher.final 
 auth_tag = cipher.auth_tag 

 auth_tag = auth_tag[0] # single byte is sufficient 

 cipher = OpenSSL::Cipher.new('aes-128-gcm') 
 cipher.decrypt 
 cipher.key = key 
 cipher.iv = iv 
 cipher.auth_tag = auth_tag 
 cipher.auth_data = 'auth_data' 
 data = cipher.update(ciphertext) + cipher.final 

 # NO error raised 
 ~~~ 

 Currently, the only way to prevent such attacks is to manually assert the correct `auth_tag` length when decrypting/authenticating. 

 ~~~ 
 raise 'incorrect auth_tag length' unless auth_tag.length cipher.auth_tag.length == 16 
 ~~~ 

 I suggest the following improvements: 

 ### Documentation should mention the importance of manually checking `auth_tag` length 

 This can/should be done immediately even if the API should not change. 

 ### Authentication tag length should be an input parameter to the cipher 

 To improve the usability of the API and unburden users from performing additional manual checks without compromising security, I suggest to add an `auth_tag_len` accessor. This can be used to determine the size of the `auth_tag` both when generating and when authenticating the `auth_tag`.    The default value should be 16 bytes (see below). 

 ### `#auth_tag` should use `auth_tag_len` to determine the output length 

 During encryption: 

 If no parameter is given, `#auth_tag` should return an authentication tag according to the length configured in `auth_tag_len`. 

 If a length parameter is given, `#auth_tag` should use the supplied parameter to determine the length of the authentication tag. Although this parameter is not as useful any more it should be kept for backwards compatibility. Maybe it should be deprecated. 

 Currently the API supports different tag lengths by passing the length parameter to `#auth_tag`.    This currently defaults to 16 bytes, which should be the default value for `auth_tag_len` in order to keep backwards compatibility. 

 ### `#final` should use `auth_tag_len` to assert the correct length of the `auth_tag` 

 During decryption: 

 `auth_tag_len` should be used to assert that the supplied `auth_tag` has the correct length. The big difference to the existing API lies here, because users need to actively change the value of    `auth_tag_len` in order to allow shorter tags. 

 When the check fails, an `OpenSSL::Cipher::CipherError` should be raised. The same type of error is already raised when authentication fails, so existing users should be fine without having to touch their error handling. A descriptive error message should be helpful. In order to distinguish between such errors and "actual" verification errors, we could also add a descriptive message for the latter. 


 I'd be happy to implement these changes, but I wanted to discuss them first.

Back