android-tools/0002-Use-an-unsized-helper-for-truncated-SHA-512-variants.patch

From a24ab549e6ae246b391155d7bed3790ac0e07de2 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Thu, 25 Mar 2021 15:26:25 -0400
Subject: [PATCH] Use an unsized helper for truncated SHA-512 variants.

Although it is strictly fine to call SHA512_Final in SHA384_Final
(array sizes in C parameters are purely decorational, according to the
language), GCC 11 reportedly checks now and gets upset about the size
mismatch. Use an unsized helper function so all our code matches the
specified bounds.

Unfortunately, the bounds in all the functions are a bit misleading
because SHA512_Final really outputs based on sha->md_len (which Init
function you called) rather than which Final function. I've fixed this
places within a library where we mismatched and added asserts to the
smaller functions. SHA512_Final is assert-less because I've seen lots of
code use SHA384_Init / SHA512_Update / SHA512_Final.

This doesn't fix the SHA256 variant since that is generated by a pile of
macros in a multiply-included file. This is probably a good opportunity
to make that code less macro-heavy.

Update-Note: There is a small chance the asserts will trip something,
but hopefully not since I've left SHA512_Final alone.

Bug: 402
Change-Id: I4c9d579a63ee0a0dea103c19ef219c13bb9aa62c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46405
Reviewed-by: Adam Langley <agl@google.com>
---
 crypto/fipsmodule/digest/digests.c | 12 ++++++++++--
 crypto/fipsmodule/sha/sha512.c     | 24 ++++++++++++++++++------
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/vendor/boringssl/crypto/fipsmodule/digest/digests.c b/vendor/boringssl/crypto/fipsmodule/digest/digests.c
index 16daeba65..f006ebbc5 100644
--- a/vendor/boringssl/crypto/fipsmodule/digest/digests.c
+++ b/vendor/boringssl/crypto/fipsmodule/digest/digests.c
@@ -247,13 +247,21 @@ static void sha512_256_init(EVP_MD_CTX *ctx) {
   CHECK(SHA512_256_Init(ctx->md_data));
 }
 
+static void sha512_256_update(EVP_MD_CTX *ctx, const void *data, size_t count) {
+  CHECK(SHA512_256_Update(ctx->md_data, data, count));
+}
+
+static void sha512_256_final(EVP_MD_CTX *ctx, uint8_t *md) {
+  CHECK(SHA512_256_Final(md, ctx->md_data));
+}
+
 DEFINE_METHOD_FUNCTION(EVP_MD, EVP_sha512_256) {
   out->type = NID_sha512_256;
   out->md_size = SHA512_256_DIGEST_LENGTH;
   out->flags = 0;
   out->init = sha512_256_init;
-  out->update = sha512_update;
-  out->final = sha512_final;
+  out->update = sha512_256_update;
+  out->final = sha512_256_final;
   out->block_size = 128;
   out->ctx_size = sizeof(SHA512_CTX);
 }
diff --git a/vendor/boringssl/crypto/fipsmodule/sha/sha512.c b/vendor/boringssl/crypto/fipsmodule/sha/sha512.c
index fd025743d..ba86c1eda 100644
--- a/vendor/boringssl/crypto/fipsmodule/sha/sha512.c
+++ b/vendor/boringssl/crypto/fipsmodule/sha/sha512.c
@@ -70,6 +70,8 @@
 // this writing, so there is no need for a common collector/padding
 // implementation yet.
 
+static int sha512_final_impl(uint8_t *out, SHA512_CTX *sha);
+
 int SHA384_Init(SHA512_CTX *sha) {
   sha->h[0] = UINT64_C(0xcbbb9d5dc1059ed8);
   sha->h[1] = UINT64_C(0x629a292a367cd507);
@@ -146,8 +148,8 @@ uint8_t *SHA512_256(const uint8_t *data, size_t len,
                     uint8_t out[SHA512_256_DIGEST_LENGTH]) {
   SHA512_CTX ctx;
   SHA512_256_Init(&ctx);
-  SHA512_Update(&ctx, data, len);
-  SHA512_Final(out, &ctx);
+  SHA512_256_Update(&ctx, data, len);
+  SHA512_256_Final(out, &ctx);
   OPENSSL_cleanse(&ctx, sizeof(ctx));
   return out;
 }
@@ -161,7 +163,8 @@ static void sha512_block_data_order(uint64_t *state, const uint8_t *in,
 int SHA384_Final(uint8_t out[SHA384_DIGEST_LENGTH], SHA512_CTX *sha) {
   // |SHA384_Init| sets |sha->md_len| to |SHA384_DIGEST_LENGTH|, so this has a
   // |smaller output.
-  return SHA512_Final(out, sha);
+  assert(sha->md_len == SHA384_DIGEST_LENGTH);
+  return sha512_final_impl(out, sha);
 }
 
 int SHA384_Update(SHA512_CTX *sha, const void *data, size_t len) {
@@ -172,11 +175,11 @@ int SHA512_256_Update(SHA512_CTX *sha, const void *data, size_t len) {
   return SHA512_Update(sha, data, len);
 }
 
-int SHA512_256_Final(uint8_t out[SHA512_256_DIGEST_LENGTH],
-                                    SHA512_CTX *sha) {
+int SHA512_256_Final(uint8_t out[SHA512_256_DIGEST_LENGTH], SHA512_CTX *sha) {
   // |SHA512_256_Init| sets |sha->md_len| to |SHA512_256_DIGEST_LENGTH|, so this
   // has a |smaller output.
-  return SHA512_Final(out, sha);
+  assert(sha->md_len == SHA512_256_DIGEST_LENGTH);
+  return sha512_final_impl(out, sha);
 }
 
 void SHA512_Transform(SHA512_CTX *c, const uint8_t block[SHA512_CBLOCK]) {
@@ -232,6 +235,15 @@ int SHA512_Update(SHA512_CTX *c, const void *in_data, size_t len) {
 }
 
 int SHA512_Final(uint8_t out[SHA512_DIGEST_LENGTH], SHA512_CTX *sha) {
+  // Ideally we would assert |sha->md_len| is |SHA512_DIGEST_LENGTH| to match
+  // the size hint, but calling code often pairs |SHA384_Init| with
+  // |SHA512_Final| and expects |sha->md_len| to carry the over.
+  //
+  // TODO(davidben): Add an assert and fix code to match them up.
+  return sha512_final_impl(out, sha);
+}
+
+static int sha512_final_impl(uint8_t *out, SHA512_CTX *sha) {
   uint8_t *p = sha->p;
   size_t n = sha->num;