Fix i2d behavior for i2d_SSL_SESSION (#1966)

`i2d_SSL_SESSION` wasn't exactly following the correct behavior of the
legacy `i2d` functions when `pp` is non-NULL, but `*pp` is NULL. This
caused issues with functions expecting a newly allocated buffer when
calling `i2d_SSL_SESSION` with the recommended documented behavior. See
`i2d_SAMPLE` for more details.

### Testing:
New test below usage of `i2d_SSL_SESSION` to verify the behavior.
 
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.

crypto/bytestring/internal.h

@@ -64,7 +64,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);
 
 
 // Unicode utilities.

ssl/ssl_asn1.cc

@@ -92,6 +92,7 @@
 #include <openssl/mem.h>
 #include <openssl/x509.h>
 
+#include "../crypto/bytestring/internal.h"
 #include "../crypto/internal.h"
 #include "internal.h"
 
@@ -841,20 +842,14 @@ int i2d_SSL_SESSION(SSL_SESSION *in, uint8_t **pp) {
   if (!SSL_SESSION_to_bytes(in, &out, &len)) {
     return -1;
   }
+  bssl::UniquePtr<uint8_t> free_out(out);
 
-  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) || !CBB_add_bytes(cbb.get(), out, len)) {
+    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

@@ -2040,6 +2040,17 @@ TEST(SSLTest, SessionEncoding) {
         << "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";
+
+    // Verify that |i2d_SSL_SESSION| works correctly when |pp| is non-NULL, but
+    // |*pp| is NULL. A newly-allocated buffer containing the result should be
+    // created. See |i2d_SAMPLE| for more details.
+    uint8_t *ptr2 = nullptr;
+    int len2 = i2d_SSL_SESSION(session.get(), &ptr2);
+    ASSERT_TRUE(ptr2);
+    ASSERT_GT(len2, 0);
+    bssl::UniquePtr<uint8_t> encoded2(ptr2);
+    EXPECT_EQ(Bytes(encoded2.get(), len2), Bytes(input))
+        << "SSL_SESSION_to_bytes did not round-trip";
   }
 
   for (const char *input_b64 : {