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.

Sign up for GitHub

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

confd: ensure admin users have full access to vtysh #946

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

confd: ensure admin users have full access to vtysh #946

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 32 additions & 7 deletions src/confd/src/ietf-system.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ static char *os = NULL;
static char *nm = NULL;
static char *id = NULL;

static const char *admin_groups[] = {
"wheel",
"frrvty",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"frrvty",
#ifdef HAVE_FRR
"frrvty",
#endif

As this list grows, having this type of structure might make it easier to keep track of which deps pull in which groups. It would also avoid the need for group_exists(). Just a thought.

Now that I think about it: Is there a podman group that we should add for builds with container support?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I'll add that to my commit 😀👍

Not that I've seen, but I'll have a look again at podman. I'm still at it with the other related issues anyway.

NULL
};

static struct { char *name, *shell; } shells[] = {
{ "infix-system:sh", "/bin/sh" },
{ "infix-system:bash", "/bin/bash" },
Expand Down Expand Up @@ -599,28 +605,41 @@ static int change_dns(sr_session_ctx_t *session, uint32_t sub_id, const char *mo
return rc;
}

static bool group_exists(const char *group)
{
if (!systemf("grep -q '^%s:' /etc/group", group))
return true;
return false;
}

static bool is_group_member(const char *user, const char *group)
{
/* Check if user is already in group */
if (!systemf("grep %s /etc/group |grep -q %s", group, user))
if (!systemf("grep '^%s:' /etc/group |grep -q %s", group, user))
return true;

return false;
}

static void add_group(const char *user, const char *group)
{
bool is_already = is_group_member(user, group);
if (!group_exists(group))
return; /* silently skip non-existent groups */

if (is_already)
return; /* already group member */
if (is_group_member(user, group))
return;

if (systemf("adduser %s %s", user, group))
AUDIT("Failed giving user \"%s\" UNIX %s permissions.", user, group);
else
AUDIT("User \"%s\" added to UNIX \"%s\" group.", user, group);
}

static void add_groups(const char *user, const char **groups)
{
for (size_t i = 0; groups[i]; i++)
add_group(user, groups[i]);
}

static void del_group(const char *user, const char *group)
{
bool is_already = is_group_member(user, group);
Expand All @@ -634,6 +653,12 @@ static void del_group(const char *user, const char *group)
AUDIT("User \"%s\" removed from UNIX \"%s\" group.", user, group);
}

static void del_groups(const char *user, const char **groups)
{
for (size_t i = 0; groups[i]; i++)
del_group(user, groups[i]);
}

/* Users with a valid shell are also allowed CLI access */
static void adjust_access(const char *user, const char *shell)
{
Expand Down Expand Up @@ -1446,9 +1471,9 @@ static int change_nacm(sr_session_ctx_t *session, uint32_t sub_id, const char *m
AUDIT("Failed adjusting shell for user \"%s\"", user);

if (is_admin)
add_group(user, "wheel");
add_groups(user, admin_groups);
else
del_group(user, "wheel");
del_groups(user, admin_groups);
}

cleanup:
Expand Down