Skip to content

feat(socket) change LuaSec ssl_protocol default options#12

Open
jeremymv2 wants to merge 1 commit intokong-devfrom
feat/ssl_version_opts
Open

feat(socket) change LuaSec ssl_protocol default options#12
jeremymv2 wants to merge 1 commit intokong-devfrom
feat/ssl_version_opts

Conversation

@jeremymv2
Copy link
Copy Markdown

The underlying LuaSec SSL library allows for setting the encryption protocol
to 'any' [1]. When set as such, the client negotiates the highest
encryption protocol available. This any protocol version setting is
widely in use in the luasec repository [2].

In addition, this change limits the lowest allowable ssl protocol
version to a version not less than TLSv1.1.

In support of the above, the implementation is two-fold:

  • add no_sslv2, no_sslv3, and no_tlsv1 LuaSec options similar to what
    has been done in lua-cassandra [3]
  • set default LuaSec ssl_protocol to 'any' also similar to what has
    been done in lua-cassandra [4]

[1] - https://github.com/brunoos/luasec/blob/711a98b7605ad87b521ba607024947113bc1f527/CHANGELOG#L101
[2] - https://github.com/brunoos/luasec/search?q=protocol+%3D+%22any%22
[3] - thibaultcha/lua-cassandra@b6dff88
[4] - thibaultcha/lua-cassandra@d742d5c

Signed-off-by: Jeremy J. Miller jeremy.miller@konghq.com

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 2, 2021

CLA assistant check
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.


Jeremy J. Miller seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@jeremymv2
Copy link
Copy Markdown
Author

jeremymv2 commented Mar 2, 2021

This closes https://konghq.atlassian.net/browse/FT-1682 cc: @Kong/team-fast-track

@jeremymv2
Copy link
Copy Markdown
Author

cc: @thibaultcha I think I could use your insight here on this change. It effectively boils down to the same fixes we made for lua-cassandra. Ultimately this will be needed to remove the pg_ssl_version (https://github.com/Kong/kong-ee/blob/fd6e2461e69976a23fdb7d7f4a2b06eae3c0aa08/kong.conf.default#L969) setting in Kong-ee.

thibaultcha
thibaultcha previously approved these changes Mar 2, 2021
Copy link
Copy Markdown
Member

@thibaultcha thibaultcha left a comment

Choose a reason for hiding this comment

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

Sounds good; we will also need to document the "no_sslv2", "no_sslv3", "no_tlsv1" options for database connections in the Kong release changelog.

The underlying LuaSec SSL library allows for setting the encryption protocol
to 'any' [1]. When set as such, the client negotiates the highest
encryption protocol available. This `any` protocol version setting is
widely in use in the luasec repository [2].

In addition, this change limits the lowest allowable ssl protocol
version to a version not less than TLSv1.1.

In support of the above, the implementation is two-fold:
 - add no_sslv2, no_sslv3, and no_tlsv1 LuaSec options similar to what
   has been done in lua-cassandra [3]
 - set default LuaSec ssl_protocol to 'any' also similar to what has
   been done in lua-cassandra [4]

[1] - https://github.com/brunoos/luasec/blob/711a98b7605ad87b521ba607024947113bc1f527/CHANGELOG#L101
[2] - https://github.com/brunoos/luasec/search?q=protocol+%3D+%22any%22
[3] - thibaultcha/lua-cassandra@b6dff88
[4] - thibaultcha/lua-cassandra@d742d5c

Signed-off-by: Jeremy J. Miller <jeremy.miller@konghq.com>

disable prefer server ciphers

Signed-off-by: Jeremy J. Miller <jeremy.miller@konghq.com>
@jeremymv2 jeremymv2 force-pushed the feat/ssl_version_opts branch from 61a1f2c to 2e89f11 Compare March 3, 2021 16:15
@jeremymv2 jeremymv2 changed the base branch from master to kong-dev March 3, 2021 16:18
@jeremymv2 jeremymv2 dismissed thibaultcha’s stale review March 3, 2021 16:18

The base branch was changed.

@jeremymv2
Copy link
Copy Markdown
Author

I wanted to do some triple checking specifically with Kong and this change. Everything checks out. With the change Kong can connect to PG9.5 and PG13 instances using TLSV1.1, TLSV1.2 and TLSV1.3 (in PG13).

When trying to connect with TLSv1 or lower you will get:
Error: [PostgreSQL error] failed to retrieve PostgreSQL server_version_num: no protocols available

I think this is ready to go!

Following this merge I will begin work on removing the pg_ssl_version Kong configuration (https://konghq.atlassian.net/browse/FT-1680) and bump the Kong pgmoon dep to the new released version of this repo such that Kong will connect at the highest TLS protocol version available when using pgmoon in the init phases.

@jeremymv2 jeremymv2 requested a review from a team March 3, 2021 20:10
@jeremymv2
Copy link
Copy Markdown
Author

@thibaultcha fyi, after some analysis and discussion with the team, I discovered we still must use this fork due to some changes we've added here that aren't in leafo/pgmoon 🙁

Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

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

would be nice to have tests for those no_ssl* options but this PR is in good state nevertheless!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants