Re: pledge(2) sndioctl(1)
On Fri, May 22, 2020 at 08:10:54AM +0100, Ricardo Mestre wrote: > Hello, > > I tried to open the raw device but now it seems I was to sleepy to > figure out that I couldn't access it due to sndiod(8) having the device > opened earlier and therefore coundn't reach that code path. > > Here's the audio promise added, but maybe it raises the question again > if these utilities should have direct access to the devices, and also > includes sndiod(8) not running when this is done? > > ratchov@ will this change eventually? Otherwise the below keeps that > code path happy. As devices nodes are disabled by default for regular users, the "audio" promise doesn't add additional possibilities for regular users. On the other hand, running sndioctl as root on the raw device is useful for testing and experimenting. ok ratchov
Re: pledge(2) sndioctl(1)
Hello,
I tried to open the raw device but now it seems I was to sleepy to
figure out that I couldn't access it due to sndiod(8) having the device
opened earlier and therefore coundn't reach that code path.
Here's the audio promise added, but maybe it raises the question again
if these utilities should have direct access to the devices, and also
includes sndiod(8) not running when this is done?
ratchov@ will this change eventually? Otherwise the below keeps that
code path happy.
Index: sndioctl.c
===
RCS file: /cvs/src/usr.bin/sndioctl/sndioctl.c,v
retrieving revision 1.10
diff -u -p -u -r1.10 sndioctl.c
--- sndioctl.c 17 May 2020 05:39:32 - 1.10
+++ sndioctl.c 22 May 2020 07:01:30 -
@@ -948,6 +948,13 @@ main(int argc, char **argv)
fprintf(stderr, "%s: can't open control device\n", devname);
exit(1);
}
+
+ if (pledge("stdio audio", NULL) == -1) {
+ fprintf(stderr, "%s: pledge: %s\n", getprogname(),
+ strerror(errno));
+ exit(1);
+ }
+
if (!sioctl_ondesc(hdl, ondesc, NULL)) {
fprintf(stderr, "%s: can't get device description\n", devname);
exit(1);
On 08:25 Fri 22 May , Sebastien Marie wrote:
> On Fri, May 22, 2020 at 06:57:00AM +0200, Sebastien Marie wrote:
> > On Thu, May 21, 2020 at 11:07:39PM +0100, Ricardo Mestre wrote:
> > > Hi,
> > >
> > > After the handle sioctl_hdl `hdl' is opened (which in itself requires rw
> > > fs
> > > access and opening an unix socket) then all operations happen over that
> > > handle
> > > so the program may be restricted to only "stdio".
> > >
> > > All options were tested successfully, including the examples from the
> > > manpage
> > > plus tweaking the audio from an app ($MYBROWSER).
> >
> > Did you only perform runtime checking ? or also auditing the code source ?
> >
> > Because depending your hardware and the way sndiod is configured, it isn't
> > necessary the same code path used.
> >
> > Just one example: with device "snd/0", the sioctl_open() will use
> > _sioctl_aucat_open() for handler initialisation, whereas with "rsnd/0", it
> > is
> > _sioctl_sun_open().
> >
> > As all functions called after sioctl_open() are using the handler, you might
> > have tested only a part of the code path.
> >
> > I didn't look deeper. So it might fine, but it could also break some
> > configurations.
>
> I looked a bit deeper.
>
> commit()
> sioctl_setval()
>[with "rsnd/0"]
>sioctl_sun_setctl()
> setvol()
>ioctl(hdl->fd, AUDIO_MIXER_WRITE, &ctrl)
>
> calling ioctl(AUDIO_MIXER_WRITE) should required "audio".
>
> whereas when using default "snd/0" device, it is calling sioctl_aucat_setctl()
> and it will write(2) a message to sndiod(8) (and "stdio" is enough).
>
> I agree that "rsnd/0" isn't the standard case (it requires root to read/write
> to
> /dev), but it means you will broke sndioctl(1) in such cases.
>
> Thanks.
> --
> Sebastien Marie
>
Re: pledge(2) sndioctl(1)
On Fri, May 22, 2020 at 06:57:00AM +0200, Sebastien Marie wrote: > On Thu, May 21, 2020 at 11:07:39PM +0100, Ricardo Mestre wrote: > > Hi, > > > > After the handle sioctl_hdl `hdl' is opened (which in itself requires rw fs > > access and opening an unix socket) then all operations happen over that > > handle > > so the program may be restricted to only "stdio". > > > > All options were tested successfully, including the examples from the > > manpage > > plus tweaking the audio from an app ($MYBROWSER). > > Did you only perform runtime checking ? or also auditing the code source ? > > Because depending your hardware and the way sndiod is configured, it isn't > necessary the same code path used. > > Just one example: with device "snd/0", the sioctl_open() will use > _sioctl_aucat_open() for handler initialisation, whereas with "rsnd/0", it is > _sioctl_sun_open(). > > As all functions called after sioctl_open() are using the handler, you might > have tested only a part of the code path. > > I didn't look deeper. So it might fine, but it could also break some > configurations. I looked a bit deeper. commit() sioctl_setval() [with "rsnd/0"] sioctl_sun_setctl() setvol() ioctl(hdl->fd, AUDIO_MIXER_WRITE, &ctrl) calling ioctl(AUDIO_MIXER_WRITE) should required "audio". whereas when using default "snd/0" device, it is calling sioctl_aucat_setctl() and it will write(2) a message to sndiod(8) (and "stdio" is enough). I agree that "rsnd/0" isn't the standard case (it requires root to read/write to /dev), but it means you will broke sndioctl(1) in such cases. Thanks. -- Sebastien Marie
Re: pledge(2) sndioctl(1)
On Thu, May 21, 2020 at 11:07:39PM +0100, Ricardo Mestre wrote: > Hi, > > After the handle sioctl_hdl `hdl' is opened (which in itself requires rw fs > access and opening an unix socket) then all operations happen over that handle > so the program may be restricted to only "stdio". > > All options were tested successfully, including the examples from the manpage > plus tweaking the audio from an app ($MYBROWSER). Did you only perform runtime checking ? or also auditing the code source ? Because depending your hardware and the way sndiod is configured, it isn't necessary the same code path used. Just one example: with device "snd/0", the sioctl_open() will use _sioctl_aucat_open() for handler initialisation, whereas with "rsnd/0", it is _sioctl_sun_open(). As all functions called after sioctl_open() are using the handler, you might have tested only a part of the code path. I didn't look deeper. So it might fine, but it could also break some configurations. Thanks. -- Sebastien Marie
Re: pledge(2) sndioctl(1)
On Thu, May 21, 2020 at 11:07:39PM +0100, Ricardo Mestre wrote:
> Hi,
>
> After the handle sioctl_hdl `hdl' is opened (which in itself requires rw fs
> access and opening an unix socket) then all operations happen over that handle
> so the program may be restricted to only "stdio".
>
> All options were tested successfully, including the examples from the manpage
> plus tweaking the audio from an app ($MYBROWSER).
>
> Comments? OK?
Works for me.
ok brynet@
> Index: sndioctl.c
> ===
> RCS file: /cvs/src/usr.bin/sndioctl/sndioctl.c,v
> retrieving revision 1.10
> diff -u -p -u -r1.10 sndioctl.c
> --- sndioctl.c17 May 2020 05:39:32 - 1.10
> +++ sndioctl.c21 May 2020 22:04:58 -
> @@ -948,6 +948,13 @@ main(int argc, char **argv)
> fprintf(stderr, "%s: can't open control device\n", devname);
> exit(1);
> }
> +
> + if (pledge("stdio", NULL) == -1) {
> + fprintf(stderr, "%s: pledge: %s\n", getprogname(),
> + strerror(errno));
> + exit(1);
> + }
> +
> if (!sioctl_ondesc(hdl, ondesc, NULL)) {
> fprintf(stderr, "%s: can't get device description\n", devname);
> exit(1);
>
>
pledge(2) sndioctl(1)
Hi,
After the handle sioctl_hdl `hdl' is opened (which in itself requires rw fs
access and opening an unix socket) then all operations happen over that handle
so the program may be restricted to only "stdio".
All options were tested successfully, including the examples from the manpage
plus tweaking the audio from an app ($MYBROWSER).
Comments? OK?
Index: sndioctl.c
===
RCS file: /cvs/src/usr.bin/sndioctl/sndioctl.c,v
retrieving revision 1.10
diff -u -p -u -r1.10 sndioctl.c
--- sndioctl.c 17 May 2020 05:39:32 - 1.10
+++ sndioctl.c 21 May 2020 22:04:58 -
@@ -948,6 +948,13 @@ main(int argc, char **argv)
fprintf(stderr, "%s: can't open control device\n", devname);
exit(1);
}
+
+ if (pledge("stdio", NULL) == -1) {
+ fprintf(stderr, "%s: pledge: %s\n", getprogname(),
+ strerror(errno));
+ exit(1);
+ }
+
if (!sioctl_ondesc(hdl, ondesc, NULL)) {
fprintf(stderr, "%s: can't get device description\n", devname);
exit(1);
