Authenticated Encryption should check for tag length

[padde]

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.

---

For reference: This issue was copied over from the Ruby Issue Tracker https://bugs.ruby-lang.org/issues/12582

[padde]

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?

@rhenium I am not sure about other ciphers, but the current implementation also has a default value of 16, see https://github.com/ruby/openssl/blob/333899460950905622de50cd11593d6034211745/ext/openssl/ossl_cipher.c#L575

If we add other AE ciphers in the future we could still change the default value depending on the used cipher.

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.

I just want to make sure that users of OpenSSL::Cipher.new('aes-128-gcm') will get the same level of security as users of OpenSSL::Cipher::AES.new(128, :GCM), if this is possible. Everything else would be confusing IMHO.

There is another problem with OpenSSL::Cipher::AES right now, because we cannot initialize it with :GCM mode currently. OpenSSL lists the mode in lowercase, but OpenSSL::Cipher::AES wants to fetch the cipher in uppercase format, which does not seem to work. If we decide to implement additional features in Ruby we need to fix this first otherwise the additions will not be usable by anyone. There already is an issue in the Ruby Bug Tracker for this: https://bugs.ruby-lang.org/issues/8221

And of course, documentation to warn about truncated authentication tags should be added.

If we make sure that the API has secure defaults, I think we can limit the amount of additional documentation to a minimum.

[owlstead]

This issue was identified at the next link http://stackoverflow.com/q/38293211/589259

It seems to me that the idea to use secure defaults set to the maximum authentication tag size (16 bytes, the block size of AES) is the right one.

Shorter tag lengths quickly degrade the security of GCM, I would advice that this information is included in the documentation for setting smaller tag sizes.

Note that this is both a very much needed bug fix as well as one that may break compatibility for shorter tag sizes (although those implementations may also be vulnerable right now). I would recommend saying so in release notes in case anything breaks functionally speaking.

Maarten http://crypto.stackexchange.com/users/1172/maarten-bodewes http://stackoverflow.com/users/589259/maarten-bodewes

[noloader]

It seems to me that the idea to use secure defaults set to the maximum authentication tag size (16 bytes, the block size of AES) is the right one.

That's one way to do it - always use a fixed size tag. But the overall scheme may have gaps.

---

Shorter tag lengths quickly degrade the security of GCM, I would advice that this information is included in the documentation for setting smaller tag sizes.

NIST allows tag sizes as small as 32-bits, but they caution against it. 64-bit is usually the floor of the range, so they range in practice is usually [64, 128]. 96-bit seems to be most popular.

---

Note that this is both a very much needed bug fix as well as one that may break compatibility for shorter tag sizes

Yes, it should derive a unique key. That is, version 1 of the protocol should derive a distinct key from version 2 of the protocol. That's by design.

---

To avoid future problems, you should consider making all security parameters contribute to the keying material. For example:

protVer = version of the protocol
tagLen  = length of the tag
secret = key used in security context
... = other security parameters

Then, when you have a secret (from Password Derivation, Key Encapsulation, Diffie-Hellman, etc), use the security parameters and a usage label:

master_key = KDF(protVer || tagLen || secret || <other security parameters> )
gcm_key = KDF(master_key || "GCM key usage")
...

---

Using the version information and providing unique keys per version is important to stop downgrade attacks. For example, see Attacking and Repairing the WinZip Encryption Scheme.

You might also want to look at Kelsey's work on Truncated Hashes. Modern hashes, like BLAKE2, use the truncated size as a security parameter, so the hashes are distinct depending on the output hash size).

[rhenium]

Thank you for creating this issue.

As a general wrapper for EVP_CIPHER_CTX, a cipher-specific code or magic numbers should be minimized. In this point, in my opinion, the current default value of 16 in Cipher#auth_tag wasn't a good decision.

It's hard to change once added API. Changing the default value will certainly break any existing applications using AES-GCM. And who decide what cipher is the most used one?

Although OpenSSL::Cipher doesn't support now (our API doesn't fit, needs consideration), there is an usage of AES-CCM with 8 bytes authentication tag in TLS.

[padde]

@rhenium

As a general wrapper for EVP_CIPHER_CTX, a cipher-specific code or magic numbers should be minimized. In this point, in my opinion, the current default value of 16 in Cipher#auth_tag wasn't a good decision.

It's hard to change once added API. Changing the default value will certainly break any existing applications using AES-GCM. And who decide what cipher is the most used one?

If I read the code correctly, the only AE cipher that is implemented is currently AES-GCM. Please correct me if I'm wrong. I honestly don't see any problem with the default value of 16 for auth_tag_len, neither now nor in the future. For AES-GCM the value is the maximum tag length so it should be fine.

Although OpenSSL::Cipher doesn't support now (our API doesn't fit, needs consideration), there is an usage of AES-CCM with 8 bytes authentication tag in TLS.

Other ciphers could set different default values for auth_tag_len without breaking backwards compatibility. For example:

cipher = OpenSSL::Cipher.new('aes-256-gcm')
cipher.auth_tag_len #=> 16
# ...

cipher = OpenSSL::Cipher.new('aes-256-ccm')
cipher.auth_tag_len #=> 8
# ...

[padde]

@rhenium I agree that having cipher specific code in the general EVP_CIPHER_CTX wrapper is not the best idea, but this already happened when adding AE features to it. So, while it is not the cleanest approach, I think this is the place where this needs to be fixed.

[rhenium]

@padde

If I read the code correctly, the only AE cipher that is implemented is currently AES-GCM. Please correct me if I'm wrong. I honestly don't see any problem with the default value of 16 for auth_tag_len, neither now nor in the future. For AES-GCM the value is the maximum tag length so it should be fine.

It was. With 557e2de45e0f688e91c748efde72ae9c1086486e, if you build ext/openssl with a capable OpenSSL, (for instance) ChaCha20-Poly1305 should work.

Other ciphers could set different default values for auth_tag_len without breaking backwards compatibility. For example:

It doesn't resolve the problem. Users may use newer OpenSSL or use engines. What should be the default value for unknown AE ciphers?

[owlstead]

16 bytes is the only sensible default which is mostly backwards compatible with the current 16 byte authentication tag generated during GCM encryption, and setting no default would break backwards compatibility. So I don't think there is any choice here.

I'd say that defining the default as the maximum value possible would create a clear indication for future AEAD ciphers, although most will likely be 128 bit anyway (as 128 bit is the most used block size at the moment).

[tarcieri]

+1

• 16-bytes should be the default
• 8-bytes is the absolute minimum (not recommended by me though)
• Anything less than 8-bytes is completely broken/insecure
• 12-bytes is a more conservative minimum I would suggest instead of 8-bytes

[rhenium]

16 bytes is the only sensible default which is mostly backwards compatible with the current 16 byte authentication tag generated during GCM encryption, and setting no default would break backwards compatibility. So I don't think there is any choice here.

For Cipher#auth_tag(n), I'm not seeing the need to change the default length now, though I don't think it should have a default value.

I'd say that defining the default as the maximum value possible would create a clear indication for future AEAD ciphers, although most will likely be 128 bit anyway (as 128 bit is the most used block size at the moment).

As far as I know OpenSSL does not give the information of the maximal possible length.

We should avoid any heuristics in this library, especially in primitives. If we introduce a tag length check, it should be in each subclass of OpenSSL::Cipher because it can't be common to all AE ciphers, and should be done always with the maximal allowed in the algorithm.

[noloader]

16 bytes is the only sensible default which is mostly backwards compatible with the current 16 byte authentication tag generated during GCM encryption, and setting no default would break backwards compatibility. So I don't think there is any choice here.

For Cipher#auth_tag(n), I'm not seeing the need to change the default length now, though I don't think it should have a default value.

I think changing tag lengths and addressing the truncation attacks and potential forgeries are two separate issues. I think the thrust of @padde's issue was due to the latter.

My apologies if I am not parsing things correctly.

[padde]

There seems to be some misunderstanding about what I am trying to suggest. I don't want to change any default values, I just care about preventing truncation attacks. The reason I suggested the default value (16 bytes/128 bits) is to keep backwards compatibility.

Users may use newer OpenSSL or use engines. What should be the default value for unknown AE ciphers?

@rhenium how about we still add auth_tag_len to the generic wrapper, without setting a default value. It should be a required input iff cipher.authenticated? == true. When the value is missing, an error should be raised in #encrypt and `#decrypt.

We can then additionally set default values for known ciphers (whitelist), which provides backwards compatibility. If possible, we can move the defaults into specialized subclasses which seems the cleanest solution. However, if this means we don't get the default values with OpenSSL::Cipher.new('aes-256-gcm'), we would break backwards compatibility (existing code would produce an error that the auth_tag_len is missing). In this case I would advise to deploy a default value mechanism for known ciphers to the generic wrapper.

• 16-bytes should be the default
• 8-bytes is the absolute minimum (not recommended by me though)
• Anything less than 8-bytes is completely broken/insecure
• 12-bytes is a more conservative minimum I would suggest instead of 8-bytes

