-
Notifications
You must be signed in to change notification settings - Fork 13.9k
[FLINK-39022][security] Set security.ssl.algorithms default value to modern cipher suite #27514
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
base: master
Are you sure you want to change the base?
Changes from all commits
c31c5d3
8bfaffc
718818e
8623f7c
1699165
315c80c
f781c47
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| --- | ||
| title: "Release Notes - Flink 2.3" | ||
| --- | ||
|
|
||
| <!-- | ||
| Licensed to the Apache Software Foundation (ASF) under one | ||
| or more contributor license agreements. See the NOTICE file | ||
| distributed with this work for additional information | ||
| regarding copyright ownership. The ASF licenses this file | ||
| to you under the Apache License, Version 2.0 (the | ||
| "License"); you may not use this file except in compliance | ||
| with the License. You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, | ||
| software distributed under the License is distributed on an | ||
| "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| KIND, either express or implied. See the License for the | ||
| specific language governing permissions and limitations | ||
| under the License. | ||
| --> | ||
|
|
||
| # Release notes - Flink 2.3 | ||
|
|
||
| These release notes discuss important aspects, such as configuration, behavior or dependencies, | ||
| that changed between Flink 2.2 and Flink 2.3. Please read these notes carefully if you are | ||
| planning to upgrade your Flink version to 2.3. | ||
|
|
||
|
|
||
| ### Core | ||
|
|
||
| #### Set security.ssl.algorithms default value to modern cipher suite | ||
|
|
||
| ### [FLINK-39022](https://issues.apache.org/jira/browse/FLINK-39022) | ||
|
|
||
| A JDK update (affecting JDK 11.0.30+, 17.0.18+, 21.0.10+, and 24+) disabled `TLS_RSA_*` cipher suites. | ||
| This was done to support forward-secrecy (RFC 9325) and comply with the IETF Draft on *Deprecating Obsolete Key Exchange Methods in TLS*. | ||
|
|
||
| To support these and future JDK versions, the default value for the Flink configuration option `security.ssl.algorithms` has been changed to a modern, widely available cipher suite: | ||
|
|
||
| `TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384` | ||
|
|
||
| This default provides strong security and wide compatibility. You can customize the cipher suites using the `security.ssl.algorithms` configuration option if your environment has different requirements. | ||
| If these cipher suites are not supported on your setup, you will see that Flink processes will not be able to connect to each other. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -498,19 +498,20 @@ public static Configuration forProvider(Configuration configuration, String prov | |
| * The standard SSL algorithms to be supported. | ||
| * | ||
| * <p>More options here - | ||
| * http://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html#ciphersuites | ||
| * https://docs.oracle.com/en/java/javase/11/docs/specs/security/standard-names.html#jsse-cipher-suite-names | ||
| */ | ||
| @Documentation.Section(Documentation.Sections.SECURITY_SSL) | ||
| public static final ConfigOption<String> SSL_ALGORITHMS = | ||
| key("security.ssl.algorithms") | ||
| .stringType() | ||
| .defaultValue("TLS_RSA_WITH_AES_128_CBC_SHA") | ||
| .defaultValue( | ||
| "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be put out a warning if we find an algorithm that we consider insecure; specifically checking for the older algs we used to use?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We just want to make it more secure which will comply with the latest RFC. Not yet understand what insecure cipher you mean here🤔
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it make sense to you, that If a custom cipher is supplied that does not comply with the latest RFC, then we put out a warning to draw the users attention to something that looks insecure, because it does not comply with the latest RFC ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding such warnig is fair point but I don't understand how do you plan to do that phisically. The latest RFC suggests 4 suites. If the user add stronger/weaker than the suggested then how do you decide from Flink code whether we should give a warning or not? If you can highlight the logic then I would buy that. |
||
| .withDescription( | ||
| Description.builder() | ||
| .text( | ||
| "The comma separated list of standard SSL algorithms to be supported. Read more %s", | ||
| link( | ||
| "http://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html#ciphersuites", | ||
| "https://docs.oracle.com/en/java/javase/11/docs/specs/security/standard-names.html#jsse-cipher-suite-names", | ||
| "here")) | ||
| .build()); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -523,11 +523,6 @@ public static String getRestCertificateFingerprint( | |
| private static void addSslProviderConfig(Configuration config, String sslProvider) { | ||
| if (sslProvider.equalsIgnoreCase("OPENSSL")) { | ||
| OpenSsl.ensureAvailability(); | ||
|
|
||
| // Flink's default algorithm set is not available for openSSL - choose a different one: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question here
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TLSv1.2 is the default for security.ssl.protocol it and TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 is the new default value of security.ssl algorithm, therefore it is safe to remove here. |
||
| config.set( | ||
| SecurityOptions.SSL_ALGORITHMS, | ||
| "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"); | ||
| } | ||
| config.set(SecurityOptions.SSL_PROVIDER, sslProvider); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it safe to remove this line. Can we have a locked down setup with a reduced set of algorithms - that this warning could still be valid for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 on this, we should keep this mitigation for users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the sentences later are touching cipher customization but not telling what are be the visible symptoms. I would say we should add the visible symptoms into the upcoming new paragpraph together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, adding back the visible warning line.