Skip to content

Commit

Permalink
Migrate more KeyManagers to use createKeyFromRandomness instead of de…
Browse files Browse the repository at this point in the history
…riveKey.

PiperOrigin-RevId: 566943270
Change-Id: Ibcf073db343d236fad96238d9bc69818762b637d
  • Loading branch information
tholenst authored and copybara-github committed Sep 20, 2023
1 parent 627ad03 commit b0b138d
Show file tree
Hide file tree
Showing 8 changed files with 163 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,18 @@

import static com.google.crypto.tink.internal.TinkBugException.exceptionIsBug;

import com.google.crypto.tink.AccessesPartialKey;
import com.google.crypto.tink.Aead;
import com.google.crypto.tink.KeyTemplate;
import com.google.crypto.tink.Mac;
import com.google.crypto.tink.Parameters;
import com.google.crypto.tink.Registry;
import com.google.crypto.tink.SecretKeyAccess;
import com.google.crypto.tink.config.internal.TinkFipsUtil;
import com.google.crypto.tink.internal.KeyTypeManager;
import com.google.crypto.tink.internal.MutableParametersRegistry;
import com.google.crypto.tink.internal.PrimitiveFactory;
import com.google.crypto.tink.internal.Util;
import com.google.crypto.tink.mac.HmacKeyManager;
import com.google.crypto.tink.proto.AesCtrHmacAeadKey;
import com.google.crypto.tink.proto.AesCtrHmacAeadKeyFormat;
Expand All @@ -44,6 +47,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;