@tarcieri although I get your point, I think we should not enforce a lower bound. Ruby-OpenSSL is supposed to be a general purpose tool, and it should be able to decrypt/authenticate any valid AES-GCM ciphertext. This includes ciphertext with auth tags shorter than what would be considered secure. For the same reason, OpenSSL will not prevent you from using ECB mode and so on. I think the point is to provide secure defaults, while keeping backwards compatibility and full flexibility.

[rhenium]

Sorry for the confusion. It seems I misread @owlstead's comment.

@rhenium how about we still add auth_tag_len to the generic wrapper, without setting a default value. It should be a required input iff cipher.authenticated? == true. When the value is missing, an error should be raised in #encrypt and `#decrypt.

Why should the auth_tag_len be required even when encrypting?

I don't think users want to be requested to set the expected tag length just for a fail-safe (and especially when the algorithm doesn't need such check). I feel it could lead lazy users to write such code to suppress exceptions:

cipher.auth_tag_len = tag.bytesize
cipher.auth_tag = tag

A white list is not an option for the generic OpenSSL::Cipher. It requires us to keep up to date forever which we want to avoid. Even if we could, it still can't cover ciphers added by engines.

So, the only possible is to add subclasses and encourage the use of them in the documentation.

By the way, I'm going to include the documentation update anyway in the next release (hopefully in a week). Could you check it?

https://github.com/ruby/openssl/commit/31f270185bd3ec2c8e7f0a8dda42ee7b595ff2fc?w=1

[bdewater]

Now you are the receiver. You know the +key+ and +nonce+, and have received +encrypted+ and +tag+ through an untrusted network.

To me this seems to imply that you have to keep the IV as secret as the key, while the "Choosing an IV" section of the docs explain this is not the case.

It also might be confusing to suddenly switch to nonce for the AEAD examples. If you follow the current documentation in IRB you have set iv = cipher.random_iv earlier, while nonce is undefined. Same goes for the change for auth_data.

[rhenium]

@bdewater Thanks for reviewing!

To me this seems to imply that you have to keep the IV as secret as the key, while the "Choosing an IV" section of the docs explain this is not the case.

Right, it is confusing.

It also might be confusing to suddenly switch to nonce for the AEAD examples. If you follow the current documentation in IRB you have set iv = cipher.random_iv earlier, while nonce is undefined. Same goes for the change for auth_data.

I intentionally renamed it to nonce because it shouldn't be randomly generated, but it should be supplied from a counter or something.

diff --git a/ext/openssl/ossl_cipher.c b/ext/openssl/ossl_cipher.c
index c09fb2d..67bbd64 100644
--- a/ext/openssl/ossl_cipher.c
+++ b/ext/openssl/ossl_cipher.c
@@ -958,9 +958,11 @@ Init_ossl_cipher(void)
      * could otherwise be exploited to modify ciphertexts in ways beneficial to
      * potential attackers.
      *
-     * If no associated data is needed for encryption and later decryption,
-     * the OpenSSL library still requires a value to be set - "" may be used in
-     * case none is available.
+     * An associated data is used where there is additional information, such as
+     * headers or some metadata, that must be also authenticated but not
+     * necessarily need to be encrypted. If no associated data is needed for
+     * encryption and later decryption, the OpenSSL library still requires a
+     * value to be set - "" may be used in case none is available.
      *
      * An example using the GCM (Galois/Counter Mode). You have 16 bytes +key+,
      * 12 bytes (96 bits) +nonce+ and the associated data +auth_data+. Be sure
@@ -975,9 +977,9 @@ Init_ossl_cipher(void)
      *   encrypted = cipher.update(data) + cipher.final
      *   tag = cipher.auth_tag # produces 16 bytes tag by default
      *
-     * Now you are the receiver. You know the +key+ and +nonce+, and have
-     * received +encrypted+ and +tag+ through an untrusted network. Note that
-     * GCM accepts an arbitrary length tag between 1 and 16 bytes. You may
+     * Now you are the receiver. You know the +key+ and have received +nonce+,
+     * +auth_data+, +encrypted+ and +tag+ through an untrusted network. Note
+     * that GCM accepts an arbitrary length tag between 1 and 16 bytes. You may
      * additionally need to check that the received tag has the correct length,
      * or you allow attackers to forge a valid single byte tag for the tampered
      * ciphertext with a probability of 1/256.

Does this fix them? Or you may have a better idea.

[tarcieri]

+1 on using nonce instead of IV on AEAD examples: IVs must be random (or else you lose CPA security ala WEP) where nonces must only be used once, but could be a counter. They are distinct concepts.