From 5826e485f40f56434e082d96ee57e5a6c40382f0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Vladim=C3=ADr=20=C4=8Cun=C3=A1t?= Date: Tue, 11 Jun 2019 11:48:52 +0200 Subject: [PATCH] modules/http: fixes around maintenance of ephemeral certs The cert was updated only once :-/ Now it's updated until the http module is unloaded. Also, each socket would create its own ephemeral certificate, so now that's all shared within the process. Technically we could synchronise even multiple instances, based on the files, but that would be much more complex, and it seems an unlikely combination to deploy. --- NEWS | 1 + modules/http/README.rst | 6 ++++++ modules/http/http.lua.in | 36 ++++++++++++---------------------- modules/http/http_tls_cert.lua | 36 +++++++++++++++++++++++++++++----- 4 files changed, 50 insertions(+), 29 deletions(-) diff --git a/NEWS b/NEWS index ded82c4f1..9c6f9687b 100644 --- a/NEWS +++ b/NEWS @@ -16,6 +16,7 @@ Improvements Bugfixes -------- - TCP to upstream: don't send wrong message length (unlikely, !816) +- http module: fix problems around maintenance of ephemeral certs (!819) Knot Resolver 4.0.0 (2019-04-18) diff --git a/modules/http/README.rst b/modules/http/README.rst index 8644a0794..89f89ef7c 100644 --- a/modules/http/README.rst +++ b/modules/http/README.rst @@ -112,6 +112,12 @@ for authentication to API etc. Safari doesn't allow WebSockets over HTTPS with a self-signed certificate. Major drawback is that current browsers won't do HTTP/2 over insecure connection.) +.. warning:: + + If you use multiple Knot Resolver instances with these automatically maintained ephemeral certificates, + they currently won't be shared. + It's assumed that you don't want a self-signed certificate for serious deployments anyway. + .. _mod-http-built-in-services: Built-in services diff --git a/modules/http/http.lua.in b/modules/http/http.lua.in index 86db45d53..0d07c4e66 100644 --- a/modules/http/http.lua.in +++ b/modules/http/http.lua.in @@ -276,25 +276,24 @@ end -- using configuration for specific "kind" of HTTP server local function add_socket(fd, kind, addr_str) assert(M.servers[fd] == nil, 'socket is already served by an HTTP instance') - local conf, certs, key - conf = mergeconf(M.configs._builtin._all, M.configs._builtin[kind], M.configs._all, M.configs[kind]) + local conf = mergeconf(M.configs._builtin._all, M.configs._builtin[kind], + M.configs._all, M.configs[kind]) conf.socket = cqueues.socket.fdopen(fd) - if conf.tls ~= false then - -- Check if a cert file was specified - -- Read or create self-signed x509 certificate + if conf.tls ~= false then -- Create a TLS context, either from files or new. if conf.ephemeral then - certs, key = tls_cert.new_ephemeral_files(conf.cert, conf.key) + if not M.ephem_state then + M.ephem_state = { servers = M.servers } + tls_cert.ephemeral_state_maintain(M.ephem_state, conf.cert, conf.key) + end + conf.ctx = M.ephem_state.ctx else - certs, key = tls_cert.load(conf.cert, conf.key) - end - -- Check loaded certificate - if not certs or not key then - panic('failed to load certificate "%s"', conf.cert) + local certs, key = tls_cert.load(conf.cert, conf.key) + conf.ctx = tls_cert.new_tls_context(certs, key) end + assert(conf.ctx) end -- Compose server handler local routes = route(conf.endpoints) - conf.ctx = certs and tls_cert.new_tls_context(certs, key) conf.onstream = routes -- Create TLS context and start listening local s, err = http_server.new(conf) @@ -306,17 +305,6 @@ local function add_socket(fd, kind, addr_str) panic('failed to listen on %s: %s', addr_str, err) end M.servers[fd] = {kind = kind, server = s, config = conf} - -- Create certificate renewal timer if ephemeral; FIXME: renew more than once, not per-socket? etc. - if certs and conf.ephemeral then - local _, expiry = certs[1]:getLifetime() - expiry = 1000 * math.max(0, expiry - (os.time() - 3 * 24 * 3600)) - event.after(expiry, function () - log('[http] refreshed ephemeral certificate') - certs, key = tls_cert.new_ephemeral_files(conf.cert, conf.key) - -- TODO servers sharing cert?! - s.ctx = tls_cert.new_tls_context(certs, key) - end) - end end -- @function Stop listening on given socket @@ -326,7 +314,6 @@ local function remove_socket(fd) instance.server:close() M.servers[fd] = nil - -- TODO stop refresh timer end -- @function Listen for config changes from net.listen()/net.close() @@ -358,6 +345,7 @@ function M.deinit() remove_socket(fd) end prometheus.deinit() + tls_cert.ephemeral_state_destroy(M.ephem_state) net.register_endpoint_kind('doh') net.register_endpoint_kind('webmgmt') end diff --git a/modules/http/http_tls_cert.lua b/modules/http/http_tls_cert.lua index 7c2291958..ea9dd3159 100644 --- a/modules/http/http_tls_cert.lua +++ b/modules/http/http_tls_cert.lua @@ -43,10 +43,8 @@ local function new_ephemeral(host) return { crt }, key end --- @function Create new self-signed certificate, write to files; return certs, key --- Well, we actually never read from these files anyway (in case of ephemeral). -function tls_cert.new_ephemeral_files(certfile, keyfile) - local certs, key = new_ephemeral() +-- @function Write certs and key to files +local function write_cert_files(certs, key, certfile, keyfile) -- Write certs local f = assert(io.open(certfile, 'w'), string.format('cannot open "%s" for writing', certfile)) for _, cert in ipairs(certs) do @@ -58,7 +56,35 @@ function tls_cert.new_ephemeral_files(certfile, keyfile) local pub, priv = key:toPEM('public', 'private') assert(f:write(pub .. priv)) f:close() - return certs, key +end + +-- @function Start maintenance of a self-signed TLS context (at ephem_state.ctx). +-- Keep updating the ephem_state.servers table. Stop updating by calling _destroy(). +-- TODO: each process maintains its own ephemeral cert ATM, and the files aren't ever read from. +function tls_cert.ephemeral_state_maintain(ephem_state, certfile, keyfile) + local certs, key = new_ephemeral() + write_cert_files(certs, key, certfile, keyfile) + ephem_state.ctx = tls_cert.new_tls_context(certs, key) + -- Each server needs to have its ctx updated. + for _, s in pairs(ephem_state.servers) do + s.server.ctx = ephem_state.ctx + s.config.ctx = ephem_state.ctx -- not required, but let's keep it synchonized + end + log('[http] created new ephemeral TLS certificate') + local _, lifetime_sec = certs[1]:getLifetime() + local wait_msec = 1000 * math.max(1, lifetime_sec - (os.time() - 3 * 24 * 3600)) + if not ephem_state.timer_id then + ephem_state.timer_id = event.after(wait_msec, function () + tls_cert.ephemeral_state_maintain(ephem_state, certfile, keyfile) + end) + else + event.reschedule(ephem_state.timer_id, wait_msec) + end +end +function tls_cert.ephemeral_state_destroy(ephem_state) + if ephem_state and ephem_state.timer_id then + event.cancel(ephem_state.timer_id) + end end -- @function Read a certificate chain and a key from files; return certs, key -- 2.47.2