lukecwik commented on code in PR #24619: URL: https://github.com/apache/beam/pull/24619#discussion_r1060896515
########## CHANGES.md: ########## @@ -80,7 +83,9 @@ ## Bugfixes +* Fixed X (Java/Python) ([#X](https://github.com/apache/beam/issues/X)). Review Comment: ```suggestion ``` ########## sdks/java/container/java11/java11-security.properties: ########## @@ -0,0 +1,36 @@ +# 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. + +# Java 11 java.security properties file override for JVM +# base properties derived from: +# openjdk version "11.0.16" 2022-07-19 +# OpenJDK Runtime Environment 18.9 (build 11.0.16+8) +# OpenJDK 64-Bit Server VM 18.9 (build 11.0.16+8, mixed mode, sharing) + +# Java has now disabled TLSv1 and TLSv1.1. We specifically put it in the +# legacy algorithms list to allow it to be used if something better is not +# available (e.g. TLSv1.2). This will prevent breakages for existing users +# (for example JDBC with MySQL). See +# https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8202343 +# for additional details. +jdk.tls.disabledAlgorithms=SSLv3, RC4, DES, MD5withRSA, \ + DH keySize < 1024, EC keySize < 224, 3DES_EDE_CBC, anon, NULL, \ + include jdk.disabled.namedCurves + +jdk.tls.legacyAlgorithms= \ + K_NULL, C_NULL, M_NULL, \ + DH_anon, ECDH_anon, \ + RC4_128, RC4_40, DES_CBC, DES40_CBC, \ + 3DES_EDE_CBC, TLSv1, TLSv1.1 Review Comment: ```suggestion 3DES_EDE_CBC, TLSv1, TLSv1.1 # /dev/random blocks in virtualized environments due to lack of # good entropy sources, which makes SecureRandom use impractical. # In particular, that affects the performance of HTTPS that relies # on SecureRandom. # # Due to that, /dev/urandom is used as the default. # # See http://www.2uo.de/myths-about-urandom/ for some background # on security of /dev/urandom on Linux. securerandom.source=file:/dev/./urandom ``` ########## sdks/java/container/java17/option-java17-security.json: ########## @@ -0,0 +1,10 @@ +{ + "name": "java-security", + "enabled": true, + "options": { + "properties": { + "java.security.properties": "/opt/apache/beam/options/java17-security.properties" + } Review Comment: fix indentation ########## sdks/java/container/java11/option-java11-security.json: ########## @@ -0,0 +1,10 @@ +{ + "name": "java-security", + "enabled": true, Review Comment: please fix indentation ########## sdks/java/core/src/test/java/org/apache/beam/sdk/SdkHarnessEnvironmentTest.java: ########## @@ -66,4 +69,30 @@ public void testJammAgentAvailable() throws Exception { PAssert.that(output).containsInAnyOrder("measured"); p.run().waitUntilFinish(); } + + /** {@link DoFn} used to validate that TLS was enabled as part of java security properties. */ + private static class TLSDoFn extends DoFn<String, String> { + @ProcessElement + public void processElement(ProcessContext c) { + assertThat( + Security.getProperty("jdk.tls.disabledAlgorithms").split(",[ ]*"), + not(hasItemInArray("TLSv1"))); + assertThat( + Security.getProperty("jdk.tls.disabledAlgorithms").split(",[ ]*"), + not(hasItemInArray("TLSv1.1"))); Review Comment: ```suggestion assertNotNull(SSLContext.getInstance("TLSv1")); assertNotNull(SSLContext.getInstance("TLSv1.1")); ``` ########## sdks/java/container/java17/java17-security.properties: ########## @@ -0,0 +1,36 @@ +# 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. + +# Java 17 java.security properties file override for JVM +# base properties derived from: +# openjdk version "17.0.2" 2022-01-18 +# OpenJDK Runtime Environment (build 17.0.2+8-86) +# OpenJDK 64-Bit Server VM (build 17.0.2+8-86, mixed mode, sharing) + +# Java has now disabled TLSv1 and TLSv1.1. We specifically put it in the +# legacy algorithms list to allow it to be used if something better is not +# available (e.g. TLSv1.2). This will prevent breakages for existing users +# (for example JDBC with MySQL). See +# https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8202343 +# for additional details. +jdk.tls.disabledAlgorithms=SSLv3, RC4, DES, MD5withRSA, \ + DH keySize < 1024, EC keySize < 224, 3DES_EDE_CBC, anon, NULL + +# The raw value from 17.0.2 for legacyAlgorithms is +# NULL, anon, RC4, DES, 3DES_EDE_CBC +# Because these values are in disabledAlgorithms, it is erroneous to include +# them in legacy (they are disabled in Java 8 and Java 11 as well). Here we +# only include TLSv1 and TLSv1.1 which were removed from disabledAlgorithms +jdk.tls.legacyAlgorithms=TLSv1, TLSv1.1 Review Comment: ```suggestion jdk.tls.legacyAlgorithms=TLSv1, TLSv1.1 # /dev/random blocks in virtualized environments due to lack of # good entropy sources, which makes SecureRandom use impractical. # In particular, that affects the performance of HTTPS that relies # on SecureRandom. # # Due to that, /dev/urandom is used as the default. # # See http://www.2uo.de/myths-about-urandom/ for some background # on security of /dev/urandom on Linux. securerandom.source=file:/dev/./urandom ``` ########## sdks/java/container/java8/option-java8-security.json: ########## @@ -0,0 +1,10 @@ +{ + "name": "java-security", + "enabled": true, + "options": { + "properties": { + "java.security.properties": "/opt/apache/beam/options/java8-security.properties" Review Comment: fix indentation ########## sdks/java/container/java8/java8-security.properties: ########## @@ -0,0 +1,36 @@ +# 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. + +# Java 8 java.security properties file override for JVM +# base properties derived from: +# openjdk version "1.8.0_342" +# OpenJDK Runtime Environment (build 1.8.0_342-b07) +# OpenJDK 64-Bit Server VM (build 25.342-b07, mixed mode) + +# Java has now disabled TLSv1 and TLSv1.1. We specifically put it in the +# legacy algorithms list to allow it to be used if something better is not +# available (e.g. TLSv1.2). This will prevent breakages for existing users +# (for example JDBC with MySQL). See +# https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8202343 +# for additional details. +jdk.tls.disabledAlgorithms=SSLv3, RC4, DES, MD5withRSA, \ + DH keySize < 1024, EC keySize < 224, 3DES_EDE_CBC, anon, NULL, \ + include jdk.disabled.namedCurves + +jdk.tls.legacyAlgorithms= \ + K_NULL, C_NULL, M_NULL, \ + DH_anon, ECDH_anon, \ + RC4_128, RC4_40, DES_CBC, DES40_CBC, \ + 3DES_EDE_CBC, TLSv1, TLSv1.1 Review Comment: ```suggestion 3DES_EDE_CBC, TLSv1, TLSv1.1 # /dev/random blocks in virtualized environments due to lack of # good entropy sources, which makes SecureRandom use impractical. # In particular, that affects the performance of HTTPS that relies # on SecureRandom. # # Due to that, /dev/urandom is used as the default. # # See http://www.2uo.de/myths-about-urandom/ for some background # on security of /dev/urandom on Linux. securerandom.source=file:/dev/./urandom ``` -- 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]
