ctubbsii commented on code in PR #3792:
URL: https://github.com/apache/accumulo/pull/3792#discussion_r1346783039
##########
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##########
@@ -797,8 +798,8 @@ public enum Property {
"A comma-separated list of disallowed SSL Ciphers, see"
+ " monitor.ssl.include.ciphers to allow ciphers",
"1.6.1"),
- MONITOR_SSL_INCLUDE_PROTOCOLS("monitor.ssl.include.protocols", "TLSv1.2",
PropertyType.STRING,
- "A comma-separate list of allowed SSL protocols", "1.5.3"),
+ MONITOR_SSL_INCLUDE_PROTOCOLS("monitor.ssl.include.protocols",
"TLSv1.2,TLSv1.3",
Review Comment:
Same comment as above. Moderately up-to-date browsers will be able to use
TLS 1.3, so this shouldn't be an issue as a default config. Users can change it
if they want.
```suggestion
MONITOR_SSL_INCLUDE_PROTOCOLS("monitor.ssl.include.protocols", "TLSv1.3",
```
##########
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##########
@@ -77,9 +77,10 @@ public enum Property {
"1.6.0"),
RPC_SSL_CIPHER_SUITES("rpc.ssl.cipher.suites", "", PropertyType.STRING,
"Comma separated list of cipher suites that can be used by accepted
connections", "1.6.1"),
- RPC_SSL_ENABLED_PROTOCOLS("rpc.ssl.server.enabled.protocols", "TLSv1.2",
PropertyType.STRING,
+ RPC_SSL_ENABLED_PROTOCOLS("rpc.ssl.server.enabled.protocols",
"TLSv1.2,TLSv1.3",
Review Comment:
We can just select TLSv1.3 by default. If users have need for both, they can
enable that themselves (for example, to support a mix of different versions of
Accumulo clients configured differently).
```suggestion
RPC_SSL_ENABLED_PROTOCOLS("rpc.ssl.server.enabled.protocols", "TLSv1.3",
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
