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>
This commit is contained in:
David Benjamin 2016-06-08 18:31:42 -04:00 committed by Adam Langley
parent 1a01e1fc88
commit 2446db0f52
7 changed files with 82 additions and 109 deletions

View File

@ -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 ChaCha20_ctr32(uint8_t *out, const uint8_t *in, size_t in_len,
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 @@ static void chacha_core(uint8_t output[64], const uint32_t input[16]) {
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;

View File

@ -218,25 +218,16 @@ static bool TestChaCha20(size_t len) {
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;

View File

@ -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 @@ void EVP_AEAD_CTX_cleanup(EVP_AEAD_CTX *ctx) {
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 @@ int EVP_AEAD_CTX_seal(const EVP_AEAD_CTX *ctx, uint8_t *out, size_t *out_len,
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 @@ 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, 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;
}

View File

@ -225,84 +225,65 @@ static bool TestWithAliasedBuffers(const EVP_AEAD *aead) {
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;

View File

@ -226,7 +226,7 @@ OPENSSL_EXPORT void EVP_AEAD_CTX_cleanup(EVP_AEAD_CTX *ctx);
* 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 @@ OPENSSL_EXPORT int EVP_AEAD_CTX_seal(const EVP_AEAD_CTX *ctx, uint8_t *out,
* 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,

View File

@ -23,8 +23,8 @@ extern "C" {
/* 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);

View File

@ -325,7 +325,7 @@ int SSL_AEAD_CTX_open(SSL_AEAD_CTX *ctx, CBS *out, uint8_t type,
* 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,