Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
dinigo opened this issue Mar 21, 2024 · 3 comments
Open
Assignees

Comments

@dinigo
Copy link

dinigo commented Mar 21, 2024

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

@tholenst
Copy link
Contributor

tholenst commented Mar 26, 2024

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 tholenst self-assigned this Apr 10, 2024
@tholenst
Copy link
Contributor

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
Copy link
Contributor

tholenst commented Jun 3, 2024

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants