NetBSD Problem Report #55815
From martin@aprisoft.de Sun Nov 22 13:21:05 2020
Return-Path: <martin@aprisoft.de>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
by mollari.NetBSD.org (Postfix) with ESMTPS id 591A31A921F
for <gnats-bugs@gnats.NetBSD.org>; Sun, 22 Nov 2020 13:21:05 +0000 (UTC)
Message-Id: <20201122132055.4E0B85CC848@emmas.aprisoft.de>
Date: Sun, 22 Nov 2020 14:20:55 +0100 (CET)
From: martin@NetBSd.org
Reply-To: martin@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: tar opens device files
X-Send-Pr-Version: 3.95
>Number: 55815
>Category: bin
>Synopsis: tar opens device files
>Confidential: no
>Severity: critical
>Priority: high
>Responsible: bin-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Sun Nov 22 13:25:00 +0000 2020
>Last-Modified: Mon Jun 07 02:40:01 +0000 2021
>Originator: Martin Husemann
>Release: NetBSD 9.99.75
>Organization:
The NetBSD Foundation, Inc.
>Environment:
System: NetBSD seven-days-to-the-wolves.aprisoft.de 9.99.75 NetBSD 9.99.75 (GENERIC) #425: Wed Nov 4 15:34:33 CET 2020 martin@seven-days-to-the-wolves.aprisoft.de:/work/src/sys/arch/amd64/compile/GENERIC amd64
Architecture: x86_64
Machine: amd64
>Description:
Tar does open device files, which is a *very* bad idea.
>How-To-Repeat:
ktrace tar cvf - /dev >/dev/null
(but carefull, might kill your machine)
>Fix:
Switch MKBSDTAR back to off.
>Audit-Trail:
From: Joerg Sonnenberger <joerg@bec.de>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/55815: tar opens device files
Date: Tue, 24 Nov 2020 21:12:19 +0100
On Sun, Nov 22, 2020 at 01:25:00PM +0000, martin@NetBSd.org wrote:
> Tar does open device files, which is a *very* bad idea.
As discussed on IRC, this is a side effect of enabling ACL and extattr
support OOB. It is avoided when using --no-acls --no-extattr. Using only
the fd-based API variants is established practise, e.g. GNU tar has the
same behavior. Other options would be racy, which is especially bad when
dealing with file permissions.
Most systems consider device open with side effect a bug. I can remember
two cases in NetBSD off the top of my head. Tapes and IOPL device on
x86. The former is harmless as side effect as far as archiving is
concerned. The former has the rewind on open. IMO that should either be
completely removed or deferred to the first actual IO operation.
Joerg
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/55815: tar opens device files
Date: Wed, 25 Nov 2020 06:54:12 +0100
On Tue, Nov 24, 2020 at 08:15:02PM +0000, Joerg Sonnenberger wrote:
> As discussed on IRC, this is a side effect of enabling ACL and extattr
> support OOB. It is avoided when using --no-acls --no-extattr.
Those should be the default then, or maybe check (is there a mount flag
or something in statvfs?) whether any ACLs or ext attrs could be there at
all.
What happens if the device can not be opened at all, will it be left out of
the tar?
Martin
From: Joerg Sonnenberger <joerg@bec.de>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, martin@NetBSD.org
Subject: Re: bin/55815: tar opens device files
Date: Wed, 25 Nov 2020 10:10:43 +0100
On Wed, Nov 25, 2020 at 05:55:01AM +0000, Martin Husemann wrote:
> The following reply was made to PR bin/55815; it has been noted by GNATS.
>
> From: Martin Husemann <martin@duskware.de>
> To: gnats-bugs@netbsd.org
> Cc:
> Subject: Re: bin/55815: tar opens device files
> Date: Wed, 25 Nov 2020 06:54:12 +0100
>
> On Tue, Nov 24, 2020 at 08:15:02PM +0000, Joerg Sonnenberger wrote:
> > As discussed on IRC, this is a side effect of enabling ACL and extattr
> > support OOB. It is avoided when using --no-acls --no-extattr.
>
> Those should be the default then, or maybe check (is there a mount flag
> or something in statvfs?) whether any ACLs or ext attrs could be there at
> all.
That would be the ostrich approach of ignoring that open with side
effects is fundamentally a bad idea. It also doesn't match what other
tar implementations do and violates POLA just as well.
>
> What happens if the device can not be opened at all, will it be left out of
> the tar?
No ACLs or extattrs will be saved in that case.
Joerg
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/55815: tar opens device files
Date: Wed, 25 Nov 2020 10:19:16 +0100
On Wed, Nov 25, 2020 at 10:10:43AM +0100, Joerg Sonnenberger wrote:
> > Those should be the default then, or maybe check (is there a mount flag
> > or something in statvfs?) whether any ACLs or ext attrs could be there at
> > all.
>
> That would be the ostrich approach of ignoring that open with side
> effects is fundamentally a bad idea. It also doesn't match what other
> tar implementations do and violates POLA just as well.
I agree that device open having an effect on the state of the device is
a bad thing in general, but it has been the case in Unix since the beginning,
or am I misremembering something? At least it was that way when dealing with
all tapes I ever dealt with.
But besides that: if there *could* be no extattr or ACL on the file system
(like I *knew* for sure for the case at hand, it was an anciend FFSv1
file system way too old for any of that) and making that information
available (like with a mount flag or a fs superblock flag or whatever) -
why is skipping the unneccessary open in that case an ostrich aproach?
Or why would it violate POLA? I would call it a smart optimization instead,
and would be very suprised to see thousands of totally unneeded open()
calls.
Martin
From: Christos Zoulas <christos@zoulas.com>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org,
"martin@netbsd.org" <martin@NetBSD.org>
Subject: Re: bin/55815: tar opens device files
Date: Wed, 25 Nov 2020 08:34:16 -0500
Unfortunately it looks like implementing a no-side-effects open for some =
devices is=20
difficult because those devices (tapes in particular) rely on the =
different semantics=20
for open depending on different minor numbers. As joerg mentions, =
deferring this=20
to the first I/O would be a good thing but it is not trivial to do and =
we have a bunch=20
of them. What we seem to want is an open() flavor that gives us access =
just to the
extended attributes and stat info, so we could just do that (open =
O_EXTATTR)?
christos=
From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/55815: tar opens device files
Date: Wed, 25 Nov 2020 22:51:59 +0700
Date: Wed, 25 Nov 2020 09:20:01 +0000 (UTC)
From: Martin Husemann <martin@duskware.de>
Message-ID: <20201125092001.974C81A921F@mollari.NetBSD.org>
| I agree that device open having an effect on the state of the device is
| a bad thing in general,
I'm not sue I really agree, though "deferred to the first I/O" would be
semantically equivalent I think (except that with tapes, just open/close
has always been a method to accomplish rewind), so that would be possible.
But not "doing things" at all would be wrong (would break a long time
interface expectation, whatever anyone believe it ought to be).
| but it has been the case in Unix since the beginning,
| or am I misremembering something?
You're not, that is the way it has always been - or at least as far back
as I can remember (I never saw any of 1st-4th edition systems).
I am not sure I understand the point here though, why is tar opening
files at all if it doesn't plan on reading from them? Is this some
kind of optimisation attempt, where since most files will be regular
filesystem files, and will need to be opened, open() then fstat() has
one less pathname lookup (namei) than stat() followed when needed by open()
(the normal case)? If so, we should just stop doing that, as it is broken,
and revert to the stat()+open() model (where the open only happens if we
want the data contained in the file, rather than just its name and attributes).
We need that for symnlinks too (ie: the stat() would actually be lstat()),
surely.
I don't know what's available for fetching extattrs, but if that requires
an open, we should fix that, and make them available without opening.
But surely there is already a way, I can't believe that ls attempts to
open files, yet it shows (when asked) extattrs, right?
kre
From: Joerg Sonnenberger <joerg@bec.de>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, martin@NetBSD.org
Subject: Re: bin/55815: tar opens device files
Date: Wed, 25 Nov 2020 19:06:26 +0100
On Wed, Nov 25, 2020 at 03:55:01PM +0000, Robert Elz wrote:
> I am not sure I understand the point here though, why is tar opening
> files at all if it doesn't plan on reading from them? Is this some
> kind of optimisation attempt, where since most files will be regular
> filesystem files, and will need to be opened, open() then fstat() has
> one less pathname lookup (namei) than stat() followed when needed by open()
> (the normal case)? If so, we should just stop doing that, as it is broken,
> and revert to the stat()+open() model (where the open only happens if we
> want the data contained in the file, rather than just its name and attributes).
> We need that for symnlinks too (ie: the stat() would actually be lstat()),
> surely.
No, it is about ensuring that the extattr and ACL actually belong to the
same object as the rest of the permissions.
Joerg
From: Christos Zoulas <christos@zoulas.com>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org,
"martin@netbsd.org" <martin@NetBSD.org>
Subject: Re: bin/55815: tar opens device files
Date: Wed, 25 Nov 2020 13:20:28 -0500
> I don't know what's available for fetching extattrs, but if that requires
> an open, we should fix that, and make them available without opening.
> But surely there is already a way, I can't believe that ls attempts to
> open files, yet it shows (when asked) extattrs, right?
There are methods by pathname and by fd. By pathname suffer from
TOCTOU, so we need to do by fd. To get an fd we need open. So the
only reasonable solution seems to be to provide an open flag that does
not have side effects on devices.
christos
From: Robert Elz <kre@munnari.OZ.AU>
To: Joerg Sonnenberger <joerg@bec.de>
Cc: gnats-bugs@netbsd.org, martin@NetBSD.org
Subject: Re: bin/55815: tar opens device files
Date: Thu, 26 Nov 2020 02:06:34 +0700
Date: Wed, 25 Nov 2020 19:06:26 +0100
From: Joerg Sonnenberger <joerg@bec.de>
Message-ID: <20201125180626.GA111551@bec.de>
| No, it is about ensuring that the extattr and ACL actually belong to the
| same object as the rest of the permissions.
Ah, OK.
Is this for creating the archive, or extracting it?
In the creating case, does it really matter? If the filesystem is
changing while attempting to create an archive, then there's got to
be race conditions, and while it would be nice to correctly add either
the before, or the after to the archive, I'm not sure that's important
enough to really matter.
In the extracting case, at least when extracting a non-openable (or
potentially non-openable) file, couldn't tar extract to a file in a
randomnly named 700 mode sub-dir, set the attributes/acl on the file in
that sub-dir, and then rename it to where it belongs?
Wouldn't any attempt to open the file (almost) necessarily hang when
the file is a fifo ? But those deserve to be archived/extracted
just as accurately as regular files, directories, and special files.
(I'm not sure the same is true of sockets, but perhaps).
kre
From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/55815: tar opens device files
Date: Fri, 4 Jun 2021 17:09:53 +0000
On Wed, Nov 25, 2020 at 07:10:01PM +0000, Robert Elz wrote:
> | No, it is about ensuring that the extattr and ACL actually belong to the
> | same object as the rest of the permissions.
>
> Ah, OK.
>
> Is this for creating the archive, or extracting it?
>
> In the creating case, does it really matter? If the filesystem is
> changing while attempting to create an archive, then there's got to
> be race conditions, and while it would be nice to correctly add either
> the before, or the after to the archive, I'm not sure that's important
> enough to really matter.
I am sure there are people who do backups by tar -c / while the system
is live, and in general even if that doesn't generate a fully
consistent snapshot it would probably be bad for it to result in wrong
and possibly malicious permissions after restore.
However, it also seems foolish to pretend this is a real issue for
device nodes, so it seems like a perfectly adequate solution is for
tar to check for device nodes and not open them. Adding another open
mode seems like severe overkill. (O_NONBLOCK is sufficient for named
pipes.)
(What does tar do with filesystem sockets? You can't open them.)
Also, devices that do things at open time aren't going away, and it's
perfectly reasonable to have ACLs on devices, so just ignoring the
situation or bypassing it for non-ACL filesystems isn't the answer.
--
David A. Holland
dholland@netbsd.org
From: Joerg Sonnenberger <joerg@bec.de>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, martin@NetBSD.org
Subject: Re: bin/55815: tar opens device files
Date: Fri, 4 Jun 2021 22:36:50 +0200
On Fri, Jun 04, 2021 at 05:10:02PM +0000, David Holland wrote:
> However, it also seems foolish to pretend this is a real issue for
> device nodes, so it seems like a perfectly adequate solution is for
> tar to check for device nodes and not open them. Adding another open
> mode seems like severe overkill. (O_NONBLOCK is sufficient for named
> pipes.)
Checking for device nodes introduces TOCTOA problems though.
Joerg
From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/55815: tar opens device files
Date: Fri, 4 Jun 2021 23:22:09 +0000
On Fri, Jun 04, 2021 at 10:36:50PM +0200, Joerg Sonnenberger wrote:
> > However, it also seems foolish to pretend this is a real issue for
> > device nodes, so it seems like a perfectly adequate solution is for
> > tar to check for device nodes and not open them. Adding another open
> > mode seems like severe overkill. (O_NONBLOCK is sufficient for named
> > pipes.)
>
> Checking for device nodes introduces TOCTOA problems though.
Like I said, pretending that this is a real issue for device nodes is
foolish. Use lstat (you have to anyway to tar up links); if it's a
device, don't open it. Otherwise, open it with O_NOFOLLOW. If you then
get a device anyway and your tape RAID starts rewinding 50 tapes at
once, it's because root was screwing around. That's not our job to
stop.
--
David A. Holland
dholland@netbsd.org
From: Joerg Sonnenberger <joerg@bec.de>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, martin@NetBSD.org
Subject: Re: bin/55815: tar opens device files
Date: Sat, 5 Jun 2021 03:11:07 +0200
On Fri, Jun 04, 2021 at 11:25:02PM +0000, David Holland wrote:
> The following reply was made to PR bin/55815; it has been noted by GNATS.
>
> From: David Holland <dholland-bugs@netbsd.org>
> To: gnats-bugs@netbsd.org
> Cc:
> Subject: Re: bin/55815: tar opens device files
> Date: Fri, 4 Jun 2021 23:22:09 +0000
>
> On Fri, Jun 04, 2021 at 10:36:50PM +0200, Joerg Sonnenberger wrote:
> > > However, it also seems foolish to pretend this is a real issue for
> > > device nodes, so it seems like a perfectly adequate solution is for
> > > tar to check for device nodes and not open them. Adding another open
> > > mode seems like severe overkill. (O_NONBLOCK is sufficient for named
> > > pipes.)
> >
> > Checking for device nodes introduces TOCTOA problems though.
>
> Like I said, pretending that this is a real issue for device nodes is
> foolish. Use lstat (you have to anyway to tar up links); if it's a
> device, don't open it. Otherwise, open it with O_NOFOLLOW. If you then
> get a device anyway and your tape RAID starts rewinding 50 tapes at
> once, it's because root was screwing around. That's not our job to
> stop.
Which part of TOCTOA wasn't clear?
Joerg
From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/55815: tar opens device files
Date: Sat, 5 Jun 2021 05:08:17 -0000 (UTC)
joerg@bec.de (Joerg Sonnenberger) writes:
>Which part of TOCTOA wasn't clear?
That the "solution" has to be worse than the problem.
You can lstat() to handle devices and also need it for symlinks.
You can re-check like now for things you have to open() and ignore
the lstat test.
Then your tapes only rewind when someone sneaks in the device
node while you are archiving the files and the race condition is hit.
From: Christos Zoulas <christos@zoulas.com>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org,
"martin@netbsd.org" <martin@NetBSD.org>
Subject: Re: bin/55815: tar opens device files
Date: Sat, 5 Jun 2021 09:17:54 -0400
--Apple-Mail=_686C3893-A0BA-4A3B-862E-E15AD9F17BEC
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
charset=us-ascii
Well, if you want to avoid TOCTOA, you need something like linux's =
O_PATH
which opens the file for "stat" like access only. But even linux does =
not have
a way to "upgrade" that fd to be able to read, short of:
fd =3D open(path, O_PATH);
fstat(fd, &st);
if (is a device)
bail;
snprintf(buf, sizeof(buf), "/proc/self/fds/%d", fd);
nfd =3D open(buf, O_RDONLY);
We do have O_EXEC, but I am not sure if that is the same as O_PATH (if =
it
does not really open the device)
christos
--Apple-Mail=_686C3893-A0BA-4A3B-862E-E15AD9F17BEC
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename=signature.asc
Content-Type: application/pgp-signature;
name=signature.asc
Content-Description: Message signed with OpenPGP
-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org
iF0EARECAB0WIQS+BJlbqPkO0MDBdsRxESqxbLM7OgUCYLt5ggAKCRBxESqxbLM7
OmQsAJ0Tc/rY72tQlDhqcx9aDr2btMGvXACdF9kIbR12q+tNUwNROD1xJXinWAY=
=fRlL
-----END PGP SIGNATURE-----
--Apple-Mail=_686C3893-A0BA-4A3B-862E-E15AD9F17BEC--
From: Jason Thorpe <thorpej@me.com>
To: Christos Zoulas <christos@zoulas.com>
Cc: gnats-bugs@netbsd.org,
gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org,
"martin@netbsd.org" <martin@NetBSD.org>
Subject: Re: bin/55815: tar opens device files
Date: Sat, 5 Jun 2021 07:14:39 -0700
> On Jun 5, 2021, at 6:17 AM, Christos Zoulas <christos@zoulas.com> =
wrote:
>=20
> Well, if you want to avoid TOCTOA, you need something like linux's =
O_PATH
> which opens the file for "stat" like access only. But even linux does =
not have
> a way to "upgrade" that fd to be able to read, short of:
>=20
> fd =3D open(path, O_PATH);
> fstat(fd, &st);
> if (is a device)
> bail;
> snprintf(buf, sizeof(buf), "/proc/self/fds/%d", fd);
> nfd =3D open(buf, O_RDONLY);
>=20
> We do have O_EXEC, but I am not sure if that is the same as O_PATH (if =
it
> does not really open the device)
If we=E2=80=99re talking about =E2=80=9Censure the file we=E2=80=99re =
opening is a regular file=E2=80=9D, won=E2=80=99t O_REGULAR do that you =
want?
-- thorpej
From: Christos Zoulas <christos@zoulas.com>
To: Jason Thorpe <thorpej@me.com>
Cc: gnats-bugs@netbsd.org,
gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org,
"martin@netbsd.org" <martin@NetBSD.org>
Subject: Re: bin/55815: tar opens device files
Date: Sat, 5 Jun 2021 10:18:11 -0400
--Apple-Mail=_C26A2F85-84A1-4D72-9B09-A7FC762DBF12
Content-Type: multipart/alternative;
boundary="Apple-Mail=_E9450DEF-2453-4F17-A8CE-22FF42309949"
--Apple-Mail=_E9450DEF-2453-4F17-A8CE-22FF42309949
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
charset=utf-8
>=20
> If we=E2=80=99re talking about =E2=80=9Censure the file we=E2=80=99re =
opening is a regular file=E2=80=9D, won=E2=80=99t O_REGULAR do that you =
want?
Yes, but that suffers from TOCTOA too:
if (open(path, O_REGULAR) =3D=3D -1)
fstat(path, &st);
But that's probably good enough.
christos
--Apple-Mail=_E9450DEF-2453-4F17-A8CE-22FF42309949
Content-Transfer-Encoding: quoted-printable
Content-Type: text/html;
charset=utf-8
<html><head><meta http-equiv=3D"Content-Type" content=3D"text/html; =
charset=3Dutf-8"></head><body style=3D"word-wrap: break-word; =
-webkit-nbsp-mode: space; line-break: after-white-space;" class=3D""><br =
class=3D""><div><blockquote type=3D"cite" class=3D""><div class=3D""><br =
style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: =
12px; font-style: normal; font-variant-caps: normal; font-weight: =
normal; letter-spacing: normal; text-align: start; text-indent: 0px; =
text-transform: none; white-space: normal; word-spacing: 0px; =
-webkit-text-stroke-width: 0px; text-decoration: none;" class=3D""><span =
style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: =
12px; font-style: normal; font-variant-caps: normal; font-weight: =
normal; letter-spacing: normal; text-align: start; text-indent: 0px; =
text-transform: none; white-space: normal; word-spacing: 0px; =
-webkit-text-stroke-width: 0px; text-decoration: none; float: none; =
display: inline !important;" class=3D"">If we=E2=80=99re talking about =
=E2=80=9Censure the file we=E2=80=99re opening is a regular file=E2=80=9D,=
won=E2=80=99t O_REGULAR do that you want?</span><br style=3D"caret-color:=
rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: =
normal; font-variant-caps: normal; font-weight: normal; letter-spacing: =
normal; text-align: start; text-indent: 0px; text-transform: none; =
white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; =
text-decoration: none;" class=3D""></div></blockquote><br =
class=3D""></div><div>Yes, but that suffers from TOCTOA =
too:</div><div><br class=3D""></div><div>if (open(path, O_REGULAR) =3D=3D =
-1)</div><div><span class=3D"Apple-tab-span" style=3D"white-space:pre"> =
</span>fstat(path, &st);</div><div><br class=3D""></div><div>But =
that's probably good enough.</div><div><br =
class=3D""></div><div>christos</div><br class=3D""></body></html>=
--Apple-Mail=_E9450DEF-2453-4F17-A8CE-22FF42309949--
--Apple-Mail=_C26A2F85-84A1-4D72-9B09-A7FC762DBF12
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename=signature.asc
Content-Type: application/pgp-signature;
name=signature.asc
Content-Description: Message signed with OpenPGP
-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org
iF0EARECAB0WIQS+BJlbqPkO0MDBdsRxESqxbLM7OgUCYLuHpAAKCRBxESqxbLM7
OrpqAJ432v6SryzMNwDsTgSbB38g62jMkACffT0kQjFwNnB4Ivb88g3Nv/u+3sE=
=ERwl
-----END PGP SIGNATURE-----
--Apple-Mail=_C26A2F85-84A1-4D72-9B09-A7FC762DBF12--
From: Jason Thorpe <thorpej@me.com>
To: Christos Zoulas <christos@zoulas.com>
Cc: gnats-bugs@netbsd.org,
gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org,
"martin@netbsd.org" <martin@NetBSD.org>
Subject: Re: bin/55815: tar opens device files
Date: Sat, 5 Jun 2021 07:27:06 -0700
> On Jun 5, 2021, at 7:18 AM, Christos Zoulas <christos@zoulas.com> =
wrote:
>=20
>=20
>>=20
>> If we=E2=80=99re talking about =E2=80=9Censure the file we=E2=80=99re =
opening is a regular file=E2=80=9D, won=E2=80=99t O_REGULAR do that you =
want?
>=20
> Yes, but that suffers from TOCTOA too:
>=20
> if (open(path, O_REGULAR) =3D=3D -1)
> fstat(path, &st);
>=20
> But that's probably good enough.
Sorry, I was looking at your example of:
fd =3D open(...);
fstat(...);
if (device)
bail;
What's the point if stat'ing the path if opening with O_REGULAR fails? =
Isn't the point here to prevent tar from opening device files?
-- thorpej
From: Christos Zoulas <christos@zoulas.com>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org,
"martin@netbsd.org" <martin@NetBSD.org>
Subject: Re: bin/55815: tar opens device files
Date: Sat, 5 Jun 2021 10:47:43 -0400
--Apple-Mail=_89CA94F4-40E4-4AE1-8664-FC96ABBC5697
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
charset=us-ascii
> On Jun 5, 2021, at 10:30 AM, Jason Thorpe <thorpej@me.com> wrote:
>=20
> Sorry, I was looking at your example of:
>=20
> fd =3D3D open(...);
> fstat(...);
> if (device)
> bail;
>=20
> What's the point if stat'ing the path if opening with O_REGULAR fails? =
=3D
> Isn't the point here to prevent tar from opening device files?
>=20
> -- thorpej
>=20
That has no issue. The problem is that if it is a device you need a =
separate
stat(2). But even if we solve that, we still need to really open the =
file to get
extended attributes (ACLs). There is always the dump(8) solution (go =
under
the hood) :-)
christos
--Apple-Mail=_89CA94F4-40E4-4AE1-8664-FC96ABBC5697
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename=signature.asc
Content-Type: application/pgp-signature;
name=signature.asc
Content-Description: Message signed with OpenPGP
-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org
iF0EARECAB0WIQS+BJlbqPkO0MDBdsRxESqxbLM7OgUCYLuOjwAKCRBxESqxbLM7
OjJPAKCV9rcVLQI9Wo0GFz5jUFpGaQQL/ACffpAyJ7WxeBFyECl/LGqW5OcB40g=
=Aq9r
-----END PGP SIGNATURE-----
--Apple-Mail=_89CA94F4-40E4-4AE1-8664-FC96ABBC5697--
From: Joerg Sonnenberger <joerg@bec.de>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, martin@NetBSD.org
Subject: Re: bin/55815: tar opens device files
Date: Sat, 5 Jun 2021 22:22:51 +0200
On Sat, Jun 05, 2021 at 02:30:03PM +0000, Jason Thorpe wrote:
> The following reply was made to PR bin/55815; it has been noted by GNATS.
>
> From: Jason Thorpe <thorpej@me.com>
> To: Christos Zoulas <christos@zoulas.com>
> Cc: gnats-bugs@netbsd.org,
> gnats-admin@netbsd.org,
> netbsd-bugs@netbsd.org,
> "martin@netbsd.org" <martin@NetBSD.org>
> Subject: Re: bin/55815: tar opens device files
> Date: Sat, 5 Jun 2021 07:27:06 -0700
>
> > On Jun 5, 2021, at 7:18 AM, Christos Zoulas <christos@zoulas.com> =
> wrote:
> >=20
> >=20
> >>=20
> >> If we=E2=80=99re talking about =E2=80=9Censure the file we=E2=80=99re =
> opening is a regular file=E2=80=9D, won=E2=80=99t O_REGULAR do that you =
> want?
> >=20
> > Yes, but that suffers from TOCTOA too:
> >=20
> > if (open(path, O_REGULAR) =3D=3D -1)
> > fstat(path, &st);
> >=20
> > But that's probably good enough.
>
> Sorry, I was looking at your example of:
>
> fd =3D open(...);
> fstat(...);
> if (device)
> bail;
>
> What's the point if stat'ing the path if opening with O_REGULAR fails? =
> Isn't the point here to prevent tar from opening device files?
If the opening with O_REGULAR fails, you are already in the side path
for objects you don't want to open. Fifos, Unix sockets, devices. This
whole discussion shouldn't be about tar either. The whole "open has side
effects" problem applies to many, many other programs, so it should
really be considered in this wider context...
Joerg
From: Martin Husemann <martin@duskware.de>
To: Joerg Sonnenberger <joerg@bec.de>
Cc: gnats-bugs@netbsd.org
Subject: Re: bin/55815: tar opens device files
Date: Sun, 6 Jun 2021 08:50:27 +0200
On Sat, Jun 05, 2021 at 10:22:51PM +0200, Joerg Sonnenberger wrote:
> whole discussion shouldn't be about tar either. The whole "open has side
> effects" problem applies to many, many other programs, so it should
> really be considered in this wider context...
Totally independent of the wider "problem" (which I believe won't go away
any time soon) we should swith tar back if we can not easily fix the new
one. Where "fixing" (from my POV) would include simple hacks like making
the no-EA/no-ACLs option the default.
Martin
From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/55815: tar opens device files
Date: Mon, 7 Jun 2021 02:36:45 +0000
On Sat, Jun 05, 2021 at 08:25:01PM +0000, Joerg Sonnenberger wrote:
>> Isn't the point here to prevent tar from opening device files?
>
> If the opening with O_REGULAR fails, you are already in the side path
> for objects you don't want to open. Fifos, Unix sockets, devices. This
> whole discussion shouldn't be about tar either. The whole "open has side
> effects" problem applies to many, many other programs, so it should
> really be considered in this wider context...
It also isn't necessary to mine the moon if all you're after is some
rock.
The proper logic is
lstat
if (ISBLK || ISCHR) {
acl_get_link()
}
else if (ISLNK) {
acl_get_link()
readlink()
}
else {
fd = open(O_NOFOLLOW|O_NONBLOCK)
if (fd < 0) {
goto retry;
}
fstat(fd)
acl_get_fd(fd)
}
If doing this on a live system, the object can get shuffled between
the lstat and the acl_get_link/readlink calls, but:
- I assume tar is using single-layer names and not long paths, as
in the latter case all these worries are trivial by comparison;
- only root can shuffle device nodes, so there's no exploit path
there;
- anyone can mess with symlinks, but that's not a new problem and
nothing related to devices affects it;
- you _can_ check if the object you opened was the same as the
object lstat inspected, but there's no point; just ignore the
lstat results completely and proceed;
- O_NONBLOCK is sufficient to make named pipes safe.
I don't see what the problem is or why we're going around in circles
about it.
Yeah, it might be nice to have a way to open an object for O_NONE or
something (so you get a fd but all you can do with it is what you
could do via its name) but it's not necessary to fix this problem.
--
David A. Holland
dholland@netbsd.org
(Contact us)
$NetBSD: query-full-pr,v 1.46 2020/01/03 16:35:01 leot Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2020
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home