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>

Author: divVerent

crypto/bytestring/internal.h

@@ -68,7 +68,7 @@ OPENSSL_EXPORT int CBS_get_asn1_implicit_string(CBS *in, CBS *out,
 // 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|.

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 @@ static int SSL_SESSION_to_bytes_full(const SSL_SESSION *in, CBB *cbb,
   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 @@ using namespace bssl;
 
 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 @@ int SSL_SESSION_to_bytes_for_ticket(const SSL_SESSION *in, uint8_t **out_data,
       !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;
-  }
-
-  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;
+  ScopedCBB cbb;
+  if (!CBB_init(cbb.get(), 256) ||
+      !SSL_SESSION_to_bytes_if_not_resumable(in, cbb.get(), 0)) {
+    return 0;
   }
-  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,

ssl/ssl_test.cc

@@ -1279,12 +1279,20 @@ TEST(SSLTest, SessionEncoding) {
     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 : {