Hey Maxim,

> Wouldn't it better to move ngx_ssl_session_ticket_md defines
> to ngx_ssl_session_ticket_key_callback() implementation?

You mean inside the function or just above it? I moved them just above it.

> Do we really need these #ifdef's? I don't think saving several
> bytes in ancient OpenSSL versions worth it (and the
> ngx_ssl_stapling_index doesn't have #ifdef's).

Probably not, removed.

While there, I moved the definitions and initialization right after
ngx_ssl_session_cache_index, so that the two session resumption
indexes are together, hopefully that's alright.

> Wouldn't ngx_ssl_ticket_key_t be better? Up to you, but
> "ngx_ssl_session_ticket_key_t" looks a bit too long.

It's a bit too long, yes, but I prefer to keep everything with the
same prefix, so either rename everything to "ngx_ssl_ticket_" (which I
don't like that much) or keep long names.

> What about something like this:
>
> ngx_log_debug3(NGX_LOG_DEBUG_HTTP, c->log, 0,
> "ssl session ticket encrypt, key: \"%*s\" (%s session)",
> ngx_hex_dump(buf, key[0].name, 16) - buf, buf,
> SSL_session_reused(ssl_conn) ? "reused" : "new");
>
> ?
>
> (Well, actually I'm not sure we need key name logged, but it
> probably doesn't matter much.)

That's much better, thanks.

It's used only for the debugging, so I think it's fine to print it.

> Just in case, using a shorter name "ngx_ssl_ticket_md()" would
> save us from wrapping here and in decrypt case.

I know, I had something shorter here originally, but I've decided to
rename it, so that it would have the same prefix... But if you really
want to get rid of the wrap, maybe we could use "ngx_ssl_md()" here?
What do you think?

> Shouldn't it be ngx_memcmp(), as it checks binary data now?

Good catch, thanks! That was leftover from my previous implementation.

> Adding "goto found" instead of "break" would allow to avoid this
> additional check. Up to you though.

Personally, I prefer the explicit "end-of-the-loop" check, but I don't
mind either way. Changed to "goto found".

Updated patch attached, with slightly changed commit message (better
file names in the example).

Please note, that it doesn't have any session timeout logic, so it
should be committed after the timeout patch :)

Best regards,
Piotr Sikora


# HG changeset patch
# User Piotr Sikora <piotr@cloudflare.com>
# Date 1381532724 25200
# Fri Oct 11 16:05:24 2013 -0700
# Node ID 296805806a43f4b222c4cf34dd6489b37394e315
# Parent 5483d9e77b3287b00b1104a07688bda37bc7351e
SSL: added ability to set keys used for Session Tickets (RFC5077).

In order to support key rollover, ssl_session_ticket_key can be defined
multiple times. The first key will be used to issue and resume Session
Tickets, while the rest will be used only to resume them.

ssl_session_ticket_key session_tickets/current.key;
ssl_session_ticket_key session_tickets/prev-1h.key;
ssl_session_ticket_key session_tickets/prev-2h.key;

Please note that nginx supports Session Tickets even without explicit
configuration of the keys and this feature should be only used in setups
where SSL traffic is distributed across multiple nginx servers.

Signed-off-by: Piotr Sikora <piotr@cloudflare.com>

diff -r 5483d9e77b32 -r 296805806a43 src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c Wed Oct 02 15:07:17 2013 +0400
+++ b/src/event/ngx_event_openssl.c Fri Oct 11 16:05:24 2013 -0700
@@ -38,6 +38,12 @@ static void ngx_ssl_expire_sessions(ngx_
static void ngx_ssl_session_rbtree_insert_value(ngx_rbtree_node_t *temp,
ngx_rbtree_node_t *node, ngx_rbtree_node_t *sentinel);

+#ifdef SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB
+static int ngx_ssl_session_ticket_key_callback(ngx_ssl_conn_t *ssl_conn,
+ unsigned char *name, unsigned char *iv, EVP_CIPHER_CTX *ectx,
+ HMAC_CTX *hctx, int enc);
+#endif
+
static void *ngx_openssl_create_conf(ngx_cycle_t *cycle);
static char *ngx_openssl_engine(ngx_conf_t *cf, ngx_command_t *cmd,
void *conf);
static void ngx_openssl_exit(ngx_cycle_t *cycle);
@@ -82,6 +88,7 @@ ngx_module_t ngx_openssl_module = {
int ngx_ssl_connection_index;
int ngx_ssl_server_conf_index;
int ngx_ssl_session_cache_index;
+int ngx_ssl_session_ticket_keys_index;
int ngx_ssl_certificate_index;
int ngx_ssl_stapling_index;

@@ -139,6 +146,14 @@ ngx_ssl_init(ngx_log_t *log)
return NGX_ERROR;
}

+ ngx_ssl_session_ticket_keys_index = SSL_CTX_get_ex_new_index(0, NULL, NULL,
+ NULL, NULL);
+ if (ngx_ssl_session_ticket_keys_index == -1) {
+ ngx_ssl_error(NGX_LOG_ALERT, log, 0,
+ "SSL_CTX_get_ex_new_index() failed");
+ return NGX_ERROR;
+ }
+
ngx_ssl_certificate_index = SSL_CTX_get_ex_new_index(0, NULL, NULL, NULL,
NULL);
if (ngx_ssl_certificate_index == -1) {
@@ -2240,6 +2255,218 @@ ngx_ssl_session_rbtree_insert_value(ngx_
}


+#ifdef SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB
+
+ngx_int_t
+ngx_ssl_session_ticket_keys(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_array_t *paths)
+{
+ u_char buf[48];
+ ssize_t n;
+ ngx_str_t *path;
+ ngx_file_t file;
+ ngx_uint_t i;
+ ngx_array_t *keys;
+ ngx_file_info_t fi;
+ ngx_ssl_session_ticket_key_t *key;
+
+ if (paths == NULL) {
+ return NGX_OK;
+ }
+
+ keys = ngx_array_create(cf->pool, paths->nelts,
+ sizeof(ngx_ssl_session_ticket_key_t));
+ if (keys == NULL) {
+ return NGX_ERROR;
+ }
+
+ path = paths->elts;
+ for (i = 0; i < paths->nelts; i++) {
+
+ if (ngx_conf_full_name(cf->cycle, &path[i], 1) != NGX_OK) {
+ return NGX_ERROR;
+ }
+
+ ngx_memzero(&file, sizeof(ngx_file_t));
+ file.name = path[i];
+ file.log = cf->log;
+
+ file.fd = ngx_open_file(file.name.data, NGX_FILE_RDONLY, 0, 0);
+ if (file.fd == NGX_INVALID_FILE) {
+ ngx_conf_log_error(NGX_LOG_EMERG, cf, ngx_errno,
+ ngx_open_file_n " \"%V\" failed", &file.name);
+ return NGX_ERROR;
+ }
+
+ if (ngx_fd_info(file.fd, &fi) == NGX_FILE_ERROR) {
+ ngx_conf_log_error(NGX_LOG_CRIT, cf, ngx_errno,
+ ngx_fd_info_n " \"%V\" failed", &file.name);
+ goto failed;
+ }
+
+ if (ngx_file_size(&fi) != 48) {
+ ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+ "\"%V\" must be 48 bytes", &file.name);
+ goto failed;
+ }
+
+ n = ngx_read_file(&file, buf, 48, 0);
+
+ if (n == NGX_ERROR) {
+ ngx_conf_log_error(NGX_LOG_CRIT, cf, ngx_errno,
+ ngx_read_file_n " \"%V\" failed", &file.name);
+ goto failed;
+ }
+
+ if (n != 48) {
+ ngx_conf_log_error(NGX_LOG_CRIT, cf, 0,
+ ngx_read_file_n " \"%V\" returned only "
+ "%z bytes instead of 48", &file.name, n);
+ goto failed;
+ }
+
+ key = ngx_array_push(keys);
+ if (key == NULL) {
+ goto failed;
+ }
+
+ ngx_memcpy(key->name, buf, 16);
+ ngx_memcpy(key->aes_key, buf + 16, 16);
+ ngx_memcpy(key->hmac_key, buf + 32, 16);
+
+ if (ngx_close_file(file.fd) == NGX_FILE_ERROR) {
+ ngx_log_error(NGX_LOG_ALERT, cf->log, ngx_errno,
+ ngx_close_file_n " \"%V\" failed", &file.name);
+ }
+ }
+
+ if (SSL_CTX_set_ex_data(ssl->ctx, ngx_ssl_session_ticket_keys_index, keys)
+ == 0)
+ {
+ ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
+ "SSL_CTX_set_ex_data() failed");
+ return NGX_ERROR;
+ }
+
+ if (SSL_CTX_set_tlsext_ticket_key_cb(ssl->ctx,
+ ngx_ssl_session_ticket_key_callback)
+ == 0)
+ {
+ ngx_log_error(NGX_LOG_WARN, cf->log, 0,
+ "nginx was built with Session Tickets support, however, "
+ "now it is linked dynamically to an OpenSSL library "
+ "which has no tlsext support, therefore Session Tickets "
+ "are not available");
+ }
+
+ return NGX_OK;
+
+failed:
+
+ if (ngx_close_file(file.fd) == NGX_FILE_ERROR) {
+ ngx_log_error(NGX_LOG_ALERT, cf->log, ngx_errno,
+ ngx_close_file_n " \"%V\" failed", &file.name);
+ }
+
+ return NGX_ERROR;
+}
+
+
+#ifdef OPENSSL_NO_SHA256
+#define ngx_ssl_session_ticket_md EVP_sha1
+#else
+#define ngx_ssl_session_ticket_md EVP_sha256
+#endif
+
+
+static int
+ngx_ssl_session_ticket_key_callback(ngx_ssl_conn_t *ssl_conn,
+ unsigned char *name, unsigned char *iv, EVP_CIPHER_CTX *ectx,
+ HMAC_CTX *hctx, int enc)
+{
+ SSL_CTX *ssl_ctx;
+ ngx_uint_t i;
+ ngx_array_t *keys;
+ ngx_ssl_session_ticket_key_t *key;
+#if (NGX_DEBUG)
+ u_char buf[32];
+ ngx_connection_t *c;
+#endif
+
+ ssl_ctx = SSL_get_SSL_CTX(ssl_conn);
+
+ keys = SSL_CTX_get_ex_data(ssl_ctx, ngx_ssl_session_ticket_keys_index);
+ if (keys == NULL) {
+ return -1;
+ }
+
+ key = keys->elts;
+
+#if (NGX_DEBUG)
+ c = ngx_ssl_get_connection(ssl_conn);
+#endif
+
+ if (enc == 1) {
+ /* encrypt session ticket */
+
+ ngx_log_debug3(NGX_LOG_DEBUG_HTTP, c->log, 0,
+ "ssl session ticket encrypt, key: \"%*s\" (%s session)",
+ ngx_hex_dump(buf, key[0].name, 16) - buf, buf,
+ SSL_session_reused(ssl_conn) ? "reused" : "new");
+
+ RAND_pseudo_bytes(iv, 16);
+ EVP_EncryptInit_ex(ectx, EVP_aes_128_cbc(), NULL, key[0].aes_key, iv);
+ HMAC_Init_ex(hctx, key[0].hmac_key, 16, ngx_ssl_session_ticket_md(),
+ NULL);
+ memcpy(name, key[0].name, 16);
+
+ return 0;
+
+ } else {
+ /* decrypt session ticket */
+
+ for (i = 0; i < keys->nelts; i++) {
+ if (ngx_memcmp(name, key[i].name, 16) == 0) {
+ goto found;
+ }
+ }
+
+ ngx_log_debug2(NGX_LOG_DEBUG_HTTP, c->log, 0,
+ "ssl session ticket decrypt, key: \"%*s\" not found",
+ ngx_hex_dump(buf, name, 16) - buf, buf);
+
+ return 0;
+
+found:
+
+ ngx_log_debug3(NGX_LOG_DEBUG_HTTP, c->log, 0,
+ "ssl session ticket decrypt, key: \"%*s\"%s",
+ ngx_hex_dump(buf, key[i].name, 16) - buf, buf,
+ (i == 0) ? " (default)" : "");
+
+ HMAC_Init_ex(hctx, key[i].hmac_key, 16, ngx_ssl_session_ticket_md(),
+ NULL);
+ EVP_DecryptInit_ex(ectx, EVP_aes_128_cbc(), NULL, key[i].aes_key, iv);
+
+ return (i == 0) ? 1 : 2 /* renew */;
+ }
+}
+
+#else
+
+ngx_int_t
+ngx_ssl_session_ticket_keys(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_array_t *paths)
+{
+ if (paths) {
+ ngx_log_error(NGX_LOG_WARN, ssl->log, 0,
+ "\"ssl_session_ticket_keys\" ignored, not supported");
+ }
+
+ return NGX_OK;
+}
+
+#endif
+
+
void
ngx_ssl_cleanup_ctx(void *data)
{
diff -r 5483d9e77b32 -r 296805806a43 src/event/ngx_event_openssl.h
--- a/src/event/ngx_event_openssl.h Wed Oct 02 15:07:17 2013 +0400
+++ b/src/event/ngx_event_openssl.h Fri Oct 11 16:05:24 2013 -0700
@@ -83,6 +83,16 @@ typedef struct {
} ngx_ssl_session_cache_t;


+#ifdef SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB
+
+typedef struct {
+ u_char name[16];
+ u_char aes_key[16];
+ u_char hmac_key[16];
+} ngx_ssl_session_ticket_key_t;
+
+#endif
+

#define NGX_SSL_SSLv2 0x0002
#define NGX_SSL_SSLv3 0x0004
@@ -116,6 +126,8 @@ ngx_int_t ngx_ssl_dhparam(ngx_conf_t *cf
ngx_int_t ngx_ssl_ecdh_curve(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *name);
ngx_int_t ngx_ssl_session_cache(ngx_ssl_t *ssl, ngx_str_t *sess_ctx,
ssize_t builtin_session_cache, ngx_shm_zone_t *shm_zone, time_t timeout);
+ngx_int_t ngx_ssl_session_ticket_keys(ngx_conf_t *cf, ngx_ssl_t *ssl,
+ ngx_array_t *paths);
ngx_int_t ngx_ssl_session_cache_init(ngx_shm_zone_t *shm_zone, void *data);
ngx_int_t ngx_ssl_create_connection(ngx_ssl_t *ssl, ngx_connection_t *c,
ngx_uint_t flags);
@@ -173,6 +185,7 @@ void ngx_ssl_cleanup_ctx(void *data);
extern int ngx_ssl_connection_index;
extern int ngx_ssl_server_conf_index;
extern int ngx_ssl_session_cache_index;
+extern int ngx_ssl_session_ticket_keys_index;
extern int ngx_ssl_certificate_index;
extern int ngx_ssl_stapling_index;

diff -r 5483d9e77b32 -r 296805806a43 src/http/modules/ngx_http_ssl_module.c
--- a/src/http/modules/ngx_http_ssl_module.c Wed Oct 02 15:07:17 2013 +0400
+++ b/src/http/modules/ngx_http_ssl_module.c Fri Oct 11 16:05:24 2013 -0700
@@ -153,6 +153,13 @@ static ngx_command_t ngx_http_ssl_comma
0,
NULL },

+ { ngx_string("ssl_session_ticket_key"),
+ NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1,
+ ngx_conf_set_str_array_slot,
+ NGX_HTTP_SRV_CONF_OFFSET,
+ offsetof(ngx_http_ssl_srv_conf_t, session_ticket_keys),
+ NULL },
+
{ ngx_string("ssl_session_timeout"),
NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1,
ngx_conf_set_sec_slot,
@@ -421,6 +428,7 @@ ngx_http_ssl_create_srv_conf(ngx_conf_t
sscf->verify_depth = NGX_CONF_UNSET_UINT;
sscf->builtin_session_cache = NGX_CONF_UNSET;
sscf->session_timeout = NGX_CONF_UNSET;
+ sscf->session_ticket_keys = NGX_CONF_UNSET_PTR;
sscf->stapling = NGX_CONF_UNSET;
sscf->stapling_verify = NGX_CONF_UNSET;

@@ -623,6 +631,15 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *
return NGX_CONF_ERROR;
}

+ ngx_conf_merge_ptr_value(conf->session_ticket_keys,
+ prev->session_ticket_keys, NULL);
+
+ if (ngx_ssl_session_ticket_keys(cf, &conf->ssl, conf->session_ticket_keys)
+ != NGX_OK)
+ {
+ return NGX_CONF_ERROR;
+ }
+
if (conf->stapling) {

if (ngx_ssl_stapling(cf, &conf->ssl, &conf->stapling_file,
diff -r 5483d9e77b32 -r 296805806a43 src/http/modules/ngx_http_ssl_module.h
--- a/src/http/modules/ngx_http_ssl_module.h Wed Oct 02 15:07:17 2013 +0400
+++ b/src/http/modules/ngx_http_ssl_module.h Fri Oct 11 16:05:24 2013 -0700
@@ -42,6 +42,8 @@ typedef struct {

ngx_shm_zone_t *shm_zone;

+ ngx_array_t *session_ticket_keys;
+
ngx_flag_t stapling;
ngx_flag_t stapling_verify;
ngx_str_t stapling_file;
diff -r 5483d9e77b32 -r 296805806a43 src/mail/ngx_mail_ssl_module.c
--- a/src/mail/ngx_mail_ssl_module.c Wed Oct 02 15:07:17 2013 +0400
+++ b/src/mail/ngx_mail_ssl_module.c Fri Oct 11 16:05:24 2013 -0700
@@ -116,6 +116,13 @@ static ngx_command_t ngx_mail_ssl_comma
0,
NULL },

+ { ngx_string("ssl_session_ticket_key"),
+ NGX_MAIL_MAIN_CONF|NGX_MAIL_SRV_CONF|NGX_CONF_TAKE1,
+ ngx_conf_set_str_array_slot,
+ NGX_MAIL_SRV_CONF_OFFSET,
+ offsetof(ngx_mail_ssl_conf_t, session_ticket_keys),
+ NULL },
+
{ ngx_string("ssl_session_timeout"),
NGX_MAIL_MAIN_CONF|NGX_MAIL_SRV_CONF|NGX_CONF_TAKE1,
ngx_conf_set_sec_slot,
@@ -184,6 +191,7 @@ ngx_mail_ssl_create_conf(ngx_conf_t *cf)
scf->prefer_server_ciphers = NGX_CONF_UNSET;
scf->builtin_session_cache = NGX_CONF_UNSET;
scf->session_timeout = NGX_CONF_UNSET;
+ scf->session_ticket_keys = NGX_CONF_UNSET_PTR;

return scf;
}
@@ -331,6 +339,15 @@ ngx_mail_ssl_merge_conf(ngx_conf_t *cf,
return NGX_CONF_ERROR;
}

+ ngx_conf_merge_ptr_value(conf->session_ticket_keys,
+ prev->session_ticket_keys, NULL);
+
+ if (ngx_ssl_session_ticket_keys(cf, &conf->ssl, conf->session_ticket_keys)
+ != NGX_OK)
+ {
+ return NGX_CONF_ERROR;
+ }
+
return NGX_CONF_OK;
}

diff -r 5483d9e77b32 -r 296805806a43 src/mail/ngx_mail_ssl_module.h
--- a/src/mail/ngx_mail_ssl_module.h Wed Oct 02 15:07:17 2013 +0400
+++ b/src/mail/ngx_mail_ssl_module.h Fri Oct 11 16:05:24 2013 -0700
@@ -41,6 +41,8 @@ typedef struct {

ngx_shm_zone_t *shm_zone;

+ ngx_array_t *session_ticket_keys;
+
u_char *file;
ngx_uint_t line;
} ngx_mail_ssl_conf_t;

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel