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

Merged
merged 1 commit into from
Feb 22, 2025
+24 −8
Merged

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
32 changes: 24 additions & 8 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,13 @@ static char *os = NULL;
static char *nm = NULL;
static char *id = NULL;

/* TODO: add `#ifdef HAVE_FOO` around optional features. */
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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dropped the group-exists check, added a comment about future optional support for FRR.

I've also looked into if podman has/requires any group -- answer is none.

So, provided this latest build passes, this feature is complete.

NULL
};

static struct { char *name, *shell; } shells[] = {
{ "infix-system:sh", "/bin/sh" },
{ "infix-system:bash", "/bin/bash" },
Expand Down Expand Up @@ -601,26 +608,29 @@ static int change_dns(sr_session_ctx_t *session, uint32_t sub_id, const char *mo

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 (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 +644,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 +1462,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