Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,12 @@ public class CommonConfigurationKeysPublic {
*/
public static final String HADOOP_RPC_PROTECTION =
"hadoop.rpc.protection";
public static final String HADOOP_SECURITY_SASL_MECHANISM_KEY
= "hadoop.security.sasl.mechanism";
public static final String HADOOP_SECURITY_SASL_MECHANISM_DEFAULT
= "DIGEST-MD5";
public static final String HADOOP_SECURITY_SASL_CUSTOMIZEDCALLBACKHANDLER_CLASS_KEY
= "hadoop.security.sasl.CustomizedCallbackHandler.class";
/** Class to override Sasl Properties for a connection */
public static final String HADOOP_SECURITY_SASL_PROPS_RESOLVER_CLASS =
"hadoop.security.saslproperties.resolver.class";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@
import org.apache.hadoop.ipc.protobuf.RpcHeaderProtos.RPCTraceInfoProto;
import org.apache.hadoop.net.NetUtils;
import org.apache.hadoop.security.AccessControlException;
import org.apache.hadoop.security.SaslConstants;
import org.apache.hadoop.security.SaslMechanismFactory;
import org.apache.hadoop.security.SaslPropertiesResolver;
import org.apache.hadoop.security.SaslRpcServer;
import org.apache.hadoop.security.SaslRpcServer.AuthMethod;
Expand Down Expand Up @@ -2143,6 +2143,10 @@ public Server getServer() {
return Server.this;
}

public Configuration getConf() {
return Server.this.getConf();
}

/* Return true if the connection has no outstanding rpc */
private boolean isIdle() {
return rpcCount.get() == 0;
Expand Down Expand Up @@ -2606,7 +2610,7 @@ private RpcSaslProto buildSaslNegotiateResponse()
// accelerate token negotiation by sending initial challenge
// in the negotiation response
if (enabledAuthMethods.contains(AuthMethod.TOKEN)
&& SaslConstants.SASL_MECHANISM_DEFAULT.equals(AuthMethod.TOKEN.getMechanismName())) {
&& SaslMechanismFactory.isDefaultMechanism(AuthMethod.TOKEN.getMechanismName())) {
saslServer = createSaslServer(AuthMethod.TOKEN);
byte[] challenge = saslServer.evaluateResponse(new byte[0]);
RpcSaslProto.Builder negotiateBuilder =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,80 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.hadoop.hdfs.protocol.datatransfer.sasl;
package org.apache.hadoop.security;

import org.apache.hadoop.conf.Configuration;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.security.auth.callback.Callback;
import javax.security.auth.callback.UnsupportedCallbackException;
import java.io.IOException;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/** For handling customized {@link Callback}. */
public interface CustomizedCallbackHandler {
class DefaultHandler implements CustomizedCallbackHandler{
Logger LOG = LoggerFactory.getLogger(CustomizedCallbackHandler.class);

class Cache {
private static final Map<String, CustomizedCallbackHandler> MAP = new HashMap<>();

private static synchronized CustomizedCallbackHandler getSynchronously(
String key, Configuration conf) {
//check again synchronously
final CustomizedCallbackHandler cached = MAP.get(key);
if (cached != null) {
return cached; //cache hit
}

//cache miss
final Class<?> clazz = conf.getClass(key, DefaultHandler.class);
LOG.info("{} = {}", key, clazz);
if (clazz == DefaultHandler.class) {
return DefaultHandler.INSTANCE;
}

final Object created;
try {
created = clazz.newInstance();
} catch (Exception e) {
LOG.warn("Failed to create a new instance of {}, fallback to {}",
clazz, DefaultHandler.class, e);
return DefaultHandler.INSTANCE;
}

final CustomizedCallbackHandler handler = created instanceof CustomizedCallbackHandler ?
(CustomizedCallbackHandler) created : CustomizedCallbackHandler.delegate(created);
MAP.put(key, handler);
return handler;
}

private static CustomizedCallbackHandler get(String key, Configuration conf) {
final CustomizedCallbackHandler cached = MAP.get(key);
return cached != null ? cached : getSynchronously(key, conf);
}

public static synchronized void clear() {
MAP.clear();
}

private Cache() { }
}

class DefaultHandler implements CustomizedCallbackHandler {
private static final DefaultHandler INSTANCE = new DefaultHandler();

@Override
public void handleCallbacks(List<Callback> callbacks, String username, char[] password)
throws UnsupportedCallbackException {
if (!callbacks.isEmpty()) {
throw new UnsupportedCallbackException(callbacks.get(0));
final Callback cb = callbacks.get(0);
throw new UnsupportedCallbackException(callbacks.get(0),
"Unsupported callback: " + (cb == null ? null : cb.getClass()));
}
}
}
Expand All @@ -55,6 +112,10 @@ static CustomizedCallbackHandler delegate(Object delegated) {
};
}

static CustomizedCallbackHandler get(String key, Configuration conf) {
return Cache.get(key, conf);
}

void handleCallbacks(List<Callback> callbacks, String name, char[] password)
throws UnsupportedCallbackException, IOException;
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* 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.HadoopIllegalArgumentException;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.conf.Configuration;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_SASL_MECHANISM_DEFAULT;
import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_SASL_MECHANISM_KEY;

/**
* SASL related constants.
*/
@InterfaceAudience.LimitedPrivate({"HDFS", "MapReduce"})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we open this open for YARN too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jojochuang , good point! We should also add hbase.

@InterfaceStability.Evolving
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;

static {
// env
final String envValue = System.getenv(SASL_MECHANISM_ENV);
LOG.debug("{} = {} (env)", SASL_MECHANISM_ENV, envValue);

// conf
final Configuration conf = new Configuration();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a little concerned about this one.
If an application loads configuration from a custom path, SaslMechanismFactory initialized this way will load configurations from default path, and results in unexpected behavior to the user.

And that is a potential security issue. I had some grief over similar problems before, see: HADOOP-13638

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I seems to recall similar problem so that I did not use conf previously. Cc @stoty

@jojochuang , If we use new Configuration(false), the value is not set when the application loads configuration from a custom path. That application must set env variable. Is it okay?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't follow the issue here.
Is it about adding config files to the classpath dynamically ?
Is SaslMechanismFactory reading the Configuration too early ?

It should be possble (with more changes) to load the mechanism lazily, when SASL is used first.

IMO having to configure this from an environment variable would be an operational nightmare.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be possble (with more changes) to load the mechanism lazily, when SASL is used first.

The is already loaded mechanism lazily. The class won't be loaded unless it is used.

In our case, it should be fine since conf will be loaded during a SASL server/client is being created. The conf path must be set at that time. (UserGroupInformation has a different story since it may have to logged in before starting a server/client.)

IMO having to configure this from an environment variable would be an operational nightmare.

Hadoop security tends to use environment variable since conf may not available.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest of PR looks good to me. This part I'm not sure. So what about the normal case, like YARN Resource Manager, do they also get the value from env variable? Then what's the use of the configuration object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... So what about the normal case, like YARN Resource Manager, do they also get the value ?

They can get it either from env variable or from conf.

Then what's the use of the configuration object?

@stoty has pointed that env variable is inconvenient since the other RPC parameters are specified using conf. He has tested it with existing code. BTW, he is working on changing HBase asyncfs for the configurable SASL mechanism.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jojochuang , @stoty , unfortunately, UserGroupInformation is statically referring to SaslMechanismFactory (via SaslRpcServer.AuthMethod) as @steveloughran discovered. We may need to remove the conf support from SaslMechanismFactory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are able to use reflection to implement lazy loading of SaslMechanismFactory; see #7173

final String confValue = conf.get(HADOOP_SECURITY_SASL_MECHANISM_KEY,
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);
}
}

SASL_MECHANISM = envValue != null ? envValue
: confValue != null ? confValue
: HADOOP_SECURITY_SASL_MECHANISM_DEFAULT;
LOG.debug("SASL_MECHANISM = {} (effective)", SASL_MECHANISM);
}

public static String getMechanism() {
return SASL_MECHANISM;
}

public static boolean isDefaultMechanism(String mechanism) {
return HADOOP_SECURITY_SASL_MECHANISM_DEFAULT.equals(mechanism);
}

private SaslMechanismFactory() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
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 javax.security.auth.callback.Callback;
Expand All @@ -43,16 +45,16 @@
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.ipc.RetriableException;
import org.apache.hadoop.ipc.Server;
import org.apache.hadoop.ipc.Server.Connection;
import org.apache.hadoop.ipc.StandbyException;
import org.apache.hadoop.security.token.SecretManager;
import org.apache.hadoop.security.token.SecretManager.InvalidToken;
import org.apache.hadoop.security.token.TokenIdentifier;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_SASL_CUSTOMIZEDCALLBACKHANDLER_CLASS_KEY;

/**
* A utility class for dealing with SASL on RPC server
*/
Expand Down Expand Up @@ -223,8 +225,8 @@ public enum AuthMethod {
SIMPLE((byte) 80, ""),
KERBEROS((byte) 81, "GSSAPI"),
@Deprecated
DIGEST((byte) 82, SaslConstants.SASL_MECHANISM),
TOKEN((byte) 82, SaslConstants.SASL_MECHANISM),
DIGEST((byte) 82, SaslMechanismFactory.getMechanism()),
TOKEN((byte) 82, SaslMechanismFactory.getMechanism()),
PLAIN((byte) 83, "PLAIN");

/** The code for this method. */
Expand All @@ -234,6 +236,8 @@ public enum AuthMethod {
private AuthMethod(byte code, String 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;
Expand Down Expand Up @@ -276,28 +280,44 @@ public void write(DataOutput out) throws IOException {
/** CallbackHandler for SASL mechanism. */
@InterfaceStability.Evolving
public static class SaslDigestCallbackHandler implements CallbackHandler {
private final CustomizedCallbackHandler customizedCallbackHandler;
private SecretManager<TokenIdentifier> secretManager;
private Server.Connection connection;

public SaslDigestCallbackHandler(
SecretManager<TokenIdentifier> secretManager,
Server.Connection connection) {
this(secretManager, connection, connection.getConf());
}

public SaslDigestCallbackHandler(
SecretManager<TokenIdentifier> secretManager,
Server.Connection connection,
Configuration conf) {
this.secretManager = secretManager;
this.connection = connection;
this.customizedCallbackHandler = CustomizedCallbackHandler.get(
HADOOP_SECURITY_SASL_CUSTOMIZEDCALLBACKHANDLER_CLASS_KEY, conf);
}

private char[] getPassword(TokenIdentifier tokenid) throws InvalidToken,
StandbyException, RetriableException, IOException {
private char[] getPassword(TokenIdentifier tokenid) throws IOException {
return encodePassword(secretManager.retriableRetrievePassword(tokenid));
}

private char[] getPassword(String name) throws IOException {
final TokenIdentifier tokenIdentifier = getIdentifier(name, secretManager);
final UserGroupInformation user = tokenIdentifier.getUser();
connection.attemptingUser = user;
LOG.debug("SASL server callback: setting password for client: {}", user);
return getPassword(tokenIdentifier);
}

@Override
public void handle(Callback[] callbacks) throws InvalidToken,
UnsupportedCallbackException, StandbyException, RetriableException,
IOException {
public void handle(Callback[] callbacks) throws UnsupportedCallbackException, IOException {
NameCallback nc = null;
PasswordCallback pc = null;
AuthorizeCallback ac = null;
List<Callback> unknownCallbacks = null;
for (Callback callback : callbacks) {
if (callback instanceof AuthorizeCallback) {
ac = (AuthorizeCallback) callback;
Expand All @@ -308,20 +328,14 @@ public void handle(Callback[] callbacks) throws InvalidToken,
} else if (callback instanceof RealmCallback) {
continue; // realm is ignored
} else {
throw new UnsupportedCallbackException(callback,
"Unrecognized SASL Callback");
if (unknownCallbacks == null) {
unknownCallbacks = new ArrayList<>();
}
unknownCallbacks.add(callback);
}
}
if (pc != null) {
TokenIdentifier tokenIdentifier = getIdentifier(nc.getDefaultName(),
secretManager);
char[] password = getPassword(tokenIdentifier);
UserGroupInformation user = null;
user = tokenIdentifier.getUser(); // may throw exception
connection.attemptingUser = user;

LOG.debug("SASL server callback: setting password for client: {}", user);
pc.setPassword(password);
pc.setPassword(getPassword(nc.getDefaultName()));
}
if (ac != null) {
String authid = ac.getAuthenticationID();
Expand All @@ -341,6 +355,11 @@ public void handle(Callback[] callbacks) throws InvalidToken,
ac.setAuthorizedID(authzid);
}
}
if (unknownCallbacks != null) {
final String name = nc != null ? nc.getDefaultName() : null;
final char[] password = name != null ? getPassword(name) : null;
customizedCallbackHandler.handleCallbacks(unknownCallbacks, name, password);
}
}
}

Expand Down
Loading
Loading