From c807a2371449998cd767826ba06adc3e122e6d4a Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 4 Jul 2023 11:18:47 -0400 Subject: [PATCH] Fix truncation warnings with the iteration count They end up in uint64_t frequently right now just because the CBS APIs use uint64_t, but we don't actually accept that large of an iteration count. Also use uint32_t instead of unsigned. This type should be sized based on how large of an iteration count we think is reasonable, not something platform-dependent. Bug: 516 Change-Id: Ie5ff379af6bc65c5e4d25f4d10774bd819f08a50 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61426 Commit-Queue: David Benjamin Auto-Submit: David Benjamin Reviewed-by: Bob Beck --- crypto/evp/pbkdf.c | 6 +++--- crypto/pkcs8/internal.h | 6 +++--- crypto/pkcs8/p5_pbev2.c | 6 +++--- crypto/pkcs8/pkcs8.c | 14 +++++++------- crypto/pkcs8/pkcs8_x509.c | 13 ++++++++----- include/openssl/evp.h | 4 ++-- 6 files changed, 26 insertions(+), 23 deletions(-) diff --git a/crypto/evp/pbkdf.c b/crypto/evp/pbkdf.c index f23a74bd8..33bfe8f0c 100644 --- a/crypto/evp/pbkdf.c +++ b/crypto/evp/pbkdf.c @@ -63,7 +63,7 @@ int PKCS5_PBKDF2_HMAC(const char *password, size_t password_len, - const uint8_t *salt, size_t salt_len, unsigned iterations, + const uint8_t *salt, size_t salt_len, uint32_t iterations, const EVP_MD *digest, size_t key_len, uint8_t *out_key) { // See RFC 8018, section 5.2. int ret = 0; @@ -98,7 +98,7 @@ int PKCS5_PBKDF2_HMAC(const char *password, size_t password_len, } OPENSSL_memcpy(out_key, digest_tmp, todo); - for (unsigned j = 1; j < iterations; j++) { + for (uint32_t j = 1; j < iterations; j++) { // Compute the remaining U_* values and XOR. if (!HMAC_Init_ex(&hctx, NULL, 0, NULL, NULL) || !HMAC_Update(&hctx, digest_tmp, md_len) || @@ -139,7 +139,7 @@ err: int PKCS5_PBKDF2_HMAC_SHA1(const char *password, size_t password_len, const uint8_t *salt, size_t salt_len, - unsigned iterations, size_t key_len, + uint32_t iterations, size_t key_len, uint8_t *out_key) { return PKCS5_PBKDF2_HMAC(password, password_len, salt, salt_len, iterations, EVP_sha1(), key_len, out_key); diff --git a/crypto/pkcs8/internal.h b/crypto/pkcs8/internal.h index ab84c82b0..483e4b647 100644 --- a/crypto/pkcs8/internal.h +++ b/crypto/pkcs8/internal.h @@ -87,13 +87,13 @@ int pkcs8_pbe_decrypt(uint8_t **out, size_t *out_len, CBS *algorithm, // key material to |out| and returns one. Otherwise, it returns zero. |id| // should be one of the |PKCS12_*_ID| values. int pkcs12_key_gen(const char *pass, size_t pass_len, const uint8_t *salt, - size_t salt_len, uint8_t id, unsigned iterations, + size_t salt_len, uint8_t id, uint32_t iterations, size_t out_len, uint8_t *out, const EVP_MD *md); // pkcs12_pbe_encrypt_init configures |ctx| for encrypting with a PBES1 scheme // defined in PKCS#12. It writes the corresponding AlgorithmIdentifier to |out|. int pkcs12_pbe_encrypt_init(CBB *out, EVP_CIPHER_CTX *ctx, int alg, - unsigned iterations, const char *pass, + uint32_t iterations, const char *pass, size_t pass_len, const uint8_t *salt, size_t salt_len); @@ -121,7 +121,7 @@ int PKCS5_pbe2_decrypt_init(const struct pbe_suite *suite, EVP_CIPHER_CTX *ctx, // as defined in RFC 2998, with the specified parameters. It writes the // corresponding AlgorithmIdentifier to |out|. int PKCS5_pbe2_encrypt_init(CBB *out, EVP_CIPHER_CTX *ctx, - const EVP_CIPHER *cipher, unsigned iterations, + const EVP_CIPHER *cipher, uint32_t iterations, const char *pass, size_t pass_len, const uint8_t *salt, size_t salt_len); diff --git a/crypto/pkcs8/p5_pbev2.c b/crypto/pkcs8/p5_pbev2.c index e58cf444d..173fa9804 100644 --- a/crypto/pkcs8/p5_pbev2.c +++ b/crypto/pkcs8/p5_pbev2.c @@ -144,7 +144,7 @@ static int add_cipher_oid(CBB *out, int nid) { } static int pkcs5_pbe2_cipher_init(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher, - const EVP_MD *pbkdf2_md, unsigned iterations, + const EVP_MD *pbkdf2_md, uint32_t iterations, const char *pass, size_t pass_len, const uint8_t *salt, size_t salt_len, const uint8_t *iv, size_t iv_len, int enc) { @@ -162,7 +162,7 @@ static int pkcs5_pbe2_cipher_init(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher, } int PKCS5_pbe2_encrypt_init(CBB *out, EVP_CIPHER_CTX *ctx, - const EVP_CIPHER *cipher, unsigned iterations, + const EVP_CIPHER *cipher, uint32_t iterations, const char *pass, size_t pass_len, const uint8_t *salt, size_t salt_len) { int cipher_nid = EVP_CIPHER_nid(cipher); @@ -310,7 +310,7 @@ int PKCS5_pbe2_decrypt_init(const struct pbe_suite *suite, EVP_CIPHER_CTX *ctx, return 0; } - return pkcs5_pbe2_cipher_init(ctx, cipher, md, (unsigned)iterations, pass, + return pkcs5_pbe2_cipher_init(ctx, cipher, md, (uint32_t)iterations, pass, pass_len, CBS_data(&salt), CBS_len(&salt), CBS_data(&iv), CBS_len(&iv), 0 /* decrypt */); } diff --git a/crypto/pkcs8/pkcs8.c b/crypto/pkcs8/pkcs8.c index 6dd111b85..4bc337bfd 100644 --- a/crypto/pkcs8/pkcs8.c +++ b/crypto/pkcs8/pkcs8.c @@ -106,7 +106,7 @@ err: } int pkcs12_key_gen(const char *pass, size_t pass_len, const uint8_t *salt, - size_t salt_len, uint8_t id, unsigned iterations, + size_t salt_len, uint8_t id, uint32_t iterations, size_t out_len, uint8_t *out, const EVP_MD *md) { // See https://tools.ietf.org/html/rfc7292#appendix-B. Quoted parts of the // specification have errata applied and other typos fixed. @@ -182,7 +182,7 @@ int pkcs12_key_gen(const char *pass, size_t pass_len, const uint8_t *salt, !EVP_DigestFinal_ex(&ctx, A, &A_len)) { goto err; } - for (unsigned iter = 1; iter < iterations; iter++) { + for (uint32_t iter = 1; iter < iterations; iter++) { if (!EVP_DigestInit_ex(&ctx, md, NULL) || !EVP_DigestUpdate(&ctx, A, A_len) || !EVP_DigestFinal_ex(&ctx, A, &A_len)) { @@ -229,7 +229,7 @@ err: } static int pkcs12_pbe_cipher_init(const struct pbe_suite *suite, - EVP_CIPHER_CTX *ctx, unsigned iterations, + EVP_CIPHER_CTX *ctx, uint32_t iterations, const char *pass, size_t pass_len, const uint8_t *salt, size_t salt_len, int is_encrypt) { @@ -271,7 +271,7 @@ static int pkcs12_pbe_decrypt_init(const struct pbe_suite *suite, return 0; } - return pkcs12_pbe_cipher_init(suite, ctx, (unsigned)iterations, pass, + return pkcs12_pbe_cipher_init(suite, ctx, (uint32_t)iterations, pass, pass_len, CBS_data(&salt), CBS_len(&salt), 0 /* decrypt */); } @@ -329,7 +329,7 @@ static const struct pbe_suite *get_pkcs12_pbe_suite(int pbe_nid) { } int pkcs12_pbe_encrypt_init(CBB *out, EVP_CIPHER_CTX *ctx, int alg, - unsigned iterations, const char *pass, + uint32_t iterations, const char *pass, size_t pass_len, const uint8_t *salt, size_t salt_len) { const struct pbe_suite *suite = get_pkcs12_pbe_suite(alg); @@ -489,10 +489,10 @@ int PKCS8_marshal_encrypted_private_key(CBB *out, int pbe_nid, // it. See 5693a30813a031d3921a016a870420e7eb93ec90 in OpenSSL. int alg_ok; if (pbe_nid == -1) { - alg_ok = PKCS5_pbe2_encrypt_init(&epki, &ctx, cipher, (unsigned)iterations, + alg_ok = PKCS5_pbe2_encrypt_init(&epki, &ctx, cipher, (uint32_t)iterations, pass, pass_len, salt, salt_len); } else { - alg_ok = pkcs12_pbe_encrypt_init(&epki, &ctx, pbe_nid, (unsigned)iterations, + alg_ok = pkcs12_pbe_encrypt_init(&epki, &ctx, pbe_nid, (uint32_t)iterations, pass, pass_len, salt, salt_len); } if (!alg_ok) { diff --git a/crypto/pkcs8/pkcs8_x509.c b/crypto/pkcs8/pkcs8_x509.c index 83d34e6a1..87c096198 100644 --- a/crypto/pkcs8/pkcs8_x509.c +++ b/crypto/pkcs8/pkcs8_x509.c @@ -87,6 +87,7 @@ int pkcs12_iterations_acceptable(uint64_t iterations) { static const uint64_t kIterationsLimit = 100 * 1000000; #endif + assert(kIterationsLimit <= UINT32_MAX); return 0 < iterations && iterations <= kIterationsLimit; } @@ -554,7 +555,7 @@ err: static int pkcs12_check_mac(int *out_mac_ok, const char *password, size_t password_len, const CBS *salt, - unsigned iterations, const EVP_MD *md, + uint32_t iterations, const EVP_MD *md, const CBS *authsafes, const CBS *expected_mac) { int ret = 0; uint8_t hmac_key[EVP_MAX_MD_SIZE]; @@ -676,13 +677,15 @@ int PKCS12_get_key_and_certs(EVP_PKEY **out_key, STACK_OF(X509) *out_certs, } // The iteration count is optional and the default is one. - uint64_t iterations = 1; + uint32_t iterations = 1; if (CBS_len(&mac_data) > 0) { - if (!CBS_get_asn1_uint64(&mac_data, &iterations) || - !pkcs12_iterations_acceptable(iterations)) { + uint64_t iterations_u64; + if (!CBS_get_asn1_uint64(&mac_data, &iterations_u64) || + !pkcs12_iterations_acceptable(iterations_u64)) { OPENSSL_PUT_ERROR(PKCS8, PKCS8_R_BAD_PKCS12_DATA); goto err; } + iterations = (uint32_t)iterations_u64; } int mac_ok; @@ -1056,7 +1059,7 @@ static int add_cert_safe_contents(CBB *cbb, X509 *cert, } static int add_encrypted_data(CBB *out, int pbe_nid, const char *password, - size_t password_len, unsigned iterations, + size_t password_len, uint32_t iterations, const uint8_t *in, size_t in_len) { uint8_t salt[PKCS5_SALT_LEN]; if (!RAND_bytes(salt, sizeof(salt))) { diff --git a/include/openssl/evp.h b/include/openssl/evp.h index 1cdaca273..6f6eaa9bc 100644 --- a/include/openssl/evp.h +++ b/include/openssl/evp.h @@ -475,7 +475,7 @@ OPENSSL_EXPORT int EVP_PKEY_print_params(BIO *out, const EVP_PKEY *pkey, // returns one on success and zero on allocation failure or if iterations is 0. OPENSSL_EXPORT int PKCS5_PBKDF2_HMAC(const char *password, size_t password_len, const uint8_t *salt, size_t salt_len, - unsigned iterations, const EVP_MD *digest, + uint32_t iterations, const EVP_MD *digest, size_t key_len, uint8_t *out_key); // PKCS5_PBKDF2_HMAC_SHA1 is the same as PKCS5_PBKDF2_HMAC, but with |digest| @@ -483,7 +483,7 @@ OPENSSL_EXPORT int PKCS5_PBKDF2_HMAC(const char *password, size_t password_len, OPENSSL_EXPORT int PKCS5_PBKDF2_HMAC_SHA1(const char *password, size_t password_len, const uint8_t *salt, size_t salt_len, - unsigned iterations, size_t key_len, + uint32_t iterations, size_t key_len, uint8_t *out_key); // EVP_PBE_scrypt expands |password| into a secret key of length |key_len| using