i2d_SSL_SESSION: support operating as allocating i2d function. Nothing in the documentation says it _cannot_ be used this way, so let's make ``` uint8_t *out = nullptr; i2d_SSL_SESSION(session, &out); ``` work. Found by #Gemini. Change-Id: Ibff50d411a51b51f74db09f087ce24af6a6a6964 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/91008 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: Rudolf Polzer <rpolzer@google.com>
diff --git a/crypto/bytestring/internal.h b/crypto/bytestring/internal.h index 7560309..35bd4c9 100644 --- a/crypto/bytestring/internal.h +++ b/crypto/bytestring/internal.h
@@ -68,7 +68,7 @@ // error, it calls |CBB_cleanup| on |cbb|. // // This function may be used to help implement legacy i2d ASN.1 functions. -int CBB_finish_i2d(CBB *cbb, uint8_t **outp); +OPENSSL_EXPORT 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|.
diff --git a/ssl/ssl_asn1.cc b/ssl/ssl_asn1.cc index 2fe50ea..f2aa404 100644 --- a/ssl/ssl_asn1.cc +++ b/ssl/ssl_asn1.cc
@@ -25,6 +25,7 @@ #include <openssl/mem.h> #include <openssl/x509.h> +#include "../crypto/bytestring/internal.h" #include "../crypto/internal.h" #include "internal.h" @@ -355,6 +356,20 @@ return CBB_flush(cbb); } +static int SSL_SESSION_to_bytes_if_not_resumable(const SSL_SESSION *in, + CBB *out, int for_ticket) { + if (in->not_resumable) { + // If the caller has an unresumable session, e.g. if |SSL_get_session| + // were called on a TLS 1.3 or False Started connection, serialize with + // a placeholder value so it is not accidentally deserialized into a + // resumable one. + const auto kNotResumableSession = StringAsBytes("NOT RESUMABLE"); + return CBB_add_bytes(out, kNotResumableSession.data(), + kNotResumableSession.size()); + } + return SSL_SESSION_to_bytes_full(in, out, for_ticket); +} + // SSL_SESSION_parse_string gets an optional ASN.1 OCTET STRING explicitly // tagged with |tag| from |cbs| and saves it in |*out|. If the element was not // found, it sets |*out| to NULL. It returns one on success, whether or not the @@ -717,29 +732,12 @@ int SSL_SESSION_to_bytes(const SSL_SESSION *in, uint8_t **out_data, size_t *out_len) { - if (in->not_resumable) { - // If the caller has an unresumable session, e.g. if |SSL_get_session| were - // called on a TLS 1.3 or False Started connection, serialize with a - // placeholder value so it is not accidentally deserialized into a resumable - // one. - static const char kNotResumableSession[] = "NOT RESUMABLE"; - - *out_len = strlen(kNotResumableSession); - *out_data = (uint8_t *)OPENSSL_memdup(kNotResumableSession, *out_len); - if (*out_data == nullptr) { - return 0; - } - - return 1; - } - ScopedCBB cbb; if (!CBB_init(cbb.get(), 256) || - !SSL_SESSION_to_bytes_full(in, cbb.get(), 0) || + !SSL_SESSION_to_bytes_if_not_resumable(in, cbb.get(), 0) || !CBB_finish(cbb.get(), out_data, out_len)) { return 0; } - return 1; } @@ -751,31 +749,16 @@ !CBB_finish(cbb.get(), out_data, out_len)) { return 0; } - return 1; } int i2d_SSL_SESSION(SSL_SESSION *in, uint8_t **pp) { - uint8_t *out; - size_t len; - - if (!SSL_SESSION_to_bytes(in, &out, &len)) { - return -1; + ScopedCBB cbb; + if (!CBB_init(cbb.get(), 256) || + !SSL_SESSION_to_bytes_if_not_resumable(in, cbb.get(), 0)) { + return 0; } - - if (len > INT_MAX) { - OPENSSL_free(out); - OPENSSL_PUT_ERROR(SSL, ERR_R_OVERFLOW); - return -1; - } - - if (pp) { - OPENSSL_memcpy(*pp, out, len); - *pp += len; - } - OPENSSL_free(out); - - return len; + return CBB_finish_i2d(cbb.get(), pp); } SSL_SESSION *SSL_SESSION_from_bytes(const uint8_t *in, size_t in_len,
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 008fd86..8c24934 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc
@@ -1279,12 +1279,20 @@ uint8_t *ptr = encoded.get(); len = i2d_SSL_SESSION(session.get(), &ptr); ASSERT_GT(len, 0) << "i2d_SSL_SESSION failed"; - ASSERT_EQ(static_cast<size_t>(len), input.size()) - << "i2d_SSL_SESSION(NULL) returned invalid length"; ASSERT_EQ(ptr, encoded.get() + input.size()) << "i2d_SSL_SESSION did not advance ptr correctly"; - EXPECT_EQ(Bytes(encoded.get(), encoded_len), Bytes(input)) - << "SSL_SESSION_to_bytes did not round-trip"; + EXPECT_EQ(Bytes(encoded.get(), len), Bytes(input)) + << "i2d_SSL_SESSION did not round-trip"; + + // Verify the SSL_SESSION encoding round-trips via the legacy allocating + // API. + uint8_t *aptr = nullptr; + len = i2d_SSL_SESSION(session.get(), &aptr); + encoded.reset(aptr); + ASSERT_GT(len, 0) << "i2d_SSL_SESSION failed"; + ASSERT_TRUE(aptr != nullptr) << "i2d_SSL_SESSION did not allocate ptr"; + EXPECT_EQ(Bytes(aptr, len), Bytes(input)) + << "i2d_SSL_SESSION did not round-trip"; } for (const char *input_b64 : {