From f466cdb5e0e2cd2a2edcfb02b031c47dacd9b297 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 14 Mar 2017 15:35:27 -0400 Subject: [PATCH] size_t the RSA padding add functions. The padding check functions will need to tweak their calling conventions and the constant-time helpers, so leaving those alone for now. These were the easy ones. BUG=22 Change-Id: Ia00e41e26a134de17d56be3def5820cb042794e1 Reviewed-on: https://boringssl-review.googlesource.com/14265 Reviewed-by: David Benjamin Commit-Queue: David Benjamin CQ-Verified: CQ bot account: commit-bot@chromium.org --- crypto/rsa/internal.h | 16 +++--- crypto/rsa/padding.c | 101 ++++++++++++++++-------------------- decrepit/rsa/rsa_decrepit.c | 6 +-- include/openssl/rsa.h | 10 ++-- 4 files changed, 60 insertions(+), 73 deletions(-) diff --git a/crypto/rsa/internal.h b/crypto/rsa/internal.h index c6ea97f09..b6a072769 100644 --- a/crypto/rsa/internal.h +++ b/crypto/rsa/internal.h @@ -95,24 +95,20 @@ int BN_BLINDING_invert(BIGNUM *n, const BN_BLINDING *b, BN_MONT_CTX *mont_ctx, BN_CTX *ctx); -int RSA_padding_add_PKCS1_type_1(uint8_t *to, unsigned to_len, - const uint8_t *from, unsigned from_len); +int RSA_padding_add_PKCS1_type_1(uint8_t *to, size_t to_len, + const uint8_t *from, size_t from_len); int RSA_padding_check_PKCS1_type_1(uint8_t *to, unsigned to_len, const uint8_t *from, unsigned from_len); -int RSA_padding_add_PKCS1_type_2(uint8_t *to, unsigned to_len, - const uint8_t *from, unsigned from_len); +int RSA_padding_add_PKCS1_type_2(uint8_t *to, size_t to_len, + const uint8_t *from, size_t from_len); int RSA_padding_check_PKCS1_type_2(uint8_t *to, unsigned to_len, const uint8_t *from, unsigned from_len); -int RSA_padding_add_PKCS1_OAEP_mgf1(uint8_t *to, unsigned to_len, - const uint8_t *from, unsigned from_len, - const uint8_t *param, unsigned plen, - const EVP_MD *md, const EVP_MD *mgf1md); int RSA_padding_check_PKCS1_OAEP_mgf1(uint8_t *to, unsigned to_len, const uint8_t *from, unsigned from_len, const uint8_t *param, unsigned plen, const EVP_MD *md, const EVP_MD *mgf1md); -int RSA_padding_add_none(uint8_t *to, unsigned to_len, const uint8_t *from, - unsigned from_len); +int RSA_padding_add_none(uint8_t *to, size_t to_len, const uint8_t *from, + size_t from_len); /* RSA_private_transform calls either the method-specific |private_transform| * function (if given) or the generic one. See the comment for diff --git a/crypto/rsa/padding.c b/crypto/rsa/padding.c index 6eb1adba6..ac583c461 100644 --- a/crypto/rsa/padding.c +++ b/crypto/rsa/padding.c @@ -71,10 +71,9 @@ /* TODO(fork): don't the check functions have to be constant time? */ -int RSA_padding_add_PKCS1_type_1(uint8_t *to, unsigned to_len, - const uint8_t *from, unsigned from_len) { - unsigned j; - +int RSA_padding_add_PKCS1_type_1(uint8_t *to, size_t to_len, + const uint8_t *from, size_t from_len) { + /* See RFC 8017, section 9.2. */ if (to_len < RSA_PKCS1_PADDING_SIZE) { OPENSSL_PUT_ERROR(RSA, RSA_R_KEY_SIZE_TOO_SMALL); return 0; @@ -85,17 +84,11 @@ int RSA_padding_add_PKCS1_type_1(uint8_t *to, unsigned to_len, return 0; } - uint8_t *p = to; - - *(p++) = 0; - *(p++) = 1; /* Private Key BT (Block Type) */ - - /* pad out with 0xff data */ - j = to_len - 3 - from_len; - OPENSSL_memset(p, 0xff, j); - p += j; - *(p++) = 0; - OPENSSL_memcpy(p, from, from_len); + to[0] = 0; + to[1] = 1; + OPENSSL_memset(to + 2, 0xff, to_len - 3 - from_len); + to[to_len - from_len - 1] = 0; + OPENSSL_memcpy(to + to_len - from_len, from, from_len); return 1; } @@ -151,10 +144,25 @@ int RSA_padding_check_PKCS1_type_1(uint8_t *to, unsigned to_len, return j; } -int RSA_padding_add_PKCS1_type_2(uint8_t *to, unsigned to_len, - const uint8_t *from, unsigned from_len) { - unsigned i, j; +static int rand_nonzero(uint8_t *out, size_t len) { + if (!RAND_bytes(out, len)) { + return 0; + } + for (size_t i = 0; i < len; i++) { + while (out[i] == 0) { + if (!RAND_bytes(out + i, 1)) { + return 0; + } + } + } + + return 1; +} + +int RSA_padding_add_PKCS1_type_2(uint8_t *to, size_t to_len, + const uint8_t *from, size_t from_len) { + /* See RFC 8017, section 7.2.1. */ if (to_len < RSA_PKCS1_PADDING_SIZE) { OPENSSL_PUT_ERROR(RSA, RSA_R_KEY_SIZE_TOO_SMALL); return 0; @@ -165,30 +173,16 @@ int RSA_padding_add_PKCS1_type_2(uint8_t *to, unsigned to_len, return 0; } - uint8_t *p = to; + to[0] = 0; + to[1] = 2; - *(p++) = 0; - *(p++) = 2; /* Public Key BT (Block Type) */ - - /* pad out with non-zero random data */ - j = to_len - 3 - from_len; - - if (!RAND_bytes(p, j)) { + size_t padding_len = to_len - 3 - from_len; + if (!rand_nonzero(to + 2, padding_len)) { return 0; } - for (i = 0; i < j; i++) { - while (*p == 0) { - if (!RAND_bytes(p, 1)) { - return 0; - } - } - p++; - } - - *(p++) = 0; - - OPENSSL_memcpy(p, from, from_len); + to[2 + padding_len] = 0; + OPENSSL_memcpy(to + to_len - from_len, from, from_len); return 1; } @@ -258,8 +252,8 @@ int RSA_padding_check_PKCS1_type_2(uint8_t *to, unsigned to_len, return (int)msg_len; } -int RSA_padding_add_none(uint8_t *to, unsigned to_len, const uint8_t *from, - unsigned from_len) { +int RSA_padding_add_none(uint8_t *to, size_t to_len, const uint8_t *from, + size_t from_len) { if (from_len > to_len) { OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE); return 0; @@ -317,15 +311,10 @@ err: return ret; } -int RSA_padding_add_PKCS1_OAEP_mgf1(uint8_t *to, unsigned to_len, - const uint8_t *from, unsigned from_len, - const uint8_t *param, unsigned param_len, +int RSA_padding_add_PKCS1_OAEP_mgf1(uint8_t *to, size_t to_len, + const uint8_t *from, size_t from_len, + const uint8_t *param, size_t param_len, const EVP_MD *md, const EVP_MD *mgf1md) { - unsigned i, emlen, mdlen; - uint8_t *db, *seed; - uint8_t *dbmask = NULL, seedmask[EVP_MAX_MD_SIZE]; - int ret = 0; - if (md == NULL) { md = EVP_sha1(); } @@ -333,14 +322,14 @@ int RSA_padding_add_PKCS1_OAEP_mgf1(uint8_t *to, unsigned to_len, mgf1md = md; } - mdlen = EVP_MD_size(md); + size_t mdlen = EVP_MD_size(md); if (to_len < 2 * mdlen + 2) { OPENSSL_PUT_ERROR(RSA, RSA_R_KEY_SIZE_TOO_SMALL); return 0; } - emlen = to_len - 1; + size_t emlen = to_len - 1; if (from_len > emlen - 2 * mdlen - 1) { OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE); return 0; @@ -352,8 +341,8 @@ int RSA_padding_add_PKCS1_OAEP_mgf1(uint8_t *to, unsigned to_len, } to[0] = 0; - seed = to + 1; - db = to + mdlen + 1; + uint8_t *seed = to + 1; + uint8_t *db = to + mdlen + 1; if (!EVP_Digest(param, param_len, db, NULL, md, NULL)) { return 0; @@ -365,23 +354,25 @@ int RSA_padding_add_PKCS1_OAEP_mgf1(uint8_t *to, unsigned to_len, return 0; } - dbmask = OPENSSL_malloc(emlen - mdlen); + uint8_t *dbmask = OPENSSL_malloc(emlen - mdlen); if (dbmask == NULL) { OPENSSL_PUT_ERROR(RSA, ERR_R_MALLOC_FAILURE); return 0; } + int ret = 0; if (!PKCS1_MGF1(dbmask, emlen - mdlen, seed, mdlen, mgf1md)) { goto out; } - for (i = 0; i < emlen - mdlen; i++) { + for (size_t i = 0; i < emlen - mdlen; i++) { db[i] ^= dbmask[i]; } + uint8_t seedmask[EVP_MAX_MD_SIZE]; if (!PKCS1_MGF1(seedmask, mdlen, db, emlen - mdlen, mgf1md)) { goto out; } - for (i = 0; i < mdlen; i++) { + for (size_t i = 0; i < mdlen; i++) { seed[i] ^= seedmask[i]; } ret = 1; diff --git a/decrepit/rsa/rsa_decrepit.c b/decrepit/rsa/rsa_decrepit.c index 0d7c5f66b..c4ef5b66f 100644 --- a/decrepit/rsa/rsa_decrepit.c +++ b/decrepit/rsa/rsa_decrepit.c @@ -95,9 +95,9 @@ int RSA_verify_PKCS1_PSS(RSA *rsa, const uint8_t *mHash, const EVP_MD *Hash, return RSA_verify_PKCS1_PSS_mgf1(rsa, mHash, Hash, NULL, EM, sLen); } -int RSA_padding_add_PKCS1_OAEP(uint8_t *to, unsigned to_len, - const uint8_t *from, unsigned from_len, - const uint8_t *param, unsigned param_len) { +int RSA_padding_add_PKCS1_OAEP(uint8_t *to, size_t to_len, + const uint8_t *from, size_t from_len, + const uint8_t *param, size_t param_len) { return RSA_padding_add_PKCS1_OAEP_mgf1(to, to_len, from, from_len, param, param_len, NULL, NULL); } diff --git a/include/openssl/rsa.h b/include/openssl/rsa.h index bad3fad81..d8f76f025 100644 --- a/include/openssl/rsa.h +++ b/include/openssl/rsa.h @@ -354,8 +354,8 @@ OPENSSL_EXPORT int RSA_padding_add_PKCS1_PSS_mgf1(RSA *rsa, uint8_t *EM, * * It returns one on success or zero on error. */ OPENSSL_EXPORT int RSA_padding_add_PKCS1_OAEP_mgf1( - uint8_t *to, unsigned to_len, const uint8_t *from, unsigned from_len, - const uint8_t *param, unsigned param_len, const EVP_MD *md, + uint8_t *to, size_t to_len, const uint8_t *from, size_t from_len, + const uint8_t *param, size_t param_len, const EVP_MD *md, const EVP_MD *mgf1md); /* RSA_add_pkcs1_prefix builds a version of |msg| prefixed with the DigestInfo @@ -519,11 +519,11 @@ OPENSSL_EXPORT int RSA_verify_PKCS1_PSS(RSA *rsa, const uint8_t *mHash, /* RSA_padding_add_PKCS1_OAEP acts like |RSA_padding_add_PKCS1_OAEP_mgf1| but * the |md| and |mgf1md| parameters of the latter are implicitly set to NULL, * which means SHA-1. */ -OPENSSL_EXPORT int RSA_padding_add_PKCS1_OAEP(uint8_t *to, unsigned to_len, +OPENSSL_EXPORT int RSA_padding_add_PKCS1_OAEP(uint8_t *to, size_t to_len, const uint8_t *from, - unsigned from_len, + size_t from_len, const uint8_t *param, - unsigned param_len); + size_t param_len); struct rsa_meth_st {