Add a CBBAsSpan internal helper function Kept it internal for now. Also make CBB_data return a non-const pointer. Unlike CBS, a CBB is meant to write into its data, so it's OK if we let the caller see a mutable buffer to write into. Change-Id: I913e40be9363a012f4ca01890e11facc63d1e4a0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/88408 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Lily Chen <chlily@google.com>
diff --git a/crypto/bytestring/cbb.cc b/crypto/bytestring/cbb.cc index 57a828a..5b6812c 100644 --- a/crypto/bytestring/cbb.cc +++ b/crypto/bytestring/cbb.cc
@@ -272,7 +272,7 @@ return 0; } -const uint8_t *CBB_data(const CBB *cbb) { +uint8_t *CBB_data(const CBB *cbb) { assert(cbb->child == nullptr); if (cbb->is_child) { return cbb->u.child.base->buf + cbb->u.child.offset +
diff --git a/crypto/bytestring/internal.h b/crypto/bytestring/internal.h index 1f6f115..20d28c0 100644 --- a/crypto/bytestring/internal.h +++ b/crypto/bytestring/internal.h
@@ -68,6 +68,15 @@ // This function may be used to help implement legacy i2d ASN.1 functions. int CBB_finish_i2d(CBB *cbb, uint8_t **outp); +// CBBAsSpan returns a span containing |cbb|'s contents. It does not flush +// |cbb|. The span is valid until the next operation to |cbb|. +// +// To avoid unfinalized length prefixes, it is a fatal error to call this on a +// CBB with any active children. +inline Span<uint8_t> CBBAsSpan(const CBB *cbb) { + return Span(CBB_data(cbb), CBB_len(cbb)); +} + // D2IFromCBS takes a functor of type |Unique<T>(CBS*)| and implements the d2i // calling convention. For compatibility with functions that don't tag their // return value (e.g. public APIs), |T*(CBS)| is also accepted. The callback can
diff --git a/crypto/evp/evp_test.cc b/crypto/evp/evp_test.cc index a99a030..613402f 100644 --- a/crypto/evp/evp_test.cc +++ b/crypto/evp/evp_test.cc
@@ -44,6 +44,7 @@ #include "../test/test_util.h" #include "../test/wycheproof_util.h" +BSSL_NAMESPACE_BEGIN namespace { // evp_test dispatches between multiple test types. PublicKey and PrivateKey // tests take a key name parameter and key information. If the test is @@ -1273,3 +1274,4 @@ "third_party/wycheproof_testvectors/rsa_pkcs1_4096_test.txt"); } } // namespace +BSSL_NAMESPACE_END
diff --git a/crypto/test/der_trailing_data.cc b/crypto/test/der_trailing_data.cc index f5dcf44..6358f27 100644 --- a/crypto/test/der_trailing_data.cc +++ b/crypto/test/der_trailing_data.cc
@@ -18,6 +18,10 @@ #include <openssl/bytestring.h> +#include "../bytestring/internal.h" + +BSSL_NAMESPACE_BEGIN + static bool RewriteWithTrailingData(CBB *cbb, CBS *cbs, std::optional<size_t> *rewrite_counter) { CBS contents; @@ -55,12 +59,12 @@ } bool TestDERTrailingData( - bssl::Span<const uint8_t> in, - std::function<void(bssl::Span<const uint8_t>, size_t)> func) { + Span<const uint8_t> in, + std::function<void(Span<const uint8_t>, size_t)> func) { for (size_t elem_to_rewrite = 0; true; elem_to_rewrite++) { std::optional<size_t> rewrite_counter = elem_to_rewrite; CBS cbs = in; - bssl::ScopedCBB cbb; + ScopedCBB cbb; if (!CBB_init(cbb.get(), in.size() + /* EOC */ 2 + /* in case lengths get larger */ 8) || !RewriteWithTrailingData(cbb.get(), &cbs, &rewrite_counter) || @@ -73,6 +77,8 @@ return true; } - func(bssl::Span(CBB_data(cbb.get()), CBB_len(cbb.get())), elem_to_rewrite); + func(CBBAsSpan(cbb.get()), elem_to_rewrite); } } + +BSSL_NAMESPACE_END
diff --git a/crypto/test/der_trailing_data.h b/crypto/test/der_trailing_data.h index 2d5b369..3919575 100644 --- a/crypto/test/der_trailing_data.h +++ b/crypto/test/der_trailing_data.h
@@ -19,6 +19,8 @@ #include <openssl/span.h> +BSSL_NAMESPACE_BEGIN + // TestDERTrailingData decodes |in| as an arbitrary DER structure. It then calls // |func| multiple times on different modified versions of |in|, each time with // extra data appended to a different constructed element. The extra data will @@ -32,7 +34,9 @@ // TestDERTrailingData returns whether it successful rewrote |in| and called // |func| for every constructed element. bool TestDERTrailingData( - bssl::Span<const uint8_t> in, - std::function<void(bssl::Span<const uint8_t> rewritten, size_t n)> func); + Span<const uint8_t> in, + std::function<void(Span<const uint8_t> rewritten, size_t n)> func); + +BSSL_NAMESPACE_END #endif // OPENSSL_HEADER_CRYPTO_TEST_DER_TRAILING_DATA_H
diff --git a/crypto/x509/x_all.cc b/crypto/x509/x_all.cc index 0ca8a2c..3d9474d 100644 --- a/crypto/x509/x_all.cc +++ b/crypto/x509/x_all.cc
@@ -28,6 +28,7 @@ #include <openssl/stack.h> #include "../asn1/internal.h" +#include "../bytestring/internal.h" #include "../internal.h" #include "internal.h" @@ -46,8 +47,7 @@ return 0; } return x509_verify_signature(&impl->sig_alg, &impl->signature, - Span(CBB_data(cbb.get()), CBB_len(cbb.get())), - pkey); + CBBAsSpan(cbb.get()), pkey); } int X509_REQ_verify(X509_REQ *req, EVP_PKEY *pkey) { @@ -85,8 +85,7 @@ if (!CBB_init(cbb.get(), 128) || !x509_marshal_tbs_cert(cbb.get(), x)) { return 0; } - return x509_sign_to_bit_string(ctx, &impl->signature, - Span(CBB_data(cbb.get()), CBB_len(cbb.get()))); + return x509_sign_to_bit_string(ctx, &impl->signature, CBBAsSpan(cbb.get())); } int X509_REQ_sign(X509_REQ *x, EVP_PKEY *pkey, const EVP_MD *md) {
diff --git a/include/openssl/bytestring.h b/include/openssl/bytestring.h index 25877c2..b484710 100644 --- a/include/openssl/bytestring.h +++ b/include/openssl/bytestring.h
@@ -527,7 +527,7 @@ // // To avoid unfinalized length prefixes, it is a fatal error to call this on a // CBB with any active children. -OPENSSL_EXPORT const uint8_t *CBB_data(const CBB *cbb); +OPENSSL_EXPORT uint8_t *CBB_data(const CBB *cbb); // CBB_len returns the number of bytes written to |cbb|. It does not flush // |cbb|.
diff --git a/ssl/d1_both.cc b/ssl/d1_both.cc index db3c6e2..31a65c0 100644 --- a/ssl/d1_both.cc +++ b/ssl/d1_both.cc
@@ -26,6 +26,7 @@ #include <openssl/rand.h> #include "../crypto/internal.h" +#include "../crypto/bytestring/internal.h" #include "internal.h" @@ -1003,8 +1004,7 @@ return -1; } - ssl_do_msg_callback(ssl, /*is_write=*/1, SSL3_RT_ACK, - Span(CBB_data(&cbb), CBB_len(&cbb))); + ssl_do_msg_callback(ssl, /*is_write=*/1, SSL3_RT_ACK, CBBAsSpan(&cbb)); int bio_ret = BIO_write(ssl->wbio.get(), record, static_cast<int>(record_len));
diff --git a/ssl/encrypted_client_hello.cc b/ssl/encrypted_client_hello.cc index e27e30a..f88e236 100644 --- a/ssl/encrypted_client_hello.cc +++ b/ssl/encrypted_client_hello.cc
@@ -28,6 +28,7 @@ #include <openssl/hpke.h> #include <openssl/rand.h> +#include "../crypto/bytestring/internal.h" #include "../crypto/internal.h" #include "internal.h" @@ -255,8 +256,7 @@ return false; } - if (!is_valid_client_hello_inner(ssl, out_alert, - Span(CBB_data(&body), CBB_len(&body)))) { + if (!is_valid_client_hello_inner(ssl, out_alert, CBBAsSpan(&body))) { return false; } @@ -817,10 +817,7 @@ return false; } // Also update the EncodedClientHelloInner. - auto encoded_binder = - Span(const_cast<uint8_t *>(CBB_data(encoded_cbb.get())), - CBB_len(encoded_cbb.get())) - .last(binder_len); + auto encoded_binder = CBBAsSpan(encoded_cbb.get()).last(binder_len); auto hello_inner_binder = Span(hello_inner).last(binder_len); OPENSSL_memcpy(encoded_binder.data(), hello_inner_binder.data(), binder_len);
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc index 5d119ef..faa20ca 100644 --- a/ssl/handshake_server.cc +++ b/ssl/handshake_server.cc
@@ -36,6 +36,7 @@ #include <openssl/x509.h> #include "../crypto/internal.h" +#include "../crypto/bytestring/internal.h" #include "internal.h" @@ -1058,8 +1059,7 @@ // If generating hints, save the ECDHE key. if (hints && hs->hints_requested) { bssl::ScopedCBB private_key_cbb; - if (!hints->ecdhe_public_key.CopyFrom( - Span(CBB_data(&child), CBB_len(&child))) || + if (!hints->ecdhe_public_key.CopyFrom(CBBAsSpan(&child)) || !CBB_init(private_key_cbb.get(), 32) || !hs->key_shares[0]->SerializePrivateKey(private_key_cbb.get()) || !CBBFinishArray(private_key_cbb.get(),
diff --git a/ssl/tls13_both.cc b/ssl/tls13_both.cc index 257e4c9..d95a902 100644 --- a/ssl/tls13_both.cc +++ b/ssl/tls13_both.cc
@@ -26,6 +26,7 @@ #include <openssl/stack.h> #include <openssl/x509.h> +#include "../crypto/bytestring/internal.h" #include "../crypto/internal.h" #include "internal.h" @@ -563,8 +564,7 @@ if (hints && hs->hints_requested) { hints->cert_compression_alg_id = hs->cert_compression_alg_id; if (!hints->cert_compression_input.CopyFrom(msg) || - !hints->cert_compression_output.CopyFrom( - Span(CBB_data(&compressed), CBB_len(&compressed)))) { + !hints->cert_compression_output.CopyFrom(CBBAsSpan(&compressed))) { return false; } }
diff --git a/util/fipstools/acvp/modulewrapper/modulewrapper.cc b/util/fipstools/acvp/modulewrapper/modulewrapper.cc index 42ff1bd..bafcf0d 100644 --- a/util/fipstools/acvp/modulewrapper/modulewrapper.cc +++ b/util/fipstools/acvp/modulewrapper/modulewrapper.cc
@@ -47,6 +47,7 @@ #include <openssl/span.h> #include <openssl/tls_prf.h> +#include "../../../../crypto/bytestring/internal.h" #include "../../../../crypto/fipsmodule/bcm_interface.h" #include "../../../../crypto/fipsmodule/ec/internal.h" #include "../../../../crypto/fipsmodule/rand/internal.h" @@ -2088,8 +2089,7 @@ return false; } - return write_reply( - {pub_key_bytes, Span(CBB_data(cbb.get()), CBB_len(cbb.get()))}); + return write_reply({pub_key_bytes, CBBAsSpan(cbb.get())}); } template <typename PrivateKey, size_t SignatureBytes, @@ -2221,8 +2221,7 @@ return false; } - return write_reply( - {pub_key_bytes, Span(CBB_data(cbb.get()), CBB_len(cbb.get()))}); + return write_reply({pub_key_bytes, CBBAsSpan(cbb.get())}); } template <typename PublicKey, bcm_status (*ParsePublic)(PublicKey *, CBS *),