diff --git a/CHANGELOG b/CHANGELOG index 4f5ab271a..7c95b3f34 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,8 @@ +2020-12-31 + - 2.27.0 + - [API] Remove keylog callbacks. See issue #188. + - Add a bit more ALPN logging. + 2020-12-23 - 2.26.2 - [BUGFIX] Do not drop incoming data when STOP_SENDING is received. diff --git a/bin/prog.c b/bin/prog.c index c197fd6af..ab8ab63d7 100644 --- a/bin/prog.c +++ b/bin/prog.c @@ -38,8 +38,10 @@ #include "prog.h" static int prog_stopped; +static const char *s_keylog_dir; static SSL_CTX * get_ssl_ctx (void *, const struct sockaddr *); +static void keylog_log_line (const SSL *, const char *); static const struct lsquic_packout_mem_if pmi = { .pmi_allocate = pba_allocate, @@ -81,7 +83,7 @@ prog_init (struct prog *prog, unsigned flags, = prog; prog->prog_api.ea_pmi = &pmi; prog->prog_api.ea_pmi_ctx = &prog->prog_pba; - prog->prog_api.ea_get_ssl_ctx = flags & LSENG_SERVER ? get_ssl_ctx : NULL; + prog->prog_api.ea_get_ssl_ctx = get_ssl_ctx; #if LSQUIC_PREFERRED_ADDR if (getenv("LSQUIC_PREFERRED_ADDR4") || getenv("LSQUIC_PREFERRED_ADDR6")) prog->prog_flags |= PROG_SEARCH_ADDRS; @@ -356,7 +358,7 @@ prog_set_opt (struct prog *prog, int opt, const char *arg) return -1; } } - prog->prog_keylog_dir = optarg; + s_keylog_dir = optarg; if (prog->prog_settings.es_ql_bits) { LSQ_NOTICE("QL loss bits turned off because of -G. If you want " @@ -425,29 +427,41 @@ get_ssl_ctx (void *peer_ctx, const struct sockaddr *unused) static int -prog_init_server (struct prog *prog) +prog_init_ssl_ctx (struct prog *prog) { - struct service_port *sport; unsigned char ticket_keys[48]; prog->prog_ssl_ctx = SSL_CTX_new(TLS_method()); - if (prog->prog_ssl_ctx) + if (!prog->prog_ssl_ctx) { - SSL_CTX_set_min_proto_version(prog->prog_ssl_ctx, TLS1_3_VERSION); - SSL_CTX_set_max_proto_version(prog->prog_ssl_ctx, TLS1_3_VERSION); - SSL_CTX_set_default_verify_paths(prog->prog_ssl_ctx); - - /* This is obviously test code: the key is just an array of NUL bytes */ - memset(ticket_keys, 0, sizeof(ticket_keys)); - if (1 != SSL_CTX_set_tlsext_ticket_keys(prog->prog_ssl_ctx, - ticket_keys, sizeof(ticket_keys))) - { - LSQ_ERROR("SSL_CTX_set_tlsext_ticket_keys failed"); - return -1; - } + LSQ_ERROR("cannot allocate SSL context"); + return -1; } - else - LSQ_WARN("cannot create SSL context"); + + SSL_CTX_set_min_proto_version(prog->prog_ssl_ctx, TLS1_3_VERSION); + SSL_CTX_set_max_proto_version(prog->prog_ssl_ctx, TLS1_3_VERSION); + SSL_CTX_set_default_verify_paths(prog->prog_ssl_ctx); + + /* This is obviously test code: the key is just an array of NUL bytes */ + memset(ticket_keys, 0, sizeof(ticket_keys)); + if (1 != SSL_CTX_set_tlsext_ticket_keys(prog->prog_ssl_ctx, + ticket_keys, sizeof(ticket_keys))) + { + LSQ_ERROR("SSL_CTX_set_tlsext_ticket_keys failed"); + return -1; + } + + if (s_keylog_dir) + SSL_CTX_set_keylog_callback(prog->prog_ssl_ctx, keylog_log_line); + + return 0; +} + + +static int +prog_init_server (struct prog *prog) +{ + struct service_port *sport; TAILQ_FOREACH(sport, prog->prog_sports, next_sport) if (0 != sport_init_server(sport, prog->prog_engine, prog->prog_eb)) @@ -582,55 +596,46 @@ prog_stop (struct prog *prog) static void * -keylog_open (void *ctx, lsquic_conn_t *conn) +keylog_open_file (const SSL *ssl) { - const struct prog *const prog = ctx; + const lsquic_conn_t *conn; const lsquic_cid_t *cid; FILE *fh; int sz; char id_str[MAX_CID_LEN * 2 + 1]; char path[PATH_MAX]; + conn = lsquic_ssl_to_conn(ssl); cid = lsquic_conn_id(conn); lsquic_hexstr(cid->idbuf, cid->len, id_str, sizeof(id_str)); - sz = snprintf(path, sizeof(path), "%s/%s.keys", prog->prog_keylog_dir, - id_str); + sz = snprintf(path, sizeof(path), "%s/%s.keys", s_keylog_dir, id_str); if ((size_t) sz >= sizeof(path)) { LSQ_WARN("%s: file too long", __func__); return NULL; } - fh = fopen(path, "w"); + fh = fopen(path, "ab"); if (!fh) - LSQ_WARN("could not open %s for writing: %s", path, strerror(errno)); + LSQ_WARN("could not open %s for appending: %s", path, strerror(errno)); return fh; } static void -keylog_log_line (void *handle, const char *line) +keylog_log_line (const SSL *ssl, const char *line) { - fputs(line, handle); - fputs("\n", handle); - fflush(handle); -} + FILE *file; - -static void -keylog_close (void *handle) -{ - fclose(handle); + file = keylog_open_file(ssl); + if (file) + { + fputs(line, file); + fputs("\n", file); + fclose(file); + } } -static const struct lsquic_keylog_if keylog_if = -{ - .kli_open = keylog_open, - .kli_log_line = keylog_log_line, - .kli_close = keylog_close, -}; - - static struct ssl_ctx_st * no_cert (void *cert_lu_ctx, const struct sockaddr *sa_UNUSED, const char *sni) { @@ -644,10 +649,17 @@ prog_prep (struct prog *prog) int s; char err_buf[100]; - if (prog->prog_keylog_dir) + if (s_keylog_dir && prog->prog_certs) { - prog->prog_api.ea_keylog_if = &keylog_if; - prog->prog_api.ea_keylog_ctx = prog; + struct lsquic_hash_elem *el; + struct server_cert *cert; + + for (el = lsquic_hash_first(prog->prog_certs); el; + el = lsquic_hash_next(prog->prog_certs)) + { + cert = lsquic_hashelem_getdata(el); + SSL_CTX_set_keylog_callback(cert->ce_ssl_ctx, keylog_log_line); + } } if (0 != lsquic_engine_check_settings(prog->prog_api.ea_settings, @@ -696,6 +708,9 @@ prog_prep (struct prog *prog) prog->prog_timer = event_new(prog->prog_eb, -1, 0, prog_timer_handler, prog); + if (0 != prog_init_ssl_ctx(prog)) + return -1; + if (prog->prog_engine_flags & LSENG_SERVER) s = prog_init_server(prog); else diff --git a/bin/prog.h b/bin/prog.h index a5ae83289..572b56a8a 100644 --- a/bin/prog.h +++ b/bin/prog.h @@ -45,7 +45,6 @@ struct prog struct lsquic_engine *prog_engine; const char *prog_hostname; int prog_ipver; /* 0, 4, or 6 */ - const char *prog_keylog_dir; enum { PROG_FLAG_COOLDOWN = 1 << 0, #if LSQUIC_PREFERRED_ADDR diff --git a/docs/apiref.rst b/docs/apiref.rst index 015c6a032..622fcc4ae 100644 --- a/docs/apiref.rst +++ b/docs/apiref.rst @@ -1981,6 +1981,10 @@ Miscellaneous Stream Functions Other Functions --------------- +.. function:: lsquic_conn_t lsquic_ssl_to_conn (const SSL *) + + Get connection associated with this SSL object. + .. function:: enum lsquic_version lsquic_str2ver (const char *str, size_t len) Translate string QUIC version to LSQUIC QUIC version representation. @@ -2103,22 +2107,6 @@ Miscellaneous Types Number of elements in the peer context pointer and connection ID arrays. -.. type:: struct lsquic_keylog_if - - SSL keylog interface. - - .. member:: void * (*kli_open) (void *keylog_ctx, lsquic_conn_t *conn) - - Return keylog handle or NULL if no key logging is desired. - - .. member:: void (*kli_log_line) (void *handle, const char *line) - - Log line. The first argument is the pointer returned by ``kli_open()``. - - .. member:: void (*kli_close) (void *handle) - - Close handle. - .. type:: enum lsquic_logger_timestamp_style Enumerate timestamp styles supported by LSQUIC logger mechanism. diff --git a/docs/conf.py b/docs/conf.py index 0e957cf02..dbe635dc6 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -24,9 +24,9 @@ author = u'LiteSpeed Technologies' # The short X.Y version -version = u'2.26' +version = u'2.27' # The full version, including alpha/beta/rc tags -release = u'2.26.2' +release = u'2.27.0' # -- General configuration --------------------------------------------------- diff --git a/docs/tutorial.rst b/docs/tutorial.rst index 296cc9afb..2e56566c2 100644 --- a/docs/tutorial.rst +++ b/docs/tutorial.rst @@ -1078,28 +1078,9 @@ Key logging and Wireshark `Wireshark`_ supports IETF QUIC. The developers have been very good at keeping up with latest versions. You will need version 3.3 of Wireshark to support Internet-Draft 29. Support for HTTP/3 is in progress. -LSQUIC supports exporting TLS secrets. For that, you need to specify a set of function pointers via -:member:`lsquic_engine_api.ea_keylog_if`. - -:: - - /* Secrets are logged per connection. Interface to open file (handle), - * log lines, and close file. - */ - struct lsquic_keylog_if { - void * (*kli_open) (void *keylog_ctx, lsquic_conn_t *); - void (*kli_log_line) (void *handle, const char *line); - void (*kli_close) (void *handle); - }; - - struct lsquic_engine_api { - /* --- 8< --- snip --- 8< --- */ - const struct lsquic_keylog_if *ea_keylog_if; - void *ea_keylog_ctx; - }; - -There are three functions: one to open a file, one to write a line into the file, and one to close the file. The lines are not interpreted. -In the engine API struct, there are two members to set: one is the pointer to the struct with the function pointers, and the other is the context passed to "kli_open" function. +To export TLS secrets, use BoringSSL's ``SSL_CTX_set_keylog_callback()``. +Use `lsquic_ssl_to_conn()` to get the connection associated +with the SSL object. Key logging example ------------------- @@ -1107,8 +1088,9 @@ Key logging example :: static void * - keylog_open (void *ctx, lsquic_conn_t *conn) + keylog_open_file (const SSL *ssl) { + const lsquic_conn_t *conn; const lsquic_cid_t *cid; FILE *fh; int sz; @@ -1117,6 +1099,7 @@ Key logging example char path[PATH_MAX]; static const char b2c[16] = "0123456789ABCDEF"; + conn = lsquic_ssl_to_conn(ssl); cid = lsquic_conn_id(conn); for (i = 0; i < cid->len; ++i) { @@ -1130,28 +1113,30 @@ Key logging example LOG("WARN: %s: file too long", __func__); return NULL; } - fh = fopen(path, "wb"); + fh = fopen(path, "ab"); if (!fh) - LOG("WARN: could not open %s for writing: %s", path, strerror(errno)); + LOG("WARN: could not open %s for appending: %s", path, strerror(errno)); return fh; } static void - keylog_log_line (void *handle, const char *line) + keylog_log_line (const SSL *ssl, const char *line) { - fputs(line, handle); - fputs("\n", handle); - fflush(handle); + file = keylog_open_file(ssl); + if (file) + { + fputs(line, file); + fputs("\n", file); + fclose(file); + } } - static void - keylog_close (void *handle) - { - fclose(handle); - } + /* ... */ + + SSL_CTX_set_keylog_callback(ssl, keylog_log_line); -The function to open the file is passed the connection object. It can be used to generate a filename -based on the connection ID. +The most involved part of this is opening the necessary file, creating it if necessary. +The connection can be used to generate a filename based on the connection ID. We see that the line logger simply writes the passed C string to the filehandle and appends a newline. Wireshark screenshot diff --git a/include/lsquic.h b/include/lsquic.h index 005bcdf32..48ba656b1 100644 --- a/include/lsquic.h +++ b/include/lsquic.h @@ -24,8 +24,8 @@ extern "C" { #endif #define LSQUIC_MAJOR_VERSION 2 -#define LSQUIC_MINOR_VERSION 26 -#define LSQUIC_PATCH_VERSION 2 +#define LSQUIC_MINOR_VERSION 27 +#define LSQUIC_PATCH_VERSION 0 /** * Engine flags: @@ -1237,26 +1237,6 @@ struct lsquic_hset_if enum lsquic_hsi_flag hsi_flags; }; -/** - * SSL keylog interface. - */ -struct lsquic_keylog_if -{ - /** Return keylog handle or NULL if no key logging is desired */ - void * (*kli_open) (void *keylog_ctx, lsquic_conn_t *); - - /** - * Log line. The first argument is the pointer returned by - * @ref kli_open. - */ - void (*kli_log_line) (void *handle, const char *line); - - /** - * Close handle. - */ - void (*kli_close) (void *handle); -}; - /** * This struct contains a list of all callbacks that are used by the engine * to communicate with the user code. Most of these are optional, while @@ -1333,12 +1313,6 @@ struct lsquic_engine_api void /* FILE, really */ *ea_stats_fh; #endif - /** - * Optional SSL key logging interface. - */ - const struct lsquic_keylog_if *ea_keylog_if; - void *ea_keylog_ctx; - /** * The optional ALPN string is used by the client if @ref LSENG_HTTP * is not set. @@ -2068,6 +2042,10 @@ lsquic_conn_status (lsquic_conn_t *, char *errbuf, size_t bufsz); extern const char *const lsquic_ver2str[N_LSQVER]; +/* Return connection associated with this SSL object */ +lsquic_conn_t * +lsquic_ssl_to_conn (const struct ssl_st *); + #ifdef __cplusplus } #endif diff --git a/src/liblsquic/lsquic_enc_sess_ietf.c b/src/liblsquic/lsquic_enc_sess_ietf.c index 71ca6f21f..dea41ef3f 100644 --- a/src/liblsquic/lsquic_enc_sess_ietf.c +++ b/src/liblsquic/lsquic_enc_sess_ietf.c @@ -109,15 +109,9 @@ no_sess_ticket (enum alarm_id alarm_id, void *ctx, static int iquic_new_session_cb (SSL *, SSL_SESSION *); -static void -keylog_callback (const SSL *, const char *); - static enum ssl_verify_result_t verify_server_cert_callback (SSL *, uint8_t *out_alert); -static void -maybe_setup_key_logging (struct enc_sess_iquic *); - static void iquic_esfi_destroy (enc_session_t *); @@ -260,7 +254,6 @@ struct enc_sess_iquic } esi_flags; enum enc_level esi_last_w; unsigned esi_trasec_sz; - void *esi_keylog_handle; #ifndef NDEBUG char *esi_sni_bypass; #endif @@ -886,6 +879,8 @@ iquic_esfi_create_client (const char *hostname, goto err; alpn_selected: enc_sess->esi_alpn = am->alpn; + LSQ_DEBUG("for QUIC version %s, ALPN is %s", + lsquic_ver2str[am->version], (char *) am->alpn + 1); } if (enc_sess->esi_enpub->enp_get_ssl_ctx) @@ -916,8 +911,6 @@ iquic_esfi_create_client (const char *hostname, SSL_CTX_set_session_cache_mode(ssl_ctx, SSL_SESS_CACHE_CLIENT); if (enc_sess->esi_enpub->enp_stream_if->on_sess_resume_info) SSL_CTX_sess_set_new_cb(ssl_ctx, iquic_new_session_cb); - if (enc_sess->esi_enpub->enp_kli) - SSL_CTX_set_keylog_callback(ssl_ctx, keylog_callback); if (enc_sess->esi_enpub->enp_verify_cert || LSQ_LOG_ENABLED_EXT(LSQ_LOG_DEBUG, LSQLM_EVENT) || LSQ_LOG_ENABLED_EXT(LSQ_LOG_DEBUG, LSQLM_QLOG)) @@ -954,8 +947,6 @@ iquic_esfi_create_client (const char *hostname, goto err; } - maybe_setup_key_logging(enc_sess); - if (enc_sess->esi_alpn && 0 != SSL_set_alpn_protos(enc_sess->esi_ssl, enc_sess->esi_alpn, enc_sess->esi_alpn[0] + 1)) @@ -1243,31 +1234,6 @@ free_handshake_keys (struct enc_sess_iquic *enc_sess) } -static void -keylog_callback (const SSL *ssl, const char *line) -{ - struct enc_sess_iquic *enc_sess; - - enc_sess = SSL_get_ex_data(ssl, s_idx); - if (enc_sess->esi_keylog_handle) - enc_sess->esi_enpub->enp_kli->kli_log_line( - enc_sess->esi_keylog_handle, line); -} - - -static void -maybe_setup_key_logging (struct enc_sess_iquic *enc_sess) -{ - if (enc_sess->esi_enpub->enp_kli) - { - enc_sess->esi_keylog_handle = enc_sess->esi_enpub->enp_kli->kli_open( - enc_sess->esi_enpub->enp_kli_ctx, enc_sess->esi_conn); - LSQ_DEBUG("SSL keys %s be logged", - enc_sess->esi_keylog_handle ? "will" : "will not"); - } -} - - static enum ssl_verify_result_t verify_server_cert_callback (SSL *ssl, uint8_t *out_alert) { @@ -1332,8 +1298,6 @@ iquic_lookup_cert (SSL *ssl, void *arg) { LSQ_DEBUG("looked up cert for %s", server_name ? server_name : ""); - if (enc_sess->esi_enpub->enp_kli) - SSL_CTX_set_keylog_callback(ssl_ctx, keylog_callback); SSL_set_verify(enc_sess->esi_ssl, SSL_CTX_get_verify_mode(ssl_ctx), NULL); SSL_set_verify_depth(enc_sess->esi_ssl, @@ -1398,6 +1362,8 @@ iquic_esfi_init_server (enc_session_t *enc_session_p) lsquic_ver2str[enc_sess->esi_conn->cn_version]); return -1; ok: enc_sess->esi_alpn = am->alpn; + LSQ_DEBUG("for QUIC version %s, ALPN is %s", + lsquic_ver2str[am->version], (char *) am->alpn + 1); } path = enc_sess->esi_conn->cn_if->ci_get_path(enc_sess->esi_conn, NULL); @@ -1429,9 +1395,6 @@ iquic_esfi_init_server (enc_session_t *enc_session_p) LSQ_INFO("could not set early data context"); return -1; } - maybe_setup_key_logging(enc_sess); - if (!enc_sess->esi_enpub->enp_lookup_cert && enc_sess->esi_keylog_handle) - SSL_CTX_set_keylog_callback(ssl_ctx, keylog_callback); transpa_len = gen_trans_params(enc_sess, u.trans_params, sizeof(u.trans_params)); @@ -1988,8 +1951,6 @@ iquic_esfi_destroy (enc_session_t *enc_session_p) + sizeof(enc_sess->esi_frals) / sizeof(enc_sess->esi_frals[0]); ++fral) lsquic_frab_list_cleanup(fral); - if (enc_sess->esi_keylog_handle) - enc_sess->esi_enpub->enp_kli->kli_close(enc_sess->esi_keylog_handle); if (enc_sess->esi_ssl) SSL_free(enc_sess->esi_ssl); @@ -3406,3 +3367,19 @@ lsquic_enc_sess_ietf_gen_quic_ctx ( errno); return len; } + + +struct lsquic_conn * +lsquic_ssl_to_conn (const struct ssl_st *ssl) +{ + struct enc_sess_iquic *enc_sess; + + if (s_idx < 0) + return NULL; + + enc_sess = SSL_get_ex_data(ssl, s_idx); + if (!enc_sess) + return NULL; + + return enc_sess->esi_conn; +} diff --git a/src/liblsquic/lsquic_engine.c b/src/liblsquic/lsquic_engine.c index 35162a079..d60514c67 100644 --- a/src/liblsquic/lsquic_engine.c +++ b/src/liblsquic/lsquic_engine.c @@ -647,8 +647,6 @@ lsquic_engine_new (unsigned flags, } engine->pub.enp_verify_cert = api->ea_verify_cert; engine->pub.enp_verify_ctx = api->ea_verify_ctx; - engine->pub.enp_kli = api->ea_keylog_if; - engine->pub.enp_kli_ctx = api->ea_keylog_ctx; engine->pub.enp_engine = engine; if (hash_conns_by_addr(engine)) engine->flags |= ENG_CONNS_BY_ADDR; diff --git a/src/liblsquic/lsquic_engine_public.h b/src/liblsquic/lsquic_engine_public.h index 4a2f79852..cc1c42a24 100644 --- a/src/liblsquic/lsquic_engine_public.h +++ b/src/liblsquic/lsquic_engine_public.h @@ -56,8 +56,6 @@ struct lsquic_engine_public { const struct lsquic_packout_mem_if *enp_pmi; void *enp_pmi_ctx; - const struct lsquic_keylog_if *enp_kli; - void *enp_kli_ctx; struct lsquic_engine *enp_engine; struct lsquic_hash *enp_srst_hash; enum {