Bug #12582
openOpenSSL Authenticated Encryption should check for tag length
Description
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 == 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.
Updated by patrick.oscity (Patrick Oscity) about 8 years ago
- Description updated (diff)
Updated by patrick.oscity (Patrick Oscity) about 8 years ago
- Description updated (diff)
Updated by rhenium (Kazuki Yamaguchi) about 8 years ago
- Assignee set to rhenium (Kazuki Yamaguchi)
The development of ext/openssl has moved to GitHub recently. If you have a GitHub account, could you make an issue there?
https://github.com/ruby/openssl
I prefer keeping OpenSSL::Cipher as general as possible. I'm not sure about the default value, 16 bytes. It's usually OK with AES-GCM but how about other AE ciphers that will be added in the future?
One possible approach would be to extend the subclasses of OpenSSL::Cipher. We already have OpenSSL::Cipher::AES (see lib/openssl/cipher.rb in the repository) but we currently do nothing in it.
And of course, documentation to warn about truncated authentication tags should be added.
Updated by patrick.oscity (Patrick Oscity) about 8 years ago
Ok let's continue the discussion on Github: https://github.com/ruby/openssl/issues/63
Kazuki Yamaguchi wrote:
The development of ext/openssl has moved to GitHub recently. If you have a GitHub account, could you make an issue there?
Updated by shyouhei (Shyouhei Urabe) over 7 years ago
- Status changed from Open to Assigned