/**
* This key manager generates new {@link AesCtrHmacAeadKey} keys and produces new instances of
Expand Down Expand Up @@ -125,44 +129,47 @@ public AesCtrHmacAeadKey createKey(AesCtrHmacAeadKeyFormat format)
.build();
}

// To ensure that the derived key can provide key commitment, the AES-CTR key must be derived
// before the HMAC key.
// Consider the following malicious scenario using a brute-forced key InputStream with a 0 as
// its 32nd byte:
// 31 bytes || 1 byte of 0s || 16 bytes
// We give this stream to party A, saying that it is 32-byte HMAC key || 16-byte AES key. We
// also give this stream to party B, saying that it is 31-byte HMAC key || 16-byte AES key.
// Since HMAC pads the key with zeroes, this same stream will lead to both parties using the
// same HMAC key but different AES keys.
@Override
public AesCtrHmacAeadKey deriveKey(
KeyTypeManager<AesCtrHmacAeadKey> keyManager,
AesCtrHmacAeadKeyFormat format,
InputStream inputStream)
public com.google.crypto.tink.aead.AesCtrHmacAeadKey createKeyFromRandomness(
Parameters parameters,
InputStream stream,
@Nullable Integer idRequirement,
SecretKeyAccess access)
throws GeneralSecurityException {
validateKeyFormat(format);
byte[] aesCtrKeyBytes = new byte[format.getAesCtrKeyFormat().getKeySize()];
readFully(inputStream, aesCtrKeyBytes);
HmacKeyManager hmacKeyManager = new HmacKeyManager();
HmacKey hmacKey =
hmacKeyManager
.keyFactory()
.deriveKey(hmacKeyManager, format.getHmacKeyFormat(), inputStream);
AesCtrKey aesCtrKey =
AesCtrKey.newBuilder()
.setParams(format.getAesCtrKeyFormat().getParams())
.setVersion(getVersion())
.setKeyValue(ByteString.copyFrom(aesCtrKeyBytes))
.build();
return AesCtrHmacAeadKey.newBuilder()
.setVersion(getVersion())
.setAesCtrKey(aesCtrKey)
.setHmacKey(hmacKey)
.build();
if (parameters instanceof AesCtrHmacAeadParameters) {
return createAesCtrHmacAeadKeyFromRandomness(
(AesCtrHmacAeadParameters) parameters, stream, idRequirement, access);
}
throw new GeneralSecurityException(
"Unexpected parameters: expected AesCtrHmacAeadParameters, but got: " + parameters);
}
};
}

// To ensure that the derived key can provide key commitment, the AES-CTR key must be derived
// before the HMAC key.
// Consider the following malicious scenario using a brute-forced key InputStream with a 0 as
// its 32nd byte:
// 31 bytes || 1 byte of 0s || 16 bytes
// We give this stream to party A, saying that it is 32-byte HMAC key || 16-byte AES key. We
// also give this stream to party B, saying that it is 31-byte HMAC key || 16-byte AES key.
// Since HMAC pads the key with zeroes, this same stream will lead to both parties using the
// same HMAC key but different AES keys.
@AccessesPartialKey
static com.google.crypto.tink.aead.AesCtrHmacAeadKey createAesCtrHmacAeadKeyFromRandomness(
AesCtrHmacAeadParameters parameters,
InputStream stream,
@Nullable Integer idRequirement,
SecretKeyAccess access)
throws GeneralSecurityException {
return com.google.crypto.tink.aead.AesCtrHmacAeadKey.builder()
.setParameters(parameters)
.setIdRequirement(idRequirement)
.setAesKeyBytes(Util.readIntoSecretBytes(stream, parameters.getAesKeySizeBytes(), access))
.setHmacKeyBytes(Util.readIntoSecretBytes(stream, parameters.getHmacKeySizeBytes(), access))
.build();
}

private static Map<String, Parameters> namedParameters() throws GeneralSecurityException {
Map<String, Parameters> result = new HashMap<>();

Expand Down
10 changes: 10 additions & 0 deletions src/main/java/com/google/crypto/tink/aead/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ java_library(
name = "aes_ctr_hmac_aead_key_manager",
srcs = ["AesCtrHmacAeadKeyManager.java"],
deps = [
":aes_ctr_hmac_aead_key",
":aes_ctr_hmac_aead_parameters",
":aes_ctr_hmac_aead_proto_serialization",
":aes_ctr_key_manager",
Expand All @@ -97,20 +98,24 @@ java_library(
"//proto:aes_ctr_java_proto",
"//proto:hmac_java_proto",
"//proto:tink_java_proto",
"//src/main/java/com/google/crypto/tink:accesses_partial_key",
"//src/main/java/com/google/crypto/tink:aead",
"//src/main/java/com/google/crypto/tink:key_template",
"//src/main/java/com/google/crypto/tink:mac",
"//src/main/java/com/google/crypto/tink:parameters",
"//src/main/java/com/google/crypto/tink:registry",
"//src/main/java/com/google/crypto/tink:secret_key_access",
"//src/main/java/com/google/crypto/tink/config/internal:tink_fips_util",
"//src/main/java/com/google/crypto/tink/internal:key_type_manager",
"//src/main/java/com/google/crypto/tink/internal:mutable_parameters_registry",
"//src/main/java/com/google/crypto/tink/internal:primitive_factory",
"//src/main/java/com/google/crypto/tink/internal:tink_bug_exception",
"//src/main/java/com/google/crypto/tink/internal:util",
"//src/main/java/com/google/crypto/tink/mac:hmac_key_manager",
"//src/main/java/com/google/crypto/tink/subtle:encrypt_then_authenticate",
"//src/main/java/com/google/crypto/tink/subtle:ind_cpa_cipher",
"//src/main/java/com/google/crypto/tink/subtle:validators",
"@maven//:com_google_code_findbugs_jsr305",
"@maven//:com_google_protobuf_protobuf_java",
],
)
Expand Down Expand Up @@ -576,6 +581,7 @@ android_library(
name = "aes_ctr_hmac_aead_key_manager-android",
srcs = ["AesCtrHmacAeadKeyManager.java"],
deps = [
":aes_ctr_hmac_aead_key-android",
":aes_ctr_hmac_aead_parameters-android",
":aes_ctr_hmac_aead_proto_serialization-android",
":aes_ctr_key_manager-android",
Expand All @@ -584,20 +590,24 @@ android_library(
"//proto:aes_ctr_java_proto_lite",
"//proto:hmac_java_proto_lite",
"//proto:tink_java_proto_lite",
"//src/main/java/com/google/crypto/tink:accesses_partial_key-android",
"//src/main/java/com/google/crypto/tink:aead-android",
"//src/main/java/com/google/crypto/tink:key_template-android",
"//src/main/java/com/google/crypto/tink:mac-android",
"//src/main/java/com/google/crypto/tink:parameters-android",
"//src/main/java/com/google/crypto/tink:registry-android",
"//src/main/java/com/google/crypto/tink:secret_key_access-android",
"//src/main/java/com/google/crypto/tink/config/internal:tink_fips_util-android",
"//src/main/java/com/google/crypto/tink/internal:key_type_manager-android",
"//src/main/java/com/google/crypto/tink/internal:mutable_parameters_registry-android",
"//src/main/java/com/google/crypto/tink/internal:primitive_factory-android",
"//src/main/java/com/google/crypto/tink/internal:tink_bug_exception-android",
"//src/main/java/com/google/crypto/tink/internal:util-android",
"//src/main/java/com/google/crypto/tink/mac:hmac_key_manager-android",
"//src/main/java/com/google/crypto/tink/subtle:encrypt_then_authenticate-android",
"//src/main/java/com/google/crypto/tink/subtle:ind_cpa_cipher-android",
"//src/main/java/com/google/crypto/tink/subtle:validators-android",
"@maven//:com_google_code_findbugs_jsr305",
"@maven//:com_google_protobuf_protobuf_javalite",
],
)
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/com/google/crypto/tink/mac/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,27 @@ java_library(
"//proto:common_java_proto",
"//proto:hmac_java_proto",
"//proto:tink_java_proto",
"//src/main/java/com/google/crypto/tink:accesses_partial_key",
"//src/main/java/com/google/crypto/tink:key_template",
"//src/main/java/com/google/crypto/tink:mac",
"//src/main/java/com/google/crypto/tink:parameters",
"//src/main/java/com/google/crypto/tink:registry",
"//src/main/java/com/google/crypto/tink:secret_key_access",
"//src/main/java/com/google/crypto/tink/config/internal:tink_fips_util",
"//src/main/java/com/google/crypto/tink/internal:key_type_manager",
"//src/main/java/com/google/crypto/tink/internal:mutable_parameters_registry",
"//src/main/java/com/google/crypto/tink/internal:mutable_primitive_registry",
"//src/main/java/com/google/crypto/tink/internal:primitive_constructor",
"//src/main/java/com/google/crypto/tink/internal:primitive_factory",
"//src/main/java/com/google/crypto/tink/internal:tink_bug_exception",
"//src/main/java/com/google/crypto/tink/internal:util",
"//src/main/java/com/google/crypto/tink/mac/internal:chunked_hmac_impl",
"//src/main/java/com/google/crypto/tink/mac/internal:hmac_proto_serialization",
"//src/main/java/com/google/crypto/tink/subtle:prf_hmac_jce",
"//src/main/java/com/google/crypto/tink/subtle:prf_mac",
"//src/main/java/com/google/crypto/tink/subtle:random",
"//src/main/java/com/google/crypto/tink/subtle:validators",
"@maven//:com_google_code_findbugs_jsr305",
"@maven//:com_google_protobuf_protobuf_java",
],
)
Expand All @@ -47,23 +51,27 @@ android_library(
"//proto:common_java_proto_lite",
"//proto:hmac_java_proto_lite",
"//proto:tink_java_proto_lite",
"//src/main/java/com/google/crypto/tink:accesses_partial_key-android",
"//src/main/java/com/google/crypto/tink:key_template-android",
"//src/main/java/com/google/crypto/tink:mac-android",
"//src/main/java/com/google/crypto/tink:parameters-android",
"//src/main/java/com/google/crypto/tink:registry-android",
"//src/main/java/com/google/crypto/tink:secret_key_access-android",
"//src/main/java/com/google/crypto/tink/config/internal:tink_fips_util-android",
"//src/main/java/com/google/crypto/tink/internal:key_type_manager-android",
"//src/main/java/com/google/crypto/tink/internal:mutable_parameters_registry-android",
"//src/main/java/com/google/crypto/tink/internal:mutable_primitive_registry-android",
"//src/main/java/com/google/crypto/tink/internal:primitive_constructor-android",
"//src/main/java/com/google/crypto/tink/internal:primitive_factory-android",
"//src/main/java/com/google/crypto/tink/internal:tink_bug_exception-android",
"//src/main/java/com/google/crypto/tink/internal:util-android",
"//src/main/java/com/google/crypto/tink/mac/internal:chunked_hmac_impl-android",
"//src/main/java/com/google/crypto/tink/mac/internal:hmac_proto_serialization-android",
"//src/main/java/com/google/crypto/tink/subtle:prf_hmac_jce-android",
"//src/main/java/com/google/crypto/tink/subtle:prf_mac-android",
"//src/main/java/com/google/crypto/tink/subtle:random-android",
"//src/main/java/com/google/crypto/tink/subtle:validators-android",
"@maven//:com_google_code_findbugs_jsr305",
"@maven//:com_google_protobuf_protobuf_javalite",
],
)
Expand Down
39 changes: 29 additions & 10 deletions src/main/java/com/google/crypto/tink/mac/HmacKeyManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,19 @@

import static com.google.crypto.tink.internal.TinkBugException.exceptionIsBug;

import com.google.crypto.tink.AccessesPartialKey;
import com.google.crypto.tink.KeyTemplate;
import com.google.crypto.tink.Mac;
import com.google.crypto.tink.Parameters;
import com.google.crypto.tink.Registry;
import com.google.crypto.tink.SecretKeyAccess;
import com.google.crypto.tink.config.internal.TinkFipsUtil;
import com.google.crypto.tink.internal.KeyTypeManager;
import com.google.crypto.tink.internal.MutableParametersRegistry;
import com.google.crypto.tink.internal.MutablePrimitiveRegistry;
import com.google.crypto.tink.internal.PrimitiveConstructor;
import com.google.crypto.tink.internal.PrimitiveFactory;
import com.google.crypto.tink.internal.Util;
import com.google.crypto.tink.mac.internal.ChunkedHmacImpl;
import com.google.crypto.tink.mac.internal.HmacProtoSerialization;
import com.google.crypto.tink.proto.HashType;
Expand All @@ -47,6 +50,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;
import javax.crypto.spec.SecretKeySpec;

/**
Expand Down Expand Up @@ -188,21 +192,36 @@ public HmacKey createKey(HmacKeyFormat format) throws GeneralSecurityException {
}

@Override
public HmacKey deriveKey(
KeyTypeManager<HmacKey> keyManager, HmacKeyFormat format, InputStream inputStream)
public com.google.crypto.tink.mac.HmacKey createKeyFromRandomness(
Parameters parameters,
InputStream stream,
@Nullable Integer idRequirement,
SecretKeyAccess access)
throws GeneralSecurityException {
Validators.validateVersion(format.getVersion(), getVersion());
byte[] pseudorandomness = new byte[format.getKeySize()];
readFully(inputStream, pseudorandomness);
return HmacKey.newBuilder()
.setVersion(getVersion())
.setParams(format.getParams())
.setKeyValue(ByteString.copyFrom(pseudorandomness))
.build();
if (parameters instanceof HmacParameters) {
return createHmacKeyFromRandomness(
(HmacParameters) parameters, stream, idRequirement, access);
}
throw new GeneralSecurityException(
"Unexpected parameters: expected HmacParameters, but got: " + parameters);
}
};
}

@AccessesPartialKey
static com.google.crypto.tink.mac.HmacKey createHmacKeyFromRandomness(
HmacParameters parameters,
InputStream stream,
@Nullable Integer idRequirement,
SecretKeyAccess access)
throws GeneralSecurityException {
return com.google.crypto.tink.mac.HmacKey.builder()
.setParameters(parameters)
.setKeyBytes(Util.readIntoSecretBytes(stream, parameters.getKeySizeBytes(), access))
.setIdRequirement(idRequirement)
.build();
}

private static Map<String, Parameters> namedParameters() throws GeneralSecurityException {
Map<String, Parameters> result = new HashMap<>();
result.put("HMAC_SHA256_128BITTAG", PredefinedMacParameters.HMAC_SHA256_128BITTAG);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;

import com.google.crypto.tink.Aead;
import com.google.crypto.tink.InsecureSecretKeyAccess;
import com.google.crypto.tink.Key;
import com.google.crypto.tink.KeyTemplate;
import com.google.crypto.tink.KeyTemplates;
import com.google.crypto.tink.KeysetHandle;
Expand All @@ -36,10 +39,12 @@
import com.google.crypto.tink.subtle.EncryptThenAuthenticate;
import com.google.crypto.tink.subtle.Hex;
import com.google.crypto.tink.subtle.Random;
import com.google.crypto.tink.util.SecretBytes;
import com.google.protobuf.ByteString;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.security.GeneralSecurityException;
import java.util.Arrays;
import java.util.Set;
import java.util.TreeSet;
import org.junit.Before;
Expand Down Expand Up @@ -391,4 +396,37 @@ public void testTemplates(@FromDataPoints("templateNames") String templateName)
assertThat(h.getAt(0).getKey().getParameters())
.isEqualTo(KeyTemplates.get(templateName).toParameters());
}

@Theory
public void testCreateKeyFromRandomness(@FromDataPoints("templateNames") String templateName)
throws Exception {
byte[] keyMaterial =
new byte[] {
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46,
47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68,
};
AesCtrHmacAeadParameters parameters =
(AesCtrHmacAeadParameters) KeyTemplates.get(templateName).toParameters();
com.google.crypto.tink.aead.AesCtrHmacAeadKey key =
AesCtrHmacAeadKeyManager.createAesCtrHmacAeadKeyFromRandomness(
parameters,
new ByteArrayInputStream(keyMaterial),
parameters.hasIdRequirement() ? 123 : null,
InsecureSecretKeyAccess.get());
byte[] expectedAesKey = Arrays.copyOf(keyMaterial, parameters.getAesKeySizeBytes());
byte[] expectedHmacKey =
Arrays.copyOfRange(
keyMaterial,
parameters.getAesKeySizeBytes(),
parameters.getAesKeySizeBytes() + parameters.getHmacKeySizeBytes());
Key expectedKey =
com.google.crypto.tink.aead.AesCtrHmacAeadKey.builder()
.setParameters(parameters)
.setIdRequirement(parameters.hasIdRequirement() ? 123 : null)
.setAesKeyBytes(SecretBytes.copyFrom(expectedAesKey, InsecureSecretKeyAccess.get()))
.setHmacKeyBytes(SecretBytes.copyFrom(expectedHmacKey, InsecureSecretKeyAccess.get()))
.build();
assertTrue(key.equalsKey(expectedKey));
}
}
4 changes: 4 additions & 0 deletions src/test/java/com/google/crypto/tink/aead/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -174,17 +174,21 @@ java_test(
"//proto:hmac_java_proto",
"//proto:tink_java_proto",
"//src/main/java/com/google/crypto/tink:aead",
"//src/main/java/com/google/crypto/tink:insecure_secret_key_access",
"//src/main/java/com/google/crypto/tink:key",
"//src/main/java/com/google/crypto/tink:key_template",
"//src/main/java/com/google/crypto/tink:key_templates",
"//src/main/java/com/google/crypto/tink:parameters",
"//src/main/java/com/google/crypto/tink:registry_cluster",
"//src/main/java/com/google/crypto/tink/aead:aead_config",
"//src/main/java/com/google/crypto/tink/aead:aes_ctr_hmac_aead_key",
"//src/main/java/com/google/crypto/tink/aead:aes_ctr_hmac_aead_key_manager",
"//src/main/java/com/google/crypto/tink/aead:aes_ctr_hmac_aead_parameters",
"//src/main/java/com/google/crypto/tink/internal:key_type_manager",
"//src/main/java/com/google/crypto/tink/subtle:encrypt_then_authenticate",
"//src/main/java/com/google/crypto/tink/subtle:hex",
"//src/main/java/com/google/crypto/tink/subtle:random",
"//src/main/java/com/google/crypto/tink/util:secret_bytes",
"@maven//:com_google_protobuf_protobuf_java",
"@maven//:com_google_truth_truth",
"@maven//:junit_junit",
Expand Down
Loading

0 comments on commit b0b138d

Please sign in to comment.