Fix: ssl_protocols does not rely on ssl_cipher_suite property #12118

d19dotca · 2023-11-29T17:43:54Z

Summary

There was a support case recently where they wanted to disable TLSv1.1. However because the ssl_protocols property had a statement about being ignored unless the ssl_cipher_suite was set to custom, this led to additional issues / time wasted troubleshooting. The ssl_protocols property does not require the ssl_cipher_suite to be custom.

Checklist

[N/A] The Pull Request has tests
A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
[N/A] There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Full changelog

Fix incorrect statement on ssl_protocols property.

Issue reference

N/A

There was a support case recently where they wanted to disable TLSv1.1. However because the ssl_protocols property had a statement about being ignored unless the ssl_cipher_suite was set to custom, this led to additional issues / time wasted troubleshooting. The ssl_protocols property does _not_ require the ssl_cipher_suite to be custom.

CLAassistant · 2023-11-29T17:44:01Z

All committers have signed the CLA.

CLAassistant · 2023-11-29T17:44:02Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

fffonion · 2023-11-30T07:14:57Z

@d19dotca Thanks for the PR. But I don't think the obversation is correct.

(kong-dev)  ~/D/K/kong-ee $ KONG_SSL_PROTOCOLS=whatever kong prepare
2023/11/30 15:13:22 [warn] ulimit is currently set to "256". For better performance set it to at least "4096" using "ulimit -n"
2023/11/30 15:13:22 [warn] ulimit is currently set to "256". For better performance set it to at least "4096" using "ulimit -n"
c⏎
(kong-dev)  ~/D/K/kong-ee $ cat $KONG_PREFIX/nginx-kong.conf |grep ssl_proto
ssl_protocols TLSv1.2 TLSv1.3;
    ssl_protocols TLSv1.1 TLSv1.2 TLSv1.3;
(kong-dev)  ~/D/K/kong-ee $ KONG_SSL_PROTOCOLS=whatever KONG_SSL_CIPHER_SUITE=custom kong prepare
2023/11/30 15:13:26 [warn] ulimit is currently set to "256". For better performance set it to at least "4096" using "ulimit -n"
2023/11/30 15:13:27 [warn] ulimit is currently set to "256". For better performance set it to at least "4096" using "ulimit -n"
Error: could not prepare Kong prefix at /Users/fffonion/Dev/Kong/kong-ee/bazel-bin/build/kong-dev/kong/servroot: nginx configuration is invalid (exit code 1):
nginx: [emerg] invalid value "whatever" in /Users/fffonion/Dev/Kong/kong-ee/bazel-bin/build/kong-dev/kong/servroot/nginx-kong.conf:47
nginx: configuration file /Users/fffonion/Dev/Kong/kong-ee/bazel-bin/build/kong-dev/kong/servroot/nginx.conf test failed


  Run with --v (verbose) or --vv (debug) for more details

which matches what the document says. Could you share the config that makes you believe the document is incorrect?

bungle · 2023-11-30T16:17:12Z

Yes, the observation is not correct, see:
https://github.com/Kong/kong/blob/master/kong/conf_loader/init.lua#L438

d19dotca · 2023-11-30T16:42:44Z

@bungle & @fffonion - so this is interesting. In my testing, I by default have no KONG_SSL_* properties explicitly defined. The use-case was to disable TLSv1.1 only but allow TLSv1.2 and TLSv1.3. Of course all three are enabled/allowed by default.

If I explicitly set kong_ssl_protocols=TLSv1.2 TLSv1.3, trying to disable TLSv1.1, and ran openssl commands to test, this seemed to work perfectly fine. A command like openssl s_client -connect localhost:8443 -tls1_2 would turn out a full certificate as expected, however running the command openssl s_client -connect localhost:8443 -tls1_1 would fail which I'd also expect given the setting I specified in the environment.

Additionally in the Kong Manager UI, I could see it recognized the change to ssl_protocols, which explicitly goes against the documentation saying it'd be ignored if the cipher suite wasn't set to custom. This was tested on 3.4.1.1 and 3.5.0.1.

Is this unexpected behaviour then and perhaps indicative of a bug elsewhere rather than the documentation? This one property change also worked for the customer as well (i.e. they didn't need to specify kong_ssl_cipher_suite for example.). So all testing so far has indicated that the documentation was incorrect, but now I'm further confused why this worked then for us in testing if the code itself is also not meant to work that way.

I guess ultimately this brings the question... how do we explain this observation in our testing?

For what it's worth.. here is my kong-ent container inspection output for the env section:

		"Env": [
			"KONG_PORTAL_GUI_PROTOCOL=http",
			"KONG_UNTRUSTED_LUA_SANDBOX_REQUIRES=resty.kong.tls,resty.openssl.x509",
			"KONG_PORTAL_GUI_LISTEN=0.0.0.0:8003, 0.0.0.0:8446 ssl",
			"KONG_LOG_LEVEL=debug",
			"KONG_STATUS_LISTEN=0.0.0.0:8100",
			"KONG_PG_HOST=postgres",
			"KONG_PORTAL_AUTH=basic-auth",
			"KONG_SSL_PROTOCOLS=TLSv1.2 TLSv1.3",
			"KONG_PASSWORD={redacted}",
			"KONG_ADMIN_GUI_ACCESS_LOG=/dev/stdout",
			"KONG_VITALS=on",
			"KONG_PORTAL_API_URL=http://localhost:8004",
			"KONG_ADMIN_GUI_ERROR_LOG=/dev/stderr",
			"KONG_PORTAL_API_ACCESS_LOG=/dev/stdout",
			"KONG_DATABASE=postgres",
			"KONG_ADMIN_ERROR_LOG=/dev/stderr",
			"KONG_LICENSE_DATA={redacted}",
			"KONG_PG_PASSWORD={redacted}",
			"KONG_PROXY_ERROR_LOG=/dev/stderr",
			"KONG_PORTAL_API_LISTEN=0.0.0.0:8004, 0.0.0.0:8447 ssl",
			"KONG_PROXY_ACCESS_LOG=/dev/stdout",
			"KONG_ADMIN_ACCESS_LOG=/dev/stdout",
			"KONG_AUDIT_LOG=on",
			"KONG_PORTAL_SESSION_CONF={redacted",
			"KONG_PROXY_LISTEN=0.0.0.0:8000, 0.0.0.0:8443 http2 ssl",
			"KONG_ADMIN_GUI_LISTEN=0.0.0.0:8002, 0.0.0.0:8445 ssl",
			"KONG_ADMIN_LISTEN=0.0.0.0:8001, 0.0.0.0:8444 ssl",
			"KONG_PORTAL_GUI_ERROR_LOG=/dev/stderr",
			"KONG_PORTAL_GUI_HOST=localhost:8003",
			"KONG_PORTAL_GUI_ACCESS_LOG=/dev/stdout",
			"KONG_PORTAL=on",
			"KONG_PORTAL_API_ERROR_LOG=/dev/stderr",
			"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
			"KONG_VERSION=",
			"KONG_PREFIX=/usr/local/kong"
		],

In my testing, if I ran openssl s_client -connect localhost:8443 -tls1_2 I'd get a successful output:

[...]
Server public key is 2048 bit
Secure Renegotiation IS supported
Compression: NONE
Expansion: NONE
No ALPN negotiated
SSL-Session:
    Protocol  : TLSv1.2
    Cipher    : ECDHE-RSA-AES256-GCM-SHA384
    Session-ID: FFF3BE99AA4873E4253EA70F8FE46B97748F6DCE127586199B1E9B293D29EEE6
    Session-ID-ctx: 
    Master-Key: 712D0AA4290790F5511558EC15FE75A228D57A1A529FA8AD73DB0BFAFE05325ED5C7F4244B46A9072B90CCAD04AFF1F7
    TLS session ticket lifetime hint: 86400 (seconds)
    TLS session ticket:
    0000 - 29 2f bf 33 4e 4f 71 ee-3a 60 17 40 29 3e f0 2c   )/.3NOq.:`.@)>.,
    0010 - a8 77 36 e5 2c a6 26 b7-62 4b 6c 2e 4d 67 91 1d   .w6.,.&.bKl.Mg..
    0020 - ee b9 11 46 5f be 42 17-49 d4 0f 97 bb 4f 1e 06   ...F_.B.I....O..
    0030 - 0f b1 68 e8 f6 12 7f 50-e2 e0 61 bc 71 17 85 90   ..h....P..a.q...
    0040 - 1f eb 62 71 9a 2b 7f 9f-44 a8 5e 54 a0 ea 1f 1c   ..bq.+..D.^T....
    0050 - ef bb 85 8c 6c d8 7c 33-10 05 47 c5 e7 89 11 50   ....l.|3..G....P
    0060 - 2a bb c2 1d 28 27 cb 88-ec d8 da 1d d7 29 f9 f8   *...('.......)..
    0070 - ad ca 95 e9 ea d2 d6 7f-82 fe 07 e2 cc 36 57 88   .............6W.
    0080 - af 26 a4 97 21 69 bf dc-db 27 aa 72 2a 6e 8c 9a   .&..!i...'.r*n..
    0090 - f4 6a bc a2 06 15 3a 45-6d d6 24 4b 08 43 ad 36   .j....:Em.$K.C.6
    00a0 - 2b e5 0e db 51 e5 0f 72-5a a0 6b f1 be 16 a9 c3   +...Q..rZ.k.....

    Start Time: 1701363404
    Timeout   : 7200 (sec)
    Verify return code: 18 (self signed certificate)

However when I run openssl s_client -connect localhost:8443 -tls1_1 instead, I get the following:

New, (NONE), Cipher is (NONE)
Secure Renegotiation IS NOT supported
Compression: NONE
Expansion: NONE
No ALPN negotiated
SSL-Session:
    Protocol  : TLSv1.1
    Cipher    : 0000
    Session-ID: 
    Session-ID-ctx: 
    Master-Key: 
    Start Time: 1701363486
    Timeout   : 7200 (sec)
    Verify return code: 0 (ok)
---

fffonion · 2023-12-01T04:29:32Z

@d19dotca ah that's because openssl 3.x disables TLSv1.1 and lower by default (See https://www.openssl.org/docs/man3.1/man3/SSL_CTX_set_security_level.html, we are on SecLevel=1). We will need to update document around it.

bungle · 2023-12-01T08:23:31Z

KONG_SSL_PROTOCOLS="TLSv1.3" \
KONG_NGINX_HTTP_SSL_PROTOCOLS="TLSv1.3" \
kong prepare

cat /usr/local/kong/nginx-kong.conf | grep ssl_protocols

ssl_protocols TLSv1.2 TLSv1.3;

Also on intermediate (the default) the TLSv1.1 is not enabled, nor in modern, only in old, though @fffonion does it work with old (do we need to do something for it, the SecLevel)?

See: https://wiki.mozilla.org/Security/Server_Side_TLS

fffonion · 2023-12-01T08:29:35Z

@bungle it does not anymore after we move to openssl 3.x. For old, even if we set TLSv1.1 in ssl_protocols it will not be effective. That means we will need
some friendly error message or a check to remind user. I created https://konghq.atlassian.net/browse/KAG-3259

…protocols

kikito · 2024-03-05T09:44:58Z

Closed by chore(conf): disable TLSv1.1 and lower in openssl 3.x

d19dotca added the skip-changelog label Nov 29, 2023

pull-request-size bot added the size/XS label Nov 29, 2023

github-actions bot assigned d19dotca Nov 29, 2023

Merge remote-tracking branch 'origin/master' into d19dotca-patch-ssl_…

2e6d8a1

…protocols

kikito closed this Mar 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: ssl_protocols does not rely on ssl_cipher_suite property #12118

Fix: ssl_protocols does not rely on ssl_cipher_suite property #12118

d19dotca commented Nov 29, 2023

CLAassistant commented Nov 29, 2023 •

edited

Loading

CLAassistant commented Nov 29, 2023

fffonion commented Nov 30, 2023

bungle commented Nov 30, 2023

d19dotca commented Nov 30, 2023 •

edited

Loading

fffonion commented Dec 1, 2023 •

edited

Loading

bungle commented Dec 1, 2023 •

edited

Loading

fffonion commented Dec 1, 2023

kikito commented Mar 5, 2024

Fix: ssl_protocols does not rely on ssl_cipher_suite property #12118

Fix: ssl_protocols does not rely on ssl_cipher_suite property #12118

Conversation

d19dotca commented Nov 29, 2023

Summary

Checklist

Full changelog

Issue reference

CLAassistant commented Nov 29, 2023 • edited Loading

CLAassistant commented Nov 29, 2023

fffonion commented Nov 30, 2023

bungle commented Nov 30, 2023

d19dotca commented Nov 30, 2023 • edited Loading

fffonion commented Dec 1, 2023 • edited Loading

bungle commented Dec 1, 2023 • edited Loading

fffonion commented Dec 1, 2023

kikito commented Mar 5, 2024

CLAassistant commented Nov 29, 2023 •

edited

Loading

d19dotca commented Nov 30, 2023 •

edited

Loading

fffonion commented Dec 1, 2023 •

edited

Loading

bungle commented Dec 1, 2023 •

edited

Loading