On 2017/6/19 13:54, Patrick Hemmer wrote: > > > On 2017/6/17 00:00, Willy Tarreau wrote: >> Hi Patrick, >> >> On Fri, Jun 16, 2017 at 09:36:30PM -0400, Patrick Hemmer wrote: >>> The main reason I had for supporting the older code is that it seems >>> many (most?) linux distros, such as the one we use (CentOS/7), still >>> ship with 1.0.1 or 1.0.2. However since this is a minor change and a >>> feature enhancement, I doubt this will get backported to 1.7, meaning >>> we'll have to manually patch it into the version we use. And since we're >>> doing that, we can just use the patch that supports older OpenSSL. >> Yes in this case I think it makes sense. And given the proliferation >> of *ssl implementations, I really prefer to avoid adding code touching >> directly internal openssl stuff. >> >>> Anyway, here is the updated patch with the support for <1.1.0 dropped, >>> as well as the BoringSSL support Emmanuel requested. >> OK cool. >> >>> One other minor thing I was unsure about was the fetch name. Currently I >>> have it as "ssl_fc_session_key", but "ssl_fc_session_master_key" might >>> be more accurate. However this is rather verbose and would make it far >>> longer than any other sample fetch name, and I was unsure if there were >>> rules around the naming. >> There are no particular rules but it's true that long names are painful, >> especially for those with "fat fingers". I'd suggest that given that it >> starts with "ssl_fc_" and "ssl_bc_", the context is already quite >> restricted and you could probably call them "smk" or "smkey". One would >> argue that looking at the doc will be necessary, but with long names you >> also have to look at the doc to find how to spell them anyway, the >> difference being that short names don't mangle your configs too much, >> especially in logformat strings. > Well my argument for keeping the name starting with `ssl_fc_session_` > is that there is also `ssl_fc_session_id`. These 2 fetches pull their > attribute from the same "session" structure. They are also closely > related as using `ssl_fc_session_key` almost requires the > `ssl_fc_session_id` value (you could technically get by without it, > but it'll make using the key rather difficult unless you have some > other way of correlating a key with a specific SSL session). >>> +ssl_bc_session_key : binary >>> + Returns the SSL session master key of the back connection when the >>> outgoing >>> + connection was made over an SSL/TLS transport layer. It is useful to >>> decrypt >>> + traffic sent using ephemeral ciphers. >> Here it would be nice to add a short sentence like "Not available before >> openssl 1.1.0" so that users don't waste too much time on something not >> working. >> >>> static int >>> +smp_fetch_ssl_fc_session_key(const struct arg *args, struct sample *smp, >>> const char *kw, void *private) >>> +{ >>> +#if OPENSSL_VERSION_NUMBER >= 0x10100000L || defined(OPENSSL_IS_BORINGSSL) >> Here I'd put the ifdef around the whole function, like we have for >> ALPN for example, so that there's no risk it could be used in a non-working >> config. > My objection to this is that most of the other sample fetches don't do > this, and so it makes the user experience inconsistent. For example > the `ssl_fc_session_id` fetch, which `ssl_fc_session_key` is strongly > related to, behaves the same way. The ALPN/NPN fetches are the only > ones that make using the fetch a config error if the underlying > support is missing. >>> + struct connection *conn = objt_conn((kw[4] != 'b') ? smp->sess->origin : >>> + smp->strm ? smp->strm->si[1].end : >>> NULL); >>> + >>> + SSL_SESSION *ssl_sess; >>> + int data_len; >>> + struct chunk *data; >>> + >>> + if (!conn || !conn->xprt_ctx || conn->xprt != &ssl_sock) >>> + return 0; >>> + >>> + ssl_sess = SSL_get_session(conn->xprt_ctx); >>> + if (!ssl_sess) >>> + return 0; >>> + >>> + data = get_trash_chunk(); >>> + data_len = SSL_SESSION_get_master_key(ssl_sess, (unsigned char >>> *)data->str, data->size); >>> + if (!data_len) >>> + return 0; >>> + >>> + smp->flags = SMP_F_CONST; >>> + smp->data.type = SMP_T_BIN; >>> + data->len = data_len; >> If you want, you can even get rid of your data_len variable by directly >> assigning SSL_SESSION_get_master_key() to data->len. >> >>> +static int >>> smp_fetch_ssl_fc_sni(const struct arg *args, struct sample *smp, const >>> char *kw, void *private) >>> { >>> #ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME >>> @@ -7841,6 +7875,7 @@ static struct sample_fetch_kw_list >>> sample_fetch_keywords = {ILH, { >>> { "ssl_bc_unique_id", smp_fetch_ssl_fc_unique_id, 0, >>> NULL, SMP_T_BIN, SMP_USE_L5SRV }, >>> { "ssl_bc_use_keysize", smp_fetch_ssl_fc_use_keysize, 0, >>> NULL, SMP_T_SINT, SMP_USE_L5SRV }, >>> { "ssl_bc_session_id", smp_fetch_ssl_fc_session_id, 0, >>> NULL, SMP_T_BIN, SMP_USE_L5SRV }, >>> + { "ssl_bc_session_key", smp_fetch_ssl_fc_session_key, 0, >>> NULL, SMP_T_BIN, SMP_USE_L5SRV }, >> And then this line an be surrounded by #ifdef. The real benefit is that >> it refuses to parse where it will not work. >> >> Thanks, >> Willy > Haven't heard anything back about the consistency aspect, so here's an updated patch with the other changes not affected by user experience consistency.
-Patrick
From 62ad353cccebe58af7f39d596f232d7b3bcd8afb Mon Sep 17 00:00:00 2001 From: Patrick Hemmer <hapr...@stormcloud9.net> Date: Mon, 12 Jun 2017 18:03:48 -0400 Subject: [PATCH] MINOR: ssl: add fetch 'ssl_fc_session_key' and 'ssl_bc_session_key' These fetches return the SSL master key of the front/back connection. This is useful to decrypt traffic encrypted with ephemeral ciphers. --- doc/configuration.txt | 13 +++++++++++++ src/ssl_sock.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/doc/configuration.txt b/doc/configuration.txt index 082b857..17c1b1d 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -13930,6 +13930,12 @@ ssl_bc_session_id : binary made over an SSL/TLS transport layer. It is useful to log if we want to know if session was reused or not. +ssl_bc_session_key : binary + Returns the SSL session master key of the back connection when the outgoing + connection was made over an SSL/TLS transport layer. It is useful to decrypt + traffic sent using ephemeral ciphers. This requires OpenSSL >= 1.1.0, or + BoringSSL. + ssl_bc_use_keysize : integer Returns the symmetric cipher key size used in bits when the outgoing connection was made over an SSL/TLS transport layer. @@ -14185,6 +14191,13 @@ ssl_fc_session_id : binary a server. It is important to note that some browsers refresh their session ID every few minutes. +ssl_fc_session_key : binary + Returns the SSL session master key of the front connection when the incoming + connection was made over an SSL/TLS transport layer. It is useful to decrypt + traffic sent using ephemeral ciphers. This requires OpenSSL >= 1.1.0, or + BoringSSL. + + ssl_fc_sni : string This extracts the Server Name Indication TLS extension (SNI) field from an incoming connection made via an SSL/TLS transport layer and locally diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 3680515..d6c28b5 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -6170,6 +6170,38 @@ smp_fetch_ssl_fc_session_id(const struct arg *args, struct sample *smp, const ch } static int +smp_fetch_ssl_fc_session_key(const struct arg *args, struct sample *smp, const char *kw, void *private) +{ +#if OPENSSL_VERSION_NUMBER >= 0x10100000L || defined(OPENSSL_IS_BORINGSSL) + struct connection *conn = objt_conn((kw[4] != 'b') ? smp->sess->origin : + smp->strm ? smp->strm->si[1].end : NULL); + + SSL_SESSION *ssl_sess; + struct chunk *data; + + if (!conn || !conn->xprt_ctx || conn->xprt != &ssl_sock) + return 0; + + ssl_sess = SSL_get_session(conn->xprt_ctx); + if (!ssl_sess) + return 0; + + data = get_trash_chunk(); + data->len = SSL_SESSION_get_master_key(ssl_sess, (unsigned char *)data->str, data->size); + if (!data->len) + return 0; + + smp->flags = SMP_F_CONST; + smp->data.type = SMP_T_BIN; + smp->data.u.str = *data; + + return 1; +#else + return 0; +#endif +} + +static int smp_fetch_ssl_fc_sni(const struct arg *args, struct sample *smp, const char *kw, void *private) { #ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME @@ -7841,6 +7873,7 @@ static struct sample_fetch_kw_list sample_fetch_keywords = {ILH, { { "ssl_bc_unique_id", smp_fetch_ssl_fc_unique_id, 0, NULL, SMP_T_BIN, SMP_USE_L5SRV }, { "ssl_bc_use_keysize", smp_fetch_ssl_fc_use_keysize, 0, NULL, SMP_T_SINT, SMP_USE_L5SRV }, { "ssl_bc_session_id", smp_fetch_ssl_fc_session_id, 0, NULL, SMP_T_BIN, SMP_USE_L5SRV }, + { "ssl_bc_session_key", smp_fetch_ssl_fc_session_key, 0, NULL, SMP_T_BIN, SMP_USE_L5SRV }, { "ssl_c_ca_err", smp_fetch_ssl_c_ca_err, 0, NULL, SMP_T_SINT, SMP_USE_L5CLI }, { "ssl_c_ca_err_depth", smp_fetch_ssl_c_ca_err_depth, 0, NULL, SMP_T_SINT, SMP_USE_L5CLI }, { "ssl_c_der", smp_fetch_ssl_x_der, 0, NULL, SMP_T_BIN, SMP_USE_L5CLI }, @@ -7882,6 +7915,7 @@ static struct sample_fetch_kw_list sample_fetch_keywords = {ILH, { { "ssl_fc_unique_id", smp_fetch_ssl_fc_unique_id, 0, NULL, SMP_T_BIN, SMP_USE_L5CLI }, { "ssl_fc_use_keysize", smp_fetch_ssl_fc_use_keysize, 0, NULL, SMP_T_SINT, SMP_USE_L5CLI }, { "ssl_fc_session_id", smp_fetch_ssl_fc_session_id, 0, NULL, SMP_T_BIN, SMP_USE_L5CLI }, + { "ssl_fc_session_key", smp_fetch_ssl_fc_session_key, 0, NULL, SMP_T_BIN, SMP_USE_L5CLI }, { "ssl_fc_sni", smp_fetch_ssl_fc_sni, 0, NULL, SMP_T_STR, SMP_USE_L5CLI }, { "ssl_fc_cipherlist_bin", smp_fetch_ssl_fc_cl_bin, 0, NULL, SMP_T_STR, SMP_USE_L5CLI }, { "ssl_fc_cipherlist_hex", smp_fetch_ssl_fc_cl_hex, 0, NULL, SMP_T_BIN, SMP_USE_L5CLI }, -- 2.7.4 (Apple Git-66)