From 2b3fa6cd8a9c2bb61e15ffa011a43d8273e8dba6 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Sat, 13 May 2017 23:54:06 +0200 Subject: [PATCH 01/23] chg: Verify SCTs before attaching them to a configured certificate --- ngx_http_ssl_ct_module.c | 8 ++ ngx_ssl_ct_module.c | 258 +++++++++++++++++++++++++++++---------- ngx_ssl_ct_module.h | 1 + 3 files changed, 203 insertions(+), 64 deletions(-) diff --git a/ngx_http_ssl_ct_module.c b/ngx_http_ssl_ct_module.c index 1f37254..9ebbaeb 100644 --- a/ngx_http_ssl_ct_module.c +++ b/ngx_http_ssl_ct_module.c @@ -51,6 +51,14 @@ static ngx_command_t ngx_http_ssl_ct_commands[] = { offsetof(ngx_ssl_ct_srv_conf_t, sct_dirs), NULL }, + { + ngx_string("ssl_ct_log"), + NGX_HTTP_MAIN_CONF | NGX_HTTP_SRV_CONF | NGX_CONF_TAKE1, + &ngx_conf_set_str_slot, + NGX_HTTP_SRV_CONF_OFFSET, + offsetof(ngx_ssl_ct_srv_conf_t, ctlog), + NULL + }, ngx_null_command }; diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index 69fce81..5477b91 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -16,6 +16,10 @@ #include "ngx_ssl_ct_module.h" +#include +#include +#include + static int ngx_ssl_ct_sct_list_index; static void *ngx_ssl_ct_create_conf(ngx_cycle_t *cycle); @@ -112,12 +116,12 @@ char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, X509 *cert = SSL_CTX_get_ex_data(ssl_ctx, ngx_ssl_certificate_index); ngx_uint_t i; - for (i = 0; i < sct_dir_count; i++) { + for (i = 0; i < certificates->nelts; i++) { /* the certificate linked list is stored in reverse order */ ngx_str_t *sct_dir = &sct_dirs[sct_dir_count - i - 1]; /* read the .sct files for this cert */ - ngx_ssl_ct_ext *sct_list = ngx_ssl_ct_read_static_scts(cf, sct_dir); + ngx_ssl_ct_ext *sct_list = ngx_ssl_ct_read_static_scts(cf, conf, cert); if (!sct_list) { /* ngx_ssl_ct_read_static_scts calls ngx_log_error */ return NGX_CONF_ERROR; @@ -233,11 +237,14 @@ int ngx_ssl_ct_ext_cb(SSL *s, unsigned int ext_type, const unsigned char **out, } #endif -static ngx_ssl_ct_ext *ngx_ssl_ct_read_static_sct(ngx_conf_t *cf, +static ngx_str_t *ngx_ssl_ct_read_static_sct(ngx_conf_t *cf, ngx_str_t *dir, u_char *file, size_t file_len, - ngx_ssl_ct_ext *sct_list) { + ngx_str_t **sct_out) { + int ok = 0; + ngx_str_t *sct = NULL; + /* join dir and file name */ size_t path_len = dir->len + file_len + 2; u_char *path = ngx_pcalloc(cf->pool, path_len); @@ -272,25 +279,31 @@ static ngx_ssl_ct_ext *ngx_ssl_ct_read_static_sct(ngx_conf_t *cf, goto out; } - const size_t len_pos = sct_list->len; - size_t sct_pos = len_pos + 2; - - /* reserve two bytes for the length and sct_len bytes for the SCT. */ - const size_t sct_and_len_size = sct_len + 2; + if (sct_len > NGX_SSL_CT_EXT_MAX_LEN) { + ngx_log_error(NGX_LOG_EMERG, cf->log, 0, + "sct structure exceeds maximum length"); + goto out; + } - if (sct_and_len_size < sct_len || - sct_list->len + sct_and_len_size < sct_list->len || - sct_list->len + sct_and_len_size > NGX_SSL_CT_EXT_MAX_LEN) { + sct = ngx_pcalloc(cf->pool, sizeof(ngx_str_t)); + if(!sct) { + ngx_log_error(NGX_LOG_EMERG, cf->log, 0, + "Failed to allocate memory for SCT"); + goto out; + } + sct->len = sct_len; + sct->data = ngx_pcalloc(cf->pool, sct->len); + if(!sct->data) { ngx_log_error(NGX_LOG_EMERG, cf->log, 0, - "sct_list structure exceeds maximum length"); + "Failed to allocate memory for SCT buffer"); goto out; } - sct_list->len += sct_and_len_size; /* read the SCT from disk */ size_t to_read = sct_len; + size_t sct_pos = 0; while (to_read > 0) { - ssize_t n = ngx_read_fd(fd, sct_list->buf + sct_pos, to_read); + ssize_t n = ngx_read_fd(fd, sct->data + sct_pos, to_read); if (n == NGX_FILE_ERROR) { ngx_log_error(NGX_LOG_EMERG, cf->log, ngx_errno, ngx_read_fd_n " \"%s\" failed", path); @@ -301,9 +314,7 @@ static ngx_ssl_ct_ext *ngx_ssl_ct_read_static_sct(ngx_conf_t *cf, sct_pos += n; } - /* fill in the length bytes and return */ - sct_list->buf[len_pos] = sct_len >> 8; - sct_list->buf[len_pos + 1] = sct_len; + // We are done here ok = 1; out: @@ -314,83 +325,201 @@ static ngx_ssl_ct_ext *ngx_ssl_ct_read_static_sct(ngx_conf_t *cf, ngx_pfree(cf->pool, path); if (!ok) { + ngx_pfree(cf->pool, sct); + sct = NULL; + return NULL; } - return sct_list; -} -ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_str_t *path) { - /* resolve relative paths */ - if (ngx_conf_full_name(cf->cycle, path, 1) != NGX_OK) { - ngx_log_error(NGX_LOG_EMERG, cf->log, ngx_errno, - "ngx_conf_full_name \"%V\" failed"); - return NULL; + if(sct_out) { + *sct_out = sct; } + return sct; +} +ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_t *ctconf, X509 *cert) +{ /* allocate sct_list structure */ ngx_ssl_ct_ext *sct_list = ngx_pcalloc(cf->pool, sizeof(*sct_list)); if (!sct_list) { return NULL; } + sct_list->buf = ngx_pcalloc(cf->pool, NGX_SSL_CT_EXT_MAX_LEN); + sct_list->len = 0; + if(!sct_list->buf) + { + return NULL; + } - /* reserve the first two bytes for the length */ - sct_list->len += 2; + CT_POLICY_EVAL_CTX* cpectx = CT_POLICY_EVAL_CTX_new(); + if(!cpectx) { + ngx_pfree(cf->pool, sct_list); + return NULL; + } - /* open directory */ - ngx_dir_t dir; - if (ngx_open_dir(path, &dir) != NGX_OK) { - ngx_log_error(NGX_LOG_EMERG, cf->log, ngx_errno, - ngx_open_dir_n " \"%V\" failed", path); + if(!CT_POLICY_EVAL_CTX_set1_cert(cpectx, cert)) { + CT_POLICY_EVAL_CTX_free(cpectx); ngx_pfree(cf->pool, sct_list); return NULL; } - /* iterate through all files */ - for (;;) { - ngx_set_errno(NGX_ENOMOREFILES); + int ctlog_load; + CTLOG_STORE* ctlogs = CTLOG_STORE_new(); + if(ctconf->ctlog) { + ctlog_load = CTLOG_STORE_load_file(ctlogs, ctconf->ctlog->data); + } else { + ctlog_load = CTLOG_STORE_load_default_file(ctlogs); + } + if(!ctlog_load) { + CT_POLICY_EVAL_CTX_free(cpectx); + CTLOG_STORE_free(ctlogs); + ngx_pfree(cf->pool, sct_list); + return NULL; + } - if (ngx_read_dir(&dir) != NGX_OK) { - ngx_err_t err = ngx_errno; + CT_POLICY_EVAL_CTX_set_shared_CTLOG_STORE(cpectx, ctlogs); - if (err == NGX_ENOMOREFILES) { - break; - } else { - ngx_log_error(NGX_LOG_EMERG, cf->log, err, - ngx_read_dir_n " \"%V\" failed", path); + /* reserve the first two bytes for the length */ + sct_list->len += 2; + + for(int i = 0; i < ctconf->sct_dirs->nelts; i++) { + /* the certificate linked list is stored in reverse order */ + ngx_str_t *path = &ctconf->sct_dirs[ctconf->sct_dirs->nelts - i - 1]; + + /* resolve relative paths */ + if (ngx_conf_full_name(cf->cycle, path, 1) != NGX_OK) { + ngx_log_error(NGX_LOG_EMERG, cf->log, ngx_errno, + "ngx_conf_full_name \"%V\" failed"); + CT_POLICY_EVAL_CTX_free(cpectx); + CTLOG_STORE_free(ctlogs); + ngx_pfree(cf->pool, sct_list); + return NULL; + } + + /* open directory */ + ngx_dir_t dir; + if (ngx_open_dir(path, &dir) != NGX_OK) { + ngx_log_error(NGX_LOG_EMERG, cf->log, ngx_errno, + ngx_open_dir_n " \"%V\" failed", path); + CT_POLICY_EVAL_CTX_free(cpectx); + CTLOG_STORE_free(ctlogs); + ngx_pfree(cf->pool, sct_list); + return NULL; + } + + /* iterate through all files */ + for (;;) { + ngx_set_errno(NGX_ENOMOREFILES); + + if (ngx_read_dir(&dir) != NGX_OK) { + ngx_err_t err = ngx_errno; + + if (err == NGX_ENOMOREFILES) { + break; + } else { + ngx_log_error(NGX_LOG_EMERG, cf->log, err, + ngx_read_dir_n " \"%V\" failed", path); + + CT_POLICY_EVAL_CTX_free(cpectx); + CTLOG_STORE_free(ctlogs); + ngx_pfree(cf->pool, sct_list); + return NULL; + } + } + + /* skip dotfiles */ + size_t file_len = ngx_de_namelen(&dir); + u_char *file = ngx_de_name(&dir); + if (file[0] == '.') { + continue; + } + + /* skip files unless the extension is .sct */ + u_char *file_ext = (u_char *) ngx_strrchr(file, '.'); + if (!file_ext || ngx_strcmp(file_ext, ".sct")) { + continue; + } + + /* add the .sct file to the sct_list */ + ngx_str_t *sct_buf = NULL; + if (!ngx_ssl_ct_read_static_sct(cf, path, file, file_len, &sct_buf)) { + /* ngx_ssl_ct_read_static_sct calls ngx_log_error */ + if(sct_buf) { + ngx_pfree(cf->pool, sct_buf); + } + + CT_POLICY_EVAL_CTX_free(cpectx); + CTLOG_STORE_free(ctlogs); ngx_pfree(cf->pool, sct_list); + return NULL; } - } - /* skip dotfiles */ - size_t file_len = ngx_de_namelen(&dir); - u_char *file = ngx_de_name(&dir); - if (file[0] == '.') { - continue; - } +#if OPENSSL_VERSION_NUMBER > 0x01010100 + + SCT* ossl_sct_buf = o2i_SCT(NULL, sct_buf->data, sct_buf->len); + + if(!ossl_sct_buf) { + ngx_pfree(cf->pool, sct_buf); + goto skip_this; + } + + int sct_status = SCT_validate(ossl_sct_buf, cpectx); + + SCT_free(ossl_sct_buf); - /* skip files unless the extension is .sct */ - u_char *file_ext = (u_char *) ngx_strrchr(file, '.'); - if (!file_ext || ngx_strcmp(file_ext, ".sct")) { - continue; + if(1 != sct_status) { + goto skip_this; + } + +#endif + + //We will use this SCT + { + //Check for enough space left in the extension buffer + if(NGX_SSL_CT_EXT_MAX_LEN - sct_list->len -2 < sct_buf->len) { + ngx_log_error(NGX_LOG_EMERG, cf->log, 0, + "SCT structure too large"); + + CT_POLICY_EVAL_CTX_free(cpectx); + CTLOG_STORE_free(ctlogs); + ngx_pfree(cf->pool, sct_list); + + return NULL; + } + + u_char* sct_write = sct_list.buf + sct_list.len; + sct_write[0] = sct_buf->len >> 8; + sct_write[1] = sct_buf->len; + + sct_write += 2; + + ngx_memcpy(sct_write, sct_buf->data, sct_buf->len); + + sct_list->len += sct_buf->len; + } + +skip_this: + ngx_pfree(cf->pool, sct_buf); } - /* add the .sct file to the sct_list */ - if (!ngx_ssl_ct_read_static_sct(cf, path, file, file_len, sct_list)) { - /* ngx_ssl_ct_read_static_sct calls ngx_log_error */ + /* close directory */ + if (ngx_close_dir(&dir) != NGX_OK) { + ngx_log_error(NGX_LOG_EMERG, cf->log, ngx_errno, + ngx_close_dir_n " \"%V\" failed", path); + + CT_POLICY_EVAL_CTX_free(cpectx); + CTLOG_STORE_free(ctlogs); ngx_pfree(cf->pool, sct_list); + return NULL; } - } - /* close directory */ - if (ngx_close_dir(&dir) != NGX_OK) { - ngx_log_error(NGX_LOG_EMERG, cf->log, ngx_errno, - ngx_close_dir_n " \"%V\" failed", path); - ngx_pfree(cf->pool, sct_list); - return NULL; } + CT_POLICY_EVAL_CTX_free(cpectx); + CTLOG_STORE_free(ctlogs); + /* fill in the length bytes and return */ size_t sct_list_len = sct_list->len - 2; if (sct_list_len > 0) { @@ -399,5 +528,6 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_str_t *path) { } else { sct_list->len = 0; } + return sct_list; } diff --git a/ngx_ssl_ct_module.h b/ngx_ssl_ct_module.h index cbd73ec..e31978e 100644 --- a/ngx_ssl_ct_module.h +++ b/ngx_ssl_ct_module.h @@ -27,6 +27,7 @@ typedef struct { ngx_flag_t enable; ngx_array_t *sct_dirs; + ngx_str_t *ctlog; } ngx_ssl_ct_srv_conf_t; typedef struct { From 843706d6b4335bb1a0226b9c851f0bea187a9663 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Tue, 11 Jul 2017 02:02:58 +0200 Subject: [PATCH 02/23] fix: Several compilation issues --- ngx_ssl_ct_module.c | 21 +++++++++++---------- ngx_ssl_ct_module.h | 2 +- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index 5477b91..ca88f75 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -19,13 +19,14 @@ #include #include #include +#include static int ngx_ssl_ct_sct_list_index; static void *ngx_ssl_ct_create_conf(ngx_cycle_t *cycle); -static ngx_ssl_ct_ext *ngx_ssl_ct_read_static_sct(ngx_conf_t *cf, +static ngx_str_t *ngx_ssl_ct_read_static_sct(ngx_conf_t *cf, ngx_str_t *dir, u_char *file, size_t file_len, - ngx_ssl_ct_ext *sct_list); + ngx_str_t **sct_out); static ngx_core_module_t ngx_ssl_ct_module_ctx = { ngx_string("ssl_ct"), @@ -112,13 +113,13 @@ char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, } /* loop through all the certs/SCT dirs */ - ngx_str_t *sct_dirs = conf->sct_dirs->elts; + //ngx_str_t *sct_dirs = conf->sct_dirs->elts; X509 *cert = SSL_CTX_get_ex_data(ssl_ctx, ngx_ssl_certificate_index); ngx_uint_t i; for (i = 0; i < certificates->nelts; i++) { /* the certificate linked list is stored in reverse order */ - ngx_str_t *sct_dir = &sct_dirs[sct_dir_count - i - 1]; + //ngx_str_t *sct_dir = &sct_dirs[sct_dir_count - i - 1]; /* read the .sct files for this cert */ ngx_ssl_ct_ext *sct_list = ngx_ssl_ct_read_static_scts(cf, conf, cert); @@ -344,7 +345,7 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ if (!sct_list) { return NULL; } - sct_list->buf = ngx_pcalloc(cf->pool, NGX_SSL_CT_EXT_MAX_LEN); + //sct_list->buf = ngx_pcalloc(cf->pool, NGX_SSL_CT_EXT_MAX_LEN); sct_list->len = 0; if(!sct_list->buf) { @@ -366,7 +367,7 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ int ctlog_load; CTLOG_STORE* ctlogs = CTLOG_STORE_new(); if(ctconf->ctlog) { - ctlog_load = CTLOG_STORE_load_file(ctlogs, ctconf->ctlog->data); + ctlog_load = CTLOG_STORE_load_file(ctlogs, (const char *)ctconf->ctlog->data); } else { ctlog_load = CTLOG_STORE_load_default_file(ctlogs); } @@ -382,9 +383,9 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ /* reserve the first two bytes for the length */ sct_list->len += 2; - for(int i = 0; i < ctconf->sct_dirs->nelts; i++) { + for(size_t i = 0; i < ctconf->sct_dirs->nelts; i++) { /* the certificate linked list is stored in reverse order */ - ngx_str_t *path = &ctconf->sct_dirs[ctconf->sct_dirs->nelts - i - 1]; + ngx_str_t *path = (ngx_str_t *)&ctconf->sct_dirs[ctconf->sct_dirs->nelts - i - 1]; /* resolve relative paths */ if (ngx_conf_full_name(cf->cycle, path, 1) != NGX_OK) { @@ -457,7 +458,7 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ #if OPENSSL_VERSION_NUMBER > 0x01010100 - SCT* ossl_sct_buf = o2i_SCT(NULL, sct_buf->data, sct_buf->len); + SCT* ossl_sct_buf = o2i_SCT(NULL, (const u_char **)&sct_buf->data, sct_buf->len); if(!ossl_sct_buf) { ngx_pfree(cf->pool, sct_buf); @@ -488,7 +489,7 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ return NULL; } - u_char* sct_write = sct_list.buf + sct_list.len; + u_char* sct_write = sct_list->buf + sct_list->len; sct_write[0] = sct_buf->len >> 8; sct_write[1] = sct_buf->len; diff --git a/ngx_ssl_ct_module.h b/ngx_ssl_ct_module.h index e31978e..559339f 100644 --- a/ngx_ssl_ct_module.h +++ b/ngx_ssl_ct_module.h @@ -46,7 +46,7 @@ int ngx_ssl_ct_ext_cb(SSL *s, unsigned int ext_type, const unsigned char **out, size_t *outlen, int *al, void *add_arg); # endif #endif -ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_str_t *path); +ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_t *ctconf, X509 *cert); void *ngx_ssl_ct_create_srv_conf(ngx_conf_t *cf); char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, SSL_CTX *ssl_ctx, ngx_array_t *certificates); From ffe9120b2fd0967bf233c8c73a32949288d82ea6 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Tue, 11 Jul 2017 10:05:56 +0200 Subject: [PATCH 03/23] fix: CT Log Information Config File directive implementation was broken --- ngx_mail_ssl_ct_module.c | 8 ++++++++ ngx_ssl_ct_module.c | 1 + 2 files changed, 9 insertions(+) diff --git a/ngx_mail_ssl_ct_module.c b/ngx_mail_ssl_ct_module.c index 3ac427d..34baff2 100644 --- a/ngx_mail_ssl_ct_module.c +++ b/ngx_mail_ssl_ct_module.c @@ -47,6 +47,14 @@ static ngx_command_t ngx_mail_ssl_ct_commands[] = { offsetof(ngx_ssl_ct_srv_conf_t, sct_dirs), NULL }, + { + ngx_string("ssl_ct_log"), + NGX_HTTP_MAIN_CONF | NGX_HTTP_SRV_CONF | NGX_CONF_TAKE1, + &ngx_conf_set_str_slot, + NGX_HTTP_SRV_CONF_OFFSET, + offsetof(ngx_ssl_ct_srv_conf_t, ctlog), + NULL + }, ngx_null_command }; diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index ca88f75..b58426c 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -70,6 +70,7 @@ void *ngx_ssl_ct_create_srv_conf(ngx_conf_t *cf) { } conf->enable = NGX_CONF_UNSET; + conf->ctlog = NGX_CONF_UNSET_PTR; conf->sct_dirs = NGX_CONF_UNSET_PTR; return conf; From ee728b6c7a2fec14b573982465baf3db2b185b84 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Tue, 11 Jul 2017 21:16:02 +0200 Subject: [PATCH 04/23] fix: Incomplete previous fix --- ngx_ssl_ct_module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index b58426c..0604537 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -367,7 +367,7 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ int ctlog_load; CTLOG_STORE* ctlogs = CTLOG_STORE_new(); - if(ctconf->ctlog) { + if(ctconf->ctlog != NGX_CONF_UNSET_PTR) { ctlog_load = CTLOG_STORE_load_file(ctlogs, (const char *)ctconf->ctlog->data); } else { ctlog_load = CTLOG_STORE_load_default_file(ctlogs); From 301d3469bc4e5dd8d59acc8a91e7bec5c21b5f0e Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Tue, 11 Jul 2017 21:30:59 +0200 Subject: [PATCH 05/23] fix: Even more fixes --- ngx_mail_ssl_ct_module.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ngx_mail_ssl_ct_module.c b/ngx_mail_ssl_ct_module.c index 34baff2..e96def3 100644 --- a/ngx_mail_ssl_ct_module.c +++ b/ngx_mail_ssl_ct_module.c @@ -49,9 +49,9 @@ static ngx_command_t ngx_mail_ssl_ct_commands[] = { }, { ngx_string("ssl_ct_log"), - NGX_HTTP_MAIN_CONF | NGX_HTTP_SRV_CONF | NGX_CONF_TAKE1, + NGX_MAIL_MAIN_CONF | NGX_MAIL_SRV_CONF | NGX_CONF_TAKE1, &ngx_conf_set_str_slot, - NGX_HTTP_SRV_CONF_OFFSET, + NGX_MAIL_SRV_CONF_OFFSET, offsetof(ngx_ssl_ct_srv_conf_t, ctlog), NULL }, From d1660569f445ed69bec1639dec177d50e68b3d19 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Tue, 11 Jul 2017 22:16:17 +0200 Subject: [PATCH 06/23] fix: Properly make the ssl_ct_log directive optional --- ngx_ssl_ct_module.c | 7 ++++--- ngx_ssl_ct_module.h | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index 0604537..ce64120 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -70,7 +70,8 @@ void *ngx_ssl_ct_create_srv_conf(ngx_conf_t *cf) { } conf->enable = NGX_CONF_UNSET; - conf->ctlog = NGX_CONF_UNSET_PTR; + conf->ctlog.data = NULL; + conf->ctlog.len = 0; conf->sct_dirs = NGX_CONF_UNSET_PTR; return conf; @@ -367,8 +368,8 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ int ctlog_load; CTLOG_STORE* ctlogs = CTLOG_STORE_new(); - if(ctconf->ctlog != NGX_CONF_UNSET_PTR) { - ctlog_load = CTLOG_STORE_load_file(ctlogs, (const char *)ctconf->ctlog->data); + if(ctconf->ctlog.data) { + ctlog_load = CTLOG_STORE_load_file(ctlogs, (const char *)ctconf->ctlog.data); } else { ctlog_load = CTLOG_STORE_load_default_file(ctlogs); } diff --git a/ngx_ssl_ct_module.h b/ngx_ssl_ct_module.h index 559339f..a80b904 100644 --- a/ngx_ssl_ct_module.h +++ b/ngx_ssl_ct_module.h @@ -27,7 +27,7 @@ typedef struct { ngx_flag_t enable; ngx_array_t *sct_dirs; - ngx_str_t *ctlog; + ngx_str_t ctlog; } ngx_ssl_ct_srv_conf_t; typedef struct { From ef16b31e2f2161466b6b955bbf7c5033c062fa9a Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Tue, 11 Jul 2017 23:25:26 +0200 Subject: [PATCH 07/23] Properly merge CT-Log config from parent context --- ngx_ssl_ct_module.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index ce64120..2341ab0 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -86,6 +86,11 @@ char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, ngx_conf_merge_value(conf->enable, prev->enable, 0); ngx_conf_merge_ptr_value(conf->sct_dirs, prev->sct_dirs, NULL); + ngx_conf_merge_str_value(conf->ctlog, prev->ctlog, ""); + if(!conf->ctlog.len) { + conf->ctlog.data = NULL; + } + /* validate config */ if (conf->enable) { if (!conf->sct_dirs) { From b730131b7f58cebf1289ef9e12c3af8a3fce86dd Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Tue, 11 Jul 2017 23:28:20 +0200 Subject: [PATCH 08/23] chg: Be less strict about number of SCT directories and where to enable --- ngx_ssl_ct_module.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index 2341ab0..c27229b 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -105,18 +105,9 @@ char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, /* check if SSL is enabled */ if (!ssl_ctx) { - ngx_log_error(NGX_LOG_EMERG, cf->log, 0, - "\"ssl_ct\" can only be enabled if ssl is enabled"); - return NGX_CONF_ERROR; - } - - /* check we have one SCT dir for each certificate */ - ngx_uint_t sct_dir_count = conf->sct_dirs->nelts; - if (sct_dir_count != certificates->nelts) { - ngx_log_error(NGX_LOG_EMERG, cf->log, 0, - "there must be exactly one \"ssl_ct_static_scts\" directive for " - "each \"ssl_certificate\" directive"); - return NGX_CONF_ERROR; + ngx_log_error(NGX_LOG_WARN, cf->log, 0, + "\"ssl_ct\" is only processed if ssl is enabled"); + return NGX_CONF_OK; } /* loop through all the certs/SCT dirs */ From 0f918eb23cba6e543e5654889e4a15d49158dde2 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Tue, 11 Jul 2017 23:29:59 +0200 Subject: [PATCH 09/23] fix: Properly access array elements --- ngx_ssl_ct_module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index c27229b..9dae256 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -383,7 +383,7 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ for(size_t i = 0; i < ctconf->sct_dirs->nelts; i++) { /* the certificate linked list is stored in reverse order */ - ngx_str_t *path = (ngx_str_t *)&ctconf->sct_dirs[ctconf->sct_dirs->nelts - i - 1]; + ngx_str_t *path = ((ngx_str_t **)ctconf->sct_dirs->elts)[ctconf->sct_dirs->nelts - i - 1]; /* resolve relative paths */ if (ngx_conf_full_name(cf->cycle, path, 1) != NGX_OK) { From f35e9846e60416e2b5cc4533a09ff56707d345a6 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Wed, 12 Jul 2017 00:02:18 +0200 Subject: [PATCH 10/23] fix: Proper underlaying array type --- ngx_ssl_ct_module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index 9dae256..99e06c7 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -383,7 +383,7 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ for(size_t i = 0; i < ctconf->sct_dirs->nelts; i++) { /* the certificate linked list is stored in reverse order */ - ngx_str_t *path = ((ngx_str_t **)ctconf->sct_dirs->elts)[ctconf->sct_dirs->nelts - i - 1]; + ngx_str_t *path = &((ngx_str_t *)ctconf->sct_dirs->elts)[ctconf->sct_dirs->nelts - i - 1]; /* resolve relative paths */ if (ngx_conf_full_name(cf->cycle, path, 1) != NGX_OK) { From 16d7ac48daf52110cf000fc6cfd6460510a2448e Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Thu, 13 Jul 2017 10:09:20 +0200 Subject: [PATCH 11/23] add: Lots of logging for errors --- ngx_ssl_ct_module.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index 99e06c7..5a68947 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -248,6 +248,8 @@ static ngx_str_t *ngx_ssl_ct_read_static_sct(ngx_conf_t *cf, size_t path_len = dir->len + file_len + 2; u_char *path = ngx_pcalloc(cf->pool, path_len); if (path == NULL) { + ngx_log_error(NGX_LOG_WARN, cf->log, 0, + "Could not join filename"); return NULL; } @@ -274,13 +276,15 @@ static ngx_str_t *ngx_ssl_ct_read_static_sct(ngx_conf_t *cf, const size_t sct_len = ngx_file_size(&stat); if (sct_len == 0) { + ngx_log_error(NGX_LOG_WARN, cf->log, 0, + "Found empty SCT in file %s", path); ok = 1; goto out; } if (sct_len > NGX_SSL_CT_EXT_MAX_LEN) { ngx_log_error(NGX_LOG_EMERG, cf->log, 0, - "sct structure exceeds maximum length"); + "SCT structure exceeds maximum length"); goto out; } @@ -321,6 +325,11 @@ static ngx_str_t *ngx_ssl_ct_read_static_sct(ngx_conf_t *cf, ngx_log_error(NGX_LOG_EMERG, cf->log, ngx_errno, ngx_close_file_n " \"%s\" failed", path); } + + if (!ok) { + ngx_log_error(NGX_LOG_WARN, cf->log, 0, + "SCT in %s was not valid", path); + } ngx_pfree(cf->pool, path); if (!ok) { @@ -341,6 +350,8 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ /* allocate sct_list structure */ ngx_ssl_ct_ext *sct_list = ngx_pcalloc(cf->pool, sizeof(*sct_list)); if (!sct_list) { + ngx_log_error(NGX_LOG_WARN, cf->log, 0, + "Could not allocate SCT list structure"); return NULL; } //sct_list->buf = ngx_pcalloc(cf->pool, NGX_SSL_CT_EXT_MAX_LEN); @@ -352,11 +363,15 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ CT_POLICY_EVAL_CTX* cpectx = CT_POLICY_EVAL_CTX_new(); if(!cpectx) { + ngx_log_error(NGX_LOG_WARN, cf->log, 0, + "Could not allocate SCT Policy Evaluation Context"); ngx_pfree(cf->pool, sct_list); return NULL; } if(!CT_POLICY_EVAL_CTX_set1_cert(cpectx, cert)) { + ngx_log_error(NGX_LOG_WARN, cf->log, 0, + "Could not set certificate for SCT Policy Context"); CT_POLICY_EVAL_CTX_free(cpectx); ngx_pfree(cf->pool, sct_list); return NULL; @@ -366,10 +381,17 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ CTLOG_STORE* ctlogs = CTLOG_STORE_new(); if(ctconf->ctlog.data) { ctlog_load = CTLOG_STORE_load_file(ctlogs, (const char *)ctconf->ctlog.data); + ngx_log_error(NGX_LOG_WARN, cf->log, 0, + "CTLOG_STORE loaded from file: %s", ctconf->ctlog.data); } else { ctlog_load = CTLOG_STORE_load_default_file(ctlogs); + ngx_log_error(NGX_LOG_WARN, cf->log, 0, + "CTLOG_STORE loaded from system default location"); } if(!ctlog_load) { + ngx_log_error(NGX_LOG_WARN, cf->log, 0, + "Failed to load CT Log information file"); + CT_POLICY_EVAL_CTX_free(cpectx); CTLOG_STORE_free(ctlogs); ngx_pfree(cf->pool, sct_list); @@ -475,6 +497,9 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ //We will use this SCT { + ngx_log_error(NGX_LOG_INFO, cf->log, 0, + "Found some SCT for inclusion into the handshake"); + //Check for enough space left in the extension buffer if(NGX_SSL_CT_EXT_MAX_LEN - sct_list->len -2 < sct_buf->len) { ngx_log_error(NGX_LOG_EMERG, cf->log, 0, @@ -525,6 +550,8 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ sct_list->buf[0] = sct_list_len >> 8; sct_list->buf[1] = sct_list_len; } else { + ngx_log_error(NGX_LOG_WARN, cf->log, 0, + "No suiteable SCT found in configured directories"); sct_list->len = 0; } From 3eeb00637de4cd881e8d607b4d01afd42024fb8e Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Thu, 13 Jul 2017 10:10:58 +0200 Subject: [PATCH 12/23] del: Remove invalid check causing early abort --- ngx_ssl_ct_module.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index 5a68947..408bb35 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -354,12 +354,7 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ "Could not allocate SCT list structure"); return NULL; } - //sct_list->buf = ngx_pcalloc(cf->pool, NGX_SSL_CT_EXT_MAX_LEN); sct_list->len = 0; - if(!sct_list->buf) - { - return NULL; - } CT_POLICY_EVAL_CTX* cpectx = CT_POLICY_EVAL_CTX_new(); if(!cpectx) { From 6ffbbac2bf1998d2c5c20d35a5d4c433bd6ef6f1 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Sat, 15 Jul 2017 19:42:36 +0200 Subject: [PATCH 13/23] add: Set explicit timestamp for verification --- ngx_ssl_ct_module.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index 408bb35..efe7a86 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -16,6 +16,8 @@ #include "ngx_ssl_ct_module.h" +#include + #include #include #include @@ -394,6 +396,7 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ } CT_POLICY_EVAL_CTX_set_shared_CTLOG_STORE(cpectx, ctlogs); + CT_POLICY_EVAL_CTX_set_time(cpectx, (time(NULL) + 300ULL) * 1000ULL); /* reserve the first two bytes for the length */ sct_list->len += 2; From 07f455cd6f7da082442b948c9ae20446c878dc40 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Sat, 15 Jul 2017 19:57:41 +0200 Subject: [PATCH 14/23] add: Make the certificate subject available for logging --- ngx_ssl_ct_module.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index efe7a86..efe95a2 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -349,6 +349,10 @@ static ngx_str_t *ngx_ssl_ct_read_static_sct(ngx_conf_t *cf, ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_t *ctconf, X509 *cert) { + char *subj = X509_NAME_oneline(X509_get_subject_name(cert), NULL, 0); + ngx_log_error(NGX_LOG_WARN, cf->log, 0, + "Looking up certificates for: %s", subj); + /* allocate sct_list structure */ ngx_ssl_ct_ext *sct_list = ngx_pcalloc(cf->pool, sizeof(*sct_list)); if (!sct_list) { @@ -553,5 +557,7 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ sct_list->len = 0; } + OPENSSL_free(subj); + return sct_list; } From acb33367dc9a7891d06c0b3fba3e3bee409306fe Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Sat, 15 Jul 2017 20:05:28 +0200 Subject: [PATCH 15/23] fix: Fill out any fields OpenSSL might expect us to fill --- ngx_ssl_ct_module.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index efe95a2..792aea1 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -480,18 +480,32 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ #if OPENSSL_VERSION_NUMBER > 0x01010100 - SCT* ossl_sct_buf = o2i_SCT(NULL, (const u_char **)&sct_buf->data, sct_buf->len); + const u_char* sct_buf_ptr = (const u_char *)sct_buf->data; + SCT* ossl_sct_buf = o2i_SCT(NULL, &sct_buf_ptr, sct_buf->len); if(!ossl_sct_buf) { + ngx_log_error(NGX_LOG_WARN, cf->log, 0, + "Could not parse SCT file %s", file); + ngx_pfree(cf->pool, sct_buf); goto skip_this; } - int sct_status = SCT_validate(ossl_sct_buf, cpectx); + SCT_set_log_entry_type(ossl_sct_buf, CT_LOG_ENTRY_TYPE_X509); - SCT_free(ossl_sct_buf); + if(-1 == SCT_validate(ossl_sct_buf, cpectx)) { + ngx_log_error(NGX_LOG_WARN, cf->log, 0, + "Validation of SCT file %s returned an internal error", file); + + goto skip_this; + } + + int sct_status = SCT_get_validation_status(ossl_sct_buf); + + if(SCT_VALIDATION_STATUS_VALID != sct_status) { + ngx_log_error(NGX_LOG_INFO, cf->log, 0, + "SCT validation on certificate %s returned %d for file %s", subj, sct_status, file); - if(1 != sct_status) { goto skip_this; } @@ -526,6 +540,8 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ } skip_this: + SCT_free(ossl_sct_buf); + ngx_pfree(cf->pool, sct_buf); } From b36d54ad47b49ce882037df4b46c927150d2a4d3 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Sat, 15 Jul 2017 20:07:02 +0200 Subject: [PATCH 16/23] fmt: Be somewhat more explicit when performing checks --- ngx_ssl_ct_module.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index 792aea1..7ae9a48 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -389,7 +389,7 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ ngx_log_error(NGX_LOG_WARN, cf->log, 0, "CTLOG_STORE loaded from system default location"); } - if(!ctlog_load) { + if(1 != ctlog_load) { ngx_log_error(NGX_LOG_WARN, cf->log, 0, "Failed to load CT Log information file"); @@ -517,7 +517,7 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ "Found some SCT for inclusion into the handshake"); //Check for enough space left in the extension buffer - if(NGX_SSL_CT_EXT_MAX_LEN - sct_list->len -2 < sct_buf->len) { + if(NGX_SSL_CT_EXT_MAX_LEN - sct_list->len - 2 < sct_buf->len) { ngx_log_error(NGX_LOG_EMERG, cf->log, 0, "SCT structure too large"); From 72416177a0b421a15a11040ce61368a631f6bd4e Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Sat, 15 Jul 2017 20:32:48 +0200 Subject: [PATCH 17/23] fix: Increase storage pointer by size of SCT plus size of length field --- ngx_ssl_ct_module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index 7ae9a48..c38a6de 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -536,7 +536,7 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ ngx_memcpy(sct_write, sct_buf->data, sct_buf->len); - sct_list->len += sct_buf->len; + sct_list->len += sct_buf->len + 2; } skip_this: From 4a637c55b43610e2583e817444d6aab5a8b99225 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Sat, 15 Jul 2017 21:21:31 +0200 Subject: [PATCH 18/23] chg: Lower the debug level for certain messages --- ngx_ssl_ct_module.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index c38a6de..5d86df5 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -382,11 +382,11 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ CTLOG_STORE* ctlogs = CTLOG_STORE_new(); if(ctconf->ctlog.data) { ctlog_load = CTLOG_STORE_load_file(ctlogs, (const char *)ctconf->ctlog.data); - ngx_log_error(NGX_LOG_WARN, cf->log, 0, + ngx_log_error(NGX_LOG_INFO, cf->log, 0, "CTLOG_STORE loaded from file: %s", ctconf->ctlog.data); } else { ctlog_load = CTLOG_STORE_load_default_file(ctlogs); - ngx_log_error(NGX_LOG_WARN, cf->log, 0, + ngx_log_error(NGX_LOG_INFO, cf->log, 0, "CTLOG_STORE loaded from system default location"); } if(1 != ctlog_load) { @@ -503,9 +503,16 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ int sct_status = SCT_get_validation_status(ossl_sct_buf); if(SCT_VALIDATION_STATUS_VALID != sct_status) { - ngx_log_error(NGX_LOG_INFO, cf->log, 0, + ngx_log_error(NGX_LOG_DEBUG, cf->log, 0, "SCT validation on certificate %s returned %d for file %s", subj, sct_status, file); + unsigned long ossl_error; + while((ossl_error = ERR_get_error())) { + char* ossl_errstr = ERR_error_string(ossl_error, NULL); + ngx_log_error(NGX_LOG_DEBUG, cf->log, 0, + "OpenSSL: %s", ossl_errstr); + } + goto skip_this; } From 3eda0e581b2647e07d1e64e18f565c5cd22aac93 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Sun, 16 Jul 2017 01:25:49 +0200 Subject: [PATCH 19/23] chg: More refined logging --- ngx_ssl_ct_module.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index 5d86df5..ed9c3e3 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -130,12 +130,21 @@ char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, if (sct_list->len == 0) { ngx_pfree(cf->pool, sct_list); + ngx_log_error(NGX_LOG_DEBUG, cf->log, 0, + "No SCTs to attach for this certificate"); goto next; } #ifndef OPENSSL_IS_BORINGSSL /* associate the sct_list with the cert */ X509_set_ex_data(cert, ngx_ssl_ct_sct_list_index, sct_list); + + unsigned long ossl_error; + while((ossl_error = ERR_get_error())) { + char* ossl_errstr = ERR_error_string(ossl_error, NULL); + ngx_log_error(NGX_LOG_DEBUG, cf->log, 0, + "OpenSSL: %s", ossl_errstr); + } #else if (SSL_CTX_set_signed_cert_timestamp_list(ssl_ctx, sct_list->buf, sct_list->len) == 0) { @@ -350,7 +359,7 @@ static ngx_str_t *ngx_ssl_ct_read_static_sct(ngx_conf_t *cf, ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_t *ctconf, X509 *cert) { char *subj = X509_NAME_oneline(X509_get_subject_name(cert), NULL, 0); - ngx_log_error(NGX_LOG_WARN, cf->log, 0, + ngx_log_error(NGX_LOG_INFO, cf->log, 0, "Looking up certificates for: %s", subj); /* allocate sct_list structure */ @@ -576,7 +585,7 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ sct_list->buf[1] = sct_list_len; } else { ngx_log_error(NGX_LOG_WARN, cf->log, 0, - "No suiteable SCT found in configured directories"); + "No suiteable SCT found in configured directories for %s", subj); sct_list->len = 0; } From 64669918e59128080d8e5d66c16aea47dedb61c2 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Sun, 16 Jul 2017 01:31:01 +0200 Subject: [PATCH 20/23] chg: Refine some conditions --- ngx_ssl_ct_module.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index ed9c3e3..60bb71b 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -179,14 +179,14 @@ char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, | SSL_EXT_TLS1_2_SERVER_HELLO | SSL_EXT_TLS1_3_CERTIFICATE; if (SSL_CTX_add_custom_ext(ssl_ctx, NGX_SSL_CT_EXT, context, - &ngx_ssl_ct_ext_cb, NULL, NULL, NULL, NULL) == 0) { + &ngx_ssl_ct_ext_cb, NULL, NULL, NULL, NULL) != 1) { ngx_log_error(NGX_LOG_EMERG, cf->log, 0, "SSL_CTX_add_custom_ext failed"); return NGX_CONF_ERROR; } # else if (SSL_CTX_add_server_custom_ext(ssl_ctx, NGX_SSL_CT_EXT, - &ngx_ssl_ct_ext_cb, NULL, NULL, NULL, NULL) == 0) { + &ngx_ssl_ct_ext_cb, NULL, NULL, NULL, NULL) != 1) { ngx_log_error(NGX_LOG_EMERG, cf->log, 0, "SSL_CTX_add_server_custom_ext failed"); return NGX_CONF_ERROR; @@ -237,7 +237,7 @@ int ngx_ssl_ct_ext_cb(SSL *s, unsigned int ext_type, const unsigned char **out, /* get sct_list for the cert OpenSSL chose to use for this connection */ ngx_ssl_ct_ext *sct_list = X509_get_ex_data(x, ngx_ssl_ct_sct_list_index); - if (sct_list) { + if (sct_list && sct_list->len) { *out = sct_list->buf; *outlen = sct_list->len; return 1; @@ -411,9 +411,6 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ CT_POLICY_EVAL_CTX_set_shared_CTLOG_STORE(cpectx, ctlogs); CT_POLICY_EVAL_CTX_set_time(cpectx, (time(NULL) + 300ULL) * 1000ULL); - /* reserve the first two bytes for the length */ - sct_list->len += 2; - for(size_t i = 0; i < ctconf->sct_dirs->nelts; i++) { /* the certificate linked list is stored in reverse order */ ngx_str_t *path = &((ngx_str_t *)ctconf->sct_dirs->elts)[ctconf->sct_dirs->nelts - i - 1]; @@ -579,11 +576,8 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ CTLOG_STORE_free(ctlogs); /* fill in the length bytes and return */ - size_t sct_list_len = sct_list->len - 2; - if (sct_list_len > 0) { - sct_list->buf[0] = sct_list_len >> 8; - sct_list->buf[1] = sct_list_len; - } else { + size_t sct_list_len = sct_list->len; + if (sct_list_len < 1) { ngx_log_error(NGX_LOG_WARN, cf->log, 0, "No suiteable SCT found in configured directories for %s", subj); sct_list->len = 0; From 2917e159cd2e7516f1102e7d09c88afff550be33 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Sat, 7 Dec 2024 16:30:24 +0100 Subject: [PATCH 21/23] chg: Make ngx_ssl_ct_ext_cb private-use only --- ngx_ssl_ct_module.c | 7 ++++++- ngx_ssl_ct_module.h | 11 +---------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index 60bb71b..7ddfca5 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -26,10 +26,15 @@ static int ngx_ssl_ct_sct_list_index; static void *ngx_ssl_ct_create_conf(ngx_cycle_t *cycle); + static ngx_str_t *ngx_ssl_ct_read_static_sct(ngx_conf_t *cf, ngx_str_t *dir, u_char *file, size_t file_len, ngx_str_t **sct_out); +static int ngx_ssl_ct_ext_cb(SSL *s, unsigned int ext_type, unsigned int context, + const unsigned char **out, size_t *outlen, X509 *x, size_t chainidx, + int *al, void *add_arg); + static ngx_core_module_t ngx_ssl_ct_module_ctx = { ngx_string("ssl_ct"), @@ -199,7 +204,7 @@ char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, #ifndef OPENSSL_IS_BORINGSSL # if OPENSSL_VERSION_NUMBER >= 0x10101000L -int ngx_ssl_ct_ext_cb(SSL *s, unsigned int ext_type, unsigned int context, +static int ngx_ssl_ct_ext_cb(SSL *s, unsigned int ext_type, unsigned int context, const unsigned char **out, size_t *outlen, X509 *x, size_t chainidx, int *al, void *add_arg) { /* only include SCTs in the end-entity certificate */ diff --git a/ngx_ssl_ct_module.h b/ngx_ssl_ct_module.h index a80b904..6df2612 100644 --- a/ngx_ssl_ct_module.h +++ b/ngx_ssl_ct_module.h @@ -36,16 +36,7 @@ typedef struct { } ngx_ssl_ct_ext; ngx_int_t ngx_ssl_ct_init(ngx_log_t *log); -#ifndef OPENSSL_IS_BORINGSSL -# if OPENSSL_VERSION_NUMBER >= 0x10101000L -int ngx_ssl_ct_ext_cb(SSL *s, unsigned int ext_type, unsigned int context, - const unsigned char **out, size_t *outlen, X509 *x, size_t chainidx, - int *al, void *add_arg); -# else -int ngx_ssl_ct_ext_cb(SSL *s, unsigned int ext_type, const unsigned char **out, - size_t *outlen, int *al, void *add_arg); -# endif -#endif + ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_t *ctconf, X509 *cert); void *ngx_ssl_ct_create_srv_conf(ngx_conf_t *cf); char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, From badf911982b076a340de10aab1a7715b1ab33630 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Sat, 7 Dec 2024 16:32:04 +0100 Subject: [PATCH 22/23] del: Drop anchient OpenSSL version support --- ngx_ssl_ct_module.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index 7ddfca5..572ef80 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -179,7 +179,6 @@ char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, #ifndef OPENSSL_IS_BORINGSSL /* add OpenSSL TLS extension */ -# if OPENSSL_VERSION_NUMBER >= 0x10101000L int context = SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO | SSL_EXT_TLS1_3_CERTIFICATE; @@ -189,21 +188,12 @@ char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, "SSL_CTX_add_custom_ext failed"); return NGX_CONF_ERROR; } -# else - if (SSL_CTX_add_server_custom_ext(ssl_ctx, NGX_SSL_CT_EXT, - &ngx_ssl_ct_ext_cb, NULL, NULL, NULL, NULL) != 1) { - ngx_log_error(NGX_LOG_EMERG, cf->log, 0, - "SSL_CTX_add_server_custom_ext failed"); - return NGX_CONF_ERROR; - } -# endif #endif return NGX_CONF_OK; } #ifndef OPENSSL_IS_BORINGSSL -# if OPENSSL_VERSION_NUMBER >= 0x10101000L static int ngx_ssl_ct_ext_cb(SSL *s, unsigned int ext_type, unsigned int context, const unsigned char **out, size_t *outlen, X509 *x, size_t chainidx, int *al, void *add_arg) { @@ -211,11 +201,6 @@ static int ngx_ssl_ct_ext_cb(SSL *s, unsigned int ext_type, unsigned int context if (context == SSL_EXT_TLS1_3_CERTIFICATE && chainidx != 0) { return 0; } -# else -int ngx_ssl_ct_ext_cb(SSL *s, unsigned int ext_type, const unsigned char **out, - size_t *outlen, int *al, void *add_arg) { - X509 *x = NULL; -# endif if (!x) { /* get the cert OpenSSL chose to use for this connection */ @@ -489,8 +474,6 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ return NULL; } -#if OPENSSL_VERSION_NUMBER > 0x01010100 - const u_char* sct_buf_ptr = (const u_char *)sct_buf->data; SCT* ossl_sct_buf = o2i_SCT(NULL, &sct_buf_ptr, sct_buf->len); @@ -527,8 +510,6 @@ ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_ goto skip_this; } -#endif - //We will use this SCT { ngx_log_error(NGX_LOG_INFO, cf->log, 0, From 71bf4d20c7a952181b4ae4d318b74c11f0406307 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Sat, 7 Dec 2024 16:41:38 +0100 Subject: [PATCH 23/23] chg: Update implementation for nginx >= 1.27.2 --- ngx_http_ssl_ct_module.c | 14 +------------ ngx_mail_ssl_ct_module.c | 14 +------------ ngx_ssl_ct_module.c | 42 ++++++++++++++------------------------ ngx_ssl_ct_module.h | 2 +- ngx_stream_ssl_ct_module.c | 16 ++------------- 5 files changed, 20 insertions(+), 68 deletions(-) diff --git a/ngx_http_ssl_ct_module.c b/ngx_http_ssl_ct_module.c index 9ebbaeb..136b3dd 100644 --- a/ngx_http_ssl_ct_module.c +++ b/ngx_http_ssl_ct_module.c @@ -82,17 +82,5 @@ static char *ngx_http_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, ngx_http_ssl_srv_conf_t *ssl_conf = ngx_http_conf_get_module_srv_conf(cf, ngx_http_ssl_module); - ngx_array_t *certificates; - -#if nginx_version >= 1011000 - certificates = ssl_conf->certificates; -#else - certificates = ngx_array_create(cf->pool, 1, sizeof(ngx_str_t)); - - ngx_str_t *certificate = ngx_array_push(certificates); - *certificate = ssl_conf->certificate; -#endif - - return ngx_ssl_ct_merge_srv_conf(cf, parent, child, ssl_conf->ssl.ctx, - certificates); + return ngx_ssl_ct_merge_srv_conf(cf, parent, child, &ssl_conf->ssl); } diff --git a/ngx_mail_ssl_ct_module.c b/ngx_mail_ssl_ct_module.c index e96def3..172bf63 100644 --- a/ngx_mail_ssl_ct_module.c +++ b/ngx_mail_ssl_ct_module.c @@ -78,17 +78,5 @@ static char *ngx_mail_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, ngx_mail_ssl_conf_t *ssl_conf = ngx_mail_conf_get_module_srv_conf(cf, ngx_mail_ssl_module); - ngx_array_t *certificates; - -#if nginx_version >= 1011000 - certificates = ssl_conf->certificates; -#else - certificates = ngx_array_create(cf->pool, 1, sizeof(ngx_str_t)); - - ngx_str_t *certificate = ngx_array_push(certificates); - *certificate = ssl_conf->certificate; -#endif - - return ngx_ssl_ct_merge_srv_conf(cf, parent, child, ssl_conf->ssl.ctx, - certificates); + return ngx_ssl_ct_merge_srv_conf(cf, parent, child, &ssl_conf->ssl); } diff --git a/ngx_ssl_ct_module.c b/ngx_ssl_ct_module.c index 572ef80..b2032b9 100644 --- a/ngx_ssl_ct_module.c +++ b/ngx_ssl_ct_module.c @@ -85,7 +85,7 @@ void *ngx_ssl_ct_create_srv_conf(ngx_conf_t *cf) { } char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, - SSL_CTX *ssl_ctx, ngx_array_t *certificates) { + ngx_ssl_t *ssl) { /* merge config */ ngx_ssl_ct_srv_conf_t *prev = parent; ngx_ssl_ct_srv_conf_t *conf = child; @@ -101,7 +101,7 @@ char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, /* validate config */ if (conf->enable) { if (!conf->sct_dirs) { - ngx_log_error(NGX_LOG_EMERG, cf->log, 0, + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, "no \"ssl_ct_static_scts\" is defined for the \"ssl_ct\"" "directive"); return NGX_CONF_ERROR; @@ -111,20 +111,15 @@ char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, } /* check if SSL is enabled */ - if (!ssl_ctx) { - ngx_log_error(NGX_LOG_WARN, cf->log, 0, - "\"ssl_ct\" is only processed if ssl is enabled"); + if (!ssl->ctx) { + ngx_ssl_error(NGX_LOG_WARN, ssl->log, 0, + "\"ssl_ct\" is only processed if SSL is enabled"); return NGX_CONF_OK; } - /* loop through all the certs/SCT dirs */ - //ngx_str_t *sct_dirs = conf->sct_dirs->elts; - X509 *cert = SSL_CTX_get_ex_data(ssl_ctx, ngx_ssl_certificate_index); - - ngx_uint_t i; - for (i = 0; i < certificates->nelts; i++) { - /* the certificate linked list is stored in reverse order */ - //ngx_str_t *sct_dir = &sct_dirs[sct_dir_count - i - 1]; + /* loop through all the certs */ + for (ngx_uint_t i = 0; i < ssl->certs.nelts; i++) { + X509 *cert = ((X509 **) ssl->certs.elts)[i]; /* read the .sct files for this cert */ ngx_ssl_ct_ext *sct_list = ngx_ssl_ct_read_static_scts(cf, conf, cert); @@ -135,9 +130,9 @@ char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, if (sct_list->len == 0) { ngx_pfree(cf->pool, sct_list); - ngx_log_error(NGX_LOG_DEBUG, cf->log, 0, + ngx_ssl_error(NGX_LOG_DEBUG, ssl->log, 0, "No SCTs to attach for this certificate"); - goto next; + continue; } #ifndef OPENSSL_IS_BORINGSSL @@ -147,20 +142,20 @@ char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, unsigned long ossl_error; while((ossl_error = ERR_get_error())) { char* ossl_errstr = ERR_error_string(ossl_error, NULL); - ngx_log_error(NGX_LOG_DEBUG, cf->log, 0, + ngx_ssl_error(NGX_LOG_DEBUG, ssl->log, 0, "OpenSSL: %s", ossl_errstr); } #else if (SSL_CTX_set_signed_cert_timestamp_list(ssl_ctx, sct_list->buf, sct_list->len) == 0) { - ngx_log_error(NGX_LOG_EMERG, cf->log, 0, + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, "SSL_CTX_set_signed_cert_timestamp_list failed"); ngx_pfree(cf->pool, sct_list); return NGX_CONF_ERROR; } if (conf->sct_dirs->nelts > 1) { - ngx_log_error(NGX_LOG_WARN, cf->log, 0, + ngx_ssl_error(NGX_LOG_WARN, ssl->log, 0, "BoringSSL does not support using SCTs with multiple " "certificates, the last non-empty \"ssl_ct_static_scts\" " "directory will be used for all certificates"); @@ -168,13 +163,6 @@ char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, break; #endif - -next: -#if nginx_version >= 1011000 - cert = X509_get_ex_data(cert, ngx_ssl_next_certificate_index); -#else - break; -#endif } #ifndef OPENSSL_IS_BORINGSSL @@ -182,9 +170,9 @@ char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, int context = SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO | SSL_EXT_TLS1_3_CERTIFICATE; - if (SSL_CTX_add_custom_ext(ssl_ctx, NGX_SSL_CT_EXT, context, + if (SSL_CTX_add_custom_ext(ssl->ctx, NGX_SSL_CT_EXT, context, &ngx_ssl_ct_ext_cb, NULL, NULL, NULL, NULL) != 1) { - ngx_log_error(NGX_LOG_EMERG, cf->log, 0, + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, "SSL_CTX_add_custom_ext failed"); return NGX_CONF_ERROR; } diff --git a/ngx_ssl_ct_module.h b/ngx_ssl_ct_module.h index 6df2612..36f1f24 100644 --- a/ngx_ssl_ct_module.h +++ b/ngx_ssl_ct_module.h @@ -40,6 +40,6 @@ ngx_int_t ngx_ssl_ct_init(ngx_log_t *log); ngx_ssl_ct_ext *ngx_ssl_ct_read_static_scts(ngx_conf_t *cf, ngx_ssl_ct_srv_conf_t *ctconf, X509 *cert); void *ngx_ssl_ct_create_srv_conf(ngx_conf_t *cf); char *ngx_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child, - SSL_CTX *ssl_ctx, ngx_array_t *certificates); + ngx_ssl_t *ssl); #endif diff --git a/ngx_stream_ssl_ct_module.c b/ngx_stream_ssl_ct_module.c index 720a58b..97ba921 100644 --- a/ngx_stream_ssl_ct_module.c +++ b/ngx_stream_ssl_ct_module.c @@ -70,20 +70,8 @@ ngx_module_t ngx_stream_ssl_ct_module = { static char *ngx_stream_ssl_ct_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child) { - ngx_stream_ssl_conf_t *ssl_conf = ngx_stream_conf_get_module_srv_conf(cf, + ngx_stream_ssl_srv_conf_t *ssl_conf = ngx_stream_conf_get_module_srv_conf(cf, ngx_stream_ssl_module); - ngx_array_t *certificates; - -#if nginx_version >= 1011000 - certificates = ssl_conf->certificates; -#else - certificates = ngx_array_create(cf->pool, 1, sizeof(ngx_str_t)); - - ngx_str_t *certificate = ngx_array_push(certificates); - *certificate = ssl_conf->certificate; -#endif - - return ngx_ssl_ct_merge_srv_conf(cf, parent, child, ssl_conf->ssl.ctx, - certificates); + return ngx_ssl_ct_merge_srv_conf(cf, parent, child, &ssl_conf->ssl); }