You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by jo...@apache.org on 2020/01/29 20:49:39 UTC
[nifi] 02/05: NIFI-7051 Protect against empty group membership in
ShellUserGroupProvider, and add differentiator to id seeding
This is an automated email from the ASF dual-hosted git repository.
joewitt pushed a commit to branch support/nifi-1.11.x
in repository https://gitbox.apache.org/repos/asf/nifi.git
commit ee91450400eea1dc697572ca68902e899cad06e6
Author: Bryan Bende <bb...@apache.org>
AuthorDate: Tue Jan 21 11:22:40 2020 -0500
NIFI-7051 Protect against empty group membership in ShellUserGroupProvider, and add differentiator to id seeding
NIFI-7051 Fixing issue where identity was being used instead of identifier, making a flag to control legacy id behavior, increasing timeout of shell command runner, and changing the NSS system check command to return less info
NIFI-7051 Updating command for getSystemCheck in NSS impl to use getent --version to improve performance
This closes #4003.
---
.../src/main/resources/conf/authorizers.xml | 4 +
.../nifi/authorization/NssShellCommands.java | 2 +-
.../nifi/authorization/ShellUserGroupProvider.java | 138 ++++++++++++++++-----
.../nifi/authorization/util/ShellRunner.java | 9 +-
4 files changed, 118 insertions(+), 35 deletions(-)
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/conf/authorizers.xml b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/conf/authorizers.xml
index 52f9bb6..ec1560f 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/conf/authorizers.xml
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/conf/authorizers.xml
@@ -178,6 +178,9 @@
'Refresh Delay' - duration to wait between subsequent refreshes. Default is '5 mins'.
'Exclude Groups' - regular expression used to exclude groups. Default is '', which means no groups are excluded.
'Exclude Users' - regular expression used to exclude users. Default is '', which means no users are excluded.
+ 'Legacy Identifier Mode' - preserves the legacy behavior for id generation. Disabling this will ensure that
+ user and group ids are differentiated to handle the case where a user and group have
+ the same identity. Default is 'true', which means users and groups are not differentiated.
-->
<!-- To enable the shell-user-group-provider remove 2 lines. This is 1 of 2.
<userGroupProvider>
@@ -186,6 +189,7 @@
<property name="Refresh Delay">5 mins</property>
<property name="Exclude Groups"></property>
<property name="Exclude Users"></property>
+ <property name="Legacy Identifier Mode">true</property>
</userGroupProvider>
To enable the shell-user-group-provider remove 2 lines. This is 2 of 2. -->
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-shell-authorizer/src/main/java/org/apache/nifi/authorization/NssShellCommands.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-shell-authorizer/src/main/java/org/apache/nifi/authorization/NssShellCommands.java
index fe49200..4339907 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-shell-authorizer/src/main/java/org/apache/nifi/authorization/NssShellCommands.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-shell-authorizer/src/main/java/org/apache/nifi/authorization/NssShellCommands.java
@@ -85,6 +85,6 @@ class NssShellCommands implements ShellCommandsProvider {
* @return Shell command string that will exit normally (0) on a suitable system.
*/
public String getSystemCheck() {
- return "getent passwd";
+ return "getent --version";
}
}
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-shell-authorizer/src/main/java/org/apache/nifi/authorization/ShellUserGroupProvider.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-shell-authorizer/src/main/java/org/apache/nifi/authorization/ShellUserGroupProvider.java
index d48255e..c31fd11 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-shell-authorizer/src/main/java/org/apache/nifi/authorization/ShellUserGroupProvider.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-shell-authorizer/src/main/java/org/apache/nifi/authorization/ShellUserGroupProvider.java
@@ -22,16 +22,18 @@ import org.apache.nifi.authorization.exception.AuthorizerDestructionException;
import org.apache.nifi.authorization.util.ShellRunner;
import org.apache.nifi.components.PropertyValue;
import org.apache.nifi.util.FormatUtils;
+import org.apache.nifi.util.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
-import java.util.Arrays;
+import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.TreeSet;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
@@ -56,10 +58,12 @@ public class ShellUserGroupProvider implements UserGroupProvider {
public static final String EXCLUDE_USER_PROPERTY = "Exclude Users";
public static final String EXCLUDE_GROUP_PROPERTY = "Exclude Groups";
+ public static final String LEGACY_IDENTIFIER_MODE = "Legacy Identifier Mode";
private long fixedDelay;
private Pattern excludeUsers;
private Pattern excludeGroups;
+ private boolean legacyIdentifierMode;
// Our scheduler has one thread for users, one for groups:
private final ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(2);
@@ -134,7 +138,7 @@ public class ShellUserGroupProvider implements UserGroupProvider {
if (user == null) {
logger.debug("getUser (by name) user not found: " + identity);
} else {
- logger.debug("getUser (by name) found user: " + user + " for name: " + identity);
+ logger.debug("getUser (by name) found user: " + user.getIdentity() + " for name: " + identity);
}
return user;
}
@@ -176,7 +180,7 @@ public class ShellUserGroupProvider implements UserGroupProvider {
if (group == null) {
logger.debug("getGroup (by id) group not found: " + identifier);
} else {
- logger.debug("getGroup (by id) found group: " + group + " for id: " + identifier);
+ logger.debug("getGroup (by id) found group: " + group.getName() + " for id: " + identifier);
}
return group;
@@ -194,10 +198,12 @@ public class ShellUserGroupProvider implements UserGroupProvider {
logger.debug("Retrieved user {} for identity {}", new Object[]{user, identity});
Set<Group> groups = new HashSet<>();
-
- for (Group g : getGroups()) {
- if (user != null && g.getUsers().contains(user.getIdentity())) {
- groups.add(g);
+ if (user != null) {
+ for (Group g : getGroups()) {
+ if (g.getUsers().contains(user.getIdentifier())) {
+ logger.debug("User {} belongs to group {}", new Object[]{user.getIdentity(), g.getName()});
+ groups.add(g);
+ }
}
}
@@ -249,9 +255,9 @@ public class ShellUserGroupProvider implements UserGroupProvider {
// will work on this host or not.
try {
ShellRunner.runShell(commands.getSystemCheck());
- } catch (final IOException ioexc) {
- logger.error("initialize exception: " + ioexc + " system check command: " + commands.getSystemCheck());
- throw new AuthorizerCreationException(SYS_CHECK_ERROR, ioexc.getCause());
+ } catch (final Exception e) {
+ logger.error("initialize exception: " + e + " system check command: " + commands.getSystemCheck());
+ throw new AuthorizerCreationException(SYS_CHECK_ERROR, e);
}
// The next step is to add the user and group exclude regexes:
@@ -262,6 +268,9 @@ public class ShellUserGroupProvider implements UserGroupProvider {
throw new AuthorizerCreationException(e);
}
+ // Get the value for Legacy Identifier Mo
+ legacyIdentifierMode = Boolean.parseBoolean(getProperty(configurationContext, LEGACY_IDENTIFIER_MODE, "true"));
+
// With our command set selected, and our system check passed, we can pull in the users and groups:
refreshUsersAndGroups();
@@ -272,7 +281,7 @@ public class ShellUserGroupProvider implements UserGroupProvider {
}catch (final Throwable t) {
logger.error("", t);
}
- }, fixedDelay, fixedDelay, TimeUnit.SECONDS);
+ }, fixedDelay, fixedDelay, TimeUnit.MILLISECONDS);
}
@@ -444,6 +453,13 @@ public class ShellUserGroupProvider implements UserGroupProvider {
synchronized (usersById) {
usersById.clear();
usersById.putAll(uidToUser);
+
+ if (logger.isTraceEnabled()) {
+ logger.trace("=== Users by id...");
+ Set<User> sortedUsers = new TreeSet<>(Comparator.comparing(User::getIdentity));
+ sortedUsers.addAll(usersById.values());
+ sortedUsers.forEach(u -> logger.trace("=== " + u.toString()));
+ }
}
synchronized (usersByName) {
@@ -456,6 +472,13 @@ public class ShellUserGroupProvider implements UserGroupProvider {
groupsById.clear();
groupsById.putAll(gidToGroup);
logger.debug("groups now size: " + groupsById.size());
+
+ if (logger.isTraceEnabled()) {
+ logger.trace("=== Groups by id...");
+ Set<Group> sortedGroups = new TreeSet<>(Comparator.comparing(Group::getName));
+ sortedGroups.addAll(groupsById.values());
+ sortedGroups.forEach(g -> logger.trace("=== " + g.toString()));
+ }
}
}
@@ -468,25 +491,35 @@ public class ShellUserGroupProvider implements UserGroupProvider {
*/
private void rebuildUsers(List<String> userLines, Map<String, User> idToUser, Map<String, User> usernameToUser, Map<String, User> gidToUser) {
userLines.forEach(line -> {
+ logger.trace("Processing user: {}", new Object[]{line});
+
String[] record = line.split(":");
if (record.length > 2) {
- String name = record[0], id = record[1], gid = record[2];
+ String userIdentity = record[0], userIdentifier = record[1], primaryGroupIdentifier = record[2];
- if (name != null && id != null && !name.equals("") && !id.equals("") && !excludeUsers.matcher(name).matches()) {
- User user = new User.Builder().identity(name).identifierGenerateFromSeed(id).build();
+ if (!StringUtils.isBlank(userIdentifier) && !StringUtils.isBlank(userIdentity) && !excludeUsers.matcher(userIdentity).matches()) {
+ User user = new User.Builder()
+ .identity(userIdentity)
+ .identifierGenerateFromSeed(getUserIdentifierSeed(userIdentity))
+ .build();
idToUser.put(user.getIdentifier(), user);
- usernameToUser.put(name, user);
+ usernameToUser.put(userIdentity, user);
+ logger.debug("Refreshed user {}", new Object[]{user});
- if (gid != null && !gid.equals("")) {
+ if (!StringUtils.isBlank(primaryGroupIdentifier)) {
// create a temporary group to deterministically generate the group id and associate this user
- Group group = new Group.Builder().name(gid).identifierGenerateFromSeed(gid).build();
+ Group group = new Group.Builder()
+ .name(primaryGroupIdentifier)
+ .identifierGenerateFromSeed(getGroupIdentifierSeed(primaryGroupIdentifier))
+ .build();
gidToUser.put(group.getIdentifier(), user);
+ logger.debug("Associated primary group {} with user {}", new Object[]{group.getIdentifier(), userIdentity});
} else {
- logger.warn("Null or empty primary group id for: " + name);
+ logger.warn("Null or empty primary group id for: " + userIdentity);
}
} else {
- logger.warn("Null, empty, or skipped user name: " + name + " or id: " + id);
+ logger.warn("Null, empty, or skipped user name: " + userIdentity + " or id: " + userIdentifier);
}
} else {
logger.warn("Unexpected record format. Expected 3 or more colon separated values per line.");
@@ -506,16 +539,34 @@ public class ShellUserGroupProvider implements UserGroupProvider {
*/
private void rebuildGroups(List<String> groupLines, Map<String, Group> groupsById) {
groupLines.forEach(line -> {
+ logger.trace("Processing group: {}", new Object[]{line});
+
String[] record = line.split(":");
if (record.length > 1) {
Set<String> users = new HashSet<>();
- String name = record[0], id = record[1];
+ String groupName = record[0], groupIdentifier = record[1];
try {
- List<String> memberLines = ShellRunner.runShell(selectedShellCommands.getGroupMembers(name));
+ String groupMembersCommand = selectedShellCommands.getGroupMembers(groupName);
+ List<String> memberLines = ShellRunner.runShell(groupMembersCommand);
// Use the first line only, and log if the line count isn't exactly one:
if (!memberLines.isEmpty()) {
- users.addAll(Arrays.asList(memberLines.get(0).split(",")));
+ String memberLine = memberLines.get(0);
+ if (!StringUtils.isBlank(memberLine)) {
+ String[] members = memberLine.split(",");
+ for (String userIdentity : members) {
+ if (!StringUtils.isBlank(userIdentity)) {
+ User tempUser = new User.Builder()
+ .identity(userIdentity)
+ .identifierGenerateFromSeed(getUserIdentifierSeed(userIdentity))
+ .build();
+ users.add(tempUser.getIdentifier());
+ logger.debug("Added temp user {} for group {}", new Object[]{tempUser, groupName});
+ }
+ }
+ } else {
+ logger.debug("list membership returned no members");
+ }
} else {
logger.debug("list membership returned zero lines.");
}
@@ -527,12 +578,16 @@ public class ShellUserGroupProvider implements UserGroupProvider {
logger.error("list membership shell exception: " + ioexc);
}
- if (name != null && id != null && !name.equals("") && !id.equals("") && !excludeGroups.matcher(name).matches()) {
- Group group = new Group.Builder().name(name).identifierGenerateFromSeed(id).addUsers(users).build();
+ if (!StringUtils.isBlank(groupIdentifier) && !StringUtils.isBlank(groupName) && !excludeGroups.matcher(groupName).matches()) {
+ Group group = new Group.Builder()
+ .name(groupName)
+ .identifierGenerateFromSeed(getGroupIdentifierSeed(groupIdentifier))
+ .addUsers(users)
+ .build();
groupsById.put(group.getIdentifier(), group);
- logger.debug("Refreshed group: " + group);
+ logger.debug("Refreshed group {}", new Object[] {group});
} else {
- logger.warn("Null, empty, or skipped group name: " + name + " or id: " + id);
+ logger.warn("Null, empty, or skipped group name: " + groupName + " or id: " + groupIdentifier);
}
} else {
logger.warn("Unexpected record format. Expected 1 or more comma separated values.");
@@ -552,19 +607,38 @@ public class ShellUserGroupProvider implements UserGroupProvider {
Group primaryGroup = gidToGroup.get(primaryGid);
if (primaryGroup == null) {
- logger.warn("user: " + primaryUser + " primary group not found");
+ logger.warn("Primary group {} not found for {}", new Object[]{primaryGid, primaryUser.getIdentity()});
} else if (!excludeGroups.matcher(primaryGroup.getName()).matches()) {
Set<String> groupUsers = primaryGroup.getUsers();
- if (!groupUsers.contains(primaryUser.getIdentity())) {
- Set<String> secondSet = new HashSet<>(groupUsers);
- secondSet.add(primaryUser.getIdentity());
- Group group = new Group.Builder().name(primaryGroup.getName()).identifierGenerateFromSeed(primaryGid).addUsers(secondSet).build();
- gidToGroup.put(group.getIdentifier(), group);
+ if (!groupUsers.contains(primaryUser.getIdentifier())) {
+ Set<String> updatedUserIdentifiers = new HashSet<>(groupUsers);
+ updatedUserIdentifiers.add(primaryUser.getIdentifier());
+
+ Group updatedGroup = new Group.Builder()
+ .identifier(primaryGroup.getIdentifier())
+ .name(primaryGroup.getName())
+ .addUsers(updatedUserIdentifiers)
+ .build();
+ gidToGroup.put(updatedGroup.getIdentifier(), updatedGroup);
+ logger.debug("Added user {} to primary group {}", new Object[]{primaryUser, updatedGroup});
+ } else {
+ logger.debug("Primary group {} already contains user {}", new Object[]{primaryGroup, primaryUser});
}
+ } else {
+ logger.debug("Primary group {} excluded from matcher for {}", new Object[]{primaryGroup.getName(), primaryUser.getIdentity()});
}
});
}
+ private String getUserIdentifierSeed(final String userIdentifier) {
+ return legacyIdentifierMode ? userIdentifier : userIdentifier + "-user";
+ }
+
+ private String getGroupIdentifierSeed(final String groupIdentifier) {
+ return legacyIdentifierMode ? groupIdentifier : groupIdentifier + "-group";
+ }
+
+
/**
* @return The fixed refresh delay.
*/
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-shell-authorizer/src/main/java/org/apache/nifi/authorization/util/ShellRunner.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-shell-authorizer/src/main/java/org/apache/nifi/authorization/util/ShellRunner.java
index 46bc1cc..d12c00b 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-shell-authorizer/src/main/java/org/apache/nifi/authorization/util/ShellRunner.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-shell-authorizer/src/main/java/org/apache/nifi/authorization/util/ShellRunner.java
@@ -33,7 +33,7 @@ public class ShellRunner {
static String SHELL = "sh";
static String OPTS = "-c";
- static Integer TIMEOUT = 30;
+ static Integer TIMEOUT = 60;
public static List<String> runShell(String command) throws IOException {
return runShell(command, "<unknown>");
@@ -46,12 +46,17 @@ public class ShellRunner {
logger.debug("Run Command '" + description + "': " + builderCommand);
final Process proc = builder.start();
+ boolean completed;
try {
- proc.waitFor(TIMEOUT, TimeUnit.SECONDS);
+ completed = proc.waitFor(TIMEOUT, TimeUnit.SECONDS);
} catch (InterruptedException irexc) {
throw new IOException(irexc.getMessage(), irexc.getCause());
}
+ if (!completed) {
+ throw new IllegalStateException("Shell command '" + command + "' did not complete during the allotted time period");
+ }
+
if (proc.exitValue() != 0) {
try (final Reader stderr = new InputStreamReader(proc.getErrorStream());
final BufferedReader reader = new BufferedReader(stderr)) {