Re: sensors hiding with pledge
On 2019/01/21 22:34, Theo de Raadt wrote: > This approach seems backwards. > > It is hiding sensors from programs which are pledged (ie. we put effort into > security, therefore a fig leaf for privacy) > > But.. in programs we cannot pledge, we continue exporting. > > Yes chrome is pledged so permanently has no access to the information. > > I am not loving this. Agreed. The way pledge works for everything else is to disable the subsystem by default and allow programs to opt in. If restricting location information is needed then an approach more like the microphone disabling might make more sense. It seems more a "per user" decision than a "per app" decision. (Of course most programs would never need it - but the browsers, i.e. what people are most worried about, arguably *do* have a reason to opt in). On 2019/01/21 23:19, Constantine A. Murenin wrote: > Wouldn't this break sensorsd? (It's already been converted to use pledge.) Yes. And using "sensors" as a proxy for "location" doesn't make a lot of sense either - that affects probably about 3 people who "ldattach nmea". To actually improve things for the majority of users, it needs to restrict bssid+nwid from wlan scan results.
Re: sensors hiding with pledge
This approach seems backwards.
It is hiding sensors from programs which are pledged (ie. we put effort into
security, therefore a fig leaf for privacy)
But.. in programs we cannot pledge, we continue exporting.
Yes chrome is pledged so permanently has no access to the information.
I am not loving this.
> We recently had a thread about adding more sensors, but then the browser will
> use them to spy on us, and everybody was sad. We allow hw.sensors even for
> pledge processes because ntpd needs to read the time. However, ntpd only needs
> to read the time.
>
> This diff zeroes out sensors other than timedeltas. Maybe some others can be
> added as needed, but that seemed a good place to start. I didn't want to
> change the code too much (i.e. hide the existence of sensors entirely) so it
> just changes them all to 0 valued plain integer sensors.
>
> Thoughts?
>
> Index: kern_sysctl.c
> ===
> RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
> retrieving revision 1.353
> diff -u -p -r1.353 kern_sysctl.c
> --- kern_sysctl.c 19 Jan 2019 01:53:44 - 1.353
> +++ kern_sysctl.c 22 Jan 2019 02:01:30 -
> @@ -137,7 +137,7 @@ int sysctl_proc_nobroadcastkill(int *, u
> struct proc *);
> int sysctl_proc_vmmap(int *, u_int, void *, size_t *, struct proc *);
> int sysctl_intrcnt(int *, u_int, void *, size_t *);
> -int sysctl_sensors(int *, u_int, void *, size_t *, void *, size_t);
> +int sysctl_sensors(int *, u_int, void *, size_t *, void *, size_t, struct
> proc *);
> int sysctl_cptime2(int *, u_int, void *, size_t *, void *, size_t);
> #if NAUDIO > 0
> int sysctl_audio(int *, u_int, void *, size_t *, void *, size_t);
> @@ -735,7 +735,7 @@ hw_sysctl(int *name, u_int namelen, void
> #ifndef SMALL_KERNEL
> case HW_SENSORS:
> return (sysctl_sensors(name + 1, namelen - 1, oldp, oldlenp,
> - newp, newlen));
> + newp, newlen, p));
> case HW_SETPERF:
> return (sysctl_hwsetperf(oldp, oldlenp, newp, newlen));
> case HW_PERFPOLICY:
> @@ -2302,7 +2302,7 @@ sysctl_intrcnt(int *name, u_int namelen,
>
> int
> sysctl_sensors(int *name, u_int namelen, void *oldp, size_t *oldlenp,
> -void *newp, size_t newlen)
> +void *newp, size_t newlen, struct proc *p)
> {
> struct ksensor *ks;
> struct sensor *us;
> @@ -2350,6 +2350,22 @@ sysctl_sensors(int *name, u_int namelen,
> us->status = ks->status;
> us->numt = ks->numt;
> us->flags = ks->flags;
> +
> + /* not all sensors exposed to pledged processes */
> + if (p->p_p->ps_flags & PS_PLEDGE) {
> + switch (us->type) {
> + case SENSOR_TIMEDELTA:
> + break;
> + default:
> + memset(us->desc, 0, sizeof(us->desc));
> + memset(>tv, 0, sizeof(us->tv));
> + us->value = 0;
> + us->type = SENSOR_INTEGER;
> + us->status = SENSOR_S_UNKNOWN;
> + us->flags = SENSOR_FUNKNOWN;
> + break;
> + }
> + }
>
> ret = sysctl_rdstruct(oldp, oldlenp, newp, us,
> sizeof(struct sensor));
>
Re: sensors hiding with pledge
Wouldn't this break sensorsd? (It's already been converted to use pledge.)
C.
On Mon, 21 Jan 2019 at 20:19, Ted Unangst wrote:
>
> We recently had a thread about adding more sensors, but then the browser will
> use them to spy on us, and everybody was sad. We allow hw.sensors even for
> pledge processes because ntpd needs to read the time. However, ntpd only needs
> to read the time.
>
> This diff zeroes out sensors other than timedeltas. Maybe some others can be
> added as needed, but that seemed a good place to start. I didn't want to
> change the code too much (i.e. hide the existence of sensors entirely) so it
> just changes them all to 0 valued plain integer sensors.
>
> Thoughts?
>
> Index: kern_sysctl.c
> ===
> RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
> retrieving revision 1.353
> diff -u -p -r1.353 kern_sysctl.c
> --- kern_sysctl.c 19 Jan 2019 01:53:44 - 1.353
> +++ kern_sysctl.c 22 Jan 2019 02:01:30 -
> @@ -137,7 +137,7 @@ int sysctl_proc_nobroadcastkill(int *, u
> struct proc *);
> int sysctl_proc_vmmap(int *, u_int, void *, size_t *, struct proc *);
> int sysctl_intrcnt(int *, u_int, void *, size_t *);
> -int sysctl_sensors(int *, u_int, void *, size_t *, void *, size_t);
> +int sysctl_sensors(int *, u_int, void *, size_t *, void *, size_t, struct
> proc *);
> int sysctl_cptime2(int *, u_int, void *, size_t *, void *, size_t);
> #if NAUDIO > 0
> int sysctl_audio(int *, u_int, void *, size_t *, void *, size_t);
> @@ -735,7 +735,7 @@ hw_sysctl(int *name, u_int namelen, void
> #ifndefSMALL_KERNEL
> case HW_SENSORS:
> return (sysctl_sensors(name + 1, namelen - 1, oldp, oldlenp,
> - newp, newlen));
> + newp, newlen, p));
> case HW_SETPERF:
> return (sysctl_hwsetperf(oldp, oldlenp, newp, newlen));
> case HW_PERFPOLICY:
> @@ -2302,7 +2302,7 @@ sysctl_intrcnt(int *name, u_int namelen,
>
> int
> sysctl_sensors(int *name, u_int namelen, void *oldp, size_t *oldlenp,
> -void *newp, size_t newlen)
> +void *newp, size_t newlen, struct proc *p)
> {
> struct ksensor *ks;
> struct sensor *us;
> @@ -2350,6 +2350,22 @@ sysctl_sensors(int *name, u_int namelen,
> us->status = ks->status;
> us->numt = ks->numt;
> us->flags = ks->flags;
> +
> + /* not all sensors exposed to pledged processes */
> + if (p->p_p->ps_flags & PS_PLEDGE) {
> + switch (us->type) {
> + case SENSOR_TIMEDELTA:
> + break;
> + default:
> + memset(us->desc, 0, sizeof(us->desc));
> + memset(>tv, 0, sizeof(us->tv));
> + us->value = 0;
> + us->type = SENSOR_INTEGER;
> + us->status = SENSOR_S_UNKNOWN;
> + us->flags = SENSOR_FUNKNOWN;
> + break;
> + }
> + }
>
> ret = sysctl_rdstruct(oldp, oldlenp, newp, us,
> sizeof(struct sensor));
>
