Skip to content

NPF : User/group id filtering #47

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

Closed
wants to merge 7 commits into from
Closed

Conversation

Emmankoko
Copy link

No description provided.

* both uid and gid are both uint32_t
*/
struct r_id
npfctl_init_rid(uint32_t id1, uint32_t id2, uint8_t op)
Copy link
Contributor

Choose a reason for hiding this comment

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

pass the rid pointer to be initialized to avoid extra copies.

@@ -548,6 +575,14 @@ npfctl_print_rule(npf_conf_info_t *ctx, nl_rule_t *rl, unsigned level)
ctx->fpos += fprintf(ctx->fp, "all ");
}

if (!npf_rule_getrid(&rid, rl, "r_user")) {
ctx->fpos += fprintf(ctx->fp, "user \"%s\" ", print_guid(rid));
Copy link
Contributor

Choose a reason for hiding this comment

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

memory leaks

@Emmankoko Emmankoko force-pushed the rid branch 6 times, most recently from 024cdd6 to 7ae56f4 Compare May 31, 2025 12:47
@@ -251,7 +251,8 @@ npfk_packet_handler(npf_t *npf, struct mbuf **mp, ifnet_t *ifp, int di)
error = npf_rule_reverse(&npc, &mi, error);
}

if (error) {
/* sidewaysh handling of rejecting packets whose addr-port pair matches no sockets */
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be done in a separate commit and "sideways"?

}

return matched;
/* make sure if both uid and gid is set on rule, both must be mtching to agree */
Copy link
Contributor

Choose a reason for hiding this comment

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

"matching"

@@ -1013,7 +1013,7 @@ int
npf_rule_match_rid(npf_rule_t *rl, npf_cache_t *npc, int dir)
{
uint32_t sock_gid, sock_uid;
int matched = 0;
int uid_matched = 0, gid_matched = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps bool and false?

@@ -0,0 +1,270 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Copyright, rcsid.

int id_match;

id_match = npf_rule_match_rid(rl, npc, t->di);
error = npf_rule_conclude(rl, &mi);
Copy link
Contributor

Choose a reason for hiding this comment

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

error gets overwritten below and not tested here.

static struct socket *
test_socket(int dir, uid_t uid, gid_t gid)
{
struct sockaddr_in Server;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why caps?

test_socket(int dir, uid_t uid, gid_t gid)
{
struct sockaddr_in Server;
struct lwp *cur = curlwp;
Copy link
Contributor

Choose a reason for hiding this comment

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

single space.


struct socket *so;
int error = socreate(AF_INET, &so, SOCK_DGRAM, 0, cur, NULL);
if(error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Space after keyword (everywhere)

Copy link
Contributor

Choose a reason for hiding this comment

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

why printf instead of warn(3)?

sounlock(so);

if((error = sobind(so, (struct sockaddr *)&Server, cur)) != 0) {
printf("bind failed %d\n", error);
Copy link
Contributor

Choose a reason for hiding this comment

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

Again warn is better (everywhere)?

static bool
test_static(bool verbose)
{
for (unsigned i = 0; i < __arraycount(test_cases); i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

size_t is prolly better.

Copy link
Contributor

@zoulasc zoulasc left a comment

Choose a reason for hiding this comment

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

see inline comments.

in this testing, a socket is crafted in kernel to listen on any address( all interfaces) and port 65000 which is ideal for testing

to test,
run : npfctl debug -c $dir_to_npftest.conf -o ./npf.plist
npftest -c ./npf.plist -T guid -v
@Emmankoko Emmankoko force-pushed the rid branch 5 times, most recently from 2d134c0 to 10b8936 Compare June 1, 2025 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants