Require in == out for in-place encryption.
While most of OpenSSL's assembly allows out < in too, some of it doesn't.
Upstream seems to not consider this a problem (or, at least, they're failing to
make a decision on whether it is a problem, so we should assume they'll stay
their course). Accordingly, require aliased buffers to exactly align so we
don't have to keep chasing this down.
Change-Id: I00eb3df3e195b249116c68f7272442918d7077eb
Reviewed-on: https://boringssl-review.googlesource.com/8231
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/chacha/chacha.c b/crypto/chacha/chacha.c
index afe1b2a..1562089 100644
--- a/crypto/chacha/chacha.c
+++ b/crypto/chacha/chacha.c
@@ -16,10 +16,13 @@
#include <openssl/chacha.h>
+#include <assert.h>
#include <string.h>
#include <openssl/cpu.h>
+#include "../internal.h"
+
#define U8TO32_LITTLE(p) \
(((uint32_t)((p)[0])) | ((uint32_t)((p)[1]) << 8) | \
@@ -36,8 +39,9 @@
void CRYPTO_chacha_20(uint8_t *out, const uint8_t *in, size_t in_len,
const uint8_t key[32], const uint8_t nonce[12],
uint32_t counter) {
- uint32_t counter_nonce[4];
- counter_nonce[0] = counter;
+ assert(!buffers_alias(out, in_len, in, in_len) || in == out);
+
+ uint32_t counter_nonce[4]; counter_nonce[0] = counter;
counter_nonce[1] = U8TO32_LITTLE(nonce + 0);
counter_nonce[2] = U8TO32_LITTLE(nonce + 4);
counter_nonce[3] = U8TO32_LITTLE(nonce + 8);
@@ -118,6 +122,8 @@
void CRYPTO_chacha_20(uint8_t *out, const uint8_t *in, size_t in_len,
const uint8_t key[32], const uint8_t nonce[12],
uint32_t counter) {
+ assert(!buffers_alias(out, in_len, in, in_len) || in == out);
+
uint32_t input[16];
uint8_t buf[64];
size_t todo, i;
diff --git a/crypto/chacha/chacha_test.cc b/crypto/chacha/chacha_test.cc
index f364f98..0a5972f 100644
--- a/crypto/chacha/chacha_test.cc
+++ b/crypto/chacha/chacha_test.cc
@@ -218,25 +218,16 @@
std::unique_ptr<uint8_t[]> buf(new uint8_t[len]);
CRYPTO_chacha_20(buf.get(), kInput, len, kKey, kNonce, kCounter);
if (memcmp(buf.get(), kOutput, len) != 0) {
- fprintf(stderr, "Mismatch at length %u.\n", static_cast<unsigned>(len));
+ fprintf(stderr, "Mismatch at length %zu.\n", len);
return false;
}
- // Test in-place at various offsets.
- static const size_t kOffsets[] = {
- 0, 1, 2, 8, 15, 16, 17, 31, 32, 33, 63,
- 64, 65, 95, 96, 97, 127, 128, 129, 255, 256, 257,
- };
- for (size_t offset : kOffsets) {
- buf.reset(new uint8_t[len + offset]);
- memcpy(buf.get() + offset, kInput, len);
- CRYPTO_chacha_20(buf.get(), buf.get() + offset, len, kKey, kNonce,
- kCounter);
- if (memcmp(buf.get(), kOutput, len) != 0) {
- fprintf(stderr, "Mismatch at length %u with in-place offset %u.\n",
- static_cast<unsigned>(len), static_cast<unsigned>(offset));
- return false;
- }
+ // Test in-place.
+ memcpy(buf.get(), kInput, len);
+ CRYPTO_chacha_20(buf.get(), buf.get(), len, kKey, kNonce, kCounter);
+ if (memcmp(buf.get(), kOutput, len) != 0) {
+ fprintf(stderr, "Mismatch at length %zu, in-place.\n", len);
+ return false;
}
return true;
diff --git a/crypto/cipher/aead.c b/crypto/cipher/aead.c
index b1db83d..57eecc1 100644
--- a/crypto/cipher/aead.c
+++ b/crypto/cipher/aead.c
@@ -20,6 +20,7 @@
#include <openssl/err.h>
#include "internal.h"
+#include "../internal.h"
size_t EVP_AEAD_key_length(const EVP_AEAD *aead) { return aead->key_len; }
@@ -80,21 +81,15 @@
ctx->aead = NULL;
}
-/* check_alias returns 0 if |out| points within the buffer determined by |in|
- * and |in_len| and 1 otherwise.
- *
- * When processing, there's only an issue if |out| points within in[:in_len]
- * and isn't equal to |in|. If that's the case then writing the output will
- * stomp input that hasn't been read yet.
- *
- * This function checks for that case. */
-static int check_alias(const uint8_t *in, size_t in_len, const uint8_t *out) {
- if (out <= in) {
- return 1;
- } else if (in + in_len <= out) {
+/* check_alias returns 1 if |out| is compatible with |in| and 0 otherwise. If
+ * |in| and |out| alias, we require that |in| == |out|. */
+static int check_alias(const uint8_t *in, size_t in_len, const uint8_t *out,
+ size_t out_len) {
+ if (!buffers_alias(in, in_len, out, out_len)) {
return 1;
}
- return 0;
+
+ return in == out;
}
int EVP_AEAD_CTX_seal(const EVP_AEAD_CTX *ctx, uint8_t *out, size_t *out_len,
@@ -108,7 +103,7 @@
goto error;
}
- if (!check_alias(in, in_len, out)) {
+ if (!check_alias(in, in_len, out, max_out_len)) {
OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_OUTPUT_ALIASES_INPUT);
goto error;
}
@@ -130,7 +125,7 @@
size_t max_out_len, const uint8_t *nonce,
size_t nonce_len, const uint8_t *in, size_t in_len,
const uint8_t *ad, size_t ad_len) {
- if (!check_alias(in, in_len, out)) {
+ if (!check_alias(in, in_len, out, max_out_len)) {
OPENSSL_PUT_ERROR(CIPHER, CIPHER_R_OUTPUT_ALIASES_INPUT);
goto error;
}
diff --git a/crypto/cipher/aead_test.cc b/crypto/cipher/aead_test.cc
index f21291e..8bad93f 100644
--- a/crypto/cipher/aead_test.cc
+++ b/crypto/cipher/aead_test.cc
@@ -225,84 +225,65 @@
return false;
}
- // First test with out > in, which we expect to fail.
- for (auto offset : offsets) {
- if (offset == 0) {
- // Will be tested in the next loop.
- continue;
- }
+ // Test with out != in which we expect to fail.
+ std::vector<uint8_t> buffer(2 + valid_encryption_len);
+ uint8_t *in = buffer.data() + 1;
+ uint8_t *out1 = buffer.data();
+ uint8_t *out2 = buffer.data() + 2;
- std::vector<uint8_t> buffer(offset + valid_encryption_len);
- memcpy(buffer.data(), kPlaintext, sizeof(kPlaintext));
- uint8_t *out = buffer.data() + offset;
+ memcpy(in, kPlaintext, sizeof(kPlaintext));
+ size_t out_len;
+ if (EVP_AEAD_CTX_seal(ctx.get(), out1, &out_len,
+ sizeof(kPlaintext) + max_overhead, nonce.data(),
+ nonce_len, in, sizeof(kPlaintext), nullptr, 0) ||
+ EVP_AEAD_CTX_seal(ctx.get(), out2, &out_len,
+ sizeof(kPlaintext) + max_overhead, nonce.data(),
+ nonce_len, in, sizeof(kPlaintext), nullptr, 0)) {
+ fprintf(stderr, "EVP_AEAD_CTX_seal unexpectedly succeeded.\n");
+ return false;
+ }
+ ERR_clear_error();
- size_t out_len;
- if (!EVP_AEAD_CTX_seal(ctx.get(), out, &out_len,
- sizeof(kPlaintext) + max_overhead, nonce.data(),
- nonce_len, buffer.data(), sizeof(kPlaintext),
- nullptr, 0)) {
- // We expect offsets where the output is greater than the input to fail.
- ERR_clear_error();
- } else {
- fprintf(stderr,
- "EVP_AEAD_CTX_seal unexpectedly succeeded for offset %u.\n",
- static_cast<unsigned>(offset));
- return false;
- }
+ memcpy(in, valid_encryption.data(), valid_encryption_len);
+ if (EVP_AEAD_CTX_open(ctx.get(), out1, &out_len, valid_encryption_len,
+ nonce.data(), nonce_len, in, valid_encryption_len,
+ nullptr, 0) ||
+ EVP_AEAD_CTX_open(ctx.get(), out2, &out_len, valid_encryption_len,
+ nonce.data(), nonce_len, in, valid_encryption_len,
+ nullptr, 0)) {
+ fprintf(stderr, "EVP_AEAD_CTX_open unexpectedly succeeded.\n");
+ return false;
+ }
+ ERR_clear_error();
- memcpy(buffer.data(), valid_encryption.data(), valid_encryption_len);
- if (!EVP_AEAD_CTX_open(ctx.get(), out, &out_len, valid_encryption_len,
- nonce.data(), nonce_len, buffer.data(),
- valid_encryption_len, nullptr, 0)) {
- // We expect offsets where the output is greater than the input to fail.
- ERR_clear_error();
- } else {
- fprintf(stderr,
- "EVP_AEAD_CTX_open unexpectedly succeeded for offset %u.\n",
- static_cast<unsigned>(offset));
- ERR_print_errors_fp(stderr);
- return false;
- }
+ // Test with out == in, which we expect to work.
+ memcpy(in, kPlaintext, sizeof(kPlaintext));
+
+ if (!EVP_AEAD_CTX_seal(ctx.get(), in, &out_len,
+ sizeof(kPlaintext) + max_overhead, nonce.data(),
+ nonce_len, in, sizeof(kPlaintext), nullptr, 0)) {
+ fprintf(stderr, "EVP_AEAD_CTX_seal failed in-place.\n");
+ return false;
}
- // Test with out <= in, which we expect to work.
- for (auto offset : offsets) {
- std::vector<uint8_t> buffer(offset + valid_encryption_len);
- uint8_t *const out = buffer.data();
- uint8_t *const in = buffer.data() + offset;
- memcpy(in, kPlaintext, sizeof(kPlaintext));
+ if (out_len != valid_encryption_len ||
+ memcmp(in, valid_encryption.data(), out_len) != 0) {
+ fprintf(stderr, "EVP_AEAD_CTX_seal produced bad output in-place.\n");
+ return false;
+ }
- size_t out_len;
- if (!EVP_AEAD_CTX_seal(ctx.get(), out, &out_len,
- sizeof(kPlaintext) + max_overhead, nonce.data(),
- nonce_len, in, sizeof(kPlaintext), nullptr, 0)) {
- fprintf(stderr, "EVP_AEAD_CTX_seal failed for offset -%u.\n",
- static_cast<unsigned>(offset));
- return false;
- }
+ memcpy(in, valid_encryption.data(), valid_encryption_len);
+ if (!EVP_AEAD_CTX_open(ctx.get(), in, &out_len, valid_encryption_len,
+ nonce.data(), nonce_len, in, valid_encryption_len,
+ nullptr, 0)) {
+ fprintf(stderr, "EVP_AEAD_CTX_open failed in-place.\n");
+ return false;
+ }
- if (out_len != valid_encryption_len ||
- memcmp(out, valid_encryption.data(), out_len) != 0) {
- fprintf(stderr, "EVP_AEAD_CTX_seal produced bad output for offset -%u.\n",
- static_cast<unsigned>(offset));
- return false;
- }
-
- memcpy(in, valid_encryption.data(), valid_encryption_len);
- if (!EVP_AEAD_CTX_open(ctx.get(), out, &out_len,
- offset + valid_encryption_len, nonce.data(),
- nonce_len, in, valid_encryption_len, nullptr, 0)) {
- fprintf(stderr, "EVP_AEAD_CTX_open failed for offset -%u.\n",
- static_cast<unsigned>(offset));
- return false;
- }
-
- if (out_len != sizeof(kPlaintext) ||
- memcmp(out, kPlaintext, out_len) != 0) {
- fprintf(stderr, "EVP_AEAD_CTX_open produced bad output for offset -%u.\n",
- static_cast<unsigned>(offset));
- return false;
- }
+ if (out_len != sizeof(kPlaintext) ||
+ memcmp(in, kPlaintext, out_len) != 0) {
+ fprintf(stderr, "EVP_AEAD_CTX_open produced bad output in-place.\n");
+ return false;
}
return true;
diff --git a/include/openssl/aead.h b/include/openssl/aead.h
index d9a640c..7895825 100644
--- a/include/openssl/aead.h
+++ b/include/openssl/aead.h
@@ -226,7 +226,7 @@
* insufficient, zero will be returned. (In this case, |*out_len| is set to
* zero.)
*
- * If |in| and |out| alias then |out| must be <= |in|. */
+ * If |in| and |out| alias then |out| must be == |in|. */
OPENSSL_EXPORT int EVP_AEAD_CTX_seal(const EVP_AEAD_CTX *ctx, uint8_t *out,
size_t *out_len, size_t max_out_len,
const uint8_t *nonce, size_t nonce_len,
@@ -251,7 +251,7 @@
* insufficient, zero will be returned. (In this case, |*out_len| is set to
* zero.)
*
- * If |in| and |out| alias then |out| must be <= |in|. */
+ * If |in| and |out| alias then |out| must be == |in|. */
OPENSSL_EXPORT int EVP_AEAD_CTX_open(const EVP_AEAD_CTX *ctx, uint8_t *out,
size_t *out_len, size_t max_out_len,
const uint8_t *nonce, size_t nonce_len,
diff --git a/include/openssl/chacha.h b/include/openssl/chacha.h
index 64713c2..3d035e6 100644
--- a/include/openssl/chacha.h
+++ b/include/openssl/chacha.h
@@ -23,8 +23,8 @@
/* CRYPTO_chacha_20 encrypts |in_len| bytes from |in| with the given key and
- * nonce and writes the result to |out|, which may be equal to |in|. The
- * initial block counter is specified by |counter|. */
+ * nonce and writes the result to |out|. If |in| and |out| alias, they must be
+ * equal. The initial block counter is specified by |counter|. */
OPENSSL_EXPORT void CRYPTO_chacha_20(uint8_t *out, const uint8_t *in,
size_t in_len, const uint8_t key[32],
const uint8_t nonce[12], uint32_t counter);
diff --git a/ssl/internal.h b/ssl/internal.h
index 6d96c6c..4cf7a2e 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -325,7 +325,7 @@
* writes the result to |out|. It returns one on success and zero on
* error. |ctx| may be NULL to denote the null cipher.
*
- * If |in| and |out| alias then |out| + |explicit_nonce_len| must be <= |in| */
+ * If |in| and |out| alias then |out| + |explicit_nonce_len| must be == |in|. */
int SSL_AEAD_CTX_seal(SSL_AEAD_CTX *ctx, uint8_t *out, size_t *out_len,
size_t max_out, uint8_t type, uint16_t wire_version,
const uint8_t seqnum[8], const uint8_t *in,
