Unwind EVP_CIPHER_CTX poisoning The original motivation here was that CBC caused the wrapper layer to call into the EVP_CIPHER twice. An error in the second call would put the EVP_CIPHER_CTX in an unpredictable state. And so https://boringssl-review.googlesource.com/c/boringssl/+/54185 added some basic "poison" tracking to try to catch egregious caller mistakes. This then got extended significantly in https://boringssl-review.googlesource.com/c/boringssl/+/83447 to catch all failures. Extending it caused some problems, both in Conscrypt: 1. This changed public API at the Conscrypt level if a Java Cipher object was called after an error. See b/504728350. While somewhat questionable caller behavior, it was an unexpected behavior change. 2. The Java Cipher APIs promise that you can retry after a ShortBufferException. See [0]. Conscrypt currently does all the bounds checks itself, but this is itself problematic, so we'd rather shift that into BoringSSL, which is better suited to calculate this. https://boringssl-review.googlesource.com/c/boringssl/+/99648 patched this with further complexity to the 'poisoned' bit, but it turns out this was all unnecessary. If we look at where cipher_update is *actually* fallible, it turns out: - Almost every cipher_update is infallible, notably all the CBC ones. - AES-GCM and AES-XTS check if their keys have been initialized. (The others should but don't. We should probably have a better notion of when the EVP_CIPHER_CTX has been initialized. Either way, this is not a pattern of failure that can trigger our problem.) - AES-XTS checks if its input is long enough. - AES-GCM checks if the input is too long. Our XTS implementation doesn't support streaming anyway, and GCM is not padded or block-buffered, so the motivation for poisoning doesn't apply. THat means this was all moot. Indeed it's telling that the only positive test for poisoning had to make a custom EVP_CIPHER, and supporting custom EVP_CIPHERs is explicitly out of scope. Unwind the whole poisoning machinery entirely. Also fix some of the tests added in 99648 to actually test what [0] needs: that retrying the operation gives the same thing. Also fix a bug, caught by asserts, in the EVP_EncryptUpdate_ex length check: EVP_CIPHER_CTX_max_next_update checks ctx->encrypt, but decrypt calls into encrypt. [0] https://docs.oracle.com/en/java/javase/21/docs/api/java.base/javax/crypto/Cipher.html#:~:text=at%20outputOffset%20inclusive.-,If%20the%20output%20buffer%20is%20too%20small%20to%20hold%20the%20result%2C%20a%20ShortBufferException%20is%20thrown.%20In%20this%20case%2C%20repeat%20this%20call%20with%20a%20larger%20output%20buffer.%20Use%20getOutputSize%20to%20determine%20how%20big%20the%20output%20buffer%20should%20be.,-If%20inputLen%20is Fixed: 520121641 Change-Id: I7c1c4138d2553fbd9ea72013962f60fa9a3e628c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/99990 Commit-Queue: David Benjamin <davidben@google.com> Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Rudolf Polzer <rpolzer@google.com> Presubmit-BoringSSL-Verified: boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com>
BoringSSL is a fork of OpenSSL that is designed to meet Google's needs.
Although BoringSSL is an open source project, it is not intended for general use, as OpenSSL is. We don't recommend that third parties depend upon it. Doing so is likely to be frustrating because there are no guarantees of API or ABI stability.
Programs ship their own copies of BoringSSL when they use it and we update everything as needed when deciding to make API changes. This allows us to mostly avoid compromises in the name of compatibility. It works for us, but it may not work for you.
BoringSSL arose because Google used OpenSSL for many years in various ways and, over time, built up a large number of patches that were maintained while tracking upstream OpenSSL. As Google's product portfolio became more complex, more copies of OpenSSL sprung up and the effort involved in maintaining all these patches in multiple places was growing steadily.
Currently BoringSSL is the SSL library in Chrome/Chromium, Android (but it's not part of the NDK) and a number of other apps/programs.
Project links:
To file a security issue, use the Chromium process and mention in the report this is for BoringSSL. You can ignore the parts of the process that are specific to Chromium/Chrome.
There are other files in this directory which might be helpful: