Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use gid from /etc/groups when creating a user group #4314

Closed
wants to merge 1 commit into from

Conversation

leres
Copy link

@leres leres commented May 9, 2020

This solves group name collisions and also allows users to be placed in group operator to be able to read filesystem partitions.
Protect such groups from deletion.

This solves group name collisions and also allows users to be placed
in group operator to be able to read filesystem partitions.
Protect such groups from deletion.
Copy link
Contributor

@jim-p jim-p left a comment

Choose a reason for hiding this comment

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

Need to think on this a bit. It seems OK from a technical point of view, but security-wise, I'm not so certain. It may be too easy for an admin to unintentionally grant someone elevated shell privileges by using a special existing group name without realizing what they are doing.

@leres
Copy link
Author

leres commented May 11, 2020

Are there any groups in /etc/groups that are not in config.xml that have special privileges? You could disallow adding/changing assigned privileges to scope=system groups. So if you wanted an admin group that had different assigned privileges than "admins", you'd have to create one with a completely new group name (that isn't in /etc/group). This would also prevent changing privileges for the admins group which might not be a bad idea.

Another approach would be to add useful /etc/groups to the default config.xml (e.g. dialer, operator, kmem) and disallow using the gui to create a new group with a name that is in /etc/group.

@jim-p
Copy link
Contributor

jim-p commented May 11, 2020

Are there any groups in /etc/groups that are not in config.xml that have special privileges?

Several. But not GUI privileges, shell/OS privileges. As you found, operator is one such group. Some other groups might be things like sys, tty, daemon groups like unbound, proxy, and www. Not to mention wheel. BSD/*nix admins may know the special meaning of those and others, but pfSense GUI users may not.

You could disallow adding/changing assigned privileges to scope=system groups. So if you wanted an admin group that had different assigned privileges than "admins", you'd have to create one with a completely new group name (that isn't in /etc/group). This would also prevent changing privileges for the admins group which might not be a bad idea.

That's not the kind of privileges I'm talking about unintentionally delegating. I'm talking about someone in the GUI making a group called kmem for example and then if that user gets shell access they can access /dev/mem and /dev/kmem without root or sudo access. With some groups the chances are low that a GUI user would make an overlapping group, like kmem, but others like operator are more problematic since some places use that name for admin groups.

Allowing the use of those groups is asking for trouble, unless there are hefty warnings about the consequences of doing so.

@dennypage
Copy link
Contributor

There is huge variance in the operating system knowledge of the pfSense user base. I honestly don't believe it's possible to provide sufficient warnings to the prevent someone from making a serious mistake here.

Copy link
Member

@rbgarga rbgarga left a comment

Choose a reason for hiding this comment

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

I don't like this change as well. IMO it's too dangerous

@rbgarga
Copy link
Member

rbgarga commented Sep 18, 2020

After internal discussion we decided this approach is too dangerous to be accepted. Thank you for the contribution anyway.

@rbgarga rbgarga closed this Sep 18, 2020
@leres
Copy link
Author

leres commented Sep 18, 2020

It's disappointing but I guess I understand why you don't want to implement my solution. But don't you think there is still a serious problem here that needs fixing? I believe currently if a user uses the gui to add a user wit the name operator, it adds a duplicate that disappears on the next reboot, leaving a mess. At a minimum I would think a check should be added to disallow group names that conflict with those that exist in the base system /etc/group file.

@rbgarga
Copy link
Member

rbgarga commented Sep 18, 2020

It's disappointing but I guess I understand why you don't want to implement my solution. But don't you think there is still a serious problem here that needs fixing? I believe currently if a user uses the gui to add a user wit the name operator, it adds a duplicate that disappears on the next reboot, leaving a mess. At a minimum I would think a check should be added to disallow group names that conflict with those that exist in the base system /etc/group file.

Yes, we have no intention of close the bug at redmine. We are going to work on a fix for that

@leres
Copy link
Author

leres commented Sep 18, 2020

That sounds great. Ideally it will at least be possible to login and edit /etc/group to add a user to group operator?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants