From e8167ceea71595a66a871e4091aeb505c415e0f8 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 18 Jul 2015 01:49:20 +0900 Subject: [PATCH] nghttpx: Add AES-256-CBC encryption for TLS session ticket --- gennghttpxfun.py | 1 + src/shrpx-unittest.cc | 2 + src/shrpx.cc | 50 ++++++++++++++++++++---- src/shrpx_config.cc | 83 +++++++++++++++++++++++++++++++++------- src/shrpx_config.h | 24 +++++++++--- src/shrpx_config_test.cc | 54 ++++++++++++++++++++++---- src/shrpx_config_test.h | 1 + src/shrpx_ssl.cc | 16 ++++---- src/util.h | 4 ++ 9 files changed, 193 insertions(+), 42 deletions(-) diff --git a/gennghttpxfun.py b/gennghttpxfun.py index 6412ae2b..9bf1c012 100755 --- a/gennghttpxfun.py +++ b/gennghttpxfun.py @@ -91,6 +91,7 @@ OPTIONS = [ "header-field-buffer", "max-header-fields", "include", + "tls-ticket-cipher", "conf", ] diff --git a/src/shrpx-unittest.cc b/src/shrpx-unittest.cc index 83695204..aea0eb33 100644 --- a/src/shrpx-unittest.cc +++ b/src/shrpx-unittest.cc @@ -122,6 +122,8 @@ int main(int argc, char *argv[]) { shrpx::test_shrpx_config_parse_log_format) || !CU_add_test(pSuite, "config_read_tls_ticket_key_file", shrpx::test_shrpx_config_read_tls_ticket_key_file) || + !CU_add_test(pSuite, "config_read_tls_ticket_key_file_aes_256", + shrpx::test_shrpx_config_read_tls_ticket_key_file_aes_256) || !CU_add_test(pSuite, "config_match_downstream_addr_group", shrpx::test_shrpx_config_match_downstream_addr_group) || !CU_add_test(pSuite, "util_streq", shrpx::test_util_streq) || diff --git a/src/shrpx.cc b/src/shrpx.cc index a5fa929f..d3b1c9fb 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -627,8 +627,21 @@ void renew_ticket_key_cb(struct ev_loop *loop, ev_timer *w, int revents) { ticket_keys->keys.resize(1); } - if (RAND_bytes(reinterpret_cast(&ticket_keys->keys[0]), - sizeof(ticket_keys->keys[0])) == 0) { + auto &new_key = ticket_keys->keys[0]; + new_key.cipher = get_config()->tls_ticket_cipher; + new_key.hmac = EVP_sha256(); + new_key.hmac_keylen = EVP_MD_size(new_key.hmac); + + assert(EVP_CIPHER_key_length(new_key.cipher) <= sizeof(new_key.data.enc_key)); + assert(new_key.hmac_keylen <= sizeof(new_key.data.hmac_key)); + + if (LOG_ENABLED(INFO)) { + LOG(INFO) << "enc_keylen=" << EVP_CIPHER_key_length(new_key.cipher) + << ", hmac_keylen=" << new_key.hmac_keylen; + } + + if (RAND_bytes(reinterpret_cast(&new_key.data), + sizeof(new_key.data)) == 0) { if (LOG_ENABLED(INFO)) { LOG(INFO) << "failed to renew ticket key"; } @@ -638,7 +651,7 @@ void renew_ticket_key_cb(struct ev_loop *loop, ev_timer *w, int revents) { if (LOG_ENABLED(INFO)) { LOG(INFO) << "ticket keys generation done"; for (auto &key : ticket_keys->keys) { - LOG(INFO) << "name: " << util::format_hex(key.name, sizeof(key.name)); + LOG(INFO) << "name: " << util::format_hex(key.data.name); } } @@ -709,8 +722,17 @@ int event_loop() { if (!get_config()->upstream_no_tls) { bool auto_tls_ticket_key = true; if (!get_config()->tls_ticket_key_files.empty()) { - auto ticket_keys = - read_tls_ticket_key_file(get_config()->tls_ticket_key_files); + if (!get_config()->tls_ticket_cipher_given) { + LOG(WARN) << "It is strongly recommended to specify " + "--tls-ticket-cipher=aes-128-cbc (or " + "tls-ticket-cipher=aes-128-cbc in configuration file) " + "when --tls-ticket-key-file is used for the smooth " + "transition when the default value of --tls-ticket-cipher " + "becomes aes-256-cbc"; + } + auto ticket_keys = read_tls_ticket_key_file( + get_config()->tls_ticket_key_files, get_config()->tls_ticket_cipher, + EVP_sha256()); if (!ticket_keys) { LOG(WARN) << "Use internal session ticket key generator"; } else { @@ -967,6 +989,8 @@ void fill_default_config() { mod_config()->header_field_buffer = 64_k; mod_config()->max_header_fields = 100; mod_config()->downstream_addr_group_catch_all = 0; + mod_config()->tls_ticket_cipher = EVP_aes_128_cbc(); + mod_config()->tls_ticket_cipher_given = false; } } // namespace @@ -1278,8 +1302,11 @@ SSL/TLS: are treated as a part of protocol string. Default: )" << DEFAULT_TLS_PROTO_LIST << R"( --tls-ticket-key-file= - Path to file that contains 48 bytes random data to - construct TLS session ticket parameters. This options + Path to file that contains random data to construct TLS + session ticket parameters. If aes-128-cbc is given in + --tls-ticket-cipher, the file must contain exactly 48 + bytes. If aes-256-cbc is given in --tls-ticket-cipher, + the file must contain exactly 80 bytes. This options can be used repeatedly to specify multiple ticket parameters. If several files are given, only the first key is used to encrypt TLS session tickets. Other keys @@ -1294,6 +1321,10 @@ SSL/TLS: while opening or reading a file, key is generated automatically and renewed every 12hrs. At most 2 keys are stored in memory. + --tls-ticket-cipher= + Specify cipher to encrypt TLS session ticket. Specify + either aes-128-cbc or aes-256-cbc. By default, + aes-128-cbc is used. --fetch-ocsp-response-file= Path to fetch-ocsp-response script file. It should be absolute path. @@ -1660,6 +1691,7 @@ int main(int argc, char **argv) { {SHRPX_OPT_MAX_HEADER_FIELDS, required_argument, &flag, 81}, {SHRPX_OPT_ADD_REQUEST_HEADER, required_argument, &flag, 82}, {SHRPX_OPT_INCLUDE, required_argument, &flag, 83}, + {SHRPX_OPT_TLS_TICKET_CIPHER, required_argument, &flag, 84}, {nullptr, 0, nullptr, 0}}; int option_index = 0; @@ -2026,6 +2058,10 @@ int main(int argc, char **argv) { // --include cmdcfgs.emplace_back(SHRPX_OPT_INCLUDE, optarg); break; + case 84: + // --tls-ticket-cipher + cmdcfgs.emplace_back(SHRPX_OPT_TLS_TICKET_CIPHER, optarg); + break; default: break; } diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index 2b20fa93..5486fda8 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -144,36 +144,72 @@ bool is_secure(const char *filename) { } // namespace std::unique_ptr -read_tls_ticket_key_file(const std::vector &files) { +read_tls_ticket_key_file(const std::vector &files, + const EVP_CIPHER *cipher, const EVP_MD *hmac) { auto ticket_keys = make_unique(); auto &keys = ticket_keys->keys; keys.resize(files.size()); + auto enc_keylen = EVP_CIPHER_key_length(cipher); + auto hmac_keylen = EVP_MD_size(hmac); + if (cipher == EVP_aes_128_cbc()) { + // backward compatibility, as a legacy of using same file format + // with nginx and apache. + hmac_keylen = 16; + } + auto expectedlen = sizeof(keys[0].data.name) + enc_keylen + hmac_keylen; + char buf[256]; + assert(sizeof(buf) >= expectedlen); + size_t i = 0; for (auto &file : files) { + struct stat fst {}; + + if (stat(file.c_str(), &fst) == -1) { + auto error = errno; + LOG(ERROR) << "tls-ticket-key-file: could not stat file " << file + << ", errno=" << error; + return nullptr; + } + + if (fst.st_size != expectedlen) { + LOG(ERROR) << "tls-ticket-key-file: the expected file size is " + << expectedlen << ", the actual file size is " << fst.st_size; + return nullptr; + } + std::ifstream f(file.c_str()); if (!f) { LOG(ERROR) << "tls-ticket-key-file: could not open file " << file; return nullptr; } - char buf[48]; - f.read(buf, sizeof(buf)); - if (f.gcount() != sizeof(buf)) { - LOG(ERROR) << "tls-ticket-key-file: want to read 48 bytes but read " - << f.gcount() << " bytes from " << file; + + f.read(buf, expectedlen); + if (f.gcount() != expectedlen) { + LOG(ERROR) << "tls-ticket-key-file: want to read " << expectedlen + << " bytes but only read " << f.gcount() << " bytes from " + << file; return nullptr; } auto &key = keys[i++]; - auto p = buf; - memcpy(key.name, p, sizeof(key.name)); - p += sizeof(key.name); - memcpy(key.aes_key, p, sizeof(key.aes_key)); - p += sizeof(key.aes_key); - memcpy(key.hmac_key, p, sizeof(key.hmac_key)); + key.cipher = cipher; + key.hmac = hmac; + key.hmac_keylen = hmac_keylen; if (LOG_ENABLED(INFO)) { - LOG(INFO) << "session ticket key: " << util::format_hex(key.name, - sizeof(key.name)); + LOG(INFO) << "enc_keylen=" << enc_keylen + << ", hmac_keylen=" << key.hmac_keylen; + } + + auto p = buf; + memcpy(key.data.name, p, sizeof(key.data.name)); + p += sizeof(key.data.name); + memcpy(key.data.enc_key, p, enc_keylen); + p += enc_keylen; + memcpy(key.data.hmac_key, p, hmac_keylen); + + if (LOG_ENABLED(INFO)) { + LOG(INFO) << "session ticket key: " << util::format_hex(key.data.name); } } return ticket_keys; @@ -668,6 +704,7 @@ enum { SHRPX_OPTID_SUBCERT, SHRPX_OPTID_SYSLOG_FACILITY, SHRPX_OPTID_TLS_PROTO_LIST, + SHRPX_OPTID_TLS_TICKET_CIPHER, SHRPX_OPTID_TLS_TICKET_KEY_FILE, SHRPX_OPTID_USER, SHRPX_OPTID_VERIFY_CLIENT, @@ -959,6 +996,11 @@ int option_lookup_token(const char *name, size_t namelen) { return SHRPX_OPTID_WORKER_WRITE_RATE; } break; + case 'r': + if (util::strieq_l("tls-ticket-ciphe", name, 16)) { + return SHRPX_OPTID_TLS_TICKET_CIPHER; + } + break; case 's': if (util::strieq_l("max-header-field", name, 16)) { return SHRPX_OPTID_MAX_HEADER_FIELDS; @@ -1805,6 +1847,19 @@ int parse_config(const char *opt, const char *optarg, return 0; } + case SHRPX_OPTID_TLS_TICKET_CIPHER: + if (util::strieq(optarg, "aes-128-cbc")) { + mod_config()->tls_ticket_cipher = EVP_aes_128_cbc(); + } else if (util::strieq(optarg, "aes-256-cbc")) { + mod_config()->tls_ticket_cipher = EVP_aes_256_cbc(); + } else { + LOG(ERROR) << opt + << ": unsupported cipher for ticket encryption: " << optarg; + return -1; + } + mod_config()->tls_ticket_cipher_given = true; + + return 0; case SHRPX_OPTID_CONF: LOG(WARN) << "conf: ignored"; diff --git a/src/shrpx_config.h b/src/shrpx_config.h index 87af15ba..1fee15f4 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -171,6 +171,7 @@ constexpr char SHRPX_OPT_NO_OCSP[] = "no-ocsp"; constexpr char SHRPX_OPT_HEADER_FIELD_BUFFER[] = "header-field-buffer"; constexpr char SHRPX_OPT_MAX_HEADER_FIELDS[] = "max-header-fields"; constexpr char SHRPX_OPT_INCLUDE[] = "include"; +constexpr char SHRPX_OPT_TLS_TICKET_CIPHER[] = "tls-ticket-cipher"; union sockaddr_union { sockaddr_storage storage; @@ -224,9 +225,17 @@ struct DownstreamAddrGroup { }; struct TicketKey { - uint8_t name[16]; - uint8_t aes_key[16]; - uint8_t hmac_key[16]; + const EVP_CIPHER *cipher; + const EVP_MD *hmac; + size_t hmac_keylen; + struct { + // name of this ticket configuration + uint8_t name[16]; + // encryption key for |cipher| + uint8_t enc_key[32]; + // hmac key for |hmac| + uint8_t hmac_key[32]; + } data; }; struct TicketKeys { @@ -300,6 +309,7 @@ struct Config { nghttp2_session_callbacks *http2_downstream_callbacks; nghttp2_option *http2_option; nghttp2_option *http2_client_option; + const EVP_CIPHER *tls_ticket_cipher; char **argv; char *cwd; size_t num_worker; @@ -376,6 +386,8 @@ struct Config { // true if host contains UNIX domain socket path bool host_unix; bool no_ocsp; + // true if --tls-ticket-cipher is used + bool tls_ticket_cipher_given; }; const Config *get_config(); @@ -447,10 +459,12 @@ int int_syslog_facility(const char *strfacility); FILE *open_file_for_write(const char *filename); // Reads TLS ticket key file in |files| and returns TicketKey which -// stores read key data. This function returns TicketKey if it +// stores read key data. The given |cipher| and |hmac| determine the +// expected file size. This function returns TicketKey if it // succeeds, or nullptr. std::unique_ptr -read_tls_ticket_key_file(const std::vector &files); +read_tls_ticket_key_file(const std::vector &files, + const EVP_CIPHER *cipher, const EVP_MD *hmac); // Selects group based on request's |hostport| and |path|. |hostport| // is the value taken from :authority or host header field, and may diff --git a/src/shrpx_config_test.cc b/src/shrpx_config_test.cc index c5116294..d91a706b 100644 --- a/src/shrpx_config_test.cc +++ b/src/shrpx_config_test.cc @@ -190,24 +190,62 @@ void test_shrpx_config_read_tls_ticket_key_file(void) { close(fd1); close(fd2); - auto ticket_keys = read_tls_ticket_key_file({file1, file2}); + auto ticket_keys = + read_tls_ticket_key_file({file1, file2}, EVP_aes_128_cbc(), EVP_sha256()); unlink(file1); unlink(file2); CU_ASSERT(ticket_keys.get() != nullptr); CU_ASSERT(2 == ticket_keys->keys.size()); auto key = &ticket_keys->keys[0]; - CU_ASSERT(0 == memcmp("0..............1", key->name, sizeof(key->name))); CU_ASSERT(0 == - memcmp("2..............3", key->aes_key, sizeof(key->aes_key))); - CU_ASSERT(0 == - memcmp("4..............5", key->hmac_key, sizeof(key->hmac_key))); + memcmp("0..............1", key->data.name, sizeof(key->data.name))); + CU_ASSERT(0 == memcmp("2..............3", key->data.enc_key, 16)); + CU_ASSERT(0 == memcmp("4..............5", key->data.hmac_key, 16)); key = &ticket_keys->keys[1]; - CU_ASSERT(0 == memcmp("6..............7", key->name, sizeof(key->name))); CU_ASSERT(0 == - memcmp("8..............9", key->aes_key, sizeof(key->aes_key))); + memcmp("6..............7", key->data.name, sizeof(key->data.name))); + CU_ASSERT(0 == memcmp("8..............9", key->data.enc_key, 16)); + CU_ASSERT(0 == memcmp("a..............b", key->data.hmac_key, 16)); +} + +void test_shrpx_config_read_tls_ticket_key_file_aes_256(void) { + char file1[] = "/tmp/nghttpx-unittest.XXXXXX"; + auto fd1 = mkstemp(file1); + assert(fd1 != -1); + assert(80 == write(fd1, "0..............12..............................34..." + "...........................5", + 80)); + char file2[] = "/tmp/nghttpx-unittest.XXXXXX"; + auto fd2 = mkstemp(file2); + assert(fd2 != -1); + assert(80 == write(fd2, "6..............78..............................9a..." + "...........................b", + 80)); + + close(fd1); + close(fd2); + auto ticket_keys = + read_tls_ticket_key_file({file1, file2}, EVP_aes_256_cbc(), EVP_sha256()); + unlink(file1); + unlink(file2); + CU_ASSERT(ticket_keys.get() != nullptr); + CU_ASSERT(2 == ticket_keys->keys.size()); + auto key = &ticket_keys->keys[0]; CU_ASSERT(0 == - memcmp("a..............b", key->hmac_key, sizeof(key->hmac_key))); + memcmp("0..............1", key->data.name, sizeof(key->data.name))); + CU_ASSERT(0 == + memcmp("2..............................3", key->data.enc_key, 32)); + CU_ASSERT(0 == + memcmp("4..............................5", key->data.hmac_key, 32)); + + key = &ticket_keys->keys[1]; + CU_ASSERT(0 == + memcmp("6..............7", key->data.name, sizeof(key->data.name))); + CU_ASSERT(0 == + memcmp("8..............................9", key->data.enc_key, 32)); + CU_ASSERT(0 == + memcmp("a..............................b", key->data.hmac_key, 32)); } void test_shrpx_config_match_downstream_addr_group(void) { diff --git a/src/shrpx_config_test.h b/src/shrpx_config_test.h index 5f9c170a..5db97392 100644 --- a/src/shrpx_config_test.h +++ b/src/shrpx_config_test.h @@ -35,6 +35,7 @@ void test_shrpx_config_parse_config_str_list(void); void test_shrpx_config_parse_header(void); void test_shrpx_config_parse_log_format(void); void test_shrpx_config_read_tls_ticket_key_file(void); +void test_shrpx_config_read_tls_ticket_key_file_aes_256(void); void test_shrpx_config_match_downstream_addr_group(void); } // namespace shrpx diff --git a/src/shrpx_ssl.cc b/src/shrpx_ssl.cc index 9f95fa2f..121f4ce3 100644 --- a/src/shrpx_ssl.cc +++ b/src/shrpx_ssl.cc @@ -213,21 +213,21 @@ int ticket_key_cb(SSL *ssl, unsigned char *key_name, unsigned char *iv, if (LOG_ENABLED(INFO)) { CLOG(INFO, handler) << "encrypt session ticket key: " - << util::format_hex(key.name, 16); + << util::format_hex(key.data.name); } - memcpy(key_name, key.name, sizeof(key.name)); + memcpy(key_name, key.data.name, sizeof(key.data.name)); - EVP_EncryptInit_ex(ctx, EVP_aes_128_cbc(), nullptr, key.aes_key, iv); - HMAC_Init_ex(hctx, key.hmac_key, sizeof(key.hmac_key), EVP_sha256(), - nullptr); + EVP_EncryptInit_ex(ctx, get_config()->tls_ticket_cipher, nullptr, + key.data.enc_key, iv); + HMAC_Init_ex(hctx, key.data.hmac_key, key.hmac_keylen, key.hmac, nullptr); return 1; } size_t i; for (i = 0; i < keys.size(); ++i) { auto &key = keys[0]; - if (memcmp(key.name, key_name, sizeof(key.name)) == 0) { + if (memcmp(key_name, key.data.name, sizeof(key.data.name)) == 0) { break; } } @@ -246,8 +246,8 @@ int ticket_key_cb(SSL *ssl, unsigned char *key_name, unsigned char *iv, } auto &key = keys[i]; - HMAC_Init_ex(hctx, key.hmac_key, sizeof(key.hmac_key), EVP_sha256(), nullptr); - EVP_DecryptInit_ex(ctx, EVP_aes_128_cbc(), nullptr, key.aes_key, iv); + HMAC_Init_ex(hctx, key.data.hmac_key, key.hmac_keylen, key.hmac, nullptr); + EVP_DecryptInit_ex(ctx, key.cipher, nullptr, key.data.enc_key, iv); return i == 0 ? 1 : 2; } diff --git a/src/util.h b/src/util.h index d43d3c76..6c04fef0 100644 --- a/src/util.h +++ b/src/util.h @@ -212,6 +212,10 @@ std::string quote_string(const std::string &target); std::string format_hex(const unsigned char *s, size_t len); +template std::string format_hex(const unsigned char (&s)[N]) { + return format_hex(s, N); +} + std::string http_date(time_t t); // Returns given time |t| from epoch in Common Log format (e.g.,