Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(conf): disable TLSv1.1 and lower in openssl 3.x #12420

Merged
merged 3 commits into from
Feb 2, 2024

Conversation

Water-Melon
Copy link
Contributor

@Water-Melon Water-Melon commented Jan 25, 2024

Summary

  1. remove unsupported TLS versions from default configurations.
  2. support communication with old versions of OpenSSL clients using TLSv1.1.

Issue reference

KAG-3259

@Water-Melon
Copy link
Contributor Author

Tested the Kong with KONG_SSL_CIPHER_SUITE=old using testssl.sh , and the results are as follows:

./bin/openssl.Linux.x86_64.krb s_client -connect localhost:8443 -tls1_1
CONNECTED(00000003)
depth=0 C = US, ST = California, L = San Francisco, O = Kong, OU = IT Department, CN = localhost
verify error:num=18:self signed certificate
verify return:1
depth=0 C = US, ST = California, L = San Francisco, O = Kong, OU = IT Department, CN = localhost
verify return:1
---
Certificate chain
 0 s:/C=US/ST=California/L=San Francisco/O=Kong/OU=IT Department/CN=localhost
   i:/C=US/ST=California/L=San Francisco/O=Kong/OU=IT Department/CN=localhost
---
Server certificate
-----BEGIN CERTIFICATE-----
MIICNzCCAd2gAwIBAgIRAM9fAU4m/tueomjlsnp8us0wCgYIKoZIzj0EAwIwdTEL
MAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcMDVNhbiBG
cmFuY2lzY28xDTALBgNVBAoMBEtvbmcxFjAUBgNVBAsMDUlUIERlcGFydG1lbnQx
EjAQBgNVBAMMCWxvY2FsaG9zdDAeFw0yNDAxMjUwOTAwNDhaFw00NDAxMjAwOTAw
NDhaMHUxCzAJBgNVBAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRYwFAYDVQQH
DA1TYW4gRnJhbmNpc2NvMQ0wCwYDVQQKDARLb25nMRYwFAYDVQQLDA1JVCBEZXBh
cnRtZW50MRIwEAYDVQQDDAlsb2NhbGhvc3QwWTATBgcqhkjOPQIBBggqhkjOPQMB
BwNCAATyzmVJAbXYS0ZVRJVo3dDf+ldyq+Cv9St8TAD+6kmYunHYtCjD4sHh75wR
grlO160YXr7bOsmI8WaT/GpNayG9o04wTDAMBgNVHRMBAf8EAjAAMB0GA1UdJQQW
MBQGCCsGAQUFBwMBBggrBgEFBQcDAjAdBgNVHQ4EFgQUt0DNiPonJFP0eSlLgJhI
AYAjjNwwCgYIKoZIzj0EAwIDSAAwRQIgVJNNSwMpf7m3bvKJR/USIRuwEEngi/3o
2oe54Kn2uCACIQCvHwo0VHHap92cB8vKS77kMsDO91qrOlUSJIiQGq7evQ==
-----END CERTIFICATE-----
subject=/C=US/ST=California/L=San Francisco/O=Kong/OU=IT Department/CN=localhost
issuer=/C=US/ST=California/L=San Francisco/O=Kong/OU=IT Department/CN=localhost
---
No client certificate CA names sent
Server Temp Key: ECDH, P-256, 256 bits
---
SSL handshake has read 1078 bytes and written 390 bytes
---
New, TLSv1/SSLv3, Cipher is ECDHE-ECDSA-AES256-SHA
Server public key is 256 bit
Secure Renegotiation IS supported
Compression: NONE
Expansion: NONE
No ALPN negotiated
SSL-Session:
    Protocol  : TLSv1.1
    Cipher    : ECDHE-ECDSA-AES256-SHA
    Session-ID: B33AE697547AA979FAF6D9C1570A90F774FD51E61FCE0B7F0170A5020F7E0AE8
    Session-ID-ctx: 
    Master-Key: D7A4C16BDBB79ED938D396B74FBCAE7948347F442ABA2AE5B39B16EB20EB6A05718AB87A2A3DA3424D5783EBCD1B4D07
    Key-Arg   : None
    Krb5 Principal: None
    PSK identity: None
    PSK identity hint: None
    SRP username: None
    TLS session ticket lifetime hint: 86400 (seconds)
    TLS session ticket:
    0000 - 02 20 4b b6 b1 4c 72 c9-d6 be 8f 0a 5d 9e df 0b   . K..Lr.....]...
    0010 - 79 3f d9 69 55 9e c3 d7-2c 4a c8 11 d3 53 cd 99   y?.iU...,J...S..
    0020 - ca ad 9c 5e 50 da a6 76-22 39 00 a2 3c ed 7a 65   ...^P..v"9..<.ze
    0030 - 43 82 95 8d 14 a5 35 d1-0e e7 2d 07 89 81 23 bb   C.....5...-...#.
    0040 - 52 09 f4 0e 28 70 99 03-46 47 7f d6 4b 9a f5 77   R...(p..FG..K..w
    0050 - 79 71 57 20 fb af b6 27-e6 4f e1 a0 b0 6c 37 90   yqW ...'.O...l7.
    0060 - 93 ad 88 05 c5 e3 9a 2d-e6 f1 2d 4f 8a 3c 01 00   .......-..-O.<..
    0070 - 37 94 15 b4 2a f0 d9 60-f9 2f 85 9b 95 3f f4 c7   7...*..`./...?..
    0080 - c6 92 30 d4 25 c7 93 99-ef 8b c7 71 c2 32 03 97   ..0.%......q.2..
    0090 - 02 c9 07 7a bf 69 a4 40-66 94 b8 3a 37 7f d4 6e   ...z.i.@f..:7..n
    00a0 - 51 b9 49 f0 b7 1f 3a ff-5e b7 3b c1 14 2d 18 38   Q.I...:.^.;..-.8

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

And if we use openssl 3.x to execute the openssl s_client command, the handshake will fail.
Moreover, if we test with openssl 3.x's s_client and s_server using the -auth_level 0 parameter, the handshake will still fail.
Therefore, this may indicate that openssl 3.x's s_client no longer supports TLSv1.x.

@Water-Melon Water-Melon marked this pull request as ready for review January 25, 2024 12:22
@bungle
Copy link
Member

bungle commented Jan 25, 2024

@Water-Melon does it mean we cannot get this old working anymore, even with SECLEVEL=0?
https://wiki.mozilla.org/Security/Server_Side_TLS#Old_backward_compatibility

@fffonion is that right?

If so, then we need to remove the whole old thing (or do we still want to somewhat support old?).

@bungle
Copy link
Member

bungle commented Jan 25, 2024

Therefore, this may indicate that openssl 3.x's s_client no longer supports TLSv1.x.

But it (3.x) works on server with TLSv1.1/TLSv1.0? Or not?

@Water-Melon
Copy link
Contributor Author

Water-Melon commented Jan 26, 2024

@bungle old is working correctly to those clients with OpenSSL 1.x whether the OpenSSL version is 3.x on server side. But if the OpenSSL version is 3.x on client, old will not working, because client does not support TLSv1.1.

This PR has two objectives: the first is to remove unsupported TLS versions from default configurations, and the second is to support communication with old versions of OpenSSL clients using TLSv1.1.

@Water-Melon Water-Melon force-pushed the chore/disable_tls1_in_v3 branch from 45e564c to 89a1150 Compare January 26, 2024 07:51
@fffonion
Copy link
Contributor

0old is working correctly to those clients with OpenSSL 1.x whether the OpenSSL version is 3.x on server side.

let's mention this in the kong.conf.default comments, so it will become docs on konghq.com

@Water-Melon Water-Melon force-pushed the chore/disable_tls1_in_v3 branch 2 times, most recently from ee92f48 to d390920 Compare January 26, 2024 09:11
@Kong Kong deleted a comment from CLAassistant Jan 26, 2024
@Water-Melon Water-Melon force-pushed the chore/disable_tls1_in_v3 branch from d390920 to 3c03154 Compare January 26, 2024 09:14
@Water-Melon Water-Melon marked this pull request as draft January 29, 2024 02:28
@Water-Melon Water-Melon force-pushed the chore/disable_tls1_in_v3 branch 2 times, most recently from 516a523 to 7305825 Compare January 30, 2024 05:10
@Water-Melon Water-Melon force-pushed the chore/disable_tls1_in_v3 branch from 7305825 to 7ba02d4 Compare January 30, 2024 06:25
@Water-Melon Water-Melon marked this pull request as ready for review January 30, 2024 07:41
kong/conf_loader/parse.lua Outdated Show resolved Hide resolved
kong/conf_loader/parse.lua Show resolved Hide resolved
kong/conf_loader/parse.lua Outdated Show resolved Hide resolved
kong/conf_loader/parse.lua Outdated Show resolved Hide resolved
changelog/unreleased/kong/disable-TLSv1_1-in-openssl3.yml Outdated Show resolved Hide resolved
kong/conf_loader/parse.lua Show resolved Hide resolved
kong/conf_loader/parse.lua Outdated Show resolved Hide resolved
@@ -432,6 +432,20 @@ local function check_and_parse(conf, opts)
conf.ssl_dhparam = suite.dhparams
conf.nginx_http_ssl_dhparam = suite.dhparams
conf.nginx_stream_ssl_dhparam = suite.dhparams
else
for _, key in ipairs({
"nginx_http_ssl_conf_command",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only usage of those conf fields I can find is here. Could you elaborate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this set of configurations is the only one related to the security level, I believe there shouldn't be any issues.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this set of configurations is the only one related to the security level, I believe there shouldn't be any issues.

It's surprising to find that we simply ignore anything other than the SECLEVEL in this field. Could we just name it "nginx_http_ssl_security_level" or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In nginx, there is no specific configuration that is solely responsible for modifying the security level.

Nginx does not support using a single ssl_conf_command to apply multiple commands.
It is acceptable to use multiple ssl_conf_command directives in the Nginx configuration.
But it seems that kong's conf_load cannot support this unless we do special processing for these instructions.

Adding a Kong configuration as you said or making the conf_loader support array-type configurations could solve this problem. However, this would require significant changes, and there might be few users who would actually configure these settings. Therefore, this is just a most cost-effective strategy. Although I still believe that enabling conf_loader to support array-type values would provide better configurability.

@team-gateway-bot
Copy link
Collaborator

Backport failed for release/3.5.x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release/3.5.x
git worktree add -d .worktree/backport-12420-to-release/3.5.x origin/release/3.5.x
cd .worktree/backport-12420-to-release/3.5.x
git switch --create backport-12420-to-release/3.5.x
git cherry-pick -x 2516c5035f8a2406a3add38370b520f54aac6a11

github-actions bot pushed a commit that referenced this pull request Feb 12, 2024
- remove unsupported TLS versions from default configurations.
- support communication with old versions of OpenSSL clients using TLSv1.1.

KAG-3259

(cherry picked from commit 2516c50)
@team-gateway-bot
Copy link
Collaborator

Backport failed for release/3.4.x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release/3.4.x
git worktree add -d .worktree/backport-12420-to-release/3.4.x origin/release/3.4.x
cd .worktree/backport-12420-to-release/3.4.x
git switch --create backport-12420-to-release/3.4.x
git cherry-pick -x 2516c5035f8a2406a3add38370b520f54aac6a11

@team-gateway-bot
Copy link
Collaborator

Backport failed for release/3.5.x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release/3.5.x
git worktree add -d .worktree/backport-12420-to-release/3.5.x origin/release/3.5.x
cd .worktree/backport-12420-to-release/3.5.x
git switch --create backport-12420-to-release/3.5.x
git cherry-pick -x 2516c5035f8a2406a3add38370b520f54aac6a11

@team-gateway-bot
Copy link
Collaborator

Successfully created backport PR for release/3.6.x:

@team-gateway-bot
Copy link
Collaborator

Git push to origin failed for release/3.6.x with exitcode 1

@team-gateway-bot
Copy link
Collaborator

Backport failed for release/3.4.x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release/3.4.x
git worktree add -d .worktree/backport-12420-to-release/3.4.x origin/release/3.4.x
cd .worktree/backport-12420-to-release/3.4.x
git switch --create backport-12420-to-release/3.4.x
git cherry-pick -x 2516c5035f8a2406a3add38370b520f54aac6a11

@team-gateway-bot
Copy link
Collaborator

Backport failed for release/3.5.x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release/3.5.x
git worktree add -d .worktree/backport-12420-to-release/3.5.x origin/release/3.5.x
cd .worktree/backport-12420-to-release/3.5.x
git switch --create backport-12420-to-release/3.5.x
git cherry-pick -x 2516c5035f8a2406a3add38370b520f54aac6a11

@team-gateway-bot
Copy link
Collaborator

Git push to origin failed for release/3.6.x with exitcode 1

bungle added a commit that referenced this pull request Feb 16, 2024
### Summary

The #12420 by @Water-Melon forgot to add `grpc_ssl_conf_command`.
This commit adds that.

Signed-off-by: Aapo Talvensaari <[email protected]>
Water-Melon added a commit that referenced this pull request Feb 19, 2024
- remove unsupported TLS versions from default configurations.
- support communication with old versions of OpenSSL clients using TLSv1.1.

KAG-3259

(cherry picked from commit 2516c50)
Water-Melon added a commit that referenced this pull request Feb 19, 2024
- remove unsupported TLS versions from default configurations.
- support communication with old versions of OpenSSL clients using TLSv1.1.

KAG-3259

(cherry picked from commit 2516c50)
bungle pushed a commit that referenced this pull request Feb 19, 2024
- remove unsupported TLS versions from default configurations.
- support communication with old versions of OpenSSL clients using TLSv1.1.

KAG-3259

(cherry picked from commit 2516c50)
bungle pushed a commit that referenced this pull request Feb 19, 2024
- remove unsupported TLS versions from default configurations.
- support communication with old versions of OpenSSL clients using TLSv1.1.

KAG-3259

(cherry picked from commit 2516c50)
bungle pushed a commit that referenced this pull request Feb 19, 2024
- remove unsupported TLS versions from default configurations.
- support communication with old versions of OpenSSL clients using TLSv1.1.

KAG-3259

(cherry picked from commit 2516c50)
Water-Melon pushed a commit that referenced this pull request Feb 20, 2024
The #12420 by @Water-Melon forgot to add `grpc_ssl_conf_command`.
This commit adds that.

Signed-off-by: Aapo Talvensaari <[email protected]>
(cherry picked from commit 84cb1be)
Water-Melon pushed a commit that referenced this pull request Feb 20, 2024
The #12420 by @Water-Melon forgot to add `grpc_ssl_conf_command`.
This commit adds that.

Signed-off-by: Aapo Talvensaari <[email protected]>
(cherry picked from commit 84cb1be)
Water-Melon pushed a commit that referenced this pull request Feb 20, 2024
### Summary

The #12420 by @Water-Melon forgot to add `grpc_ssl_conf_command`.
This commit adds that.

Signed-off-by: Aapo Talvensaari <[email protected]>
(cherry picked from commit 84cb1be)
ms2008 pushed a commit that referenced this pull request Feb 21, 2024
### Summary

The #12420 by @Water-Melon forgot to add `grpc_ssl_conf_command`.
This commit adds that.

Signed-off-by: Aapo Talvensaari <[email protected]>
(cherry picked from commit 84cb1be)
ms2008 pushed a commit that referenced this pull request Feb 21, 2024
The #12420 by @Water-Melon forgot to add `grpc_ssl_conf_command`.
This commit adds that.

Signed-off-by: Aapo Talvensaari <[email protected]>
(cherry picked from commit 84cb1be)
ms2008 pushed a commit that referenced this pull request Feb 21, 2024
The #12420 by @Water-Melon forgot to add `grpc_ssl_conf_command`.
This commit adds that.

Signed-off-by: Aapo Talvensaari <[email protected]>
(cherry picked from commit 84cb1be)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants