Skip to content

Key destroyed too soon #23

@overheadhunter

Description

@overheadhunter

The fix for #20 will pass in a copy of a key to Cipher.init(...), because the Cipher implementation may destroy this key:

try (DestroyableSecretKey clone = key.clone()) {
cipher.init(ciphermode, clone, params); // use cloned key, as this may destroy key.getEncoded()
return cipher;
} catch (InvalidKeyException e) {

However, this causes a regression on other implementations that expect the key to outlive the Cipher. For example, on Android the key is still used in some operations like Cipher.doFinal():

javax.crypto.AEADBadTagException: error:1e000065:Cipher functions:OPENSSL_internal:BAD_DECRYPT
	at java.lang.reflect.Constructor.newInstance0(Native Method)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:343)
	at com.android.org.conscrypt.OpenSSLAeadCipher.throwAEADBadTagExceptionIfAvailable(OpenSSLAeadCipher.java:273)
	at com.android.org.conscrypt.OpenSSLAeadCipher.doFinalInternal(OpenSSLAeadCipher.java:302)
	at com.android.org.conscrypt.OpenSSLCipher.engineDoFinal(OpenSSLCipher.java:374)
	at javax.crypto.Cipher.doFinal(Cipher.java:2055)

This means we have to deal with three kinds of Cipher implementations:

  1. Ciphers that read the key only during init (and may keep an internal copy)
  2. Ciphers that destroy the key at some point (as in JDK 17)
  3. Ciphers that keep a reference to the original key and access it after initialization (as in Android)

Our current solution only covers 1+2. I.e., we need to partially revert 8139787 and make a short-lived copy of DestroyableSecretKey in KeyWrap itself.

Metadata

Metadata

Labels

No labels
No labels

Type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions