tink-java icon indicating copy to clipboard operation
tink-java copied to clipboard

AesSivKey supports 32, 48, and 64 bytes. But AesSivKeyManager only 64

Open dinigo opened this issue 1 year ago • 3 comments

Describe the bug:

AesSivParameters allows and validates keys of lengths 32, 48, and 64 bytes https://github.com/tink-crypto/tink-java/blob/main/src/main/java/com/google/crypto/tink/daead/AesSivParameters.java#L54-L69

However AesSivKeyManager Supports only 64

https://github.com/tink-crypto/tink-java/blob/main/src/main/java/com/google/crypto/tink/daead/AesSivKeyManager.java#L79-L87

What was the expected behavior?

For keys of 128 bit (32byte) to work. I suppose validateKeyFormat in keyFactory would need to accept all of the allowed lengths

  public void validateKeyFormat(AesSivKeyFormat format) throws GeneralSecurityException {
    final int[] VALID_KEY_SIZE_IN_BYTES = {32, 48, 64};
    for (int validKeySize : VALID_KEY_SIZE_IN_BYTES) {
      if (format.getKeySize() == validKeySize) {
        return;
      }
    }
    throw new InvalidAlgorithmParameterException(
        "invalid key size: "
            + format.getKeySize()
            + ". Valid keys must have "
            + VALID_KEY_SIZE_IN_BYTES
            + " bytes.");
  }

How can we reproduce the bug?

This code breaks when building the KeysetHandle aesSivKeyHandle because the key I'm using is 32 bytes long.

  public static DeterministicAead getCypherFromKey(String base64Key) throws GeneralSecurityException {
    DeterministicAeadConfig.register();
    SecretBytes keyMaterial = SecretBytes.copyFrom(Base64.decode(base64Key), InsecureSecretKeyAccess.get());
    int keyLength = Base64.decode(base64Key).length;
    // Needs to be registered before any key instantiation or configuration
    AesSivParameters aesSivParams = AesSivParameters.builder()
        .setKeySizeBytes(keyLength)
        .setVariant(AesSivParameters.Variant.NO_PREFIX)
        .build();
    AesSivKey aesSivKey = AesSivKey.builder()
        .setParameters(aesSivParams)
        .setKeyBytes(keyMaterial)
        .build();
    KeysetHandle.Builder.Entry keyEntry = KeysetHandle.importKey(aesSivKey)
        .withRandomId()
        .makePrimary()
        .setStatus(KeyStatus.ENABLED);
    KeysetHandle aesSivKeyHandle = KeysetHandle.newBuilder()
        .addEntry(keyEntry)
        .build();
    return aesSivKeyHandle.getPrimitive(DeterministicAead.class);
  }

Do you have any debugging information?

Exception in thread "main" java.security.InvalidKeyException: invalid key size: 32. Valid keys must have 64 bytes.
	at com.google.crypto.tink.daead.AesSivKeyManager.validateKey(AesSivKeyManager.java:106)
	at com.google.crypto.tink.daead.AesSivKeyManager.validateKey(AesSivKeyManager.java:58)
	at com.google.crypto.tink.internal.KeyManagerImpl.validateKeyAndGetPrimitive(KeyManagerImpl.java:138)
	at com.google.crypto.tink.internal.KeyManagerImpl.getPrimitive(KeyManagerImpl.java:66)
	at com.google.crypto.tink.Registry.getPrimitive(Registry.java:508)
	at com.google.crypto.tink.Registry.getPrimitive(Registry.java:534)
	at com.google.crypto.tink.internal.RegistryConfiguration.getLegacyPrimitive(RegistryConfiguration.java:47)
	at com.google.crypto.tink.KeysetHandle.getLegacyPrimitiveOrNull(KeysetHandle.java:1173)
	at com.google.crypto.tink.KeysetHandle.getPrimitiveWithKnownInputPrimitive(KeysetHandle.java:1095)
	at com.google.crypto.tink.KeysetHandle.getPrimitive(KeysetHandle.java:1134)
	at com.google.crypto.tink.KeysetHandle.getPrimitive(KeysetHandle.java:1143)
	at com.acme.crypto.AesSivCipher.getCypherFromKey(TinkExample.java:69)

What version of Tink are you using?

1.12.0.

Can you tell us more about your development environment?

JDK 11

dinigo avatar Mar 21 '24 12:03 dinigo

The fact that Tink only supports 64 byte keys is intentional. See https://developers.google.com/tink/deterministic-aead#choose_a_key_type.

However, we could create a separate configuration which supports 32 byte keys. The code would then look as follows:

    AesSivParameters aesSivParams = AesSivParameters.builder()
        .setKeySizeBytes(keyLength)
        .setVariant(AesSivParameters.Variant.NO_PREFIX)
        .build();
    AesSivKey aesSivKey = AesSivKey.builder()
        .setParameters(aesSivParams)
        .setKeyBytes(keyMaterial)
        .build();
    KeysetHandle.Builder.Entry keyEntry = KeysetHandle.importKey(aesSivKey)
        .withRandomId()
        .makePrimary()
        .setStatus(KeyStatus.ENABLED);
    KeysetHandle aesSivKeyHandle = KeysetHandle.newBuilder()
        .addEntry(keyEntry)
        .build();
    return aesSivKeyHandle.getPrimitive(AesSiv32ByteConfig.get(), DeterministicAead.class);

(Note: it only differs in the last line). The name of the config would have to be decided.

tholenst avatar Mar 26 '24 07:03 tholenst

The question is whether this satisfies users.

However, looking at the code it also doesn't seem right that https://cloud.google.com/sensitive-data-protection/docs/pseudonymization simply supports 32 byte keys (though I don't really understand the documentation)

tholenst avatar May 14 '24 08:05 tholenst

We currently don't plan to change this. Please comment if you need this, or maybe ping the team who is interested in this to contact us internally (I wouldn't know how to find the people who want this).

tholenst avatar Jun 03 '24 12:06 tholenst