]> git.ipfire.org Git - thirdparty/knot-resolver.git/commitdiff
Fix regression in HTTP module which broke custom certs.
authorPetr Špaček <petr.spacek@nic.cz>
Tue, 24 Jul 2018 16:12:26 +0000 (18:12 +0200)
committerVladimír Čunát <vladimir.cunat@nic.cz>
Tue, 31 Jul 2018 15:43:01 +0000 (17:43 +0200)
This is now covered by test suite.
fixup! b2cefdcf350e846492579e3308f234a696350e01 (regressed in 2.4.0).

Parameter cert=false did not work even in 2.3.0 so it was replaced with cleaner
tls=false.

modules/http/README.rst
modules/http/http.lua
modules/http/test_tls/broken.crt [new file with mode: 0644]
modules/http/test_tls/broken.key [new file with mode: 0644]
modules/http/test_tls/test.crt [new file with mode: 0644]
modules/http/test_tls/test.key [new file with mode: 0644]
modules/http/test_tls/tls.test.lua [new file with mode: 0644]

index 5a64b81c42e3a8672cb9a8235c9755e7fa2b937f..f5af30d338ebe543247e30a30d1442e2e37fb76c 100644 (file)
@@ -42,15 +42,18 @@ Now you can reach the web services and APIs, done!
        $ curl -k https://localhost:8053
        $ curl -k https://localhost:8053/stats
 
-It is possible to disable HTTPS altogether by passing ``cert = false`` option.
+It is possible to disable HTTPS altogether by passing ``tls = false`` option.
 While it's not recommended, it could be fine for localhost tests as, for example,
 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.
 
+Alternatively you can disable unecrypted HTTP and enforce HTTPS by passing
+``tls = true`` option.
+
 .. code-block:: lua
 
        http = {
-               cert = false,
+               tls = true,
        }
 
 If you want to provide your own certificate and key, you're welcome to do so:
index 11b8c9cae09a21bcaaac1dc55d5433b0b75e5e79..f3c4e493690105354163d4f866f8e96dcd54ecaf 100644 (file)
@@ -274,32 +274,32 @@ end
 -- @function Listen on given HTTP(s) host
 function M.add_interface(conf)
        local crt, key, ephemeral
-       if conf.crtfile ~= false then
-               -- Check if the cert file exists
-               if not conf.crtfile then
-                       conf.crtfile = 'self.crt'
-                       conf.keyfile = 'self.key'
+       if conf.tls ~= false then
+               -- Check if a cert file was specified
+               if not conf.cert then
+                       conf.cert = 'self.crt'
+                       conf.key = 'self.key'
                        ephemeral = true
-               elseif not conf.keyfile then
+               elseif not conf.key then
                        error('certificate provided, but missing key')
                end
                -- Read or create self-signed x509 certificate
-               local f = io.open(conf.crtfile, 'r')
+               local f = io.open(conf.cert, 'r')
                if f then
                        crt = assert(x509.new(f:read('*all')))
                        f:close()
                        -- Continue reading key file
                        if crt then
-                               f = io.open(conf.keyfile, 'r')
+                               f = io.open(conf.key, 'r')
                                key = assert(pkey.new(f:read('*all')))
                                f:close()
                        end
                elseif ephemeral then
-                       crt, key = updatecert(conf.crtfile, conf.keyfile)
+                       crt, key = updatecert(conf.cert, conf.key)
                end
                -- Check loaded certificate
                if not crt or not key then
-                       panic('failed to load certificate "%s"', conf.crtfile)
+                       panic('failed to load certificate "%s"', conf.cert)
                end
        end
        -- Compose server handler
@@ -320,6 +320,7 @@ function M.add_interface(conf)
                reuseport = reuseport,
                client_timeout = conf.client_timeout or 5,
                ctx = crt and tlscontext(crt, key),
+               tls = conf.tls,
                onstream = routes,
        }
        -- Manually call :listen() so that we are bound before calling :localname()
@@ -336,7 +337,7 @@ function M.add_interface(conf)
                expiry = math.max(0, expiry - (os.time() - 3 * 24 * 3600))
                event.after(expiry, function ()
                        log('[http] refreshed ephemeral certificate')
-                       crt, key = updatecert(conf.crtfile, conf.keyfile)
+                       crt, key = updatecert(conf.cert, conf.key)
                        s.ctx = tlscontext(crt, key)
                end)
        end
@@ -348,8 +349,8 @@ function M.interface(host, port, endpoints, crtfile, keyfile)
                host = host,
                port = port,
                endpoints = endpoints,
-               crtfile = crtfile,
-               keyfile = keyfile,
+               cert = crtfile,
+               key = keyfile,
        }
 end
 
