Skip to content

Commit

Permalink
boringssl: Fix compilation with gcc 11.X
Browse files Browse the repository at this point in the history
This is a backport of various upstream patches. These patches can be
removed after android updates its vendored boringssl version. In the
meantime, we supply them manually.

See: https://bugs.chromium.org/p/boringssl/issues/detail?id=402#c4

Fixes #16
Fixes #26
  • Loading branch information
vanaf committed Jun 2, 2021
1 parent ec9a74f commit df7bb55
Show file tree
Hide file tree
Showing 11 changed files with 4,068 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
From 139adff9b27eaf0bdaac664ec4c9a7db2fe3f920 Mon Sep 17 00:00:00 2001
From: David Benjamin <[email protected]>
Date: Thu, 25 Mar 2021 01:41:51 -0400
Subject: [PATCH] Fix mismatch between header and implementation of bn_sqr_comba8.

Bug: 402
Change-Id: I6de879f44f6e3eca26f2f49c500769d944fa9bc0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46404
Reviewed-by: Adam Langley <[email protected]>
---

diff --git a/crypto/fipsmodule/bn/internal.h b/crypto/fipsmodule/bn/internal.h
index 623e0c6..3d368db 100644
--- a/crypto/fipsmodule/bn/internal.h
+++ b/crypto/fipsmodule/bn/internal.h
@@ -297,7 +297,7 @@
void bn_mul_comba8(BN_ULONG r[16], const BN_ULONG a[8], const BN_ULONG b[8]);

// bn_sqr_comba8 sets |r| to |a|^2.
-void bn_sqr_comba8(BN_ULONG r[16], const BN_ULONG a[4]);
+void bn_sqr_comba8(BN_ULONG r[16], const BN_ULONG a[8]);

// bn_sqr_comba4 sets |r| to |a|^2.
void bn_sqr_comba4(BN_ULONG r[8], const BN_ULONG a[4]);
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
From a24ab549e6ae246b391155d7bed3790ac0e07de2 Mon Sep 17 00:00:00 2001
From: David Benjamin <[email protected]>
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 <[email protected]>
---

diff --git a/crypto/fipsmodule/digest/digests.c b/crypto/fipsmodule/digest/digests.c
index 16daeba..f006ebb 100644
--- a/crypto/fipsmodule/digest/digests.c
+++ b/crypto/fipsmodule/digest/digests.c
@@ -247,13 +247,21 @@
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/crypto/fipsmodule/sha/sha512.c b/crypto/fipsmodule/sha/sha512.c
index fd02574..ba86c1e 100644
--- a/crypto/fipsmodule/sha/sha512.c
+++ b/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 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 @@
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 @@
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_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;

Loading

2 comments on commit df7bb55

@brianjmurrell
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any estimates on when a release will be made that includes these fixes, needed to build with gcc 11?

@nmeum
Copy link
Owner

@nmeum nmeum commented on df7bb55 Jul 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just need to fix #44 and #38 before making a new release. I just didn't have the time yet to do so, I hope I get around to it in the next few days.

Please sign in to comment.