Move cipher note from kerb to tls page by Ordinant · Pull Request #10579 · trinodb/trino

Ordinant · 2022-01-12T20:11:20Z

No description provided.

docs/src/main/sphinx/security/kerberos.rst

docs/src/main/sphinx/security/tls.rst

mosabua · 2022-01-21T17:58:15Z

@dain @electrum @hashhar can we move this forward?

dain

looks good to me

Ordinant · 2022-01-31T22:19:39Z

Dain approved! Who's going to merge this in, please?

hashhar

LGTM since this is a lift-and-shift.

Left some comments since this could use much work - for someone unfamiliar with TLS configs there is some "not to the point" talk + some contradictions.

hashhar · 2022-02-01T00:15:15Z

docs/src/main/sphinx/security/tls.rst


+.. _tls-version-and-ciphers:
+
+Supported standards


Very weird title IMO.

"Supported TLS versions and Cipher suites" is more factual and actually conveys information.

hashhar · 2022-02-01T00:18:59Z

docs/src/main/sphinx/security/tls.rst

+  echo "java.util.Arrays.asList(((javax.net.ssl.SSLServerSocketFactory) \
+  javax.net.ssl.SSLServerSocketFactory.getDefault()).getSupportedCipherSuites()).stream().forEach(System.out::println)" | jshell -
+
+The default Trino server specifies a set of regular expressions that exclude


Suggested change

The default Trino server specifies a set of regular expressions that exclude

By default Trino is configured to exclude

The configs don't use regex at all. It's just a list.

hashhar · 2022-02-01T00:38:53Z

docs/src/main/sphinx/security/tls.rst

+using TLS 1.2 and TLS 1.3 certificates. The server rejects TLS 1.1, TLS 1.0, and
+all SSL format certificates.
+
+The Trino server does not specify a set of supported ciphers, instead deferring


This statement and the next paragraph are conflicting. Either Trino can exclude and include ciphers or it cannot.

hashhar · 2022-02-01T00:39:35Z

docs/src/main/sphinx/security/tls.rst

+to the defaults set by the JVM version in use. The documentation for Java 11
+lists its `supported cipher suites


Since we talk vendors we should be extra clear when referring to examples here.

Suggested change

to the defaults set by the JVM version in use. The documentation for Java 11

lists its `supported cipher suites

to the defaults set by the JVM version in use. For example Oracle JDK 11

lists its `supported cipher suites

hashhar · 2022-02-01T00:40:16Z

docs/src/main/sphinx/security/tls.rst

+Run the following two-line code on the same JVM from the same vendor as
+configured on the coordinator to determine that JVM's default cipher list.


Suggested change

Run the following two-line code on the same JVM from the same vendor as

configured on the coordinator to determine that JVM's default cipher list.

Run the following code on the same JVM as used by

the coordinator to determine that JVM's default cipher list.

hashhar · 2022-02-01T00:43:14Z

docs/src/main/sphinx/security/tls.rst

+older cipher suites that do not support forward secrecy (FS).
+
+Use the ``http-server.https.included-cipher`` property to specify a
+comma-separated list of ciphers in preferred use order. If one of your preferred


Suggested change

comma-separated list of ciphers in preferred use order. If one of your preferred

comma-separated list of ciphers in order of preferred use. If one of your preferred

hashhar · 2022-02-01T00:44:07Z

docs/src/main/sphinx/security/tls.rst

+
+.. note::
+
+    If you manage the coordinator's direct TLS implementatation, monitor the CPU


What a "direct TLS implementation"?

hashhar · 2022-02-01T00:46:26Z

docs/src/main/sphinx/security/tls.rst

+
+    If you manage the coordinator's direct TLS implementatation, monitor the CPU
+    usage on the Trino coordinator after enabling HTTPS. Java prefers the more
+    CPU-intensive cipher suites, if you allow it to choose from a big list of


this comma is redundant

hashhar · 2022-02-01T00:47:07Z

docs/src/main/sphinx/security/tls.rst

+    usage on the Trino coordinator after enabling HTTPS. Java prefers the more
+    CPU-intensive cipher suites, if you allow it to choose from a big list of
+    ciphers. If the CPU usage is unacceptably high after enabling HTTPS, you can
+    configure Java to use specific cipher suites as described in this section.


Can be read to mean to verbatim set the config shared above which I don't beleive is what we intend to mean here.

hashhar · 2022-02-01T00:48:00Z

docs/src/main/sphinx/security/tls.rst

+    However, best practice is to instead use an external load balancer, as
+    discussed next.


Suggested change

However, best practice is to instead use an external load balancer, as

discussed next.

However, best practice is to terminate TLS at a load balancer instead as

discussed next.

hashhar · 2022-02-01T00:49:54Z

@jhlodin Can you kindly create an issue so that we remember to follow up on this?

cla-bot bot added the cla-signed label Jan 12, 2022

Ordinant requested review from dain, electrum and mosabua January 12, 2022 20:14

Ordinant added the docs label Jan 12, 2022

mosabua requested changes Jan 13, 2022

View reviewed changes

Ordinant force-pushed the bw/mvcipher branch 4 times, most recently from c84f02d to f24b10b Compare January 13, 2022 21:48

mosabua approved these changes Jan 14, 2022

View reviewed changes

electrum reviewed Jan 14, 2022

View reviewed changes

docs/src/main/sphinx/security/tls.rst Outdated Show resolved Hide resolved

Ordinant force-pushed the bw/mvcipher branch from f24b10b to e294486 Compare January 14, 2022 21:44

electrum reviewed Jan 14, 2022

View reviewed changes

docs/src/main/sphinx/security/tls.rst Outdated Show resolved Hide resolved

Ordinant force-pushed the bw/mvcipher branch 2 times, most recently from b15140a to ff11970 Compare January 18, 2022 21:53

Move cipher note from kerb to tls pages

e8dcb58

Ordinant force-pushed the bw/mvcipher branch from ff11970 to e8dcb58 Compare January 24, 2022 18:56

dain reviewed Jan 31, 2022

View reviewed changes

hashhar approved these changes Feb 1, 2022

View reviewed changes

hashhar merged commit 1db50ca into trinodb:master Feb 1, 2022

github-actions bot added this to the 370 milestone Feb 1, 2022

mosabua mentioned this pull request Feb 2, 2022

Add Trino 370 release notes #10793

Merged

	The default Trino server specifies a set of regular expressions that exclude
	By default Trino is configured to exclude

		to the defaults set by the JVM version in use. The documentation for Java 11
		lists its `supported cipher suites

		Run the following two-line code on the same JVM from the same vendor as
		configured on the coordinator to determine that JVM's default cipher list.

	comma-separated list of ciphers in preferred use order. If one of your preferred
	comma-separated list of ciphers in order of preferred use. If one of your preferred


		.. note::

		If you manage the coordinator's direct TLS implementatation, monitor the CPU

		However, best practice is to instead use an external load balancer, as
		discussed next.

Conversation

Ordinant commented Jan 12, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mosabua commented Jan 21, 2022

Uh oh!

dain left a comment

Choose a reason for hiding this comment

Uh oh!

Ordinant commented Jan 31, 2022

Uh oh!

hashhar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hashhar Feb 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hashhar commented Feb 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

hashhar Feb 1, 2022 •

edited

Loading