Make bssl_shim and fuzzer BIOs use the public APIs Converting libcrypto's is actually kind of hairy because we need to either use CRYPTO_once or make sure downstream callers have __cxa_guard_acquire. (Also bio->num is not part of public API.) But let's at least demonstrate better hygiene for these few. Bug: 412269080 Change-Id: I6a7e1fa063e4857b9f584413443dd7f5f8ae7d00 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/78848 Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/ssl/test/async_bio.cc b/ssl/test/async_bio.cc index b829d8b..6e2009b 100644 --- a/ssl/test/async_bio.cc +++ b/ssl/test/async_bio.cc
@@ -25,24 +25,32 @@ namespace { -extern const BIO_METHOD g_async_bio_method; - struct AsyncBio { - bool datagram; - size_t read_quota; - size_t write_quota; + bool datagram = false; + size_t read_quota = 0; + size_t write_quota = 0; }; +static int AsyncBioMethodType() { + static int type = [] { + int idx = BIO_get_new_index(); + BSSL_CHECK(idx > 0); + return idx | BIO_TYPE_FILTER; + }(); + return type; +} + AsyncBio *GetData(BIO *bio) { - if (bio->method != &g_async_bio_method) { - return NULL; + if (BIO_method_type(bio) != AsyncBioMethodType()) { + return nullptr; } - return (AsyncBio *)bio->ptr; + return static_cast<AsyncBio *>(BIO_get_data(bio)); } static int AsyncWrite(BIO *bio, const char *in, int inl) { AsyncBio *a = GetData(bio); - if (a == NULL || bio->next_bio == NULL) { + BIO *next = BIO_next(bio); + if (a == nullptr || next == nullptr) { return 0; } @@ -57,7 +65,7 @@ if (!a->datagram && static_cast<size_t>(inl) > a->write_quota) { inl = static_cast<int>(a->write_quota); } - int ret = BIO_write(bio->next_bio, in, inl); + int ret = BIO_write(next, in, inl); if (ret <= 0) { BIO_copy_next_retry(bio); } else { @@ -68,7 +76,8 @@ static int AsyncRead(BIO *bio, char *out, int outl) { AsyncBio *a = GetData(bio); - if (a == NULL || bio->next_bio == NULL) { + BIO *next = BIO_next(bio); + if (a == nullptr || next == nullptr) { return 0; } @@ -83,7 +92,7 @@ if (!a->datagram && static_cast<size_t>(outl) > a->read_quota) { outl = static_cast<int>(a->read_quota); } - int ret = BIO_read(bio->next_bio, out, outl); + int ret = BIO_read(next, out, outl); if (ret <= 0) { BIO_copy_next_retry(bio); } else { @@ -93,65 +102,64 @@ } static long AsyncCtrl(BIO *bio, int cmd, long num, void *ptr) { - if (bio->next_bio == NULL) { + BIO *next = BIO_next(bio); + if (next == nullptr) { return 0; } BIO_clear_retry_flags(bio); - long ret = BIO_ctrl(bio->next_bio, cmd, num, ptr); + long ret = BIO_ctrl(next, cmd, num, ptr); BIO_copy_next_retry(bio); return ret; } static int AsyncNew(BIO *bio) { - AsyncBio *a = (AsyncBio *)OPENSSL_zalloc(sizeof(*a)); - if (a == NULL) { - return 0; - } - bio->init = 1; - bio->ptr = (char *)a; + BIO_set_data(bio, new AsyncBio); + BIO_set_init(bio, 1); return 1; } static int AsyncFree(BIO *bio) { - if (bio == NULL) { + if (bio == nullptr) { return 0; } - OPENSSL_free(bio->ptr); - bio->ptr = NULL; - bio->init = 0; - bio->flags = 0; + delete GetData(bio); + BIO_set_data(bio, nullptr); + BIO_set_init(bio, 0); return 1; } static long AsyncCallbackCtrl(BIO *bio, int cmd, BIO_info_cb *fp) { - if (bio->next_bio == NULL) { + BIO *next = BIO_next(bio); + if (next == nullptr) { return 0; } - return BIO_callback_ctrl(bio->next_bio, cmd, fp); + return BIO_callback_ctrl(next, cmd, fp); } -const BIO_METHOD g_async_bio_method = { - BIO_TYPE_FILTER, - "async bio", - AsyncWrite, - AsyncRead, - NULL /* puts */, - NULL /* gets */, - AsyncCtrl, - AsyncNew, - AsyncFree, - AsyncCallbackCtrl, -}; +static const BIO_METHOD *AsyncBioMethod() { + static const BIO_METHOD *method = [] { + BIO_METHOD *ret = BIO_meth_new(AsyncBioMethodType(), "async bio"); + BSSL_CHECK(ret); + BSSL_CHECK(BIO_meth_set_write(ret, AsyncWrite)); + BSSL_CHECK(BIO_meth_set_read(ret, AsyncRead)); + BSSL_CHECK(BIO_meth_set_ctrl(ret, AsyncCtrl)); + BSSL_CHECK(BIO_meth_set_create(ret, AsyncNew)); + BSSL_CHECK(BIO_meth_set_destroy(ret, AsyncFree)); + BSSL_CHECK(BIO_meth_set_callback_ctrl(ret, AsyncCallbackCtrl)); + return ret; + }(); + return method; +} } // namespace bssl::UniquePtr<BIO> AsyncBioCreate() { - return bssl::UniquePtr<BIO>(BIO_new(&g_async_bio_method)); + return bssl::UniquePtr<BIO>(BIO_new(AsyncBioMethod())); } bssl::UniquePtr<BIO> AsyncBioCreateDatagram() { - bssl::UniquePtr<BIO> ret(BIO_new(&g_async_bio_method)); + bssl::UniquePtr<BIO> ret(BIO_new(AsyncBioMethod())); if (!ret) { return nullptr; } @@ -161,7 +169,7 @@ void AsyncBioAllowRead(BIO *bio, size_t count) { AsyncBio *a = GetData(bio); - if (a == NULL) { + if (a == nullptr) { return; } a->read_quota += count; @@ -169,7 +177,7 @@ void AsyncBioAllowWrite(BIO *bio, size_t count) { AsyncBio *a = GetData(bio); - if (a == NULL) { + if (a == nullptr) { return; } a->write_quota += count;
diff --git a/ssl/test/fuzzer.h b/ssl/test/fuzzer.h index 79217e5..9360fee 100644 --- a/ssl/test/fuzzer.h +++ b/ssl/test/fuzzer.h
@@ -602,15 +602,14 @@ b->protocol = protocol_; CBS_init(&b->cbs, in, len); - bssl::UniquePtr<BIO> bio(BIO_new(&kBIOMethod)); - bio->init = 1; - bio->ptr = b; + bssl::UniquePtr<BIO> bio(BIO_new(BIOMethod())); + BIO_set_init(bio.get(), 1); + BIO_set_data(bio.get(), b); return bio; } static int BIORead(BIO *bio, char *out, int len) { - assert(bio->method == &kBIOMethod); - BIOData *b = reinterpret_cast<BIOData *>(bio->ptr); + BIOData *b = reinterpret_cast<BIOData *>(BIO_get_data(bio)); if (b->protocol == kTLS) { len = std::min(static_cast<size_t>(len), CBS_len(&b->cbs)); memcpy(out, CBS_data(&b->cbs), len); @@ -629,13 +628,21 @@ } static int BIODestroy(BIO *bio) { - assert(bio->method == &kBIOMethod); - BIOData *b = reinterpret_cast<BIOData *>(bio->ptr); + BIOData *b = reinterpret_cast<BIOData *>(BIO_get_data(bio)); delete b; return 1; } - static const BIO_METHOD kBIOMethod; + static const BIO_METHOD *BIOMethod() { + static const BIO_METHOD *method = [] { + BIO_METHOD *ret = BIO_meth_new(0, nullptr); + BSSL_CHECK(ret); + BSSL_CHECK(BIO_meth_set_read(ret, BIORead)); + BSSL_CHECK(BIO_meth_set_destroy(ret, BIODestroy)); + return ret; + }(); + return method; + } bool debug_; Protocol protocol_; @@ -644,19 +651,6 @@ std::vector<uint8_t> handoff_, handback_; }; -const BIO_METHOD TLSFuzzer::kBIOMethod = { - 0, // type - nullptr, // name - nullptr, // bwrite - TLSFuzzer::BIORead, - nullptr, // bputs - nullptr, // bgets - nullptr, // ctrl - nullptr, // create - TLSFuzzer::BIODestroy, - nullptr, // callback_ctrl -}; - } // namespace
diff --git a/ssl/test/packeted_bio.cc b/ssl/test/packeted_bio.cc index 9cec7fe..dfd8c91 100644 --- a/ssl/test/packeted_bio.cc +++ b/ssl/test/packeted_bio.cc
@@ -31,8 +31,6 @@ namespace { -extern const BIO_METHOD g_packeted_bio_method; - constexpr uint8_t kOpcodePacket = 'P'; constexpr uint8_t kOpcodeTimeout = 'T'; constexpr uint8_t kOpcodeTimeoutAck = 't'; @@ -59,11 +57,20 @@ std::function<bool(uint32_t)> set_mtu; }; +static int PacketedBioMethodType() { + static int type = [] { + int idx = BIO_get_new_index(); + BSSL_CHECK(idx > 0); + return idx | BIO_TYPE_FILTER; + }(); + return type; +} + PacketedBio *GetData(BIO *bio) { - if (bio->method != &g_packeted_bio_method) { + if (BIO_method_type(bio) != PacketedBioMethodType()) { return NULL; } - return (PacketedBio *)bio->ptr; + return static_cast<PacketedBio *>(BIO_get_data(bio)); } // ReadAll reads |len| bytes from |bio| into |out|. It returns 1 on success and @@ -85,8 +92,9 @@ } static int PacketedWrite(BIO *bio, const char *in, int inl) { - if (bio->next_bio == NULL) { - return 0; + BIO *next = BIO_next(bio); + if (next == nullptr) { + return -1; } BIO_clear_retry_flags(bio); @@ -98,14 +106,14 @@ header[2] = (inl >> 16) & 0xff; header[3] = (inl >> 8) & 0xff; header[4] = inl & 0xff; - int ret = BIO_write(bio->next_bio, header, sizeof(header)); + int ret = BIO_write(next, header, sizeof(header)); if (ret <= 0) { BIO_copy_next_retry(bio); return ret; } // Write the buffer. - ret = BIO_write(bio->next_bio, in, inl); + ret = BIO_write(next, in, inl); if (ret < 0 || (inl > 0 && ret == 0)) { BIO_copy_next_retry(bio); return ret; @@ -116,8 +124,9 @@ static int PacketedRead(BIO *bio, char *out, int outl) { PacketedBio *data = GetData(bio); - if (bio->next_bio == NULL) { - return 0; + BIO *next = BIO_next(bio); + if (next == nullptr) { + return -1; } BIO_clear_retry_flags(bio); @@ -125,7 +134,7 @@ for (;;) { // Read the opcode. uint8_t opcode; - int ret = ReadAll(bio->next_bio, &opcode, sizeof(opcode)); + int ret = ReadAll(next, &opcode, sizeof(opcode)); if (ret <= 0) { BIO_copy_next_retry(bio); return ret; @@ -141,7 +150,7 @@ // Process the timeout. uint8_t buf[8]; - ret = ReadAll(bio->next_bio, buf, sizeof(buf)); + ret = ReadAll(next, buf, sizeof(buf)); if (ret <= 0) { BIO_copy_next_retry(bio); return ret; @@ -153,7 +162,7 @@ data->timeout.tv_sec = timeout / 1000000; // Send an ACK to the peer. - ret = BIO_write(bio->next_bio, &kOpcodeTimeoutAck, 1); + ret = BIO_write(next, &kOpcodeTimeoutAck, 1); if (ret <= 0) { return ret; } @@ -166,7 +175,7 @@ if (opcode == kOpcodeMTU) { uint8_t buf[4]; - ret = ReadAll(bio->next_bio, buf, sizeof(buf)); + ret = ReadAll(next, buf, sizeof(buf)); if (ret <= 0) { BIO_copy_next_retry(bio); return ret; @@ -182,7 +191,7 @@ if (opcode == kOpcodeExpectNextTimeout) { uint8_t buf[8]; - ret = ReadAll(bio->next_bio, buf, sizeof(buf)); + ret = ReadAll(next, buf, sizeof(buf)); if (ret <= 0) { BIO_copy_next_retry(bio); return ret; @@ -231,14 +240,14 @@ // Read the length prefix. uint8_t len_bytes[4]; - ret = ReadAll(bio->next_bio, len_bytes, sizeof(len_bytes)); + ret = ReadAll(next, len_bytes, sizeof(len_bytes)); if (ret <= 0) { BIO_copy_next_retry(bio); return ret; } std::vector<uint8_t> buf(CRYPTO_load_u32_be(len_bytes), 0); - ret = ReadAll(bio->next_bio, buf.data(), buf.size()); + ret = ReadAll(next, buf.data(), buf.size()); if (ret <= 0) { fprintf(stderr, "Packeted BIO was truncated\n"); return -1; @@ -253,61 +262,65 @@ } static long PacketedCtrl(BIO *bio, int cmd, long num, void *ptr) { - if (bio->next_bio == NULL) { + BIO *next = BIO_next(bio); + if (next == nullptr) { return 0; } BIO_clear_retry_flags(bio); - long ret = BIO_ctrl(bio->next_bio, cmd, num, ptr); + long ret = BIO_ctrl(next, cmd, num, ptr); BIO_copy_next_retry(bio); return ret; } static int PacketedNew(BIO *bio) { - bio->init = 1; + BIO_set_init(bio, 1); return 1; } static int PacketedFree(BIO *bio) { - if (bio == NULL) { + if (bio == nullptr) { return 0; } delete GetData(bio); - bio->init = 0; return 1; } static long PacketedCallbackCtrl(BIO *bio, int cmd, BIO_info_cb *fp) { - if (bio->next_bio == NULL) { + BIO *next = BIO_next(bio); + if (next == nullptr) { return 0; } - return BIO_callback_ctrl(bio->next_bio, cmd, fp); + return BIO_callback_ctrl(next, cmd, fp); } -const BIO_METHOD g_packeted_bio_method = { - BIO_TYPE_FILTER, - "packeted bio", - PacketedWrite, - PacketedRead, - NULL /* puts */, - NULL /* gets */, - PacketedCtrl, - PacketedNew, - PacketedFree, - PacketedCallbackCtrl, -}; +static const BIO_METHOD *PacketedBioMethod() { + static const BIO_METHOD *method = [] { + BIO_METHOD *ret = BIO_meth_new(PacketedBioMethodType(), "packeted bio"); + BSSL_CHECK(ret); + BSSL_CHECK(BIO_meth_set_write(ret, PacketedWrite)); + BSSL_CHECK(BIO_meth_set_read(ret, PacketedRead)); + BSSL_CHECK(BIO_meth_set_ctrl(ret, PacketedCtrl)); + BSSL_CHECK(BIO_meth_set_create(ret, PacketedNew)); + BSSL_CHECK(BIO_meth_set_destroy(ret, PacketedFree)); + BSSL_CHECK(BIO_meth_set_callback_ctrl(ret, PacketedCallbackCtrl)); + return ret; + }(); + return method; +} } // namespace bssl::UniquePtr<BIO> PacketedBioCreate( timeval *clock, std::function<bool(timeval *)> get_timeout, std::function<bool(uint32_t)> set_mtu) { - bssl::UniquePtr<BIO> bio(BIO_new(&g_packeted_bio_method)); + bssl::UniquePtr<BIO> bio(BIO_new(PacketedBioMethod())); if (!bio) { return nullptr; } - bio->ptr = new PacketedBio(clock, std::move(get_timeout), std::move(set_mtu)); + BIO_set_data(bio.get(), new PacketedBio(clock, std::move(get_timeout), + std::move(set_mtu))); return bio; }