Add SSL_SESSION_to_bytes to replace i2d_SSL_SESSION.
Deprecate the old two-pass version of the function. If the ticket is too long,
replace it with a placeholder value but keep the connection working.
Change-Id: Ib9fdea66389b171862143d79b5540ea90a9bd5fb
Reviewed-on: https://boringssl-review.googlesource.com/2011
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 858d2fd..37521bd 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1957,14 +1957,50 @@
OPENSSL_EXPORT int SSL_SESSION_print(BIO *fp,const SSL_SESSION *ses);
#endif
OPENSSL_EXPORT void SSL_SESSION_free(SSL_SESSION *ses);
-OPENSSL_EXPORT int i2d_SSL_SESSION(SSL_SESSION *in,unsigned char **pp);
OPENSSL_EXPORT int SSL_set_session(SSL *to, SSL_SESSION *session);
OPENSSL_EXPORT int SSL_CTX_add_session(SSL_CTX *s, SSL_SESSION *c);
OPENSSL_EXPORT int SSL_CTX_remove_session(SSL_CTX *,SSL_SESSION *c);
OPENSSL_EXPORT int SSL_CTX_set_generate_session_id(SSL_CTX *, GEN_SESSION_CB);
OPENSSL_EXPORT int SSL_set_generate_session_id(SSL *, GEN_SESSION_CB);
OPENSSL_EXPORT int SSL_has_matching_session_id(const SSL *ssl, const unsigned char *id, unsigned int id_len);
-OPENSSL_EXPORT SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **a,const unsigned char **pp, long length);
+
+/* SSL_SESSION_to_bytes serializes |in| into a newly allocated buffer
+ * and sets |*out_data| to that buffer and |*out_len| to its
+ * length. The caller takes ownership of the buffer and must call
+ * |OPENSSL_free| when done. It returns one on success and zero on
+ * error. */
+OPENSSL_EXPORT int SSL_SESSION_to_bytes(SSL_SESSION *in, uint8_t **out_data,
+ size_t *out_len);
+
+/* SSL_SESSION_to_bytes_for_ticket serializes |in|, but excludes the
+ * session ID which is not necessary in a session ticket. */
+OPENSSL_EXPORT int SSL_SESSION_to_bytes_for_ticket(SSL_SESSION *in,
+ uint8_t **out_data,
+ size_t *out_len);
+
+/* Deprecated: i2d_SSL_SESSION serializes |in| to the bytes pointed to
+ * by |*pp|. On success, it returns the number of bytes written and
+ * advances |*pp| by that many bytes. On failure, it returns -1. If
+ * |pp| is NULL, no bytes are written and only the length is
+ * returned.
+ *
+ * Use SSL_SESSION_to_bytes instead. */
+OPENSSL_EXPORT int i2d_SSL_SESSION(SSL_SESSION *in, uint8_t **pp);
+
+/* d2i_SSL_SESSION deserializes a serialized buffer contained in the
+ * |length| bytes pointed to by |*pp|. It returns the new SSL_SESSION
+ * and advances |*pp| by the number of bytes consumed on success and
+ * NULL on failure. If |a| is NULL, the caller takes ownership of the
+ * new session and must call |SSL_SESSION_free| when done.
+ *
+ * If |a| and |*a| are not NULL, the SSL_SESSION at |*a| is overridden
+ * with the deserialized session rather than allocating a new one. In
+ * addition, |a| is not NULL, but |*a| is, |*a| is set to the new
+ * SSL_SESSION.
+ *
+ * Passing a value other than NULL to |a| is deprecated. */
+OPENSSL_EXPORT SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **a, const uint8_t **pp,
+ long length);
OPENSSL_EXPORT X509 * SSL_get_peer_certificate(const SSL *s);
@@ -2440,6 +2476,10 @@
#define SSL_F_ssl_ctx_log_master_secret 286
#define SSL_F_d2i_SSL_SESSION 287
#define SSL_F_i2d_SSL_SESSION 288
+#define SSL_F_d2i_SSL_SESSION_get_octet_string 289
+#define SSL_F_d2i_SSL_SESSION_get_string 290
+#define SSL_F_ssl3_send_new_session_ticket 291
+#define SSL_F_SSL_SESSION_to_bytes_full 292
#define SSL_R_UNABLE_TO_FIND_ECDH_PARAMETERS 100
#define SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC 101
#define SSL_R_INVALID_NULL_CMD_NAME 102
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index edcdc03..7240211 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -2535,61 +2535,62 @@
{
if (s->state == SSL3_ST_SW_SESSION_TICKET_A)
{
- unsigned char *p, *senc, *macstart;
- const unsigned char *const_p;
- int len, slen_full, slen;
- SSL_SESSION *sess;
+ uint8_t *session;
+ size_t session_len;
+ uint8_t *p, *macstart;
+ int len;
unsigned int hlen;
EVP_CIPHER_CTX ctx;
HMAC_CTX hctx;
SSL_CTX *tctx = s->initial_ctx;
unsigned char iv[EVP_MAX_IV_LENGTH];
unsigned char key_name[16];
+ /* The maximum overhead of encrypting the session is 16 (key
+ * name) + IV + one block of encryption overhead + HMAC. */
+ const size_t max_ticket_overhead = 16 + EVP_MAX_IV_LENGTH +
+ EVP_MAX_BLOCK_LENGTH + EVP_MAX_MD_SIZE;
- /* get session encoding length */
- slen_full = i2d_SSL_SESSION(s->session, NULL);
- /* Some length values are 16 bits, so forget it if session is
- * too long
- */
- if (slen_full > 0xFF00)
- return -1;
- senc = OPENSSL_malloc(slen_full);
- if (!senc)
- return -1;
- p = senc;
- i2d_SSL_SESSION(s->session, &p);
-
- /* create a fresh copy (not shared with other threads) to clean up */
- const_p = senc;
- sess = d2i_SSL_SESSION(NULL, &const_p, slen_full);
- if (sess == NULL)
+ /* Serialize the SSL_SESSION to be encoded into the ticket. */
+ if (!SSL_SESSION_to_bytes_for_ticket(s->session, &session,
+ &session_len))
{
- OPENSSL_free(senc);
return -1;
}
- sess->session_id_length = 0; /* ID is irrelevant for the ticket */
- slen = i2d_SSL_SESSION(sess, NULL);
- if (slen > slen_full) /* shouldn't ever happen */
+ /* If the session is too long, emit a dummy value rather than
+ * abort the connection. */
+ if (session_len > 0xFFFF - max_ticket_overhead)
{
- OPENSSL_free(senc);
- return -1;
+ const char kTicketPlaceholder[] = "TICKET TOO LARGE";
+ size_t placeholder_len = strlen(kTicketPlaceholder);
+
+ OPENSSL_free(session);
+
+ p = ssl_handshake_start(s);
+ /* Emit ticket_lifetime_hint. */
+ l2n(0, p);
+ /* Emit ticket. */
+ s2n(placeholder_len, p);
+ memcpy(p, kTicketPlaceholder, placeholder_len);
+ p += placeholder_len;
+
+ len = p - ssl_handshake_start(s);
+ ssl_set_handshake_header(s, SSL3_MT_NEWSESSION_TICKET, len);
+ s->state = SSL3_ST_SW_SESSION_TICKET_B;
+ return ssl_do_write(s);
}
- p = senc;
- i2d_SSL_SESSION(sess, &p);
- SSL_SESSION_free(sess);
/* Grow buffer if need be: the length calculation is as
- * follows handshake_header_length +
+ * follows: handshake_header_length +
* 4 (ticket lifetime hint) + 2 (ticket length) +
- * 16 (key name) + max_iv_len (iv length) +
- * session_length + max_enc_block_size (max encrypted session
- * length) + max_md_size (HMAC).
- */
+ * max_ticket_overhead + * session_length */
if (!BUF_MEM_grow(s->init_buf,
- SSL_HM_HEADER_LENGTH(s) + 22 + EVP_MAX_IV_LENGTH +
- EVP_MAX_BLOCK_LENGTH + EVP_MAX_MD_SIZE + slen))
+ SSL_HM_HEADER_LENGTH(s) + 6 +
+ max_ticket_overhead + session_len))
+ {
+ OPENSSL_free(session);
return -1;
+ }
p = ssl_handshake_start(s);
EVP_CIPHER_CTX_init(&ctx);
HMAC_CTX_init(&hctx);
@@ -2602,7 +2603,7 @@
if (tctx->tlsext_ticket_key_cb(s, key_name, iv, &ctx,
&hctx, 1) < 0)
{
- OPENSSL_free(senc);
+ OPENSSL_free(session);
return -1;
}
}
@@ -2632,7 +2633,7 @@
memcpy(p, iv, EVP_CIPHER_CTX_iv_length(&ctx));
p += EVP_CIPHER_CTX_iv_length(&ctx);
/* Encrypt session data */
- EVP_EncryptUpdate(&ctx, p, &len, senc, slen);
+ EVP_EncryptUpdate(&ctx, p, &len, session, session_len);
p += len;
EVP_EncryptFinal_ex(&ctx, p, &len);
p += len;
@@ -2651,7 +2652,7 @@
p = ssl_handshake_start(s) + 4;
s2n(len - 6, p);
s->state=SSL3_ST_SW_SESSION_TICKET_B;
- OPENSSL_free(senc);
+ OPENSSL_free(session);
}
/* SSL3_ST_SW_SESSION_TICKET_B */
diff --git a/ssl/ssl_asn1.c b/ssl/ssl_asn1.c
index bfa064f..ef7ebdc 100644
--- a/ssl/ssl_asn1.c
+++ b/ssl/ssl_asn1.c
@@ -127,7 +127,7 @@
CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 2;
static const int kPeerTag =
CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 3;
-static const int kSessionIDContextTag =
+ static const int kSessionIDContextTag =
CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 4;
static const int kVerifyResultTag =
CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 5;
@@ -152,11 +152,10 @@
static const int kExtendedMasterSecretTag =
CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 17;
-int i2d_SSL_SESSION(SSL_SESSION *in, uint8_t **pp) {
+static int SSL_SESSION_to_bytes_full(SSL_SESSION *in, uint8_t **out_data,
+ size_t *out_len, int for_ticket) {
CBB cbb, session, child, child2;
uint16_t cipher_id;
- uint8_t *out;
- size_t len;
if (in == NULL || (in->cipher == NULL && in->cipher_id == 0)) {
return 0;
@@ -178,7 +177,9 @@
!CBB_add_asn1(&session, &child, CBS_ASN1_OCTETSTRING) ||
!CBB_add_u16(&child, cipher_id) ||
!CBB_add_asn1(&session, &child, CBS_ASN1_OCTETSTRING) ||
- !CBB_add_bytes(&child, in->session_id, in->session_id_length) ||
+ /* The session ID is irrelevant for a session ticket. */
+ !CBB_add_bytes(&child, in->session_id,
+ for_ticket ? 0 : in->session_id_length) ||
!CBB_add_asn1(&session, &child, CBS_ASN1_OCTETSTRING) ||
!CBB_add_bytes(&child, in->master_key, in->master_key_length)) {
OPENSSL_PUT_ERROR(SSL, i2d_SSL_SESSION, ERR_R_MALLOC_FAILURE);
@@ -330,10 +331,33 @@
}
}
- if (!CBB_finish(&cbb, &out, &len)) {
+ if (!CBB_finish(&cbb, out_data, out_len)) {
OPENSSL_PUT_ERROR(SSL, i2d_SSL_SESSION, ERR_R_MALLOC_FAILURE);
goto err;
}
+ return 1;
+
+ err:
+ CBB_cleanup(&cbb);
+ return 0;
+}
+
+int SSL_SESSION_to_bytes(SSL_SESSION *in, uint8_t **out_data, size_t *out_len) {
+ return SSL_SESSION_to_bytes_full(in, out_data, out_len, 0);
+}
+
+int SSL_SESSION_to_bytes_for_ticket(SSL_SESSION *in, uint8_t **out_data,
+ size_t *out_len) {
+ return SSL_SESSION_to_bytes_full(in, out_data, out_len, 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);
@@ -341,7 +365,6 @@
return -1;
}
- /* TODO(davidben): Provide a safer API and deprecate this one. */
if (pp) {
memcpy(*pp, out, len);
*pp += len;
@@ -349,10 +372,6 @@
OPENSSL_free(out);
return len;
-
-err:
- CBB_cleanup(&cbb);
- return -1;
}
/* d2i_SSL_SESSION_get_string gets an optional ASN.1 OCTET STRING
diff --git a/ssl/ssl_error.c b/ssl/ssl_error.c
index 247fa0f..b070b5f 100644
--- a/ssl/ssl_error.c
+++ b/ssl/ssl_error.c
@@ -39,6 +39,7 @@
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SESSION_new, 0), "SSL_SESSION_new"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SESSION_print_fp, 0), "SSL_SESSION_print_fp"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SESSION_set1_id_context, 0), "SSL_SESSION_set1_id_context"},
+ {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_SESSION_to_bytes_full, 0), "SSL_SESSION_to_bytes_full"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_add_dir_cert_subjects_to_stack, 0), "SSL_add_dir_cert_subjects_to_stack"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_add_file_cert_subjects_to_stack, 0), "SSL_add_file_cert_subjects_to_stack"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_check_private_key, 0), "SSL_check_private_key"},
@@ -71,6 +72,8 @@
{ERR_PACK(ERR_LIB_SSL, SSL_F_authz_find_data, 0), "authz_find_data"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_check_suiteb_cipher_list, 0), "check_suiteb_cipher_list"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_d2i_SSL_SESSION, 0), "d2i_SSL_SESSION"},
+ {ERR_PACK(ERR_LIB_SSL, SSL_F_d2i_SSL_SESSION_get_octet_string, 0), "d2i_SSL_SESSION_get_octet_string"},
+ {ERR_PACK(ERR_LIB_SSL, SSL_F_d2i_SSL_SESSION_get_string, 0), "d2i_SSL_SESSION_get_string"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_do_dtls1_write, 0), "do_dtls1_write"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_do_ssl3_write, 0), "do_ssl3_write"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_dtls1_accept, 0), "dtls1_accept"},
@@ -138,6 +141,7 @@
{ERR_PACK(ERR_LIB_SSL, SSL_F_ssl3_send_client_certificate, 0), "ssl3_send_client_certificate"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_ssl3_send_client_hello, 0), "ssl3_send_client_hello"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_ssl3_send_client_key_exchange, 0), "ssl3_send_client_key_exchange"},
+ {ERR_PACK(ERR_LIB_SSL, SSL_F_ssl3_send_new_session_ticket, 0), "ssl3_send_new_session_ticket"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_ssl3_send_server_certificate, 0), "ssl3_send_server_certificate"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_ssl3_send_server_hello, 0), "ssl3_send_server_hello"},
{ERR_PACK(ERR_LIB_SSL, SSL_F_ssl3_send_server_key_exchange, 0), "ssl3_send_server_key_exchange"},
diff --git a/ssl/ssl_test.c b/ssl/ssl_test.c
index edf509e..da9ba4b 100644
--- a/ssl/ssl_test.c
+++ b/ssl/ssl_test.c
@@ -343,7 +343,7 @@
static int test_ssl_session_asn1(const char *input_b64) {
int ret = 0, len;
- size_t input_len;
+ size_t input_len, encoded_len;
uint8_t *input = NULL, *encoded = NULL;
const uint8_t *cptr;
uint8_t *ptr;
@@ -363,6 +363,19 @@
}
/* Verify the SSL_SESSION encoding round-trips. */
+ if (!SSL_SESSION_to_bytes(session, &encoded, &encoded_len)) {
+ fprintf(stderr, "SSL_SESSION_to_bytes failed\n");
+ goto done;
+ }
+ if (encoded_len != input_len ||
+ memcmp(input, encoded, input_len) != 0) {
+ fprintf(stderr, "SSL_SESSION_to_bytes did not round-trip\n");
+ goto done;
+ }
+ OPENSSL_free(encoded);
+ encoded = NULL;
+
+ /* Verify the SSL_SESSION encoding round-trips via the legacy API. */
len = i2d_SSL_SESSION(session, NULL);
if (len < 0 || (size_t)len != input_len) {
fprintf(stderr, "i2d_SSL_SESSION(NULL) returned invalid length\n");
