Skip to content

Require internal shared secret is set when authentication is enabled#11944

Merged
dain merged 1 commit intotrinodb:masterfrom
dain:require-internal-secret
Apr 16, 2022
Merged

Require internal shared secret is set when authentication is enabled#11944
dain merged 1 commit intotrinodb:masterfrom
dain:require-internal-secret

Conversation

@dain
Copy link
Member

@dain dain commented Apr 13, 2022

Description

Require internal shared secret is set when authentication is enabled. This prevents configuration errors where the client to server interface is secured, but the server to server interface is not.

Documentation

( ) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
(X) Documentation issue #issuenumber is filed, and can be handled later.

The existing security documentation will need to be updated to note that the secret is required when configuring authentication. There are many pages with instructions for setting up authentication and it is not clear where this note should go.

Release notes

( ) No release notes entries required.
(X) Release notes entries required with the following suggested text:

# General
* The ``internal-communication.shared-secret`` must now be set when authentication installed. ({issue}`11944`)

@cla-bot cla-bot bot added the cla-signed label Apr 13, 2022
@dain dain requested a review from electrum April 13, 2022 23:05
@dain dain force-pushed the require-internal-secret branch from 18752e0 to 270f7ad Compare April 14, 2022 00:46
@hashhar hashhar requested a review from kokosing April 14, 2022 06:21
@findepi
Copy link
Member

findepi commented Apr 14, 2022

This prevents configuration errors where the client to server interface is secured, but the server to server interface is not.

It used to be an allowed configuration. Is it no longer so?

@dain
Copy link
Member Author

dain commented Apr 14, 2022

This prevents configuration errors where the client to server interface is secured, but the server to server interface is not.

It used to be an allowed configuration. Is it no longer so?

You can still have HTTPS client/server and HTTP server/server, but you now must explicitly set the shared secret when authentication is installed (no defaults)

Copy link
Member

Choose a reason for hiding this comment

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

Nit: don’t use “is” for Boolean fields, only for getters.

Copy link
Member

Choose a reason for hiding this comment

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

Capitalize HTTPS

@dain dain force-pushed the require-internal-secret branch from 270f7ad to 03bfd1a Compare April 14, 2022 22:23
@dain dain merged commit feb3b3d into trinodb:master Apr 16, 2022
@dain dain deleted the require-internal-secret branch April 16, 2022 01:17
@github-actions github-actions bot added this to the 378 milestone Apr 16, 2022
@@ -13,6 +13,7 @@
*/
Copy link
Member

Choose a reason for hiding this comment

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

Should we update docs that this is now required?

CC: @mosabua

Copy link
Member

Choose a reason for hiding this comment

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

yes .. on it

Copy link
Member

Choose a reason for hiding this comment

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

nmahadevuni added a commit to nmahadevuni/presto that referenced this pull request Jun 16, 2023
Supports JWT for authentication of internal requests.
This is required for secure internal communication,
especially when used in conjunction with external
user authentication such as PASSWORD, LDAP etc

Cherry-pick of trinodb/trino#2032
Cherry-pick of trinodb/trino#2090
Cherry-pick of trinodb/trino#2093
Cherry-pick of trinodb/trino#2202
Cherry-pick of trinodb/trino#11944

Co-authored-by: Dain Sundstrom <dain@iq80.com>
tdcmeehan pushed a commit to prestodb/presto that referenced this pull request Jun 22, 2023
Supports JWT for authentication of internal requests.
This is required for secure internal communication,
especially when used in conjunction with external
user authentication such as PASSWORD, LDAP etc

Cherry-pick of trinodb/trino#2032
Cherry-pick of trinodb/trino#2090
Cherry-pick of trinodb/trino#2093
Cherry-pick of trinodb/trino#2202
Cherry-pick of trinodb/trino#11944

Co-authored-by: Dain Sundstrom <dain@iq80.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants