On 2019/01/18 10:59, Peter J. Philipp wrote: > I have "covered" up PPPoE Session ID's from users because it is a value that > is only gotten on the Data Link layer and historically non-root users did not > have access to that. It really is a value that doesn't concern them. I have > wrapped the display with a suser() conditional. The magic value 0xFFFF is > not used/reserved according to RFC 2516.
No real comment on whether we should do this or not (it feels a bit like restricting arp to root..) but if it is done, it would seem better to use a value which cannot be sent on the wire, rather than one which is just reserved. (And actually hide it from ifconfig rather than displaying a bogus value). > as root: > > beta# ifconfig pppoe0 > pppoe0: flags=8810<POINTOPOINT,SIMPLEX,MULTICAST> mtu 1492 > index 12 priority 0 llprio 3 > dev: state: initial > sid: 0x0 PADI retries: 0 PADR retries: 0 > groups: pppoe > > as non-root: > > beta$ ifconfig pppoe0 > pppoe0: flags=8810<POINTOPOINT,SIMPLEX,MULTICAST> mtu 1492 > index 12 priority 0 llprio 3 > dev: state: initial > sid: 0xffff PADI retries: 0 PADR retries: 0 > groups: pppoe > > > patch follows: > > Index: if_pppoe.c > =================================================================== > RCS file: /cvs/src/sys/net/if_pppoe.c,v > retrieving revision 1.67 > diff -u -p -u -r1.67 if_pppoe.c > --- if_pppoe.c 19 Feb 2018 08:59:52 -0000 1.67 > +++ if_pppoe.c 18 Jan 2019 09:50:58 -0000 > @@ -874,7 +876,12 @@ pppoe_ioctl(struct ifnet *ifp, unsigned > struct pppoeconnectionstate *state = > (struct pppoeconnectionstate *)data; > state->state = sc->sc_state; > - state->session_id = sc->sc_session; > + > + if (! suser(p)) > + state->session_id = sc->sc_session; > + else > + state->session_id = 0xffff; /* reserved sid */ > + > state->padi_retry_no = sc->sc_padi_retried; > state->padr_retry_no = sc->sc_padr_retried; > state->session_time.tv_sec = sc->sc_session_time.tv_sec; >
