feat(ssl) configurable ssl cipher suites #2555
Conversation
| @@ -228,6 +229,15 @@ local function check_and_infer(conf) | |||
| end | |||
| end | |||
|
|
|||
| if conf.ssl_cipher_suite ~= "custom" then | |||
| local ok, err = pcall(function() | |||
There was a problem hiding this comment.
This unused ok will fail linting. We prefer:
if not ok then
error[#errors +1] = err
end
kong/tools/ciphers.lua
Outdated
| "!EDH-DSS-DES-CBC3-SHA", | ||
| "!KRB5-DES-CBC3-SHA", | ||
| "!SRP" | ||
| } |
There was a problem hiding this comment.
(style: we need trailing commas in several locations in this table)
kong.conf.default
Outdated
| # Note: See https://wiki.mozilla.org/Security/Server_Side_TLS for detailed | ||
| # descriptions of each cipher suite. | ||
|
|
||
| #ssl_ciphers = # Define a custom list of TLS ciphers to be |
There was a problem hiding this comment.
Defines, as per the convention followed in this configuration file (and the ssl_cipher_suite property).
| ssl_session_cache shared:SSL:10m; | ||
| ssl_session_timeout 10m; | ||
| ssl_prefer_server_ciphers on; | ||
| ssl_ciphers ${{SSL_CIPHERS}}; |
There was a problem hiding this comment.
This either needs to be added to the Admin server as well, or the property should reflect that those values are for the Proxy port only (the former is probably what we are after here).
kong/tools/ciphers.lua
Outdated
| } | ||
|
|
||
| local ciphers = setmetatable(_ciphers, { | ||
| __index = function(t, k) error("Undefined cipher suite " .. tostring(k)) end, |
There was a problem hiding this comment.
style: this looks like a 4 spaces indent
kong/tools/ciphers.lua
Outdated
| @@ -0,0 +1,114 @@ | |||
| local cat = table.concat | |||
There was a problem hiding this comment.
We try to avoid such globals caching when not necessary (cold paths) - this looks like such a case, since this is only to be called by the CLI.
spec/01-unit/02-conf_loader_spec.lua
Outdated
| it("defines ssl_ciphers by default", function() | ||
| local conf, err = conf_loader(nil, {}) | ||
| assert.is_nil(err) | ||
| assert.matches(":", conf.ssl_ciphers) -- looks kinda like a cipher suite |
There was a problem hiding this comment.
assert.matches(":", conf.ssl_ciphers, nil, true) might be preferable here. Same for other assert.matches in this suite.
spec/01-unit/02-conf_loader_spec.lua
Outdated
| assert.matches("Undefined cipher suite foo", errors[1]) | ||
| end) | ||
| it("overrides ssl_ciphers when ssl_cipher_suite is custom", function() | ||
| local conf,err = conf_loader(nil, { |
There was a problem hiding this comment.
very very minor but missing a space - it's fine
spec/01-unit/02-conf_loader_spec.lua
Outdated
| ssl_cipher_suite = "custom", | ||
| }) | ||
| assert.is_nil(err) | ||
| assert.equals(ciphers("modern"), conf.ssl_ciphers) |
There was a problem hiding this comment.
Better to use assert.same() when comparing tables, at any time except rare exceptions.
spec/01-unit/02-conf_loader_spec.lua
Outdated
| }) | ||
| assert.is_nil(err) | ||
| assert.equals(ciphers("modern"), conf.ssl_ciphers) | ||
| end) |
There was a problem hiding this comment.
I think we are missing a test case when the cipher suite is other than the default one - and other than custom?
spec/01-unit/02-conf_loader_spec.lua
Outdated
| assert.matches(":", conf.ssl_ciphers) -- looks kinda like a cipher suite | ||
| end) | ||
| it("errors on invalid ssl_cipher_suite", function() | ||
| local conf, _, errors = conf_loader(nil, { |
There was a problem hiding this comment.
This unused conf failed linting. Better asserting it is nil.
|
Eagle-eyed! Re-up'd addressing concerns (and manually linting, I gotta learn to do that beforehand...) |
Provide a shorthand config option to define cipher suites based on Mozilla's recommended cipher list, and an optional config to define custom ciphers.
Introduced in Kong/kong#2555
Introduced in Kong/kong#2555
Summary
The increasing ubiquity of TLS in the HTTP ecosystem necessitates that Kong be a good citizen and provide a secure default TLS configuration for users. This commit provides a shorthand config option to define cipher suites based on Mozilla's recommended cipher list, and an optional config to define custom ciphers.
Further optimizations to the default TLS configuration, such as OSCP stapling/verification and non-default Diffie-Hellman params, should be implemented, but these changes are less trivial, and so belong in discrete PRs.
Full changelog
ssl_ciphersandssl_cipher_suitesconfig directives.kong.tools.ciphersmodule, based on https://wiki.mozilla.org/Security/Server_Side_TLS