From 9cdfccc36a5673f223a593d9d1cc6615b74197fb Mon Sep 17 00:00:00 2001
From: paulavidas <paulavidas@google.com>
Date: Thu, 8 Aug 2019 03:02:55 -0700
Subject: [PATCH] Fix ciphertext segment size validation.
We changed the segment size validation in all Tink streaming keys such that the first block always needs to have room for at least 1 byte of ciphertext. Previously it was not consistent. The decision whether to allow empty ciphertexts in the first segment is somewhat arbitrary, but it should be consistent everywhere.
PiperOrigin-RevId: 262315914
---
cc/streamingaead/BUILD.bazel | 1 +
cc/streamingaead/CMakeLists.txt | 1 +
cc/streamingaead/aes_gcm_hkdf_streaming_key_manager.cc | 8 ++++++--
cc/subtle/aes_ctr_hmac_stream_segment_encrypter.cc | 2 +-
cc/subtle/aes_ctr_hmac_stream_segment_encrypter_test.cc | 2 +-
cc/subtle/aes_gcm_hkdf_stream_segment_decrypter.cc | 2 +-
cc/subtle/aes_gcm_hkdf_stream_segment_decrypter_test.cc | 4 +++-
cc/subtle/aes_gcm_hkdf_stream_segment_encrypter.cc | 2 +-
cc/subtle/aes_gcm_hkdf_stream_segment_encrypter_test.cc | 3 ++-
cc/subtle/aes_gcm_hkdf_streaming.cc | 5 +++--
.../streamingaead/AesCtrHmacStreamingKeyManager.java | 8 ++++++--
.../streamingaead/AesGcmHkdfStreamingKeyManager.java | 9 +++++++--
12 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/cc/streamingaead/BUILD.bazel b/cc/streamingaead/BUILD.bazel
index c18e48f16..ef6cde3e1 100644
--- a/cc/streamingaead/BUILD.bazel
+++ b/cc/streamingaead/BUILD.bazel
@@ -66,6 +66,7 @@ cc_library(
"//cc:core/key_type_manager",
"//cc:key_manager",
"//cc:streaming_aead",
+ "//cc/subtle:aes_gcm_hkdf_stream_segment_encrypter",
"//cc/subtle:aes_gcm_hkdf_streaming",
"//cc/subtle:random",
"//cc/util:constants",
diff --git a/cc/streamingaead/CMakeLists.txt b/cc/streamingaead/CMakeLists.txt
index 88ea605ca..4adeaddb9 100644
--- a/cc/streamingaead/CMakeLists.txt
+++ b/cc/streamingaead/CMakeLists.txt
@@ -61,6 +61,7 @@ tink_cc_library(
tink::core::streaming_aead
tink::proto::aes_gcm_hkdf_streaming_cc_proto
tink::proto::tink_cc_proto
+ tink::subtle::aes_gcm_hkdf_stream_segment_encrypter
tink::subtle::aes_gcm_hkdf_streaming
tink::subtle::random
tink::util::constants
diff --git a/cc/streamingaead/aes_gcm_hkdf_streaming_key_manager.cc b/cc/streamingaead/aes_gcm_hkdf_streaming_key_manager.cc
index a3e2181a1..1bb7dc4eb 100644
--- a/cc/streamingaead/aes_gcm_hkdf_streaming_key_manager.cc
+++ b/cc/streamingaead/aes_gcm_hkdf_streaming_key_manager.cc
@@ -16,12 +16,14 @@
#include "tink/streamingaead/aes_gcm_hkdf_streaming_key_manager.h"
+#include "tink/subtle/aes_gcm_hkdf_stream_segment_encrypter.h"
#include "tink/subtle/random.h"
#include "tink/util/validation.h"
namespace crypto {
namespace tink {
+using ::crypto::tink::subtle::AesGcmHkdfStreamSegmentEncrypter;
using ::crypto::tink::util::Status;
using ::crypto::tink::util::StatusOr;
using ::google::crypto::tink::AesGcmHkdfStreamingKey;
@@ -38,8 +40,10 @@ Status ValidateParams(const AesGcmHkdfStreamingParams& params) {
params.hkdf_hash_type() == HashType::SHA512)) {
return Status(util::error::INVALID_ARGUMENT, "unsupported hkdf_hash_type");
}
- if (params.ciphertext_segment_size() <
- (params.derived_key_size() + 8) + 16) { // header_size + tag_size
+ int header_size = 1 + params.derived_key_size() +
+ AesGcmHkdfStreamSegmentEncrypter::kNoncePrefixSizeInBytes;
+ if (params.ciphertext_segment_size() <=
+ header_size + AesGcmHkdfStreamSegmentEncrypter::kTagSizeInBytes) {
return Status(util::error::INVALID_ARGUMENT,
"ciphertext_segment_size too small");
}
diff --git a/cc/subtle/aes_ctr_hmac_stream_segment_encrypter.cc b/cc/subtle/aes_ctr_hmac_stream_segment_encrypter.cc
index 60e04e849..67323f0a6 100644
--- a/cc/subtle/aes_ctr_hmac_stream_segment_encrypter.cc
+++ b/cc/subtle/aes_ctr_hmac_stream_segment_encrypter.cc
@@ -65,7 +65,7 @@ util::Status Validate(const AesCtrHmacStreamSegmentEncrypter::Params& params) {
}
int header_size = 1 + params.salt.size() +
AesCtrHmacStreamSegmentEncrypter::kNoncePrefixSizeInBytes;
- if (params.ciphertext_segment_size <
+ if (params.ciphertext_segment_size <=
params.ciphertext_offset + header_size + params.tag_size) {
return util::Status(util::error::INVALID_ARGUMENT,
"ciphertext_segment_size too small");
diff --git a/cc/subtle/aes_ctr_hmac_stream_segment_encrypter_test.cc b/cc/subtle/aes_ctr_hmac_stream_segment_encrypter_test.cc
index 6744e4b03..4589a84e1 100644
--- a/cc/subtle/aes_ctr_hmac_stream_segment_encrypter_test.cc
+++ b/cc/subtle/aes_ctr_hmac_stream_segment_encrypter_test.cc
@@ -175,7 +175,7 @@ TEST(AesCtrHmacStreamSegmentEncrypterTest, WrongCiphertextSegmentSize) {
for (int ciphertext_offset : {0, 1, 5, 10}) {
int min_ct_segment_size = key_size + ciphertext_offset +
8 + // nonce_prefix_size + 1
- tag_size;
+ tag_size + 1;
for (int ct_segment_size :
{min_ct_segment_size - 5, min_ct_segment_size - 1,
min_ct_segment_size, min_ct_segment_size + 1,
diff --git a/cc/subtle/aes_gcm_hkdf_stream_segment_decrypter.cc b/cc/subtle/aes_gcm_hkdf_stream_segment_decrypter.cc
index 5bbf85edb..b690a22a3 100644
--- a/cc/subtle/aes_gcm_hkdf_stream_segment_decrypter.cc
+++ b/cc/subtle/aes_gcm_hkdf_stream_segment_decrypter.cc
@@ -79,7 +79,7 @@ util::Status Validate(const AesGcmHkdfStreamSegmentDecrypter::Params& params) {
}
int header_size = 1 + params.derived_key_size +
AesGcmHkdfStreamSegmentEncrypter::kNoncePrefixSizeInBytes;
- if (params.ciphertext_segment_size <
+ if (params.ciphertext_segment_size <=
params.ciphertext_offset + header_size +
AesGcmHkdfStreamSegmentEncrypter::kTagSizeInBytes) {
return util::Status(util::error::INVALID_ARGUMENT,
diff --git a/cc/subtle/aes_gcm_hkdf_stream_segment_decrypter_test.cc b/cc/subtle/aes_gcm_hkdf_stream_segment_decrypter_test.cc
index 195654c17..0ea741bfb 100644
--- a/cc/subtle/aes_gcm_hkdf_stream_segment_decrypter_test.cc
+++ b/cc/subtle/aes_gcm_hkdf_stream_segment_decrypter_test.cc
@@ -231,7 +231,9 @@ TEST(AesGcmHkdfStreamSegmentDecrypterTest, testWrongCiphertextSegmentSize) {
for (int ciphertext_offset : {0, 1, 5, 10}) {
int min_ct_segment_size = derived_key_size + ciphertext_offset +
8 + // nonce_prefix_size + 1
- 16; // tag_size
+ 16 + // tag_size
+ 1;
+
for (int ct_segment_size : {min_ct_segment_size - 5,
min_ct_segment_size - 1, min_ct_segment_size,
min_ct_segment_size + 1, min_ct_segment_size + 10}) {
diff --git a/cc/subtle/aes_gcm_hkdf_stream_segment_encrypter.cc b/cc/subtle/aes_gcm_hkdf_stream_segment_encrypter.cc
index 7fdd52848..5f94686a4 100644
--- a/cc/subtle/aes_gcm_hkdf_stream_segment_encrypter.cc
+++ b/cc/subtle/aes_gcm_hkdf_stream_segment_encrypter.cc
@@ -77,7 +77,7 @@ util::Status Validate(const AesGcmHkdfStreamSegmentEncrypter::Params& params) {
}
int header_size = 1 + params.salt.size() +
AesGcmHkdfStreamSegmentEncrypter::kNoncePrefixSizeInBytes;
- if (params.ciphertext_segment_size <
+ if (params.ciphertext_segment_size <=
params.ciphertext_offset + header_size +
AesGcmHkdfStreamSegmentEncrypter::kTagSizeInBytes) {
return util::Status(util::error::INVALID_ARGUMENT,
diff --git a/cc/subtle/aes_gcm_hkdf_stream_segment_encrypter_test.cc b/cc/subtle/aes_gcm_hkdf_stream_segment_encrypter_test.cc
index 6c7be6352..a60b81146 100644
--- a/cc/subtle/aes_gcm_hkdf_stream_segment_encrypter_test.cc
+++ b/cc/subtle/aes_gcm_hkdf_stream_segment_encrypter_test.cc
@@ -163,7 +163,8 @@ TEST(AesGcmHkdfStreamSegmentEncrypterTest, testWrongCiphertextSegmentSize) {
for (int ciphertext_offset : {0, 1, 5, 10}) {
int min_ct_segment_size = key_size + ciphertext_offset +
8 + // nonce_prefix_size + 1
- 16; // tag_size
+ 16 + // tag_size
+ 1;
for (int ct_segment_size : {min_ct_segment_size - 5,
min_ct_segment_size - 1, min_ct_segment_size,
min_ct_segment_size + 1, min_ct_segment_size + 10}) {
diff --git a/cc/subtle/aes_gcm_hkdf_streaming.cc b/cc/subtle/aes_gcm_hkdf_streaming.cc
index 1da6fc9a3..fb1299336 100644
--- a/cc/subtle/aes_gcm_hkdf_streaming.cc
+++ b/cc/subtle/aes_gcm_hkdf_streaming.cc
@@ -57,8 +57,9 @@ Status Validate(absl::string_view ikm,
return util::Status(util::error::INVALID_ARGUMENT,
"ciphertext_offset must be non-negative");
}
- if (ciphertext_segment_size <
- ciphertext_offset + derived_key_size) {
+ if (ciphertext_segment_size <=
+ ciphertext_offset + derived_key_size +
+ AesGcmHkdfStreamSegmentEncrypter::kTagSizeInBytes) {
return util::Status(util::error::INVALID_ARGUMENT,
"ciphertext_segment_size too small");
}
diff --git a/java/src/main/java/com/google/crypto/tink/streamingaead/AesCtrHmacStreamingKeyManager.java b/java/src/main/java/com/google/crypto/tink/streamingaead/AesCtrHmacStreamingKeyManager.java
index 329f6c720..53d74595d 100644
--- a/java/src/main/java/com/google/crypto/tink/streamingaead/AesCtrHmacStreamingKeyManager.java
+++ b/java/src/main/java/com/google/crypto/tink/streamingaead/AesCtrHmacStreamingKeyManager.java
@@ -52,6 +52,8 @@ class AesCtrHmacStreamingKeyManager
/** Minimum tag size in bytes. This provides minimum 80-bit security strength. */
private static final int MIN_TAG_SIZE_IN_BYTES = 10;
+
+ private static final int NONCE_PREFIX_IN_BYTES = 7;
/** @param serializedKey serialized {@code AesCtrHmacStreamingKey} proto */
@Override
@@ -138,9 +140,11 @@ class AesCtrHmacStreamingKeyManager
validateHmacParams(params.getHmacParams());
if (params.getCiphertextSegmentSize()
- < params.getDerivedKeySize() + params.getHmacParams().getTagSize() + 8) {
+ <= params.getDerivedKeySize() + params.getHmacParams().getTagSize() + 1
+ + NONCE_PREFIX_IN_BYTES) {
throw new GeneralSecurityException(
- "ciphertext_segment_size must be at least (derived_key_size + tag_size + 8)");
+ "ciphertext_segment_size must be at least (derived_key_size + tag_size + "
+ + "NONCE_PREFIX_IN_BYTES + 2)");
}
}
diff --git a/java/src/main/java/com/google/crypto/tink/streamingaead/AesGcmHkdfStreamingKeyManager.java b/java/src/main/java/com/google/crypto/tink/streamingaead/AesGcmHkdfStreamingKeyManager.java
index bfad9233c..2a948b464 100644
--- a/java/src/main/java/com/google/crypto/tink/streamingaead/AesGcmHkdfStreamingKeyManager.java
+++ b/java/src/main/java/com/google/crypto/tink/streamingaead/AesGcmHkdfStreamingKeyManager.java
@@ -48,6 +48,9 @@ class AesGcmHkdfStreamingKeyManager
public static final String TYPE_URL =
"type.googleapis.com/google.crypto.tink.AesGcmHkdfStreamingKey";
+
+ private static final int NONCE_PREFIX_IN_BYTES = 7;
+ private static final int TAG_SIZE_IN_BYTES = 16;
/** @param serializedKey serialized {@code AesGcmHkdfStreamingKey} proto */
@Override
@@ -118,9 +121,11 @@ class AesGcmHkdfStreamingKeyManager
if (params.getHkdfHashType() == HashType.UNKNOWN_HASH) {
throw new GeneralSecurityException("unknown HKDF hash type");
}
- if (params.getCiphertextSegmentSize() < params.getDerivedKeySize() + 8) {
+ if (params.getCiphertextSegmentSize() <= params.getDerivedKeySize() + 1 + NONCE_PREFIX_IN_BYTES
+ + TAG_SIZE_IN_BYTES) {
throw new GeneralSecurityException(
- "ciphertext_segment_size must be at least (derived_key_size + 8)");
+ "ciphertext_segment_size must be at least (derived_key_size + NONCE_PREFIX_IN_BYTES + "
+ + "TAG_SIZE_IN_BYTES + 2)");
}
}
}
--
GitLab