Project

General

Profile

Bug #6012

Groups with spaces in names not handled correctly in group file

Added by Chris Buechler about 3 years ago. Updated about 3 years ago.

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

100%

Estimated time:
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

Associated revisions

Revision 78d168ce (diff)
Added by Steve Beaver about 3 years ago

Partially fixed #6012 by removing space from chars allowed in group names

Revision b76cc978 (diff)
Added by Steve Beaver about 3 years ago

Fixed #6012
Config upgrade function replaces space with underscore in group names

Revision e5ef7ae2 (diff)
Added by Chris Buechler about 3 years ago

Save changes to config in 148 config upgrade. Ticket #6012

Revision d3f3b75f (diff)
Added by Chris Buechler about 3 years ago

If there was a group with a space, delete all the user-defined groups before hitting local_sync_accounts so pw doesn't get hung up. Ticket #6012

Revision f788b1e2 (diff)
Added by Chris Buechler about 3 years ago

Rather than renaming groups with spaces, mark their scope as remote. Ticket #6012

Revision 792adb45 (diff)
Added by Chris Buechler about 3 years ago

Don't modify the group file for scope remote. Ticket #6012

History

#1 Updated by Steve Beaver about 3 years ago

Space removed from chars allowed in group name

#2 Updated by Steve Beaver about 3 years ago

  • Status changed from Confirmed to Feedback
  • % Done changed from 0 to 100

#3 Updated by Steve Beaver about 3 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.

#4 Updated by Chris Buechler about 3 years ago

  • Status changed from Feedback to Resolved

fixed after e5ef7ae26b32d18b7aa1a117605ccbbfafefca14

#5 Updated by Chris Buechler about 3 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.

#6 Updated by Chris Buechler about 3 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.

#7 Updated by Chris Buechler about 3 years ago

  • Status changed from Feedback to Resolved

post-upgrade fixed too.

#8 Updated by Jim Pingle about 3 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.

#9 Updated by Jim Pingle about 3 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.

#10 Updated by Chris Buechler about 3 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.

#11 Updated by Chris Buechler about 3 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.

Also available in: Atom PDF