You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by James Peach <jp...@apache.org> on 2015/02/06 05:17:51 UTC
Re: trafficserver git commit: TS-2480: Fix to work in the case where there are no ticket key files but tickets have not been disabled. Also fix RHEL 5 compile error.
> On Feb 5, 2015, at 7:32 PM, shinrich@apache.org wrote:
>
> Repository: trafficserver
> Updated Branches:
> refs/heads/master 1d617582b -> 5fe69772a
>
>
> TS-2480: Fix to work in the case where there are no ticket key files but tickets have not been disabled.
Doesn't OpenSSL do this implicitly? Or is the problem that we end up setting the callback without having a keyblock?
> Also fix RHEL 5 compile error.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/5fe69772
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/5fe69772
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/5fe69772
>
> Branch: refs/heads/master
> Commit: 5fe69772aa7e5e841349f3426a997930b44c0ff5
> Parents: 1d61758
> Author: shinrich <sh...@yahoo-inc.com>
> Authored: Thu Feb 5 19:24:08 2015 -0600
> Committer: shinrich <sh...@yahoo-inc.com>
> Committed: Thu Feb 5 21:32:26 2015 -0600
>
> ----------------------------------------------------------------------
> iocore/net/SSLUtils.cc | 45 ++++++++++++++++++++++++++-------------------
> 1 file changed, 26 insertions(+), 19 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/5fe69772/iocore/net/SSLUtils.cc
> ----------------------------------------------------------------------
> diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc
> index 055d396..f0265c6 100644
> --- a/iocore/net/SSLUtils.cc
> +++ b/iocore/net/SSLUtils.cc
> @@ -543,28 +543,34 @@ ssl_context_enable_tickets(SSL_CTX * ctx, const char * ticket_key_path)
> Error("failed to read SSL session ticket key from %s", (const char *)ticket_key_path);
> goto fail;
> }
> + } else {
> + // Generate a random ticket key
> + ticket_key_len = 48;
> + ticket_key_data = (char *)ats_malloc(ticket_key_len);
> + char *tmp_ptr = ticket_key_data;
> + RAND_bytes(reinterpret_cast<unsigned char *>(tmp_ptr), ticket_key_len);
> + }
>
> - num_ticket_keys = ticket_key_len / sizeof(ssl_ticket_key_t);
> - if (num_ticket_keys == 0) {
> - Error("SSL session ticket key from %s is too short (>= 48 bytes are required)", (const char *)ticket_key_path);
> - goto fail;
> - }
> + num_ticket_keys = ticket_key_len / sizeof(ssl_ticket_key_t);
> + if (num_ticket_keys == 0) {
> + Error("SSL session ticket key from %s is too short (>= 48 bytes are required)", (const char *)ticket_key_path);
> + goto fail;
> + }
>
> - // Increase the stats.
> - if (ssl_rsb != NULL) { // ssl_rsb is not initialized during the first run.
> - SSL_INCREMENT_DYN_STAT(ssl_total_ticket_keys_renewed_stat);
> - }
> + // Increase the stats.
> + if (ssl_rsb != NULL) { // ssl_rsb is not initialized during the first run.
> + SSL_INCREMENT_DYN_STAT(ssl_total_ticket_keys_renewed_stat);
> + }
>
> - keyblock = ticket_block_alloc(num_ticket_keys);
> + keyblock = ticket_block_alloc(num_ticket_keys);
>
> - // Slurp all the keys in the ticket key file. We will encrypt with the first key, and decrypt
> - // with any key (for rotation purposes).
> - for (unsigned i = 0; i < num_ticket_keys; ++i) {
> - const char * data = (const char *)ticket_key_data + (i * sizeof(ssl_ticket_key_t));
> - memcpy(keyblock->keys[i].key_name, data, sizeof(ssl_ticket_key_t::key_name));
> - memcpy(keyblock->keys[i].hmac_secret, data + sizeof(ssl_ticket_key_t::key_name), sizeof(ssl_ticket_key_t::hmac_secret));
> - memcpy(keyblock->keys[i].aes_key, data + sizeof(ssl_ticket_key_t::key_name) + sizeof(ssl_ticket_key_t::hmac_secret), sizeof(ssl_ticket_key_t::aes_key));
> - }
> + // Slurp all the keys in the ticket key file. We will encrypt with the first key, and decrypt
> + // with any key (for rotation purposes).
> + for (unsigned i = 0; i < num_ticket_keys; ++i) {
> + const char * data = (const char *)ticket_key_data + (i * sizeof(ssl_ticket_key_t));
> + memcpy(keyblock->keys[i].key_name, data, sizeof(ssl_ticket_key_t::key_name));
> + memcpy(keyblock->keys[i].hmac_secret, data + sizeof(ssl_ticket_key_t::key_name), sizeof(ssl_ticket_key_t::hmac_secret));
> + memcpy(keyblock->keys[i].aes_key, data + sizeof(ssl_ticket_key_t::key_name) + sizeof(ssl_ticket_key_t::hmac_secret), sizeof(ssl_ticket_key_t::aes_key));
> }
>
> // Setting the callback can only fail if OpenSSL does not recognize the
> @@ -1771,10 +1777,11 @@ ssl_store_ssl_context(
> if (SSLConfigParams::init_ssl_ctx_cb) {
> SSLConfigParams::init_ssl_ctx_cb(ctx, true);
> }
> +#if HAVE_OPENSSL_SESSION_TICKETS
> if (!inserted && keyblock != NULL) {
> ticket_block_free(keyblock);
> }
> -
> +#endif
> return ctx;
> }
>
>
Re: trafficserver git commit: TS-2480: Fix to work in the case where
there are no ticket key files but tickets have not been disabled. Also fix
RHEL 5 compile error.
Posted by Susan Hinrichs <sh...@network-geographics.com>.
On 2/5/2015 10:17 PM, James Peach wrote:
>> On Feb 5, 2015, at 7:32 PM, shinrich@apache.org wrote:
>>
>> Repository: trafficserver
>> Updated Branches:
>> refs/heads/master 1d617582b -> 5fe69772a
>>
>>
>> TS-2480: Fix to work in the case where there are no ticket key files but tickets have not been disabled.
> Doesn't OpenSSL do this implicitly? Or is the problem that we end up setting the callback without having a keyblock?
Once you have the call back you must provide a key. Since we bootstrap
on the default ctx, we either have the callback or we don't.
>
>> Also fix RHEL 5 compile error.
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/5fe69772
>> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/5fe69772
>> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/5fe69772
>>
>> Branch: refs/heads/master
>> Commit: 5fe69772aa7e5e841349f3426a997930b44c0ff5
>> Parents: 1d61758
>> Author: shinrich <sh...@yahoo-inc.com>
>> Authored: Thu Feb 5 19:24:08 2015 -0600
>> Committer: shinrich <sh...@yahoo-inc.com>
>> Committed: Thu Feb 5 21:32:26 2015 -0600
>>
>> ----------------------------------------------------------------------
>> iocore/net/SSLUtils.cc | 45 ++++++++++++++++++++++++++-------------------
>> 1 file changed, 26 insertions(+), 19 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/5fe69772/iocore/net/SSLUtils.cc
>> ----------------------------------------------------------------------
>> diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc
>> index 055d396..f0265c6 100644
>> --- a/iocore/net/SSLUtils.cc
>> +++ b/iocore/net/SSLUtils.cc
>> @@ -543,28 +543,34 @@ ssl_context_enable_tickets(SSL_CTX * ctx, const char * ticket_key_path)
>> Error("failed to read SSL session ticket key from %s", (const char *)ticket_key_path);
>> goto fail;
>> }
>> + } else {
>> + // Generate a random ticket key
>> + ticket_key_len = 48;
>> + ticket_key_data = (char *)ats_malloc(ticket_key_len);
>> + char *tmp_ptr = ticket_key_data;
>> + RAND_bytes(reinterpret_cast<unsigned char *>(tmp_ptr), ticket_key_len);
>> + }
>>
>> - num_ticket_keys = ticket_key_len / sizeof(ssl_ticket_key_t);
>> - if (num_ticket_keys == 0) {
>> - Error("SSL session ticket key from %s is too short (>= 48 bytes are required)", (const char *)ticket_key_path);
>> - goto fail;
>> - }
>> + num_ticket_keys = ticket_key_len / sizeof(ssl_ticket_key_t);
>> + if (num_ticket_keys == 0) {
>> + Error("SSL session ticket key from %s is too short (>= 48 bytes are required)", (const char *)ticket_key_path);
>> + goto fail;
>> + }
>>
>> - // Increase the stats.
>> - if (ssl_rsb != NULL) { // ssl_rsb is not initialized during the first run.
>> - SSL_INCREMENT_DYN_STAT(ssl_total_ticket_keys_renewed_stat);
>> - }
>> + // Increase the stats.
>> + if (ssl_rsb != NULL) { // ssl_rsb is not initialized during the first run.
>> + SSL_INCREMENT_DYN_STAT(ssl_total_ticket_keys_renewed_stat);
>> + }
>>
>> - keyblock = ticket_block_alloc(num_ticket_keys);
>> + keyblock = ticket_block_alloc(num_ticket_keys);
>>
>> - // Slurp all the keys in the ticket key file. We will encrypt with the first key, and decrypt
>> - // with any key (for rotation purposes).
>> - for (unsigned i = 0; i < num_ticket_keys; ++i) {
>> - const char * data = (const char *)ticket_key_data + (i * sizeof(ssl_ticket_key_t));
>> - memcpy(keyblock->keys[i].key_name, data, sizeof(ssl_ticket_key_t::key_name));
>> - memcpy(keyblock->keys[i].hmac_secret, data + sizeof(ssl_ticket_key_t::key_name), sizeof(ssl_ticket_key_t::hmac_secret));
>> - memcpy(keyblock->keys[i].aes_key, data + sizeof(ssl_ticket_key_t::key_name) + sizeof(ssl_ticket_key_t::hmac_secret), sizeof(ssl_ticket_key_t::aes_key));
>> - }
>> + // Slurp all the keys in the ticket key file. We will encrypt with the first key, and decrypt
>> + // with any key (for rotation purposes).
>> + for (unsigned i = 0; i < num_ticket_keys; ++i) {
>> + const char * data = (const char *)ticket_key_data + (i * sizeof(ssl_ticket_key_t));
>> + memcpy(keyblock->keys[i].key_name, data, sizeof(ssl_ticket_key_t::key_name));
>> + memcpy(keyblock->keys[i].hmac_secret, data + sizeof(ssl_ticket_key_t::key_name), sizeof(ssl_ticket_key_t::hmac_secret));
>> + memcpy(keyblock->keys[i].aes_key, data + sizeof(ssl_ticket_key_t::key_name) + sizeof(ssl_ticket_key_t::hmac_secret), sizeof(ssl_ticket_key_t::aes_key));
>> }
>>
>> // Setting the callback can only fail if OpenSSL does not recognize the
>> @@ -1771,10 +1777,11 @@ ssl_store_ssl_context(
>> if (SSLConfigParams::init_ssl_ctx_cb) {
>> SSLConfigParams::init_ssl_ctx_cb(ctx, true);
>> }
>> +#if HAVE_OPENSSL_SESSION_TICKETS
>> if (!inserted && keyblock != NULL) {
>> ticket_block_free(keyblock);
>> }
>> -
>> +#endif
>> return ctx;
>> }
>>
>>