From 1bec25297ce6315271b45a54cfbb46f4e5ad2108 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Mon, 30 Nov 2020 12:24:42 -0800 Subject: [PATCH] Poly1305: Use |size_t|; assert |poly1305_state| is large enough. Clarify that there are no truncation issues on targets where the range of |unsigned| is smaller than the range of |size_t|. Ensure that |poly1305_state| is (still) large enough. This is a good idea independently of this change, but is especially important because switching the fields to |size_t| might have enlarged the structures. Change-Id: I16e408229c28fcba6c3592603ddb9431cf1f142d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44244 Commit-Queue: Adam Langley Reviewed-by: Adam Langley --- crypto/poly1305/poly1305.c | 17 ++++++++++------- crypto/poly1305/poly1305_arm.c | 24 +++++++++++++----------- crypto/poly1305/poly1305_vec.c | 4 ++++ 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/crypto/poly1305/poly1305.c b/crypto/poly1305/poly1305.c index a6dd14501..31a567d76 100644 --- a/crypto/poly1305/poly1305.c +++ b/crypto/poly1305/poly1305.c @@ -46,10 +46,14 @@ struct poly1305_state_st { uint32_t s1, s2, s3, s4; uint32_t h0, h1, h2, h3, h4; uint8_t buf[16]; - unsigned int buf_used; + size_t buf_used; uint8_t key[16]; }; +OPENSSL_STATIC_ASSERT( + sizeof(struct poly1305_state_st) + 63 <= sizeof(poly1305_state), + "poly1305_state isn't large enough to hold aligned poly1305_state_st"); + static inline struct poly1305_state_st *poly1305_aligned_state( poly1305_state *state) { return (struct poly1305_state_st *)(((uintptr_t)state + 63) & ~63); @@ -200,7 +204,6 @@ void CRYPTO_poly1305_init(poly1305_state *statep, const uint8_t key[32]) { void CRYPTO_poly1305_update(poly1305_state *statep, const uint8_t *in, size_t in_len) { - unsigned int i; struct poly1305_state_st *state = poly1305_aligned_state(statep); #if defined(OPENSSL_POLY1305_NEON) @@ -211,11 +214,11 @@ void CRYPTO_poly1305_update(poly1305_state *statep, const uint8_t *in, #endif if (state->buf_used) { - unsigned todo = 16 - state->buf_used; + size_t todo = 16 - state->buf_used; if (todo > in_len) { - todo = (unsigned)in_len; + todo = in_len; } - for (i = 0; i < todo; i++) { + for (size_t i = 0; i < todo; i++) { state->buf[state->buf_used + i] = in[i]; } state->buf_used += todo; @@ -236,10 +239,10 @@ void CRYPTO_poly1305_update(poly1305_state *statep, const uint8_t *in, } if (in_len) { - for (i = 0; i < in_len; i++) { + for (size_t i = 0; i < in_len; i++) { state->buf[i] = in[i]; } - state->buf_used = (unsigned)in_len; + state->buf_used = in_len; } } diff --git a/crypto/poly1305/poly1305_arm.c b/crypto/poly1305/poly1305_arm.c index 004221ded..d6f034c65 100644 --- a/crypto/poly1305/poly1305_arm.c +++ b/crypto/poly1305/poly1305_arm.c @@ -36,7 +36,7 @@ extern void addmulmod(fe1305x2 *r, const fe1305x2 *x, const fe1305x2 *y, const fe1305x2 *c); extern int blocks(fe1305x2 *h, const fe1305x2 *precomp, const uint8_t *in, - unsigned int inlen); + size_t inlen); static void freeze(fe1305x2 *r) { int i; @@ -136,7 +136,7 @@ static void fe1305x2_tobytearray(uint8_t r[16], fe1305x2 *x) { } static void fe1305x2_frombytearray(fe1305x2 *r, const uint8_t *x, size_t xlen) { - unsigned i; + size_t i; uint8_t t[17]; for (i = 0; (i < 16) && (i < xlen); i++) { @@ -179,17 +179,20 @@ static const alignas(16) fe1305x2 zero; struct poly1305_state_st { uint8_t data[sizeof(fe1305x2[5]) + 128]; uint8_t buf[32]; - unsigned int buf_used; + size_t buf_used; uint8_t key[16]; }; +OPENSSL_STATIC_ASSERT( + sizeof(struct poly1305_state_st) + 63 <= sizeof(poly1305_state), + "poly1305_state isn't large enough to hold aligned poly1305_state_st."); + void CRYPTO_poly1305_init_neon(poly1305_state *state, const uint8_t key[32]) { struct poly1305_state_st *st = (struct poly1305_state_st *)(state); fe1305x2 *const r = (fe1305x2 *)(st->data + (15 & (-(int)st->data))); fe1305x2 *const h = r + 1; fe1305x2 *const c = h + 1; fe1305x2 *const precomp = c + 1; - unsigned int j; r->v[1] = r->v[0] = 0x3ffffff & load32(key); r->v[3] = r->v[2] = 0x3ffff03 & (load32(key + 3) >> 2); @@ -197,7 +200,7 @@ void CRYPTO_poly1305_init_neon(poly1305_state *state, const uint8_t key[32]) { r->v[7] = r->v[6] = 0x3f03fff & (load32(key + 9) >> 6); r->v[9] = r->v[8] = 0x00fffff & (load32(key + 12) >> 8); - for (j = 0; j < 10; j++) { + for (size_t j = 0; j < 10; j++) { h->v[j] = 0; // XXX: should fast-forward a bit } @@ -215,14 +218,13 @@ void CRYPTO_poly1305_update_neon(poly1305_state *state, const uint8_t *in, fe1305x2 *const h = r + 1; fe1305x2 *const c = h + 1; fe1305x2 *const precomp = c + 1; - unsigned int i; if (st->buf_used) { - unsigned int todo = 32 - st->buf_used; + size_t todo = 32 - st->buf_used; if (todo > in_len) { todo = in_len; } - for (i = 0; i < todo; i++) { + for (size_t i = 0; i < todo; i++) { st->buf[st->buf_used + i] = in[i]; } st->buf_used += todo; @@ -232,7 +234,7 @@ void CRYPTO_poly1305_update_neon(poly1305_state *state, const uint8_t *in, if (st->buf_used == sizeof(st->buf) && in_len) { addmulmod(h, h, precomp, &zero); fe1305x2_frombytearray(c, st->buf, sizeof(st->buf)); - for (i = 0; i < 10; i++) { + for (size_t i = 0; i < 10; i++) { h->v[i] += c->v[i]; } st->buf_used = 0; @@ -240,7 +242,7 @@ void CRYPTO_poly1305_update_neon(poly1305_state *state, const uint8_t *in, } while (in_len > 32) { - unsigned int tlen = 1048576; + size_t tlen = 1048576; if (in_len < tlen) { tlen = in_len; } @@ -250,7 +252,7 @@ void CRYPTO_poly1305_update_neon(poly1305_state *state, const uint8_t *in, } if (in_len) { - for (i = 0; i < in_len; i++) { + for (size_t i = 0; i < in_len; i++) { st->buf[i] = in[i]; } st->buf_used = in_len; diff --git a/crypto/poly1305/poly1305_vec.c b/crypto/poly1305/poly1305_vec.c index 29cd5c3f3..83f1efee3 100644 --- a/crypto/poly1305/poly1305_vec.c +++ b/crypto/poly1305/poly1305_vec.c @@ -92,6 +92,10 @@ typedef struct poly1305_state_internal_t { } poly1305_state_internal; /* 448 bytes total + 63 bytes for alignment = 511 bytes raw */ +OPENSSL_STATIC_ASSERT( + sizeof(struct poly1305_state_internal_t) + 63 <= sizeof(poly1305_state), + "poly1305_state isn't large enough to hold aligned poly1305_state_internal_t"); + static inline poly1305_state_internal *poly1305_aligned_state( poly1305_state *state) { return (poly1305_state_internal *)(((uint64_t)state + 63) & ~63);