[ https://issues.apache.org/jira/browse/KNOX-3048?focusedWorklogId=972663&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-972663 ]
ASF GitHub Bot logged work on KNOX-3048: ---------------------------------------- Author: ASF GitHub Bot Created on: 11/Jun/25 20:27 Start Date: 11/Jun/25 20:27 Worklog Time Spent: 10m Work Description: pzampino commented on code in PR #1043: URL: https://github.com/apache/knox/pull/1043#discussion_r2140990876 ########## gateway-spi/src/main/java/org/apache/knox/gateway/util/GroupBasedImpersonationProvider.java: ########## @@ -0,0 +1,263 @@ +/* + * 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 + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * 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.knox.gateway.util; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.security.UserGroupInformation; +import org.apache.hadoop.security.authorize.AccessControlList; +import org.apache.hadoop.security.authorize.AuthorizationException; +import org.apache.hadoop.security.authorize.DefaultImpersonationProvider; +import org.apache.hadoop.util.MachineList; +import org.apache.knox.gateway.i18n.GatewaySpiMessages; +import org.apache.knox.gateway.i18n.messages.MessagesFactory; + +import java.net.InetAddress; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +import static org.apache.knox.gateway.util.AuthFilterUtils.PROXYGROUP_PREFIX; +import static org.apache.knox.gateway.util.AuthFilterUtils.PROXYUSER_PREFIX; + +/** + * An extension of Hadoop's DefaultImpersonationProvider that adds support for group-based impersonation. + * This provider allows users who belong to specific groups to impersonate other users. + */ +public class GroupBasedImpersonationProvider extends DefaultImpersonationProvider { + private static final GatewaySpiMessages LOG = MessagesFactory.get(GatewaySpiMessages.class); + private static final String CONF_HOSTS = ".hosts"; + private static final String CONF_USERS = ".users"; + private static final String CONF_GROUPS = ".groups"; + private static final String PREFIX_REGEX_EXP = "\\."; + private static final String USERS_GROUPS_REGEX_EXP = "[\\S]*(" + + Pattern.quote(CONF_USERS) + "|" + Pattern.quote(CONF_GROUPS) + ")"; + private static final String HOSTS_REGEX_EXP = "[\\S]*" + Pattern.quote(CONF_HOSTS); + private final Map<String, AccessControlList> proxyGroupsAcls = new HashMap<>(); + private Map<String, MachineList> groupProxyHosts = new HashMap<>(); + private String groupConfigPrefix; + private boolean doesProxyUserConfigExist = true; + static final String IMPERSONATION_ENABLED_PARAM = "impersonation.enabled"; + + public GroupBasedImpersonationProvider() { + super(); + } + + @Override + public Configuration getConf() { + return super.getConf(); + } + + @Override + public void setConf(Configuration conf) { + super.setConf(conf); + } + + @Override + public void init(String configurationPrefix) { + super.init(configurationPrefix); + + /* Check if user proxy configs are provided */ + final Map<String, String> filteredProps = Optional.ofNullable(getConf().getPropsWithPrefix(PROXYUSER_PREFIX + ".")) + .orElse(Collections.emptyMap()) // handle null map defensively + .entrySet() + .stream() + .filter(entry -> !IMPERSONATION_ENABLED_PARAM.equals(entry.getKey())) // avoid NPE by reversing equals + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + + doesProxyUserConfigExist = !filteredProps.isEmpty(); + + initGroupBasedProvider(PROXYGROUP_PREFIX); + } + + private void initGroupBasedProvider(final String proxyGroupPrefix) { + groupConfigPrefix = proxyGroupPrefix + + (proxyGroupPrefix.endsWith(".") ? "" : "."); + + String prefixRegEx = groupConfigPrefix.replace(".", PREFIX_REGEX_EXP); + String usersGroupsRegEx = prefixRegEx + USERS_GROUPS_REGEX_EXP; + String hostsRegEx = prefixRegEx + HOSTS_REGEX_EXP; + + // get list of users and groups per proxygroup + // Map of <hadoop.proxygroup.[VIRTUAL_GROUP].users|groups, group1,group2> + Map<String, String> allMatchKeys = + getConf().getValByRegex(usersGroupsRegEx); + + for (Map.Entry<String, String> entry : allMatchKeys.entrySet()) { + //aclKey = hadoop.proxygroup.[VIRTUAL_GROUP] + String aclKey = getAclKey(entry.getKey()); + + if (!proxyGroupsAcls.containsKey(aclKey)) { + proxyGroupsAcls.put(aclKey, new AccessControlList( + allMatchKeys.get(aclKey + CONF_USERS), + allMatchKeys.get(aclKey + CONF_GROUPS))); + } + } + + // get hosts per proxygroup + allMatchKeys = getConf().getValByRegex(hostsRegEx); + for (Map.Entry<String, String> entry : allMatchKeys.entrySet()) { + groupProxyHosts.put(entry.getKey(), + new MachineList(entry.getValue())); + } + } + + private String getAclKey(String key) { + int endIndex = key.lastIndexOf('.'); + if (endIndex != -1) { + return key.substring(0, endIndex); + } + return key; + } + + /** + * Authorization based on user and group impersonation policies. + * + * @param user the user information attempting the operation, which includes the real + * user and the effective impersonated user. + * @param remoteAddress the remote address from which the user is connecting. + * @throws AuthorizationException if the user is not authorized based on the + * configured impersonation and group policies. + */ + @Override + public void authorize(UserGroupInformation user, InetAddress remoteAddress) throws AuthorizationException { + authorize(user, remoteAddress, Collections.emptyList()); + } + + /** + * Authorization based on groups that are provided as a function argument + * + * @param user the user information attempting the operation, which includes the real + * user and the effective impersonated user. + * @param groups the list of groups to check for authorization. + * @param remoteAddress the remote address from which the user is connecting. + * @throws AuthorizationException if the user is not authorized based on the + * configured impersonation and group policies. + */ + public void authorize(UserGroupInformation user, InetAddress remoteAddress, List<String> groups) throws AuthorizationException { + + /* Proxy user configuration takes precedence over proxy group configuration. */ + if (doesProxyUserConfigExist) { + try{ + /* check for proxy user authorization */ + super.authorize(user, remoteAddress); + } catch (final AuthorizationException e) { + /* + * Log and try group based impersonation. + * Since this provider is for groups no need to check if Review Comment: Is this comment true? We only employ this implementation if there is group-level proxy config? That seems wrong to me. ########## gateway-spi/src/main/java/org/apache/knox/gateway/util/AuthFilterUtils.java: ########## @@ -42,212 +44,247 @@ import org.apache.knox.gateway.i18n.messages.MessagesFactory; public class AuthFilterUtils { - public static final String DEFAULT_AUTH_UNAUTHENTICATED_PATHS_PARAM = "/knoxtoken/api/v1/jwks.json"; - public static final String PROXYUSER_PREFIX = "hadoop.proxyuser"; - public static final String QUERY_PARAMETER_DOAS = "doAs"; - public static final String REAL_USER_NAME_ATTRIBUTE = "real.user.name"; - public static final String DO_GLOBAL_LOGOUT_ATTRIBUTE = "do.global.logout"; - - private static final GatewaySpiMessages LOG = MessagesFactory.get(GatewaySpiMessages.class); - private static final Map<String, Map<String, ImpersonationProvider>> TOPOLOGY_IMPERSONATION_PROVIDERS = new ConcurrentHashMap<>(); - private static final Lock refreshSuperUserGroupsLock = new ReentrantLock(); - - /** - * A helper method that checks whether request contains - * unauthenticated path - * @param request - * @return - */ - public static boolean doesRequestContainUnauthPath( - final Set<String> unAuthenticatedPaths, final ServletRequest request) { - /* make sure the path matches EXACTLY to prevent auth bypass */ - return unAuthenticatedPaths.contains(((HttpServletRequest) request).getPathInfo()); - } - - /** - * A helper method that parses a string and adds to the - * provided unauthenticated set. - * @param unAuthenticatedPaths - * @param list - */ - public static void parseStringThenAdd(final Set<String> unAuthenticatedPaths, final String list) { - final StringTokenizer tokenizer = new StringTokenizer(list, ";,"); - while (tokenizer.hasMoreTokens()) { - unAuthenticatedPaths.add(tokenizer.nextToken()); - } - } - - /** - * A method that parses a string (delimiters = ;,) and adds them to the - * provided un-authenticated path set. - * @param unAuthenticatedPaths - * @param list - * @param defaultList - */ - public static void addUnauthPaths(final Set<String> unAuthenticatedPaths, final String list, final String defaultList) { - /* add default unauthenticated paths list */ - parseStringThenAdd(unAuthenticatedPaths, defaultList); - /* add provided unauthenticated paths list if specified */ - if (!StringUtils.isBlank(list)) { - AuthFilterUtils.parseStringThenAdd(unAuthenticatedPaths, list); - } - } - - public static void refreshSuperUserGroupsConfiguration(ServletContext context, List<String> initParameterNames, String topologyName, String role) { - if (context == null) { - throw new IllegalArgumentException("Cannot get proxyuser configuration from NULL context"); - } - refreshSuperUserGroupsConfiguration(context, null, initParameterNames, topologyName, role); - } - - public static void refreshSuperUserGroupsConfiguration(FilterConfig filterConfig, List<String> initParameterNames, String topologyName, String role) { - if (filterConfig == null) { - throw new IllegalArgumentException("Cannot get proxyuser configuration from NULL filter config"); - } - refreshSuperUserGroupsConfiguration(null, filterConfig, initParameterNames, topologyName, role); - } - - private static void refreshSuperUserGroupsConfiguration(ServletContext context, FilterConfig filterConfig, List<String> initParameterNames, String topologyName, String role) { - final Configuration conf = new Configuration(false); - if (initParameterNames != null) { - initParameterNames.stream().filter(name -> name.startsWith(PROXYUSER_PREFIX + ".")).forEach(name -> { - String value = context == null ? filterConfig.getInitParameter(name) : context.getInitParameter(name); - conf.set(name, value); - }); - } - - saveImpersonationProvider(topologyName, role, conf); - } - - private static void saveImpersonationProvider(String topologyName, String role, final Configuration conf) { - refreshSuperUserGroupsLock.lock(); - try { - final ImpersonationProvider impersonationProvider = new DefaultImpersonationProvider(); - impersonationProvider.setConf(conf); - impersonationProvider.init(PROXYUSER_PREFIX); - LOG.createImpersonationProvider(topologyName, role, PROXYUSER_PREFIX, conf.getPropsWithPrefix(PROXYUSER_PREFIX + ".").toString()); - TOPOLOGY_IMPERSONATION_PROVIDERS.putIfAbsent(topologyName, new ConcurrentHashMap<String, ImpersonationProvider>()); - TOPOLOGY_IMPERSONATION_PROVIDERS.get(topologyName).put(role, impersonationProvider); - } finally { - refreshSuperUserGroupsLock.unlock(); - } - } - - public static HttpServletRequest getProxyRequest(HttpServletRequest request, String doAsUser, String topologyName, String role) throws AuthorizationException { - return getProxyRequest(request, request.getUserPrincipal().getName(), doAsUser, topologyName, role); - } - - public static HttpServletRequest getProxyRequest(HttpServletRequest request, String remoteUser, String doAsUser, String topologyName, String role) throws AuthorizationException { - final UserGroupInformation remoteRequestUgi = getRemoteRequestUgi(remoteUser, doAsUser); - if (remoteRequestUgi != null) { - authorizeImpersonationRequest(request, remoteRequestUgi, topologyName, role); - - return new HttpServletRequestWrapper(request) { - @Override - public String getRemoteUser() { - return remoteRequestUgi.getShortUserName(); + public static final String DEFAULT_AUTH_UNAUTHENTICATED_PATHS_PARAM = "/knoxtoken/api/v1/jwks.json"; + public static final String PROXYUSER_PREFIX = "hadoop.proxyuser"; + public static final String QUERY_PARAMETER_DOAS = "doAs"; + public static final String REAL_USER_NAME_ATTRIBUTE = "real.user.name"; + public static final String DO_GLOBAL_LOGOUT_ATTRIBUTE = "do.global.logout"; + + private static final GatewaySpiMessages LOG = MessagesFactory.get(GatewaySpiMessages.class); + private static final Map<String, Map<String, ImpersonationProvider>> TOPOLOGY_IMPERSONATION_PROVIDERS = new ConcurrentHashMap<>(); + private static final Lock refreshSuperUserGroupsLock = new ReentrantLock(); + + public static final String PROXYGROUP_PREFIX = "hadoop.proxygroup"; + /** + * A helper method that checks whether request contains + * unauthenticated path + * @param request + * @return + */ + public static boolean doesRequestContainUnauthPath( + final Set<String> unAuthenticatedPaths, final ServletRequest request) { + /* make sure the path matches EXACTLY to prevent auth bypass */ + return unAuthenticatedPaths.contains(((HttpServletRequest) request).getPathInfo()); + } + + /** + * A helper method that parses a string and adds to the + * provided unauthenticated set. + * @param unAuthenticatedPaths + * @param list + */ + public static void parseStringThenAdd(final Set<String> unAuthenticatedPaths, final String list) { + final StringTokenizer tokenizer = new StringTokenizer(list, ";,"); + while (tokenizer.hasMoreTokens()) { + unAuthenticatedPaths.add(tokenizer.nextToken()); + } + } + + /** + * A method that parses a string (delimiters = ;,) and adds them to the + * provided un-authenticated path set. + * @param unAuthenticatedPaths + * @param list + * @param defaultList + */ + public static void addUnauthPaths(final Set<String> unAuthenticatedPaths, final String list, final String defaultList) { + /* add default unauthenticated paths list */ + parseStringThenAdd(unAuthenticatedPaths, defaultList); + /* add provided unauthenticated paths list if specified */ + if (!StringUtils.isBlank(list)) { + AuthFilterUtils.parseStringThenAdd(unAuthenticatedPaths, list); + } + } + + public static void refreshSuperUserGroupsConfiguration(ServletContext context, List<String> initParameterNames, String topologyName, String role) { + if (context == null) { + throw new IllegalArgumentException("Cannot get proxyuser configuration from NULL context"); + } + refreshSuperUserGroupsConfiguration(context, null, initParameterNames, topologyName, role); + } + + public static void refreshSuperUserGroupsConfiguration(FilterConfig filterConfig, List<String> initParameterNames, String topologyName, String role) { + if (filterConfig == null) { + throw new IllegalArgumentException("Cannot get proxyuser configuration from NULL filter config"); + } + refreshSuperUserGroupsConfiguration(null, filterConfig, initParameterNames, topologyName, role); + } + + private static void refreshSuperUserGroupsConfiguration(ServletContext context, FilterConfig filterConfig, List<String> initParameterNames, String topologyName, String role) { + final Configuration conf = new Configuration(false); + boolean hasProxyGroupParams = false; + + if (initParameterNames != null) { + initParameterNames.stream().filter(name -> name.startsWith(PROXYUSER_PREFIX + ".")).forEach(name -> { + String value = context == null ? filterConfig.getInitParameter(name) : context.getInitParameter(name); + conf.set(name, value); + }); + /* For proxy groups */ + hasProxyGroupParams = initParameterNames.stream().anyMatch(name -> name.startsWith(PROXYGROUP_PREFIX + ".")); + if(hasProxyGroupParams) { + initParameterNames.stream().filter(name -> name.startsWith(PROXYGROUP_PREFIX + ".")).forEach(name -> { + String value = context == null ? filterConfig.getInitParameter(name) : context.getInitParameter(name); + conf.set(name, value); + }); + } } - @Override - public Principal getUserPrincipal() { - return remoteRequestUgi::getUserName; + ImpersonationProvider impersonationProvider; + if(hasProxyGroupParams) { Review Comment: Why can't this be our default implementation that supports both user-level and the new group-level impersonation config? It would simplify a lot of things. So, rather than GroupBasedImpersonationProvider, it would be someyhing like KnoxImpersonationProvider (our implementation that also supports group-level config). ########## gateway-spi/src/main/java/org/apache/knox/gateway/i18n/GatewaySpiMessages.java: ########## @@ -93,4 +96,22 @@ public interface GatewaySpiMessages { @Message(level=MessageLevel.DEBUG, text="Ignoring cookie path scope filter for default topology") void ignoringCookiePathScopeForDefaultTopology(); + + @Message(level=MessageLevel.DEBUG, text="Loaded proxy groups ACLs: {0}") + void loadedProxyGroupsAcls(String acls); + + @Message(level = MessageLevel.ERROR, text = "Unauthorized connection for super-user: {0} from IP remoteAddress {1}") Review Comment: Suggestion: "User impersonation failed for user {0}. Connections from remote address {1} are not authorized." Issue Time Tracking ------------------- Worklog Id: (was: 972663) Time Spent: 5h (was: 4h 50m) > Surrogate proxy user configuration for user groups > -------------------------------------------------- > > Key: KNOX-3048 > URL: https://issues.apache.org/jira/browse/KNOX-3048 > Project: Apache Knox > Issue Type: Improvement > Components: Server > Affects Versions: 2.0.0 > Reporter: Philip Zampino > Assignee: Sandeep More > Priority: Major > Fix For: 2.1.0 > > Time Spent: 5h > Remaining Estimate: 0h > > *Problem Statement:* > Currently Knox has the ability for specific users (say for e.g. {{sp_user}}) > to impersonate other users (say for e.g.{{ot_user}}). This is driven by > configs defined in a topology. Currently these configs are needed for each > user that impersonates other users (i.e. {{sp_user}}), this can get out of > hand quickly and can be difficult to maintain. > To solve this problem the proposed solution uses a group level impersonation > configuration. This configuration will be based on the virtual groups feature > that is already available in Knox. With this new configuration we can have > specific users who belong to a virtual group/s (based on conditions such as > {{(match groups 'analyst|scientist') }}) impersonate other users. This will > significantly cut down on the config properties and keep them readable and > maintainable. -- This message was sent by Atlassian Jira (v8.20.10#820010)