Give SSL_SESSION a destructor. Previously we'd partially attempted the ssl_st / bssl::SSLConnection subclassing split, but that gets messy when we actually try to add a destructor, because CRYPTO_EX_DATA's cleanup function needs an ssl_st*, not a bssl::SSLConnection*. Downcasting is technically undefined at this point and will likely offend some CFI-like check. Moreover, it appears that even with today's subclassing split, New<SSL>() emits symbols like: W ssl_st*& std::forward<ssl_st*&>(std::remove_reference<ssl_st*&>::type&) The compiler does not bother emitting them in optimized builds, but it does suggest we can't really avoid claiming the ssl_st type name at the symbol level, short of doing reinterpret_casts at all API boundaries. And, of course, we've already long claimed it at the #include level. So I've just left this defining directly on ssl_session_st. The cost is we need to write some silly "bssl::" prefixes in the headers, but so it goes. In the likely event we change our minds again, we can always revise this. Change-Id: Ieb429e8eaabe7c2961ef7f8d9234fb71f19a5e2a Reviewed-on: https://boringssl-review.googlesource.com/29587 Commit-Queue: David Benjamin <davidben@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/handshake.cc b/ssl/handshake.cc index cfba839..c1837a8 100644 --- a/ssl/handshake.cc +++ b/ssl/handshake.cc
@@ -281,16 +281,6 @@ return 1; } -static void set_crypto_buffer(CRYPTO_BUFFER **dest, CRYPTO_BUFFER *src) { - // TODO(davidben): Remove this helper once |SSL_SESSION| can use |UniquePtr| - // and |UniquePtr| has up_ref helpers. - CRYPTO_BUFFER_free(*dest); - *dest = src; - if (src != nullptr) { - CRYPTO_BUFFER_up_ref(src); - } -} - enum ssl_verify_result_t ssl_verify_peer_cert(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; const SSL_SESSION *prev_session = ssl->s3->established_session.get(); @@ -300,18 +290,19 @@ // so this check is sufficient to ensure the reported peer certificate never // changes on renegotiation. assert(!ssl->server); - if (sk_CRYPTO_BUFFER_num(prev_session->certs) != - sk_CRYPTO_BUFFER_num(hs->new_session->certs)) { + if (sk_CRYPTO_BUFFER_num(prev_session->certs.get()) != + sk_CRYPTO_BUFFER_num(hs->new_session->certs.get())) { OPENSSL_PUT_ERROR(SSL, SSL_R_SERVER_CERT_CHANGED); ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); return ssl_verify_invalid; } - for (size_t i = 0; i < sk_CRYPTO_BUFFER_num(hs->new_session->certs); i++) { + for (size_t i = 0; i < sk_CRYPTO_BUFFER_num(hs->new_session->certs.get()); + i++) { const CRYPTO_BUFFER *old_cert = - sk_CRYPTO_BUFFER_value(prev_session->certs, i); + sk_CRYPTO_BUFFER_value(prev_session->certs.get(), i); const CRYPTO_BUFFER *new_cert = - sk_CRYPTO_BUFFER_value(hs->new_session->certs, i); + sk_CRYPTO_BUFFER_value(hs->new_session->certs.get(), i); if (CRYPTO_BUFFER_len(old_cert) != CRYPTO_BUFFER_len(new_cert) || OPENSSL_memcmp(CRYPTO_BUFFER_data(old_cert), CRYPTO_BUFFER_data(new_cert), @@ -326,10 +317,9 @@ // certificate. Since we only authenticated the previous one, copy other // authentication from the established session and ignore what was newly // received. - set_crypto_buffer(&hs->new_session->ocsp_response, - prev_session->ocsp_response); - set_crypto_buffer(&hs->new_session->signed_cert_timestamp_list, - prev_session->signed_cert_timestamp_list); + hs->new_session->ocsp_response = UpRef(prev_session->ocsp_response); + hs->new_session->signed_cert_timestamp_list = + UpRef(prev_session->signed_cert_timestamp_list); hs->new_session->verify_result = prev_session->verify_result; return ssl_verify_ok; }
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc index c649eba..eba21f3 100644 --- a/ssl/handshake_client.cc +++ b/ssl/handshake_client.cc
@@ -402,7 +402,7 @@ if (ssl->session->is_server || !ssl_supports_version(hs, ssl->session->ssl_version) || (ssl->session->session_id_length == 0 && - ssl->session->tlsext_ticklen == 0) || + ssl->session->ticket.empty()) || ssl->session->not_resumable || !ssl_session_is_time_valid(ssl, ssl->session)) { ssl_set_session(ssl, NULL); @@ -772,16 +772,13 @@ CBS body = msg.body; uint8_t alert = SSL_AD_DECODE_ERROR; - UniquePtr<STACK_OF(CRYPTO_BUFFER)> chain; - if (!ssl_parse_cert_chain(&alert, &chain, &hs->peer_pubkey, NULL, &body, - ssl->ctx->pool)) { + if (!ssl_parse_cert_chain(&alert, &hs->new_session->certs, &hs->peer_pubkey, + NULL, &body, ssl->ctx->pool)) { ssl_send_alert(ssl, SSL3_AL_FATAL, alert); return ssl_hs_error; } - sk_CRYPTO_BUFFER_pop_free(hs->new_session->certs, CRYPTO_BUFFER_free); - hs->new_session->certs = chain.release(); - if (sk_CRYPTO_BUFFER_num(hs->new_session->certs) == 0 || + if (sk_CRYPTO_BUFFER_num(hs->new_session->certs.get()) == 0 || CBS_len(&body) != 0 || !ssl->ctx->x509_method->session_cache_objects(hs->new_session.get())) { OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); @@ -791,7 +788,7 @@ if (!ssl_check_leaf_certificate( hs, hs->peer_pubkey.get(), - sk_CRYPTO_BUFFER_value(hs->new_session->certs, 0))) { + sk_CRYPTO_BUFFER_value(hs->new_session->certs.get(), 0))) { ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); return ssl_hs_error; } @@ -838,9 +835,8 @@ return ssl_hs_error; } - CRYPTO_BUFFER_free(hs->new_session->ocsp_response); - hs->new_session->ocsp_response = - CRYPTO_BUFFER_new_from_CBS(&ocsp_response, ssl->ctx->pool); + hs->new_session->ocsp_response.reset( + CRYPTO_BUFFER_new_from_CBS(&ocsp_response, ssl->ctx->pool)); if (hs->new_session->ocsp_response == nullptr) { ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); return ssl_hs_error; @@ -1225,9 +1221,8 @@ } assert(psk_len <= PSK_MAX_PSK_LEN); - OPENSSL_free(hs->new_session->psk_identity); - hs->new_session->psk_identity = BUF_strdup(identity); - if (hs->new_session->psk_identity == NULL) { + hs->new_session->psk_identity.reset(BUF_strdup(identity)); + if (hs->new_session->psk_identity == nullptr) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); return ssl_hs_error; } @@ -1526,8 +1521,8 @@ } CBS new_session_ticket = msg.body, ticket; - uint32_t tlsext_tick_lifetime_hint; - if (!CBS_get_u32(&new_session_ticket, &tlsext_tick_lifetime_hint) || + uint32_t ticket_lifetime_hint; + if (!CBS_get_u32(&new_session_ticket, &ticket_lifetime_hint) || !CBS_get_u16_length_prefixed(&new_session_ticket, &ticket) || CBS_len(&new_session_ticket) != 0) { ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); @@ -1561,14 +1556,13 @@ session = renewed_session.get(); } - // |tlsext_tick_lifetime_hint| is measured from when the ticket was issued. + // |ticket_lifetime_hint| is measured from when the ticket was issued. ssl_session_rebase_time(ssl, session); - if (!CBS_stow(&ticket, &session->tlsext_tick, &session->tlsext_ticklen)) { - OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); + if (!session->ticket.CopyFrom(ticket)) { return ssl_hs_error; } - session->tlsext_tick_lifetime_hint = tlsext_tick_lifetime_hint; + session->ticket_lifetime_hint = ticket_lifetime_hint; // Generate a session ID for this session based on the session ticket. We use // the session ID mechanism for detecting ticket resumption. This also fits in
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc index 65a6299..edbe617 100644 --- a/ssl/handshake_server.cc +++ b/ssl/handshake_server.cc
@@ -965,17 +965,14 @@ CBS certificate_msg = msg.body; uint8_t alert = SSL_AD_DECODE_ERROR; - UniquePtr<STACK_OF(CRYPTO_BUFFER)> chain; - if (!ssl_parse_cert_chain(&alert, &chain, &hs->peer_pubkey, + if (!ssl_parse_cert_chain(&alert, &hs->new_session->certs, &hs->peer_pubkey, hs->config->retain_only_sha256_of_client_certs ? hs->new_session->peer_sha256 - : NULL, + : nullptr, &certificate_msg, ssl->ctx->pool)) { ssl_send_alert(ssl, SSL3_AL_FATAL, alert); return ssl_hs_error; } - sk_CRYPTO_BUFFER_pop_free(hs->new_session->certs, CRYPTO_BUFFER_free); - hs->new_session->certs = chain.release(); if (CBS_len(&certificate_msg) != 0 || !ssl->ctx->x509_method->session_cache_objects(hs->new_session.get())) { @@ -984,7 +981,7 @@ return ssl_hs_error; } - if (sk_CRYPTO_BUFFER_num(hs->new_session->certs) == 0) { + if (sk_CRYPTO_BUFFER_num(hs->new_session->certs.get()) == 0) { // No client certificate so the handshake buffer may be discarded. hs->transcript.FreeBuffer(); @@ -1009,7 +1006,7 @@ } static enum ssl_hs_wait_t do_verify_client_certificate(SSL_HANDSHAKE *hs) { - if (sk_CRYPTO_BUFFER_num(hs->new_session->certs) > 0) { + if (sk_CRYPTO_BUFFER_num(hs->new_session->certs.get()) > 0) { switch (ssl_verify_peer_cert(hs)) { case ssl_verify_ok: break; @@ -1058,12 +1055,13 @@ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); return ssl_hs_error; } - - if (!CBS_strdup(&psk_identity, &hs->new_session->psk_identity)) { + char *raw = nullptr; + if (!CBS_strdup(&psk_identity, &raw)) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); return ssl_hs_error; } + hs->new_session->psk_identity.reset(raw); } // Depending on the key exchange method, compute |premaster_secret|. @@ -1178,7 +1176,7 @@ // Look up the key for the identity. uint8_t psk[PSK_MAX_PSK_LEN]; unsigned psk_len = hs->config->psk_server_callback( - ssl, hs->new_session->psk_identity, psk, sizeof(psk)); + ssl, hs->new_session->psk_identity.get(), psk, sizeof(psk)); if (psk_len > PSK_MAX_PSK_LEN) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); @@ -1468,8 +1466,7 @@ // If we aren't retaining peer certificates then we can discard it now. if (hs->new_session != NULL && hs->config->retain_only_sha256_of_client_certs) { - sk_CRYPTO_BUFFER_pop_free(hs->new_session->certs, CRYPTO_BUFFER_free); - hs->new_session->certs = NULL; + hs->new_session->certs.reset(); ssl->ctx->x509_method->session_clear(hs->new_session.get()); }
diff --git a/ssl/internal.h b/ssl/internal.h index d66b418..52320f2 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -3195,113 +3195,112 @@ struct ssl_ctx_st : public bssl::SSLContext {}; struct ssl_st : public bssl::SSLConnection {}; -// TODO(davidben): This type was recently made opaque. After 2018-06-13, move it -// into a namespace and make it C++. struct ssl_session_st { - CRYPTO_refcount_t references; - uint16_t ssl_version; // what ssl version session info is being kept in here? + explicit ssl_session_st(const bssl::SSL_X509_METHOD *method); + ssl_session_st(const ssl_session_st &) = delete; + ssl_session_st &operator=(const ssl_session_st &) = delete; + + CRYPTO_refcount_t references = 1; + uint16_t ssl_version = 0; // what ssl version session info is being kept in here? // group_id is the ID of the ECDH group used to establish this session or zero // if not applicable or unknown. - uint16_t group_id; + uint16_t group_id = 0; // peer_signature_algorithm is the signature algorithm used to authenticate // the peer, or zero if not applicable or unknown. - uint16_t peer_signature_algorithm; + uint16_t peer_signature_algorithm = 0; // master_key, in TLS 1.2 and below, is the master secret associated with the // session. In TLS 1.3 and up, it is the resumption secret. - int master_key_length; - uint8_t master_key[SSL_MAX_MASTER_KEY_LENGTH]; + int master_key_length = 0; + uint8_t master_key[SSL_MAX_MASTER_KEY_LENGTH] = {0}; // session_id - valid? - unsigned int session_id_length; - uint8_t session_id[SSL_MAX_SSL_SESSION_ID_LENGTH]; + unsigned session_id_length = 0; + uint8_t session_id[SSL_MAX_SSL_SESSION_ID_LENGTH] = {0}; // this is used to determine whether the session is being reused in // the appropriate context. It is up to the application to set this, // via SSL_new - uint8_t sid_ctx_length; - uint8_t sid_ctx[SSL_MAX_SID_CTX_LENGTH]; + uint8_t sid_ctx_length = 0; + uint8_t sid_ctx[SSL_MAX_SID_CTX_LENGTH] = {0}; - char *psk_identity; + bssl::UniquePtr<char> psk_identity; // certs contains the certificate chain from the peer, starting with the leaf // certificate. - STACK_OF(CRYPTO_BUFFER) *certs; + bssl::UniquePtr<STACK_OF(CRYPTO_BUFFER)> certs; - const bssl::SSL_X509_METHOD *x509_method; + const bssl::SSL_X509_METHOD *x509_method = nullptr; // x509_peer is the peer's certificate. - X509 *x509_peer; + X509 *x509_peer = nullptr; // x509_chain is the certificate chain sent by the peer. NOTE: for historical // reasons, when a client (so the peer is a server), the chain includes // |peer|, but when a server it does not. - STACK_OF(X509) *x509_chain; + STACK_OF(X509) *x509_chain = nullptr; // x509_chain_without_leaf is a lazily constructed copy of |x509_chain| that // omits the leaf certificate. This exists because OpenSSL, historically, // didn't include the leaf certificate in the chain for a server, but did for // a client. The |x509_chain| always includes it and, if an API call requires // a chain without, it is stored here. - STACK_OF(X509) *x509_chain_without_leaf; + STACK_OF(X509) *x509_chain_without_leaf = nullptr; // verify_result is the result of certificate verification in the case of // non-fatal certificate errors. - long verify_result; + long verify_result = X509_V_ERR_INVALID_CALL; // timeout is the lifetime of the session in seconds, measured from |time|. // This is renewable up to |auth_timeout|. - uint32_t timeout; + uint32_t timeout = SSL_DEFAULT_SESSION_TIMEOUT; // auth_timeout is the non-renewable lifetime of the session in seconds, // measured from |time|. - uint32_t auth_timeout; + uint32_t auth_timeout = SSL_DEFAULT_SESSION_TIMEOUT; // time is the time the session was issued, measured in seconds from the UNIX // epoch. - uint64_t time; + uint64_t time = 0; - const SSL_CIPHER *cipher; + const SSL_CIPHER *cipher = nullptr; CRYPTO_EX_DATA ex_data; // application specific data // These are used to make removal of session-ids more efficient and to // implement a maximum cache size. - SSL_SESSION *prev, *next; + SSL_SESSION *prev = nullptr, *next = nullptr; - // RFC4507 info - uint8_t *tlsext_tick; // Session ticket - size_t tlsext_ticklen; // Session ticket length + bssl::Array<uint8_t> ticket; - CRYPTO_BUFFER *signed_cert_timestamp_list; + bssl::UniquePtr<CRYPTO_BUFFER> signed_cert_timestamp_list; // The OCSP response that came with the session. - CRYPTO_BUFFER *ocsp_response; + bssl::UniquePtr<CRYPTO_BUFFER> ocsp_response; // peer_sha256 contains the SHA-256 hash of the peer's certificate if // |peer_sha256_valid| is true. - uint8_t peer_sha256[SHA256_DIGEST_LENGTH]; + uint8_t peer_sha256[SHA256_DIGEST_LENGTH] = {0}; // original_handshake_hash contains the handshake hash (either SHA-1+MD5 or // SHA-2, depending on TLS version) for the original, full handshake that // created a session. This is used by Channel IDs during resumption. - uint8_t original_handshake_hash[EVP_MAX_MD_SIZE]; - uint8_t original_handshake_hash_len; + uint8_t original_handshake_hash[EVP_MAX_MD_SIZE] = {0}; + uint8_t original_handshake_hash_len = 0; - uint32_t tlsext_tick_lifetime_hint; // Session lifetime hint in seconds + uint32_t ticket_lifetime_hint = 0; // Session lifetime hint in seconds - uint32_t ticket_age_add; + uint32_t ticket_age_add = 0; // ticket_max_early_data is the maximum amount of data allowed to be sent as // early data. If zero, 0-RTT is disallowed. - uint32_t ticket_max_early_data; + uint32_t ticket_max_early_data = 0; // early_alpn is the ALPN protocol from the initial handshake. This is only // stored for TLS 1.3 and above in order to enforce ALPN matching for 0-RTT // resumptions. - uint8_t *early_alpn; - size_t early_alpn_len; + bssl::Array<uint8_t> early_alpn; // extended_master_secret is whether the master secret in this session was // generated using EMS and thus isn't vulnerable to the Triple Handshake @@ -3319,6 +3318,10 @@ // is_server is whether this session was created by a server. bool is_server:1; + + private: + ~ssl_session_st(); + friend void SSL_SESSION_free(SSL_SESSION *); };
diff --git a/ssl/ssl_asn1.cc b/ssl/ssl_asn1.cc index 54e87bf..5dfacb2 100644 --- a/ssl/ssl_asn1.cc +++ b/ssl/ssl_asn1.cc
@@ -224,8 +224,8 @@ // The peer certificate is only serialized if the SHA-256 isn't // serialized instead. - if (sk_CRYPTO_BUFFER_num(in->certs) > 0 && !in->peer_sha256_valid) { - const CRYPTO_BUFFER *buffer = sk_CRYPTO_BUFFER_value(in->certs, 0); + if (sk_CRYPTO_BUFFER_num(in->certs.get()) > 0 && !in->peer_sha256_valid) { + const CRYPTO_BUFFER *buffer = sk_CRYPTO_BUFFER_value(in->certs.get(), 0); if (!CBB_add_asn1(&session, &child, kPeerTag) || !CBB_add_bytes(&child, CRYPTO_BUFFER_data(buffer), CRYPTO_BUFFER_len(buffer))) { @@ -252,25 +252,26 @@ if (in->psk_identity) { if (!CBB_add_asn1(&session, &child, kPSKIdentityTag) || - !CBB_add_asn1_octet_string(&child, (const uint8_t *)in->psk_identity, - strlen(in->psk_identity))) { + !CBB_add_asn1_octet_string(&child, + (const uint8_t *)in->psk_identity.get(), + strlen(in->psk_identity.get()))) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); return 0; } } - if (in->tlsext_tick_lifetime_hint > 0) { + if (in->ticket_lifetime_hint > 0) { if (!CBB_add_asn1(&session, &child, kTicketLifetimeHintTag) || - !CBB_add_asn1_uint64(&child, in->tlsext_tick_lifetime_hint)) { + !CBB_add_asn1_uint64(&child, in->ticket_lifetime_hint)) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); return 0; } } - if (in->tlsext_tick && !for_ticket) { + if (!in->ticket.empty() && !for_ticket) { if (!CBB_add_asn1(&session, &child, kTicketTag) || - !CBB_add_asn1_octet_string(&child, in->tlsext_tick, - in->tlsext_ticklen)) { + !CBB_add_asn1_octet_string(&child, in->ticket.data(), + in->ticket.size())) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); return 0; } @@ -297,8 +298,8 @@ if (in->signed_cert_timestamp_list != nullptr) { if (!CBB_add_asn1(&session, &child, kSignedCertTimestampListTag) || !CBB_add_asn1_octet_string( - &child, CRYPTO_BUFFER_data(in->signed_cert_timestamp_list), - CRYPTO_BUFFER_len(in->signed_cert_timestamp_list))) { + &child, CRYPTO_BUFFER_data(in->signed_cert_timestamp_list.get()), + CRYPTO_BUFFER_len(in->signed_cert_timestamp_list.get()))) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); return 0; } @@ -306,9 +307,9 @@ if (in->ocsp_response != nullptr) { if (!CBB_add_asn1(&session, &child, kOCSPResponseTag) || - !CBB_add_asn1_octet_string(&child, - CRYPTO_BUFFER_data(in->ocsp_response), - CRYPTO_BUFFER_len(in->ocsp_response))) { + !CBB_add_asn1_octet_string( + &child, CRYPTO_BUFFER_data(in->ocsp_response.get()), + CRYPTO_BUFFER_len(in->ocsp_response.get()))) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); return 0; } @@ -333,13 +334,13 @@ // serialized instead. if (in->certs != NULL && !in->peer_sha256_valid && - sk_CRYPTO_BUFFER_num(in->certs) >= 2) { + sk_CRYPTO_BUFFER_num(in->certs.get()) >= 2) { if (!CBB_add_asn1(&session, &child, kCertChainTag)) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); return 0; } - for (size_t i = 1; i < sk_CRYPTO_BUFFER_num(in->certs); i++) { - const CRYPTO_BUFFER *buffer = sk_CRYPTO_BUFFER_value(in->certs, i); + for (size_t i = 1; i < sk_CRYPTO_BUFFER_num(in->certs.get()); i++) { + const CRYPTO_BUFFER *buffer = sk_CRYPTO_BUFFER_value(in->certs.get(), i); if (!CBB_add_bytes(&child, CRYPTO_BUFFER_data(buffer), CRYPTO_BUFFER_len(buffer))) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); @@ -386,10 +387,10 @@ return 0; } - if (in->early_alpn) { + if (!in->early_alpn.empty()) { if (!CBB_add_asn1(&session, &child, kEarlyALPNTag) || - !CBB_add_asn1_octet_string(&child, (const uint8_t *)in->early_alpn, - in->early_alpn_len)) { + !CBB_add_asn1_octet_string(&child, in->early_alpn.data(), + in->early_alpn.size())) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); return 0; } @@ -398,13 +399,11 @@ return CBB_flush(cbb); } -// SSL_SESSION_parse_string gets an optional ASN.1 OCTET STRING -// explicitly tagged with |tag| from |cbs| and saves it in |*out|. On -// entry, if |*out| is not NULL, it frees the existing contents. If -// the element was not found, it sets |*out| to NULL. It returns one -// on success, whether or not the element was found, and zero on -// decode error. -static int SSL_SESSION_parse_string(CBS *cbs, char **out, unsigned tag) { +// 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 +// element was found, and zero on decode error. +static int SSL_SESSION_parse_string(CBS *cbs, UniquePtr<char> *out, unsigned tag) { CBS value; int present; if (!CBS_get_optional_asn1_octet_string(cbs, &value, &present, tag)) { @@ -416,38 +415,33 @@ OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION); return 0; } - if (!CBS_strdup(&value, out)) { + char *raw = nullptr; + if (!CBS_strdup(&value, &raw)) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); return 0; } + out->reset(raw); } else { - OPENSSL_free(*out); - *out = NULL; + out->reset(); } return 1; } -// SSL_SESSION_parse_string gets an optional ASN.1 OCTET STRING -// explicitly tagged with |tag| from |cbs| and stows it in |*out_ptr| -// and |*out_len|. If |*out_ptr| is not NULL, it frees the existing -// contents. On entry, if the element was not found, it sets -// |*out_ptr| to NULL. It returns one on success, whether or not the -// element was found, and zero on decode error. -static int SSL_SESSION_parse_octet_string(CBS *cbs, uint8_t **out_ptr, - size_t *out_len, unsigned tag) { +// SSL_SESSION_parse_octet_string gets an optional ASN.1 OCTET STRING explicitly +// tagged with |tag| from |cbs| and stows it in |*out|. It returns one on +// success, whether or not the element was found, and zero on decode error. +static bool SSL_SESSION_parse_octet_string(CBS *cbs, Array<uint8_t> *out, + unsigned tag) { CBS value; if (!CBS_get_optional_asn1_octet_string(cbs, &value, NULL, tag)) { OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION); - return 0; + return false; } - if (!CBS_stow(&value, out_ptr, out_len)) { - OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); - return 0; - } - return 1; + return out->CopyFrom(value); } -static int SSL_SESSION_parse_crypto_buffer(CBS *cbs, CRYPTO_BUFFER **out, +static int SSL_SESSION_parse_crypto_buffer(CBS *cbs, + UniquePtr<CRYPTO_BUFFER> *out, unsigned tag, CRYPTO_BUFFER_POOL *pool) { if (!CBS_peek_asn1_tag(cbs, tag)) { @@ -461,8 +455,7 @@ OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION); return 0; } - CRYPTO_BUFFER_free(*out); - *out = CRYPTO_BUFFER_new_from_CBS(&value, pool); + out->reset(CRYPTO_BUFFER_new_from_CBS(&value, pool)); if (*out == nullptr) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); return 0; @@ -617,10 +610,9 @@ if (!SSL_SESSION_parse_string(&session, &ret->psk_identity, kPSKIdentityTag) || - !SSL_SESSION_parse_u32(&session, &ret->tlsext_tick_lifetime_hint, + !SSL_SESSION_parse_u32(&session, &ret->ticket_lifetime_hint, kTicketLifetimeHintTag, 0) || - !SSL_SESSION_parse_octet_string(&session, &ret->tlsext_tick, - &ret->tlsext_ticklen, kTicketTag)) { + !SSL_SESSION_parse_octet_string(&session, &ret->ticket, kTicketTag)) { return nullptr; } @@ -680,8 +672,8 @@ return nullptr; } if (has_peer || has_cert_chain) { - ret->certs = sk_CRYPTO_BUFFER_new_null(); - if (ret->certs == NULL) { + ret->certs.reset(sk_CRYPTO_BUFFER_new_null()); + if (ret->certs == nullptr) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); return nullptr; } @@ -689,7 +681,7 @@ if (has_peer) { UniquePtr<CRYPTO_BUFFER> buffer(CRYPTO_BUFFER_new_from_CBS(&peer, pool)); if (!buffer || - !PushToStack(ret->certs, std::move(buffer))) { + !PushToStack(ret->certs.get(), std::move(buffer))) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); return nullptr; } @@ -705,7 +697,7 @@ UniquePtr<CRYPTO_BUFFER> buffer(CRYPTO_BUFFER_new_from_CBS(&cert, pool)); if (buffer == nullptr || - !PushToStack(ret->certs, std::move(buffer))) { + !PushToStack(ret->certs.get(), std::move(buffer))) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); return nullptr; } @@ -746,7 +738,7 @@ !SSL_SESSION_parse_u32(&session, &ret->auth_timeout, kAuthTimeoutTag, ret->timeout) || !SSL_SESSION_parse_octet_string(&session, &ret->early_alpn, - &ret->early_alpn_len, kEarlyALPNTag) || + kEarlyALPNTag) || CBS_len(&session) != 0) { OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION); return nullptr;
diff --git a/ssl/ssl_cert.cc b/ssl/ssl_cert.cc index 8b869c8..04c77ad 100644 --- a/ssl/ssl_cert.cc +++ b/ssl/ssl_cert.cc
@@ -808,7 +808,7 @@ return NULL; } - return session->certs; + return session->certs.get(); } const STACK_OF(CRYPTO_BUFFER) *SSL_get0_server_requested_CAs(const SSL *ssl) {
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index a153c60..0363e32 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc
@@ -2048,8 +2048,8 @@ return; } - *out = CRYPTO_BUFFER_data(session->signed_cert_timestamp_list); - *out_len = CRYPTO_BUFFER_len(session->signed_cert_timestamp_list); + *out = CRYPTO_BUFFER_data(session->signed_cert_timestamp_list.get()); + *out_len = CRYPTO_BUFFER_len(session->signed_cert_timestamp_list.get()); } void SSL_get0_ocsp_response(const SSL *ssl, const uint8_t **out, @@ -2061,8 +2061,8 @@ return; } - *out = CRYPTO_BUFFER_data(session->ocsp_response); - *out_len = CRYPTO_BUFFER_len(session->ocsp_response); + *out = CRYPTO_BUFFER_data(session->ocsp_response.get()); + *out_len = CRYPTO_BUFFER_len(session->ocsp_response.get()); } int SSL_set_tlsext_host_name(SSL *ssl, const char *name) { @@ -2191,8 +2191,8 @@ void SSL_get0_alpn_selected(const SSL *ssl, const uint8_t **out_data, unsigned *out_len) { if (SSL_in_early_data(ssl) && !ssl->server) { - *out_data = ssl->s3->hs->early_session->early_alpn; - *out_len = ssl->s3->hs->early_session->early_alpn_len; + *out_data = ssl->s3->hs->early_session->early_alpn.data(); + *out_len = ssl->s3->hs->early_session->early_alpn.size(); } else { *out_data = ssl->s3->alpn_selected.data(); *out_len = ssl->s3->alpn_selected.size(); @@ -2545,7 +2545,7 @@ if (session == NULL) { return NULL; } - return session->psk_identity; + return session->psk_identity.get(); } void SSL_set_psk_client_callback(
diff --git a/ssl/ssl_session.cc b/ssl/ssl_session.cc index a2a1482..379cea7 100644 --- a/ssl/ssl_session.cc +++ b/ssl/ssl_session.cc
@@ -166,22 +166,7 @@ static int remove_session_lock(SSL_CTX *ctx, SSL_SESSION *session, int lock); UniquePtr<SSL_SESSION> ssl_session_new(const SSL_X509_METHOD *x509_method) { - UniquePtr<SSL_SESSION> session( - (SSL_SESSION *)OPENSSL_malloc(sizeof(SSL_SESSION))); - if (!session) { - OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); - return 0; - } - OPENSSL_memset(session.get(), 0, sizeof(SSL_SESSION)); - - session->x509_method = x509_method; - session->verify_result = X509_V_ERR_INVALID_CALL; - session->references = 1; - session->timeout = SSL_DEFAULT_SESSION_TIMEOUT; - session->auth_timeout = SSL_DEFAULT_SESSION_TIMEOUT; - session->time = time(NULL); - CRYPTO_new_ex_data(&session->ex_data); - return session; + return MakeUnique<SSL_SESSION>(x509_method); } uint32_t ssl_hash_session_id(Span<const uint8_t> session_id) { @@ -222,20 +207,20 @@ new_session->cipher = session->cipher; // Copy authentication state. - if (session->psk_identity != NULL) { - new_session->psk_identity = BUF_strdup(session->psk_identity); - if (new_session->psk_identity == NULL) { + if (session->psk_identity != nullptr) { + new_session->psk_identity.reset(BUF_strdup(session->psk_identity.get())); + if (new_session->psk_identity == nullptr) { return nullptr; } } - if (session->certs != NULL) { + if (session->certs != nullptr) { auto buf_up_ref = [](CRYPTO_BUFFER *buf) { CRYPTO_BUFFER_up_ref(buf); return buf; }; - new_session->certs = sk_CRYPTO_BUFFER_deep_copy(session->certs, buf_up_ref, - CRYPTO_BUFFER_free); - if (new_session->certs == NULL) { + new_session->certs.reset(sk_CRYPTO_BUFFER_deep_copy( + session->certs.get(), buf_up_ref, CRYPTO_BUFFER_free)); + if (new_session->certs == nullptr) { return nullptr; } } @@ -246,16 +231,9 @@ new_session->verify_result = session->verify_result; - if (session->ocsp_response != NULL) { - new_session->ocsp_response = session->ocsp_response; - CRYPTO_BUFFER_up_ref(new_session->ocsp_response); - } - - if (session->signed_cert_timestamp_list != NULL) { - new_session->signed_cert_timestamp_list = - session->signed_cert_timestamp_list; - CRYPTO_BUFFER_up_ref(new_session->signed_cert_timestamp_list); - } + new_session->ocsp_response = UpRef(session->ocsp_response); + new_session->signed_cert_timestamp_list = + UpRef(session->signed_cert_timestamp_list); OPENSSL_memcpy(new_session->peer_sha256, session->peer_sha256, SHA256_DIGEST_LENGTH); @@ -280,31 +258,20 @@ session->original_handshake_hash_len); new_session->original_handshake_hash_len = session->original_handshake_hash_len; - new_session->tlsext_tick_lifetime_hint = session->tlsext_tick_lifetime_hint; + new_session->ticket_lifetime_hint = session->ticket_lifetime_hint; new_session->ticket_age_add = session->ticket_age_add; new_session->ticket_max_early_data = session->ticket_max_early_data; new_session->extended_master_secret = session->extended_master_secret; - if (session->early_alpn != NULL) { - new_session->early_alpn = - (uint8_t *)BUF_memdup(session->early_alpn, session->early_alpn_len); - if (new_session->early_alpn == NULL) { - return nullptr; - } + if (!new_session->early_alpn.CopyFrom(session->early_alpn)) { + return nullptr; } - new_session->early_alpn_len = session->early_alpn_len; } // Copy the ticket. - if (dup_flags & SSL_SESSION_INCLUDE_TICKET) { - if (session->tlsext_tick != NULL) { - new_session->tlsext_tick = - (uint8_t *)BUF_memdup(session->tlsext_tick, session->tlsext_ticklen); - if (new_session->tlsext_tick == NULL) { - return nullptr; - } - } - new_session->tlsext_ticklen = session->tlsext_ticklen; + if (dup_flags & SSL_SESSION_INCLUDE_TICKET && + !new_session->ticket.CopyFrom(session->ticket)) { + return nullptr; } // The new_session does not get a copy of the ex_data. @@ -667,7 +634,7 @@ // If the session contains a client certificate (either the full // certificate or just the hash) then require that the form of the // certificate matches the current configuration. - ((sk_CRYPTO_BUFFER_num(session->certs) == 0 && + ((sk_CRYPTO_BUFFER_num(session->certs.get()) == 0 && !session->peer_sha256_valid) || session->peer_sha256_valid == hs->config->retain_only_sha256_of_client_certs); @@ -883,6 +850,22 @@ using namespace bssl; +ssl_session_st::ssl_session_st(const SSL_X509_METHOD *method) + : x509_method(method), + extended_master_secret(false), + peer_sha256_valid(false), + not_resumable(false), + ticket_age_add_valid(false), + is_server(false) { + CRYPTO_new_ex_data(&ex_data); + time = ::time(nullptr); +} + +ssl_session_st::~ssl_session_st() { + CRYPTO_free_ex_data(&g_ex_data_class, this, &ex_data); + x509_method->session_clear(this); +} + SSL_SESSION *SSL_SESSION_new(const SSL_CTX *ctx) { return ssl_session_new(ctx->x509_method).release(); } @@ -898,17 +881,7 @@ return; } - CRYPTO_free_ex_data(&g_ex_data_class, session, &session->ex_data); - - OPENSSL_cleanse(session->master_key, sizeof(session->master_key)); - OPENSSL_cleanse(session->session_id, sizeof(session->session_id)); - sk_CRYPTO_BUFFER_pop_free(session->certs, CRYPTO_BUFFER_free); - session->x509_method->session_clear(session); - OPENSSL_free(session->tlsext_tick); - CRYPTO_BUFFER_free(session->signed_cert_timestamp_list); - CRYPTO_BUFFER_free(session->ocsp_response); - OPENSSL_free(session->psk_identity); - OPENSSL_free(session->early_alpn); + session->~ssl_session_st(); OPENSSL_free(session); } @@ -951,15 +924,15 @@ const STACK_OF(CRYPTO_BUFFER) * SSL_SESSION_get0_peer_certificates(const SSL_SESSION *session) { - return session->certs; + return session->certs.get(); } void SSL_SESSION_get0_signed_cert_timestamp_list(const SSL_SESSION *session, const uint8_t **out, size_t *out_len) { if (session->signed_cert_timestamp_list) { - *out = CRYPTO_BUFFER_data(session->signed_cert_timestamp_list); - *out_len = CRYPTO_BUFFER_len(session->signed_cert_timestamp_list); + *out = CRYPTO_BUFFER_data(session->signed_cert_timestamp_list.get()); + *out_len = CRYPTO_BUFFER_len(session->signed_cert_timestamp_list.get()); } else { *out = nullptr; *out_len = 0; @@ -969,8 +942,8 @@ void SSL_SESSION_get0_ocsp_response(const SSL_SESSION *session, const uint8_t **out, size_t *out_len) { if (session->ocsp_response) { - *out = CRYPTO_BUFFER_data(session->ocsp_response); - *out_len = CRYPTO_BUFFER_len(session->ocsp_response); + *out = CRYPTO_BUFFER_data(session->ocsp_response.get()); + *out_len = CRYPTO_BUFFER_len(session->ocsp_response.get()); } else { *out = nullptr; *out_len = 0; @@ -1040,31 +1013,24 @@ } int SSL_SESSION_has_ticket(const SSL_SESSION *session) { - return session->tlsext_ticklen > 0; + return !session->ticket.empty(); } void SSL_SESSION_get0_ticket(const SSL_SESSION *session, const uint8_t **out_ticket, size_t *out_len) { if (out_ticket != nullptr) { - *out_ticket = session->tlsext_tick; + *out_ticket = session->ticket.data(); } - *out_len = session->tlsext_ticklen; + *out_len = session->ticket.size(); } int SSL_SESSION_set_ticket(SSL_SESSION *session, const uint8_t *ticket, size_t ticket_len) { - uint8_t *copy = (uint8_t *)BUF_memdup(ticket, ticket_len); - if (copy == nullptr) { - return 0; - } - OPENSSL_free(session->tlsext_tick); - session->tlsext_tick = copy; - session->tlsext_ticklen = ticket_len; - return 1; + return session->ticket.CopyFrom(MakeConstSpan(ticket, ticket_len)); } uint32_t SSL_SESSION_get_ticket_lifetime_hint(const SSL_SESSION *session) { - return session->tlsext_tick_lifetime_hint; + return session->ticket_lifetime_hint; } const SSL_CIPHER *SSL_SESSION_get0_cipher(const SSL_SESSION *session) {
diff --git a/ssl/ssl_x509.cc b/ssl/ssl_x509.cc index 0af4167..4f2fc9e 100644 --- a/ssl/ssl_x509.cc +++ b/ssl/ssl_x509.cc
@@ -283,7 +283,7 @@ static int ssl_crypto_x509_session_cache_objects(SSL_SESSION *sess) { bssl::UniquePtr<STACK_OF(X509)> chain; - if (sk_CRYPTO_BUFFER_num(sess->certs) > 0) { + if (sk_CRYPTO_BUFFER_num(sess->certs.get()) > 0) { chain.reset(sk_X509_new_null()); if (!chain) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); @@ -292,7 +292,7 @@ } X509 *leaf = nullptr; - for (CRYPTO_BUFFER *cert : sess->certs) { + for (CRYPTO_BUFFER *cert : sess->certs.get()) { UniquePtr<X509> x509(X509_parse_from_buffer(cert)); if (!x509) { OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc index 6db7de7..49c8955 100644 --- a/ssl/t1_lib.cc +++ b/ssl/t1_lib.cc
@@ -943,8 +943,7 @@ return true; } - const uint8_t *ticket_data = NULL; - int ticket_len = 0; + Span<const uint8_t> ticket; // Renegotiation does not participate in session resumption. However, still // advertise the extension to avoid potentially breaking servers which carry @@ -952,17 +951,16 @@ // without upstream's 3c3f0259238594d77264a78944d409f2127642c4. if (!ssl->s3->initial_handshake_complete && ssl->session != NULL && - ssl->session->tlsext_tick != NULL && + !ssl->session->ticket.empty() && // Don't send TLS 1.3 session tickets in the ticket extension. ssl_session_protocol_version(ssl->session) < TLS1_3_VERSION) { - ticket_data = ssl->session->tlsext_tick; - ticket_len = ssl->session->tlsext_ticklen; + ticket = ssl->session->ticket; } - CBB ticket; + CBB ticket_cbb; if (!CBB_add_u16(out, TLSEXT_TYPE_session_ticket) || - !CBB_add_u16_length_prefixed(out, &ticket) || - !CBB_add_bytes(&ticket, ticket_data, ticket_len) || + !CBB_add_u16_length_prefixed(out, &ticket_cbb) || + !CBB_add_bytes(&ticket_cbb, ticket.data(), ticket.size()) || !CBB_flush(out)) { return false; } @@ -1343,9 +1341,8 @@ // // TODO(davidben): Enforce this anyway. if (!ssl->s3->session_reused) { - CRYPTO_BUFFER_free(hs->new_session->signed_cert_timestamp_list); - hs->new_session->signed_cert_timestamp_list = - CRYPTO_BUFFER_new_from_CBS(contents, ssl->ctx->pool); + hs->new_session->signed_cert_timestamp_list.reset( + CRYPTO_BUFFER_new_from_CBS(contents, ssl->ctx->pool)); if (hs->new_session->signed_cert_timestamp_list == nullptr) { *out_alert = SSL_AD_INTERNAL_ERROR; return false; @@ -1875,7 +1872,7 @@ } size_t binder_len = EVP_MD_size(ssl_session_get_digest(ssl->session)); - return 15 + ssl->session->tlsext_ticklen + binder_len; + return 15 + ssl->session->ticket.size() + binder_len; } static bool ext_pre_shared_key_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) { @@ -1909,8 +1906,8 @@ !CBB_add_u16_length_prefixed(out, &contents) || !CBB_add_u16_length_prefixed(&contents, &identity) || !CBB_add_u16_length_prefixed(&identity, &ticket) || - !CBB_add_bytes(&ticket, ssl->session->tlsext_tick, - ssl->session->tlsext_ticklen) || + !CBB_add_bytes(&ticket, ssl->session->ticket.data(), + ssl->session->ticket.size()) || !CBB_add_u32(&identity, obfuscated_ticket_age) || !CBB_add_u16_length_prefixed(&contents, &binders) || !CBB_add_u8_length_prefixed(&binders, &binder) || @@ -2076,10 +2073,8 @@ hs->received_hello_retry_request || // In case ALPN preferences changed since this session was established, // avoid reporting a confusing value in |SSL_get0_alpn_selected|. - (ssl->session->early_alpn_len != 0 && - !ssl_is_alpn_protocol_allowed( - hs, MakeConstSpan(ssl->session->early_alpn, - ssl->session->early_alpn_len)))) { + (!ssl->session->early_alpn.empty() && + !ssl_is_alpn_protocol_allowed(hs, ssl->session->early_alpn))) { return true; }
diff --git a/ssl/tls13_both.cc b/ssl/tls13_both.cc index 495838c..9149e76 100644 --- a/ssl/tls13_both.cc +++ b/ssl/tls13_both.cc
@@ -256,9 +256,8 @@ } if (sk_CRYPTO_BUFFER_num(certs.get()) == 1) { - CRYPTO_BUFFER_free(hs->new_session->ocsp_response); - hs->new_session->ocsp_response = - CRYPTO_BUFFER_new_from_CBS(&ocsp_response, ssl->ctx->pool); + hs->new_session->ocsp_response.reset( + CRYPTO_BUFFER_new_from_CBS(&ocsp_response, ssl->ctx->pool)); if (hs->new_session->ocsp_response == nullptr) { ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); return 0; @@ -280,9 +279,8 @@ } if (sk_CRYPTO_BUFFER_num(certs.get()) == 1) { - CRYPTO_BUFFER_free(hs->new_session->signed_cert_timestamp_list); - hs->new_session->signed_cert_timestamp_list = - CRYPTO_BUFFER_new_from_CBS(&sct, ssl->ctx->pool); + hs->new_session->signed_cert_timestamp_list.reset( + CRYPTO_BUFFER_new_from_CBS(&sct, ssl->ctx->pool)); if (hs->new_session->signed_cert_timestamp_list == nullptr) { ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); return 0; @@ -298,9 +296,7 @@ } hs->peer_pubkey = std::move(pkey); - - sk_CRYPTO_BUFFER_pop_free(hs->new_session->certs, CRYPTO_BUFFER_free); - hs->new_session->certs = certs.release(); + hs->new_session->certs = std::move(certs); if (!ssl->ctx->x509_method->session_cache_objects(hs->new_session.get())) { OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); @@ -308,7 +304,7 @@ return 0; } - if (sk_CRYPTO_BUFFER_num(hs->new_session->certs) == 0) { + if (sk_CRYPTO_BUFFER_num(hs->new_session->certs.get()) == 0) { if (!allow_anonymous) { OPENSSL_PUT_ERROR(SSL, SSL_R_PEER_DID_NOT_RETURN_A_CERTIFICATE); ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_CERTIFICATE_REQUIRED);
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc index 317b4d3..1f1d4b4 100644 --- a/ssl/tls13_client.cc +++ b/ssl/tls13_client.cc
@@ -429,20 +429,14 @@ } // Store the negotiated ALPN in the session. - if (!ssl->s3->alpn_selected.empty()) { - hs->new_session->early_alpn = (uint8_t *)BUF_memdup( - ssl->s3->alpn_selected.data(), ssl->s3->alpn_selected.size()); - if (hs->new_session->early_alpn == NULL) { - ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); - return ssl_hs_error; - } - hs->new_session->early_alpn_len = ssl->s3->alpn_selected.size(); + if (!hs->new_session->early_alpn.CopyFrom(ssl->s3->alpn_selected)) { + ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + return ssl_hs_error; } if (ssl->s3->early_data_accepted) { if (hs->early_session->cipher != hs->new_session->cipher || - MakeConstSpan(hs->early_session->early_alpn, - hs->early_session->early_alpn_len) != + MakeConstSpan(hs->early_session->early_alpn) != ssl->s3->alpn_selected) { OPENSSL_PUT_ERROR(SSL, SSL_R_ALPN_MISMATCH_ON_EARLY_DATA); return ssl_hs_error; @@ -849,7 +843,7 @@ !CBS_get_u32(&body, &session->ticket_age_add) || !CBS_get_u8_length_prefixed(&body, &ticket_nonce) || !CBS_get_u16_length_prefixed(&body, &ticket) || - !CBS_stow(&ticket, &session->tlsext_tick, &session->tlsext_ticklen) || + !session->ticket.CopyFrom(ticket) || !CBS_get_u16_length_prefixed(&body, &extensions) || CBS_len(&body) != 0) { ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc index 481dc54..a113504 100644 --- a/ssl/tls13_server.cc +++ b/ssl/tls13_server.cc
@@ -395,8 +395,7 @@ // Custom extensions is incompatible with 0-RTT. hs->custom_extensions.received == 0 && // The negotiated ALPN must match the one in the ticket. - ssl->s3->alpn_selected == - MakeConstSpan(session->early_alpn, session->early_alpn_len)) { + MakeConstSpan(ssl->s3->alpn_selected) == session->early_alpn) { ssl->s3->early_data_accepted = true; } @@ -425,14 +424,9 @@ hs->new_session->cipher = hs->new_cipher; // Store the initial negotiated ALPN in the session. - if (!ssl->s3->alpn_selected.empty()) { - hs->new_session->early_alpn = (uint8_t *)BUF_memdup( - ssl->s3->alpn_selected.data(), ssl->s3->alpn_selected.size()); - if (hs->new_session->early_alpn == NULL) { - ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); - return ssl_hs_error; - } - hs->new_session->early_alpn_len = ssl->s3->alpn_selected.size(); + if (!hs->new_session->early_alpn.CopyFrom(ssl->s3->alpn_selected)) { + ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + return ssl_hs_error; } if (ssl->ctx->dos_protection_cb != NULL && @@ -824,7 +818,7 @@ static enum ssl_hs_wait_t do_read_client_certificate_verify( SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; - if (sk_CRYPTO_BUFFER_num(hs->new_session->certs) == 0) { + if (sk_CRYPTO_BUFFER_num(hs->new_session->certs.get()) == 0) { // Skip this state. hs->tls13_state = state_read_channel_id; return ssl_hs_ok;