diff --git a/modules/http/test_tls/broken.crt b/modules/http/test_tls/broken.crt
new file mode 100644 (file)
index 0000000..d93d1f8
--- /dev/null
@@ -0,0 +1,3 @@
+Æ¿¯\88\9e\ 1³psâ$Ðëí¯Ö¡«ÆÈ¼[\8cî\17Ø1´Ç\v =fl\a:°Êl\9d=z\86=M}\ 5i\81É»¤Ñ­\86÷*7\83\vÑ5
\ 4\10óîÒj\a\13¨\9aIÜWÈÀ­\ 6®·´\r\bMwÚf[´H\1fï\8eí\1cÙÑ\aÅ-\ 3Á¯Ëìý\81Eȼë\80f\11\19û¦Ö\r\ 2   \1a       2fTKÊq\18ýF\14Ñ\ 2¬U\9dÖ        \83\93\15(jÀ¤\9caà,Õ\9c¥*²X:\8dlF\12ÌÍ¿£Mî©>\ 6ó3\8c\9a
\91áD<^O\99q\ 1\88Q\8d\8eÉ\ eý\91κ\ 4M\85\1dgÄ]p\ 6\aUNÉö¥MÝ\9d>\8c\9fÂ\ 5\1aÿº¹á(\9d\ 5EI\94®\ 5µ'ÌGÅ\80÷mÕÅ÷Ã\ 1:\9d\9e3\rº_\94!\82ÁÎ\92?\v3\8cú\ 3$H[\84E\86¡M¡4è\90\bòR\ 2\1e¨\86\ 3\1aÒA+\140w\12\18¥\17ö0Ï\10ÓÁ\8d\a\93%é¸eo¤a\9aåëØó(\ eöw;¥oÇ¥î¯$\19\97!ZèÀ\88\90r%Äùàü&ßýh;1¾@\88ñý-9((Ób7±\ 4á\\84U\81\ fâoÃJÊ\ 4\10`»:Þª\80\86¼\99~\7f\9f\8ea\95\9c\81\8d\83ì\81ÜÐ\83Ç\10\8c\9c\97PB*l\ 1«Î}!q\1a7ü;+\16QRL\91¢¶v\1cÉÃ\12»Ë\ eQ\87[Kã\7f\19XðRà\v2(\18¬íùªå7ÀÜ+$ëÃE,óõ­Ûý³IRÄÙï·\r§^4µD \90¼_r\85,i\9aÁè\ 4Á\ðhξ
\ No newline at end of file
diff --git a/modules/http/test_tls/broken.key b/modules/http/test_tls/broken.key
new file mode 100644 (file)
index 0000000..ebcbfcf
Binary files /dev/null and b/modules/http/test_tls/broken.key differ
diff --git a/modules/http/test_tls/test.crt b/modules/http/test_tls/test.crt
new file mode 100644 (file)
index 0000000..01c36f8
--- /dev/null
@@ -0,0 +1,10 @@
+-----BEGIN CERTIFICATE-----
+MIIBXDCCAQKgAwIBAgIRAONzB6ou1Lh79QSlofsBtBMwCgYIKoZIzj0EAwIwGTEX
+MBUGA1UEAwwOcm9oYW4tcmVzb2x2ZXIwHhcNMTgwNzIzMTA1MjE4WhcNMTgxMDIx
+MTA1MjE4WjAZMRcwFQYDVQQDDA5yb2hhbi1yZXNvbHZlcjBZMBMGByqGSM49AgEG
+CCqGSM49AwEHA0IABJPYWceFJkbjORCrO8aIhMk3Bw2PpTzuPC27O/rjojBjmadO
+vZyFxIKgfYiHp4uMr1z81K+cqq1s/q0+kW+tNaejKzApMBkGA1UdEQQSMBCCDnJv
+aGFuLXJlc29sdmVyMAwGA1UdEwEB/wQCMAAwCgYIKoZIzj0EAwIDSAAwRQIhAL+Z
+IElUAmI0nQdaSRLZw5LCZeC/OIFx9JfaoDzMNkW5AiABXCWYzR+/uyYV7KDucwtW
+LGh/LrjC/FZGK3Drefbu0A==
+-----END CERTIFICATE-----
diff --git a/modules/http/test_tls/test.key b/modules/http/test_tls/test.key
new file mode 100644 (file)
index 0000000..1256b5a
--- /dev/null
@@ -0,0 +1,9 @@
+-----BEGIN PUBLIC KEY-----
+MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEk9hZx4UmRuM5EKs7xoiEyTcHDY+l
+PO48Lbs7+uOiMGOZp069nIXEgqB9iIeni4yvXPzUr5yqrWz+rT6Rb601pw==
+-----END PUBLIC KEY-----
+-----BEGIN PRIVATE KEY-----
+MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgtgNJHFS7+jlibs3b
+4NMYVLgZvVgOh5ouMn/ujQrAbouhRANCAAST2FnHhSZG4zkQqzvGiITJNwcNj6U8
+7jwtuzv646IwY5mnTr2chcSCoH2Ih6eLjK9c/NSvnKqtbP6tPpFvrTWn
+-----END PRIVATE KEY-----
diff --git a/modules/http/test_tls/tls.test.lua b/modules/http/test_tls/tls.test.lua
new file mode 100644 (file)
index 0000000..9eac382
--- /dev/null
@@ -0,0 +1,164 @@
+-- check prerequisites
+local has_http = pcall(require, 'http') and pcall(require, 'http.request')
+if not has_http then
+       pass('skipping http module test because its not installed')
+       done()
+else
+       local request = require('http.request')
+       local openssl_ctx = require('openssl.ssl.context')
+
+       local function setup_module(desc, config)
+               if http then
+                       modules.unload('http')
+               end
+               modules.load('http')
+               same(http.config(config), nil, desc .. ' can be configured')
+
+               local server = http.servers[1]
+               ok(server ~= nil, desc .. ' creates server instance')
+               local _, host, port = server:localname()
+               ok(host and port, desc .. ' binds to an interface')
+               return host, port
+       end
+
+       local function http_get(uri)
+               -- disable certificate verification in this test
+               local req = request.new_from_uri(uri)
+               local idxstart = string.find(uri, 'https://')
+               if idxstart == 1 then
+                       req.ctx = openssl_ctx.new()
+                       assert(req.ctx, 'OpenSSL cert verification must be disabled')
+                       req.ctx:setVerify(openssl_ctx.VERIFY_NONE)
+               end
+
+               local headers = assert(req:go())
+               return tonumber(headers:get(':status'))
+       end
+
+       -- test whether http interface responds and binds
+       local function check_protocol(uri, description, ok_expected)
+               if ok_expected then
+                       local code = http_get(uri)
+                       same(code, 200, description)
+               else
+                       boom(http_get, {uri}, description)
+               end
+       end
+
+       local function test_defaults()
+               local host, port = setup_module('HTTP module default config', {})
+
+               local uri = string.format('http://%s:%d', host, port)
+               check_protocol(uri, 'HTTP is enabled by default', true)
+               uri = string.format('https://%s:%d', host, port)
+               check_protocol(uri, 'HTTPS is enabled by default', true)
+
+               modules.unload('http')
+               uri = string.format('http://%s:%d', host, port)
+               check_protocol(uri, 'HTTP stops working after module unload', false)
+               uri = string.format('https://%s:%d', host, port)
+               check_protocol(uri, 'HTTPS stops working after module unload', false)
+
+       end
+
+       local function test_http_only()
+               local desc = 'HTTP-only config'
+               local host, port = setup_module(desc,
+                       {
+                               port = 0, -- Select random port
+                               tls = false,
+                       })
+
+               local uri = string.format('http://%s:%d', host, port)
+               check_protocol(uri, 'HTTP works in ' .. desc, true)
+               uri = string.format('https://%s:%d', host, port)
+               check_protocol(uri, 'HTTPS does not work in ' .. desc, false)
+       end
+
+       local function test_https_only()
+               local desc = 'HTTPS-only config'
+               local host, port = setup_module(desc,
+                       {
+                               port = 0, -- Select random port
+                               tls = true,
+                       })
+
+               local uri = string.format('http://%s:%d', host, port)
+               check_protocol(uri, 'HTTP does not work in ' .. desc, false)
+               uri = string.format('https://%s:%d', host, port)
+               check_protocol(uri, 'HTTPS works in ' .. desc, true)
+       end
+
+       local function test_custom_cert()
+               desc = 'config with custom certificate'
+               local host, port = setup_module(desc, {{
+                               host = host,
+                               port = port,
+                               cert = 'test.crt',
+                               key = 'test.key'
+                       }})
+
+               uri = string.format('https://%s:%d', host, port)
+               check_protocol(uri, 'HTTPS works for ' .. desc, true)
+       end
+
+       local function test_nonexistent_cert()
+               desc = 'config with non-existing certificate file'
+               boom(http.config, {{
+                               port = 0,
+                               cert = '/tmp/surely_nonexistent_cert_1532432095',
+                               key = 'test.key'
+                       }}, desc)
+       end
+
+       local function test_nonexistent_key()
+               desc = 'config with non-existing key file'
+               boom(http.config, {{
+                               port = 0,
+                               cert = 'test.crt',
+                               key = '/tmp/surely_nonexistent_cert_1532432095'
+                       }}, desc)
+       end
+
+       local function test_missing_key_param()
+               desc = 'config with missing key= param'
+               boom(http.config, {{
+                               port = 0,
+                               cert = 'test.crt'
+                       }}, desc)
+       end
+
+       local function test_broken_cert()
+               desc = 'config with broken file in cert= param'
+               boom(http.config, {{
+                               port = 0,
+                               cert = 'broken.crt',
+                               key = 'test.key'
+                       }}, desc)
+       end
+
+       local function test_broken_key()
+               desc = 'config with broken file in key= param'
+               boom(http.config, {{
+                               port = 0,
+                               cert = 'test.crt',
+                               key = 'broken.key'
+                       }}, desc)
+       end
+
+
+       -- plan tests
+       local tests = {
+               test_defaults,
+               test_http_only,
+               test_https_only,
+               test_custom_cert,
+               test_nonexistent_cert,
+               test_nonexistent_key,
+               test_missing_key_param,
+               test_broken_cert,
+               test_broken_key
+       }
+
+       return tests
+end