Remove CRYPTO_IOVEC_MAX. Instead, allocate on the heap when it is exceeded. There should already be no more code using the constant in the wild. Bug: 544930636 Change-Id: I9a84c791e3aae2e451d8b806220cfb636a6a6964 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/100727 Reviewed-by: David Benjamin <davidben@google.com> Presubmit-BoringSSL-Verified: boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Commit-Queue: Rudolf Polzer <rpolzer@google.com>
diff --git a/crypto/cipher/e_tls.cc b/crypto/cipher/e_tls.cc index 0edf61a..8f93522 100644 --- a/crypto/cipher/e_tls.cc +++ b/crypto/cipher/e_tls.cc
@@ -332,12 +332,15 @@ // Split the decrypted record into `iovecs_without_trailer` and `trailer`, // based on the public lower bound of where the plaintext ends. The plaintext // is followed by `mac_len` and then at most 256 bytes of padding. - InplaceVector<CRYPTO_IOVEC, CRYPTO_IOVEC_MAX> iovecs_without_trailer; - iovecs_without_trailer.CopyFrom(iovecs); + bssl::iovec::MaybeInplaceArray iovecs_without_trailer; + if (!iovecs_without_trailer.CopyFrom(iovecs)) { + return 0; + } uint8_t trailer_buf[EVP_MAX_MD_SIZE + 256]; const size_t trailer_len = std::min(in_len, mac_len + 256); - std::optional<Span<const uint8_t>> trailer = bssl::iovec::GetAndRemoveOutSuffix( - Span(trailer_buf).first(trailer_len), Span(iovecs_without_trailer)); + std::optional<Span<const uint8_t>> trailer = + bssl::iovec::GetAndRemoveOutSuffix(Span(trailer_buf).first(trailer_len), + Span(iovecs_without_trailer)); BSSL_CHECK(trailer.has_value()); // Remove CBC padding. Code from here on is timing-sensitive with respect to
diff --git a/crypto/fipsmodule/cipher/aead.cc.inc b/crypto/fipsmodule/cipher/aead.cc.inc index c27e4a7..cfbd914 100644 --- a/crypto/fipsmodule/cipher/aead.cc.inc +++ b/crypto/fipsmodule/cipher/aead.cc.inc
@@ -492,8 +492,10 @@ if (!ctx->aead->openv) { if (ctx->tag_len && ctx->aead->openv_detached) { // Try with a detached tag. - InplaceVector<CRYPTO_IOVEC, CRYPTO_IOVEC_MAX> detached_iovecs; - detached_iovecs.CopyFrom(iovecs); + bssl::iovec::MaybeInplaceArray detached_iovecs; + if (!detached_iovecs.CopyFrom(iovecs)) { + return 0; + } uint8_t tagbuf[EVP_AEAD_MAX_OVERHEAD]; std::optional<Span<const uint8_t>> tag = bssl::iovec::GetAndRemoveSuffix(
diff --git a/crypto/fipsmodule/cipher/internal.h b/crypto/fipsmodule/cipher/internal.h index a8316dd..70ecf5e 100644 --- a/crypto/fipsmodule/cipher/internal.h +++ b/crypto/fipsmodule/cipher/internal.h
@@ -163,13 +163,10 @@ // IsValid returns whether the given `CRYPTO_IVEC` or `CRYPTO_IOVEC` is // valid for use with public APIs, i.e. does not contain more than `SIZE_MAX` -// bytes and not more than `CRYPTO_IOVEC_MAX` chunks. Note that the `EVP_AEAD` -// methods need to accept an arbitrary number of chunks. +// bytes. Note that the `EVP_AEAD` methods need to accept an arbitrary number +// of chunks. template <typename IVec> inline bool IsValid(Span<IVec> ivecs) { - if (ivecs.size() > CRYPTO_IOVEC_MAX) { - return false; - } size_t allowed = SIZE_MAX; for (const IVec &ivec : ivecs) { size_t len = ivec.len; @@ -413,6 +410,10 @@ return call_func(f_final, current_range_head); } +// MaybeInplaceArray can hold a copy of a CRYPTO_IOVEC. If it is a low +// amount of entries, it will be stored on the stack, otherwise on the heap. +using MaybeInplaceArray = bssl::MaybeInplaceArray<CRYPTO_IOVEC, 16>; + // ForEachOutBlockRange is like `ForEachBlockRange` but reads from a // `CRYPTO_IOVEC`'s `out` member instead. template <
diff --git a/crypto/mem_internal.h b/crypto/mem_internal.h index d58b7f3..59927e1 100644 --- a/crypto/mem_internal.h +++ b/crypto/mem_internal.h
@@ -665,6 +665,41 @@ PackedSize<N> size_ = 0; }; +// A MaybeInplaceArray is like an `Array`, but backed by an `InplaceVector` if +// `size() <= N`, and by an `Array` if `size() > N`. +template <typename T, size_t N> +class MaybeInplaceArray { + public: + // CopyFrom replaces the array with a newly-allocated copy of `in`. It returns + // true on success and false on error. + // + // `in` may not alias `this`. + [[nodiscard]] bool CopyFrom(Span<const T> in) { + if (in.size() <= N) { + small_.CopyFrom(in); + large_.Reset(); + } else { + if (!large_.CopyFrom(in)) { + return false; + } + small_.clear(); + } + return true; + } + + // Minimal methods to allow conversion to a `Span`. + const T *data() const { return IsSmall() ? small_.data() : large_.data(); } + T *data() { return IsSmall() ? small_.data() : large_.data(); } + size_t size() const { return IsSmall() ? small_.size() : large_.size(); } + + private: + bool IsSmall() const { return large_.empty(); } + + // TODO(crbug.com/548222332): Optimize storage by putting this on the stack. + // Invariant: at least one of these two is empty. + Array<T> large_; + InplaceVector<T, N> small_; +}; BSSL_NAMESPACE_END
diff --git a/include/openssl/aead.h b/include/openssl/aead.h index 6b8ad57..af90b26 100644 --- a/include/openssl/aead.h +++ b/include/openssl/aead.h
@@ -398,16 +398,14 @@ size_t in_tag_len, const uint8_t *ad, size_t ad_len); // crypto_ivec_st (aka `CRYPTO_IVEC`) combines a pointer to input data with its -// length. It is usually passed as an array of length of at most -// `CRYPTO_IOVEC_MAX`. +// length. It is usually passed as an array. struct crypto_ivec_st { const uint8_t *in; size_t len; }; // crypto_iovec_st (aka `CRYPTO_IOVEC` combines a pointer to input data and a -// pointer to an output buffer with their common length. It is usually passed -// as an array of length of at most `CRYPTO_IOVEC_MAX`. +// pointer to an output buffer with their common length. struct crypto_iovec_st { // `out` and `in` must be disjoint or equal uint8_t *out; @@ -415,10 +413,6 @@ size_t len; }; -// CRYPTO_IOVEC_MAX is the maximum number of entries in an `CRYPTO_IOVEC` or -// `CRYPTO_IVEC` parameter. -#define CRYPTO_IOVEC_MAX 16 - // EVP_AEAD_CTX_sealv encrypts and authenticates the `in` bytes from `iovec` // and authenticates the `aadvec` bytes. It writes the same amount of // ciphertext to the `out` pointers of `iovec` and the authentication tag to @@ -450,8 +444,6 @@ // directly or via `iovec` and `aadvec`, with the one exception that it is // permitted for the same `iovec` member's `in` and `out` members to be equal // (in-place operation). -// -// `num_iovec` and `num_aadvec` must be <= `CRYPTO_IOVEC_MAX`. OPENSSL_EXPORT int EVP_AEAD_CTX_sealv(const EVP_AEAD_CTX *ctx, const CRYPTO_IOVEC *iovec, size_t num_iovec, uint8_t *out_tag, size_t *out_tag_len, @@ -493,8 +485,6 @@ // directly or via `iovec` and `aadvec`, with the one exception that it is // permitted for the same `iovec` member's `in` and `out` members to be equal // (in-place operation). -// -// `num_iovec` and `num_aadvec` must be <= `CRYPTO_IOVEC_MAX`. OPENSSL_EXPORT int EVP_AEAD_CTX_openv(const EVP_AEAD_CTX *ctx, const CRYPTO_IOVEC *iovec, size_t num_iovec, size_t *out_total_bytes, @@ -531,8 +521,6 @@ // directly or via `iovec` and `aadvec`, with the one exception that it is // permitted for the same `iovec` member's `in` and `out` members to be equal // (in-place operation). -// -// `num_iovec` and `num_aadvec` must be <= `CRYPTO_IOVEC_MAX`. OPENSSL_EXPORT int EVP_AEAD_CTX_openv_detached(const EVP_AEAD_CTX *ctx, const CRYPTO_IOVEC *iovec, size_t num_iovec,