Date: Thu, 16 Sep 2004 04:08:50 -0000 From: Max Laier <max@love2party.net> To: pf4freebsd@freelists.org Cc: tech@openbsd.org Subject: [pf4freebsd] Comments? FreeBSD-only change (group -> groupmember) Message-ID: <200407210045.06274.max@love2party.net>
next in thread | raw e-mail | index | archive | help
--Boundary-03=_yBa/Av/NmIRhcls
Content-Type: multipart/mixed;
boundary="Boundary-01=_qBa/AZ1dqu+XxAS"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
--Boundary-01=_qBa/AZ1dqu+XxAS
Content-Type: text/plain;
charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
Hi,
during a discussion about ipfw's user/group filter capabilities and it's=20
implementation with Christian S.J. Peron we found that pf only applies grou=
p=20
filter based on the primary (effective) group of the user, rather than taki=
ng=20
all member groups into account. This is a major backdraw for multiuser=20
environments, where you want to allow/deny a large group of people certain=
=20
network access, say:
pass out on $dmz from $dmz to $cvsserver port 22 group cvsuser keep sta=
te
In FreeBSD it's quite easy to change the behavior to check for member-group=
s=20
(see attachment). For OpenBSD, however, this is not possible as it does not=
=20
store/reference full credentials on the socket pcb, but rather just copies=
=20
uid/gid and euid/egid.
A patch to the same effect as what was done in FreeBSD:
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/kern/uipc_socket.c.diff?r1=3D=
1.59&r2=3D1.62
would change the situation and provide a couple of benefits (imo):
Not only would it allow pf to check groupmembership, but it will also give=
=20
*everything* that has to enforce per-socket permissions access to the *full=
*=20
credentials. Moreover, it will simply provide any future extensions of the=
=20
credentials to everything that might be interested.
I plan to commit the "fix" for FreeBSD unless you tell me, that you (=3Dthe=
=20
=46reeBSD pf-userbase) prefer to old behavior (and give me valid reasons wh=
y).=20
Other than that I am willing to help with the conversion in OpenBSD if you=
=20
(=3Dthe OpenBSD users/developers^W^W^WTheo) are interested.
Looking forward to your feedback. Thanks.
=46inal note on the patch: You will see a new cache variable "jid" that see=
ms to=20
have no sense or application. This will turn into a "jailed" filter, which=
=20
allows you to check whether a given socket was created inside a jail or not=
=20
This is FreeBSD-specific as well and will follow in a bit.
=2D-=20
/"\ Best regards, | mlaier@freebsd.org
\ / Max Laier | ICQ #67774661
X http://pf4freebsd.love2party.net/ | mlaier@EFnet
/ \ ASCII Ribbon Campaign | Against HTML Mail and News
--Boundary-01=_qBa/AZ1dqu+XxAS
Content-Type: text/x-diff;
charset="us-ascii";
name="pf_groupmember.patch"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
filename="pf_groupmember.patch"
Index: pf.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /usr/store/mlaier/fcvs/src/sys/contrib/pf/net/pf.c,v
retrieving revision 1.15
diff -u -r1.15 pf.c
=2D-- pf.c 18 Jul 2004 14:25:48 -0000 1.15
+++ pf.c 20 Jul 2004 22:07:46 -0000
@@ -64,6 +64,8 @@
#ifdef __FreeBSD__
#include <sys/sysctl.h>
#include <sys/endian.h>
+#include <sys/ucred.h>
+#include <sys/jail.h>
#else
#include <sys/pool.h>
#endif
@@ -230,8 +232,13 @@
struct ifnet *, struct pf_state *);
void pf_route6(struct mbuf **, struct pf_rule *, int,
struct ifnet *, struct pf_state *);
+#ifdef __FreeBSD__
+int pf_socket_lookup(struct xucred *, int *, int,
+ struct pf_pdesc *);
+#else
int pf_socket_lookup(uid_t *, gid_t *,
int, struct pf_pdesc *);
+#endif
u_int8_t pf_get_wscale(struct mbuf *, int, u_int16_t,
sa_family_t);
u_int16_t pf_get_mss(struct mbuf *, int, u_int16_t,
@@ -1703,9 +1710,40 @@
return (pf_match(op, a1, a2, p));
}
=20
+#ifdef __FreeBSD__
int
=2Dpf_match_uid(u_int8_t op, uid_t a1, uid_t a2, uid_t u)
+pf_match_uid(u_int8_t op, uid_t a1, uid_t a2, struct xucred *cr)
{
+ if (cr->cr_uid =3D=3D UID_MAX && op !=3D PF_OP_EQ && op !=3D PF_OP_NE)
+ return (0);
+ return (pf_match(op, a1, a2, cr->cr_uid));
+}
+
+int
+pf_match_gid(u_int8_t op, gid_t a1, gid_t a2, struct xucred *cr)
+{
+ gid_t *gp, *lg =3D &(cr->cr_groups[cr->cr_ngroups]);
+
+ switch (op) {
+ case PF_OP_EQ:
+ case PF_OP_NE:
+ for (gp =3D cr->cr_groups; gp < lg; gp++)
+ if (a1 =3D=3D *gp)
+ return ((op =3D=3D PF_OP_NE)? 0 : 1);
+ break;
+ default:
+ if (cr->cr_gid =3D=3D GID_MAX)
+ return (0);
+ for (gp =3D cr->cr_groups; gp < lg; gp++)
+ if (!pf_match(op, a1, a2, *gp))
+ return (0); /* all groups must be in range */
+ }
+
+ return (1);
+}
+#else
+int
+pf_match_uid(u_int8_t op, uid_t a1, uid_t a2, uid_t u)
if (u =3D=3D UID_MAX && op !=3D PF_OP_EQ && op !=3D PF_OP_NE)
return (0);
return (pf_match(op, a1, a2, u));
@@ -1718,6 +1756,7 @@
return (0);
return (pf_match(op, a1, a2, g));
}
+#endif
=20
struct pf_tag *
pf_get_tag(struct mbuf *m)
@@ -2367,7 +2406,12 @@
}
=20
int
+#ifdef __FreeBSD__
+pf_socket_lookup(struct xucred *cred_cache, int *jid, int direction,
+ struct pf_pdesc *pd)
+#else
pf_socket_lookup(uid_t *uid, gid_t *gid, int direction, struct pf_pdesc *p=
d)
+#endif
{
struct pf_addr *saddr, *daddr;
u_int16_t sport, dport;
@@ -2378,8 +2422,15 @@
#endif
struct inpcb *inp;
=20
+#ifdef __FreeBSD__
+ cred_cache->cr_uid =3D UID_MAX;
+ cred_cache->cr_gid =3D GID_MAX;
+ cred_cache->cr_ngroups =3D 1;
+ *jid =3D 0;
+#else
*uid =3D UID_MAX;
*gid =3D GID_MAX;
+#endif
switch (pd->proto) {
case IPPROTO_TCP:
sport =3D pd->hdr.tcp->th_sport;
@@ -2468,8 +2519,9 @@
}
#ifdef __FreeBSD__
INP_LOCK(inp);
=2D *uid =3D inp->inp_socket->so_cred->cr_uid;
=2D *gid =3D inp->inp_socket->so_cred->cr_groups[0];
+ if (jailed(inp->inp_socket->so_cred))
+ *jid =3D inp->inp_socket->so_cred->cr_prison->pr_id;
+ cru2x(inp->inp_socket->so_cred, cred_cache);
INP_UNLOCK(inp);
INP_INFO_RUNLOCK(pi);
#else
@@ -2662,8 +2714,13 @@
u_int16_t bport, nport =3D 0;
sa_family_t af =3D pd->af;
int lookup =3D -1;
+#ifdef __FreeBSD__
+ struct xucred cred_cache;
+ int jid;
+#else
uid_t uid;
gid_t gid;
+#endif
struct pf_rule *r, *a =3D NULL;
struct pf_ruleset *ruleset =3D NULL;
struct pf_src_node *nsn =3D NULL;
@@ -2733,14 +2790,26 @@
else if ((r->flagset & th->th_flags) !=3D r->flags)
r =3D TAILQ_NEXT(r, entries);
else if (r->uid.op && (lookup !=3D -1 || (lookup =3D
+#ifdef __FreeBSD__
+ pf_socket_lookup(&cred_cache, &jid, direction, pd), 1))
+ && !pf_match_uid(r->uid.op, r->uid.uid[0], r->uid.uid[1],
+ &cred_cache))
+#else
pf_socket_lookup(&uid, &gid, direction, pd), 1)) &&
!pf_match_uid(r->uid.op, r->uid.uid[0], r->uid.uid[1],
uid))
+#endif
r =3D TAILQ_NEXT(r, entries);
else if (r->gid.op && (lookup !=3D -1 || (lookup =3D
+#ifdef __FreeBSD__
+ pf_socket_lookup(&cred_cache, &jid, direction, pd), 1))
+ && !pf_match_gid(r->gid.op, r->gid.gid[0], r->gid.gid[1],
+ &cred_cache))
+#else
pf_socket_lookup(&uid, &gid, direction, pd), 1)) &&
!pf_match_gid(r->gid.op, r->gid.gid[0], r->gid.gid[1],
gid))
+#endif
r =3D TAILQ_NEXT(r, entries);
else if (r->match_tag && !pf_match_tag(m, r, nr, pftag, &tag))
r =3D TAILQ_NEXT(r, entries);
@@ -3022,8 +3091,13 @@
u_int16_t bport, nport =3D 0;
sa_family_t af =3D pd->af;
int lookup =3D -1;
+#ifdef __FreeBSD__
+ struct xucred cred_cache;
+ int jid;
+#else
uid_t uid;
gid_t gid;
+#endif
struct pf_rule *r, *a =3D NULL;
struct pf_ruleset *ruleset =3D NULL;
struct pf_src_node *nsn =3D NULL;
@@ -3090,14 +3164,26 @@
else if (r->rule_flag & PFRULE_FRAGMENT)
r =3D TAILQ_NEXT(r, entries);
else if (r->uid.op && (lookup !=3D -1 || (lookup =3D
+#ifdef __FreeBSD__
+ pf_socket_lookup(&cred_cache, &jid, direction, pd), 1))
+ && !pf_match_uid(r->uid.op, r->uid.uid[0], r->uid.uid[1],
+ &cred_cache))
+#else
pf_socket_lookup(&uid, &gid, direction, pd), 1)) &&
!pf_match_uid(r->uid.op, r->uid.uid[0], r->uid.uid[1],
uid))
+#endif
r =3D TAILQ_NEXT(r, entries);
else if (r->gid.op && (lookup !=3D -1 || (lookup =3D
+#ifdef __FreeBSD__
+ pf_socket_lookup(&cred_cache, &jid, direction, pd), 1))
+ && !pf_match_gid(r->gid.op, r->gid.gid[0], r->gid.gid[1],
+ &cred_cache))
+#else
pf_socket_lookup(&uid, &gid, direction, pd), 1)) &&
!pf_match_gid(r->gid.op, r->gid.gid[0], r->gid.gid[1],
gid))
+#endif
r =3D TAILQ_NEXT(r, entries);
else if (r->match_tag && !pf_match_tag(m, r, nr, pftag, &tag))
r =3D TAILQ_NEXT(r, entries);
Index: pfvar.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /usr/store/mlaier/fcvs/src/sys/contrib/pf/net/pfvar.h,v
retrieving revision 1.7
diff -u -r1.7 pfvar.h
=2D-- pfvar.h 22 Jun 2004 20:13:25 -0000 1.7
+++ pfvar.h 19 Jul 2004 00:56:59 -0000
@@ -58,6 +58,8 @@
struct sockaddr_in sin;
struct sockaddr_in6 sin6;
};
+
+struct xucred;
#endif
=20
#include <netinet/tcp_fsm.h>
@@ -1470,8 +1472,13 @@
struct pf_addr *, sa_family_t);
int pf_match(u_int8_t, u_int32_t, u_int32_t, u_int32_t);
int pf_match_port(u_int8_t, u_int16_t, u_int16_t, u_int16_t);
+#ifdef __FreeBSD__
+int pf_match_uid(u_int8_t, uid_t, uid_t, struct xucred *);
+int pf_match_gid(u_int8_t, gid_t, gid_t, struct xucred *);
+#else
int pf_match_uid(u_int8_t, uid_t, uid_t, uid_t);
int pf_match_gid(u_int8_t, gid_t, gid_t, gid_t);
+#endif
=20
void pf_normalize_init(void);
int pf_normalize_ip(struct mbuf **, int, struct pfi_kif *, u_short *);
--Boundary-01=_qBa/AZ1dqu+XxAS--
--Boundary-03=_yBa/Av/NmIRhcls
Content-Type: application/pgp-signature
Content-Description: signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (FreeBSD)
iD8DBQBA/aByXyyEoT62BG0RAuKRAJ9iQKwlrRRtl8pj3NlJmlhz2z1KVgCfa9p4
bDl5Gt009rjImO0b9c+fUiY=
=H9EW
-----END PGP SIGNATURE-----
--Boundary-03=_yBa/Av/NmIRhcls--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200407210045.06274.max>