From e4e04b6767436a2369ff1bf7a42a338e1cc3c6ba Mon Sep 17 00:00:00 2001 From: Tsz-Wo Nicholas Sze Date: Mon, 18 Nov 2024 15:00:52 -0800 Subject: [PATCH 1/4] HADOOP-19342. SaslRpcServer.AuthMethod print INFO messages in client side. --- .../hadoop/security/SaslMechanismFactory.java | 2 +- .../apache/hadoop/security/SaslRpcServer.java | 33 +++++++-- .../hadoop/security/TestAuthMethod.java | 73 +++++++++++++++++++ 3 files changed, 101 insertions(+), 7 deletions(-) create mode 100644 hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestAuthMethod.java diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java index 65d336b32c6e4..9a913998091c0 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java @@ -44,7 +44,7 @@ public final class SaslMechanismFactory { LOG.debug("{} = {} (env)", SASL_MECHANISM_ENV, envValue); // conf - final Configuration conf = new Configuration(false); + final Configuration conf = new Configuration(); final String confValue = conf.get(HADOOP_SECURITY_SASL_MECHANISM_KEY, HADOOP_SECURITY_SASL_MECHANISM_DEFAULT); LOG.debug("{} = {} (conf)", HADOOP_SECURITY_SASL_MECHANISM_KEY, confValue); diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java index d7e1831b90cb4..0940ce2671a43 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java @@ -23,6 +23,7 @@ import java.io.DataInputStream; import java.io.DataOutput; import java.io.IOException; +import java.lang.reflect.Method; import java.nio.charset.StandardCharsets; import java.security.PrivilegedExceptionAction; import java.security.Security; @@ -41,6 +42,8 @@ import javax.security.sasl.SaslServer; import javax.security.sasl.SaslServerFactory; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import org.apache.commons.codec.binary.Base64; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; @@ -219,25 +222,43 @@ public static String[] splitKerberosName(String fullName) { return fullName.split("[/@]"); } + private static String getMechanism() { + // use reflection to avoid loading the class + final String className = "org.apache.hadoop.security.SaslMechanismFactory"; + final String methodName = "getMechanism"; + try { + final Method method = Class.forName(className).getMethod(methodName); + return method.invoke(null).toString(); + } catch (Exception e) { + throw new IllegalStateException("Failed to invoke " + methodName + " from " + className, e); + } + } + + /** Memoize the mechanism value. */ + private static final Supplier MECHANISM_SUPPLIER + = Suppliers.memoize(SaslRpcServer::getMechanism); + /** Authentication method */ @InterfaceStability.Evolving public enum AuthMethod { SIMPLE((byte) 80, ""), KERBEROS((byte) 81, "GSSAPI"), @Deprecated - DIGEST((byte) 82, SaslMechanismFactory.getMechanism()), - TOKEN((byte) 82, SaslMechanismFactory.getMechanism()), + DIGEST((byte) 82, MECHANISM_SUPPLIER), + TOKEN((byte) 82, MECHANISM_SUPPLIER), PLAIN((byte) 83, "PLAIN"); /** The code for this method. */ public final byte code; - public final String mechanismName; + private final Supplier mechanismName; private AuthMethod(byte code, String mechanismName) { + this(code, () -> mechanismName); + } + + AuthMethod(byte code, Supplier mechanismName) { this.code = code; this.mechanismName = mechanismName; - LOG.info("{} {}: code={}, mechanism=\"{}\"", - getClass().getSimpleName(), name(), code, mechanismName); } private static final int FIRST_CODE = values()[0].code; @@ -253,7 +274,7 @@ private static AuthMethod valueOf(byte code) { * @return mechanismName. */ public String getMechanismName() { - return mechanismName; + return mechanismName.get(); } /** diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestAuthMethod.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestAuthMethod.java new file mode 100644 index 0000000000000..cabbfa7029bdb --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestAuthMethod.java @@ -0,0 +1,73 @@ +/* + * 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. + */ +package org.apache.hadoop.security; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.test.GenericTestUtils; +import org.junit.Assert; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.slf4j.event.Level; + +import java.io.OutputStream; +import java.net.URLDecoder; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.nio.file.StandardOpenOption; + +import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_SASL_MECHANISM_KEY; + +/** Test {@link SaslRpcServer.AuthMethod}. */ +public class TestAuthMethod { + @Test + public void testConfiguration() throws Exception { + final Logger log = LoggerFactory.getLogger("org.apache.hadoop.security.SaslMechanismFactory"); + GenericTestUtils.setLogLevel(log, Level.TRACE); + + // init the mechanism + final String mechanism = "TEST-MECHANISM"; + initConf(mechanism, log); + + // test get from new conf + final Configuration conf = new Configuration(); + final String confValue = conf.get(HADOOP_SECURITY_SASL_MECHANISM_KEY); + log.info("getConf: {} = {}", HADOOP_SECURITY_SASL_MECHANISM_KEY, confValue); + Assert.assertEquals(mechanism, confValue); + + // test AuthMethod + Assert.assertEquals(mechanism, SaslRpcServer.AuthMethod.TOKEN.getMechanismName()); + + // the mechanism won't change after initialized + final String newMechanism = "NEW-MECHANISM"; + initConf(newMechanism, log); + Assert.assertEquals(mechanism, SaslRpcServer.AuthMethod.TOKEN.getMechanismName()); + } + + static void initConf(String mechanism, Logger log) throws Exception { + final Configuration conf = new Configuration(); + conf.set(HADOOP_SECURITY_SASL_MECHANISM_KEY, mechanism); + log.info("setConf: {} = {}", HADOOP_SECURITY_SASL_MECHANISM_KEY, mechanism); + + final String coreSite = URLDecoder.decode(conf.getResource("core-site.xml").getPath(), "UTF-8"); + try (OutputStream out = Files.newOutputStream(Paths.get(coreSite), StandardOpenOption.CREATE)) { + conf.writeXml(out); + } + Configuration.addDefaultResource(coreSite); + log.info("writeXml: {}", coreSite); + } +} \ No newline at end of file From c2b6ab49bc3f27c32fbfe29f643af707d48adb9e Mon Sep 17 00:00:00 2001 From: Tsz-Wo Nicholas Sze Date: Mon, 18 Nov 2024 18:53:58 -0800 Subject: [PATCH 2/4] Avoid using Guava. --- .../hadoop/security/SaslMechanismFactory.java | 8 ++++-- .../apache/hadoop/security/SaslRpcServer.java | 28 ++++--------------- 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java index 9a913998091c0..dc058781e319b 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java @@ -36,9 +36,9 @@ public final class SaslMechanismFactory { static final Logger LOG = LoggerFactory.getLogger(SaslMechanismFactory.class); private static final String SASL_MECHANISM_ENV = "HADOOP_SASL_MECHANISM"; - private static final String SASL_MECHANISM; + private static volatile String SASL_MECHANISM; - static { + private static synchronized String getSynchronously() { // env final String envValue = System.getenv(SASL_MECHANISM_ENV); LOG.debug("{} = {} (env)", SASL_MECHANISM_ENV, envValue); @@ -61,10 +61,12 @@ public final class SaslMechanismFactory { : confValue != null ? confValue : HADOOP_SECURITY_SASL_MECHANISM_DEFAULT; LOG.debug("SASL_MECHANISM = {} (effective)", SASL_MECHANISM); + return SASL_MECHANISM; } public static String getMechanism() { - return SASL_MECHANISM; + final String value = SASL_MECHANISM; + return value != null ? value : getSynchronously(); } public static boolean isDefaultMechanism(String mechanism) { diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java index 0940ce2671a43..62d115b018ae7 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java @@ -23,13 +23,13 @@ import java.io.DataInputStream; import java.io.DataOutput; import java.io.IOException; -import java.lang.reflect.Method; import java.nio.charset.StandardCharsets; import java.security.PrivilegedExceptionAction; import java.security.Security; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.function.Supplier; import javax.security.auth.callback.Callback; import javax.security.auth.callback.CallbackHandler; @@ -42,8 +42,6 @@ import javax.security.sasl.SaslServer; import javax.security.sasl.SaslServerFactory; -import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; import org.apache.commons.codec.binary.Base64; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; @@ -222,35 +220,19 @@ public static String[] splitKerberosName(String fullName) { return fullName.split("[/@]"); } - private static String getMechanism() { - // use reflection to avoid loading the class - final String className = "org.apache.hadoop.security.SaslMechanismFactory"; - final String methodName = "getMechanism"; - try { - final Method method = Class.forName(className).getMethod(methodName); - return method.invoke(null).toString(); - } catch (Exception e) { - throw new IllegalStateException("Failed to invoke " + methodName + " from " + className, e); - } - } - - /** Memoize the mechanism value. */ - private static final Supplier MECHANISM_SUPPLIER - = Suppliers.memoize(SaslRpcServer::getMechanism); - /** Authentication method */ @InterfaceStability.Evolving public enum AuthMethod { SIMPLE((byte) 80, ""), KERBEROS((byte) 81, "GSSAPI"), @Deprecated - DIGEST((byte) 82, MECHANISM_SUPPLIER), - TOKEN((byte) 82, MECHANISM_SUPPLIER), + DIGEST((byte) 82, SaslMechanismFactory::getMechanism), + TOKEN((byte) 82, SaslMechanismFactory::getMechanism), PLAIN((byte) 83, "PLAIN"); /** The code for this method. */ public final byte code; - private final Supplier mechanismName; + public final Supplier mechanismName; private AuthMethod(byte code, String mechanismName) { this(code, () -> mechanismName); @@ -259,6 +241,8 @@ private AuthMethod(byte code, String mechanismName) { AuthMethod(byte code, Supplier mechanismName) { this.code = code; this.mechanismName = mechanismName; + LOG.debug("{} {}: code={}, mechanism=\"{}\"", + getClass().getSimpleName(), name(), code, mechanismName); } private static final int FIRST_CODE = values()[0].code; From 5d3699f79163ce95f4457b8aa4fe4323ba7d96ce Mon Sep 17 00:00:00 2001 From: Tsz-Wo Nicholas Sze Date: Tue, 19 Nov 2024 00:21:51 -0800 Subject: [PATCH 3/4] Fix checkstyle. --- .../apache/hadoop/security/SaslMechanismFactory.java | 10 +++++----- .../java/org/apache/hadoop/security/SaslRpcServer.java | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java index dc058781e319b..6dbfb2c2c3c29 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java @@ -36,7 +36,7 @@ public final class SaslMechanismFactory { static final Logger LOG = LoggerFactory.getLogger(SaslMechanismFactory.class); private static final String SASL_MECHANISM_ENV = "HADOOP_SASL_MECHANISM"; - private static volatile String SASL_MECHANISM; + private static volatile String mechanism; private static synchronized String getSynchronously() { // env @@ -57,15 +57,15 @@ private static synchronized String getSynchronously() { } } - SASL_MECHANISM = envValue != null ? envValue + mechanism = envValue != null ? envValue : confValue != null ? confValue : HADOOP_SECURITY_SASL_MECHANISM_DEFAULT; - LOG.debug("SASL_MECHANISM = {} (effective)", SASL_MECHANISM); - return SASL_MECHANISM; + LOG.debug("SASL_MECHANISM = {} (effective)", mechanism); + return mechanism; } public static String getMechanism() { - final String value = SASL_MECHANISM; + final String value = mechanism; return value != null ? value : getSynchronously(); } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java index 62d115b018ae7..4144cb087ba54 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java @@ -232,7 +232,7 @@ public enum AuthMethod { /** The code for this method. */ public final byte code; - public final Supplier mechanismName; + private final Supplier mechanismName; private AuthMethod(byte code, String mechanismName) { this(code, () -> mechanismName); From 6d001e5a8795b1c13715c17521c03a96ec79b7e2 Mon Sep 17 00:00:00 2001 From: Tsz-Wo Nicholas Sze Date: Tue, 19 Nov 2024 09:48:24 -0800 Subject: [PATCH 4/4] Address review comments. --- .../hadoop/security/SaslMechanismFactory.java | 14 +--- .../apache/hadoop/security/SaslRpcServer.java | 2 - .../hadoop/security/TestAuthMethod.java | 73 ------------------- 3 files changed, 3 insertions(+), 86 deletions(-) delete mode 100644 hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestAuthMethod.java diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java index 6dbfb2c2c3c29..5b529e2a6052c 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslMechanismFactory.java @@ -17,7 +17,6 @@ */ package org.apache.hadoop.security; -import org.apache.hadoop.HadoopIllegalArgumentException; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; import org.apache.hadoop.conf.Configuration; @@ -49,14 +48,7 @@ private static synchronized String getSynchronously() { HADOOP_SECURITY_SASL_MECHANISM_DEFAULT); LOG.debug("{} = {} (conf)", HADOOP_SECURITY_SASL_MECHANISM_KEY, confValue); - if (envValue != null && confValue != null) { - if (!envValue.equals(confValue)) { - throw new HadoopIllegalArgumentException("SASL Mechanism mismatched: env " - + SASL_MECHANISM_ENV + " is " + envValue + " but conf " - + HADOOP_SECURITY_SASL_MECHANISM_KEY + " is " + confValue); - } - } - + // env has a higher precedence than conf mechanism = envValue != null ? envValue : confValue != null ? confValue : HADOOP_SECURITY_SASL_MECHANISM_DEFAULT; @@ -69,8 +61,8 @@ public static String getMechanism() { return value != null ? value : getSynchronously(); } - public static boolean isDefaultMechanism(String mechanism) { - return HADOOP_SECURITY_SASL_MECHANISM_DEFAULT.equals(mechanism); + public static boolean isDefaultMechanism(String saslMechanism) { + return HADOOP_SECURITY_SASL_MECHANISM_DEFAULT.equals(saslMechanism); } private SaslMechanismFactory() {} diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java index 4144cb087ba54..bb306836dc15e 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/SaslRpcServer.java @@ -241,8 +241,6 @@ private AuthMethod(byte code, String mechanismName) { AuthMethod(byte code, Supplier mechanismName) { this.code = code; this.mechanismName = mechanismName; - LOG.debug("{} {}: code={}, mechanism=\"{}\"", - getClass().getSimpleName(), name(), code, mechanismName); } private static final int FIRST_CODE = values()[0].code; diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestAuthMethod.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestAuthMethod.java deleted file mode 100644 index cabbfa7029bdb..0000000000000 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestAuthMethod.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * 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. - */ -package org.apache.hadoop.security; - -import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.test.GenericTestUtils; -import org.junit.Assert; -import org.junit.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.slf4j.event.Level; - -import java.io.OutputStream; -import java.net.URLDecoder; -import java.nio.file.Files; -import java.nio.file.Paths; -import java.nio.file.StandardOpenOption; - -import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_SASL_MECHANISM_KEY; - -/** Test {@link SaslRpcServer.AuthMethod}. */ -public class TestAuthMethod { - @Test - public void testConfiguration() throws Exception { - final Logger log = LoggerFactory.getLogger("org.apache.hadoop.security.SaslMechanismFactory"); - GenericTestUtils.setLogLevel(log, Level.TRACE); - - // init the mechanism - final String mechanism = "TEST-MECHANISM"; - initConf(mechanism, log); - - // test get from new conf - final Configuration conf = new Configuration(); - final String confValue = conf.get(HADOOP_SECURITY_SASL_MECHANISM_KEY); - log.info("getConf: {} = {}", HADOOP_SECURITY_SASL_MECHANISM_KEY, confValue); - Assert.assertEquals(mechanism, confValue); - - // test AuthMethod - Assert.assertEquals(mechanism, SaslRpcServer.AuthMethod.TOKEN.getMechanismName()); - - // the mechanism won't change after initialized - final String newMechanism = "NEW-MECHANISM"; - initConf(newMechanism, log); - Assert.assertEquals(mechanism, SaslRpcServer.AuthMethod.TOKEN.getMechanismName()); - } - - static void initConf(String mechanism, Logger log) throws Exception { - final Configuration conf = new Configuration(); - conf.set(HADOOP_SECURITY_SASL_MECHANISM_KEY, mechanism); - log.info("setConf: {} = {}", HADOOP_SECURITY_SASL_MECHANISM_KEY, mechanism); - - final String coreSite = URLDecoder.decode(conf.getResource("core-site.xml").getPath(), "UTF-8"); - try (OutputStream out = Files.newOutputStream(Paths.get(coreSite), StandardOpenOption.CREATE)) { - conf.writeXml(out); - } - Configuration.addDefaultResource(coreSite); - log.info("writeXml: {}", coreSite); - } -} \ No newline at end of file