]> git.ipfire.org Git - thirdparty/knot-resolver.git/commitdiff
modules/http: fixes around maintenance of ephemeral certs
authorVladimír Čunát <vladimir.cunat@nic.cz>
Tue, 11 Jun 2019 09:48:52 +0000 (11:48 +0200)
committerTomas Krizek <tomas.krizek@nic.cz>
Tue, 18 Jun 2019 08:02:02 +0000 (10:02 +0200)
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
modules/http/README.rst
modules/http/http.lua.in
modules/http/http_tls_cert.lua

diff --git a/NEWS b/NEWS
index ded82c4f18104f80c450fd4bd6275c2a559794a0..9c6f9687b5f8692a51452225b459f38915c781d6 100644 (file)
--- 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)
index 8644a0794f381a044de8457da4eed12e88ead1a0..89f89ef7cb27541b97b36b3d4f80eda2c43bd0fb 100644 (file)
@@ -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
index 86db45d5312edb6b3ab6dfa8236408b3ffcfeb81..0d07c4e6658e687429ac8b352aca3a1b863b9b7a 100644 (file)
@@ -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
index 7c2291958d661c89b22b107d8295332ae7d46f7c..ea9dd315990c14de261323853ba319bdb2d10ec0 100644 (file)
@@ -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