On Sun, Jan 20, 2019 at 12:56:22PM +0000, Stuart Henderson wrote:
> 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

Not like restricting arp to root..., it's more like TCP hiding its ISN.

> 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).

I'll try to get around to it tomorrow if I can.  Otherwise just drop the
request. :-)

Best Regards,
-peter

> > 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;
> > 

Reply via email to