Project

General

Profile

Actions

Bug #6012

closed

Groups with spaces in names not handled correctly in group file

Added by Chris Buechler over 5 years ago. Updated over 5 years ago.

Status:
Resolved
Priority:
Normal
Category:
User Manager / Privileges
Target version:
Start date:
03/19/2016
Due date:
% Done:

100%

Estimated time:
Plus Target Version:
Release Notes:
Affected Version:
All
Affected Architecture:

Description

Group names that contain spaces are added to the group file with only the last word of the group name. So if you have, say, "My Users" and "VPN Users", you end up with just "Users" in the group file.

In 2.2.x, you have multiple groups with the same name. In 2.3 it's handled more sane, in that you won't end up with a conflicting group name, but only end up with one of the conflicting groups in the group file.

This causes a serious problem on upgrade to 2.3 where there are conflicts because pw in FreeBSD 10.3 ends up stuck in a never-ending loop when trying to modify a user in one of the affected groups. Then local_sync_users function in rc.bootup never finishes, which leaves the bootup incomplete.

Need to:
1) prohibit spaces in group names, since they go straight into the group file
2) add config upgrade code to replace spaces in group names with _ to avoid problems

Actions #1

Updated by Steve Beaver over 5 years ago

Space removed from chars allowed in group name

Actions #2

Updated by Steve Beaver over 5 years ago

  • Status changed from Confirmed to Feedback
  • % Done changed from 0 to 100
Actions #3

Updated by Steve Beaver over 5 years ago

Config upgrade function added to replace spaces with underscores in group names

This has been tested from the cmd line, but not as part of a real upgrade.

Actions #4

Updated by Chris Buechler over 5 years ago

  • Status changed from Feedback to Resolved

fixed after e5ef7ae26b32d18b7aa1a117605ccbbfafefca14

Actions #5

Updated by Chris Buechler over 5 years ago

  • Status changed from Resolved to Feedback
  • Assignee set to Chris Buechler

still need to verify in the 2.2.x->2.3 upgrade situation once that makes it to a snapshot.

Actions #6

Updated by Chris Buechler over 5 years ago

That wasn't enough to prevent the problem post-upgrade as pw hits the never-ending loop when modifying users before it gets to fixing the groups. Fix for that pushed.

Actions #7

Updated by Chris Buechler over 5 years ago

  • Status changed from Feedback to Resolved

post-upgrade fixed too.

Actions #8

Updated by Jim Pingle over 5 years ago

Changing the group names with spaces to underscores breaks some working setups that relied on the groups to match external authentication sources such as LDAP (e.g. "Domain Admins" group in AD), without caring how it was handled at the OS level. We might want to consider changing the group matching code for LDAP/RADIUS to treat the underscores the same as spaces to avoid breaking too many setups.

Actions #9

Updated by Jim Pingle over 5 years ago

  • Status changed from Resolved to New

Kicking this back open due to the problem above.

I'm tempted to make the LDAP/RADIUS group check both _ and ' ' when testing remote groups, but there is a small chance that would allow something it shouldn't if someone had two groups, one "Group_Name" and one "Group Name" but with different permissions, which seems like it might be sufficiently rare/inadvisable to not care about.

Line 1517 of src/etc/inc/auth.inc could be changed to this, which works:

            if (in_array($group['name'], $allowed_groups) || in_array(str_replace("_", " ", $group['name']), $allowed_groups)) {

I have a test setup available if needed.

Actions #10

Updated by Chris Buechler over 5 years ago

We already have <scope> on groups for system groups. It'd be best to extend that to user-defined groups so they're configurable as Local or Remote. That scope needs to be forced static for system groups.

Actions #11

Updated by Chris Buechler over 5 years ago

  • Status changed from New to Resolved

All good. Group scope "remote" is omitted from /etc/group, group names containing spaces are config upgraded to remote rather than renamed, and the input validation changed accordingly.

Actions

Also available in: Atom PDF