From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Gibson <david@gibson.dropbear.id.au>
To: passt-dev@passt.top
Subject:
 Re: [PATCH v2 02/10] Split checking for root from dropping root privilege
Date: Sat, 10 Sep 2022 17:09:30 +1000
Message-ID: <Yxw4KmQ0ZkPir4zF@yekko>
In-Reply-To: <20220909163327.1e2dbc57@elisabeth>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============0392317290992836258=="

--===============0392317290992836258==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Sep 09, 2022 at 04:33:27PM +0200, Stefano Brivio wrote:
> Just some ridiculous nitpicking on this one:
>=20
> On Thu,  8 Sep 2022 13:58:59 +1000
> David Gibson <david(a)gibson.dropbear.id.au> wrote:
>=20
> > [...]
> >
> > +++ b/passt.1
> > @@ -104,9 +104,10 @@ terminal, and to both system logger and standard err=
or otherwise.
> > =20
> >  .TP
> >  .BR \-\-runas " " \fIUID\fR|\fIUID:GID\fR|\fILOGIN\fR|\fILOGIN:GROUP\fR
> > -If started as root, change to given UID and corresponding group if UID i=
s given,
> > +Attempt to change to given UID and corresponding group if UID is given,
> >  or to given UID and given GID if both are given. Alternatively, login na=
me, or
> > -login name and group name can be passed.
> > +login name and group name can be passed.  This requires privilege (either
>=20
> I'd change this to "privileges", right?

Hmm.. I think either would work, but I'll change it.


> Also, adding two spaces following a period, as you do, seems to have
> some merits:
>=20
>   https://link.springer.com/article/10.3758/s13414-018-1527-6
>=20
>   Johnson, R.L., Bui, B. & Schmitt, L.L. Are two spaces better than
>   one? The effect of spacing following periods and commas during reading.
>   Atten Percept Psychophys 80, 1504=E2=80=931511 (2018)
>=20
> ...but in man pages, nroff might turn that into three or more spaces,
> inconsistently, in a justified paragraph.
>=20
> I'd stick to one. Or change all of them (I almost never use two, so
> here it's all single spaces).

Fair enough.  I believe the question of spaces after a full stop
(British/Australian English for "period") is a bit of a contentious
issue.  At least in the English speaking world - I gather it's rarely
used outside that.  It's mostly irrelevant for modern typesetting,
because the typesetter will adjust anyways, I think I developed the
habit from writing text documents, at least some in the age of
monospaced printing.

I've removed it, anyway.

> > +initial effective UID 0 or CAP_SETUID capability) to work.
> >  Default is to change to user \fInobody\fR if started as root.
> > =20
> >  .TP
> > diff --git a/util.c b/util.c
> > index b2ccb3d..17595c1 100644
> > --- a/util.c
> > +++ b/util.c
> > @@ -492,7 +492,13 @@ void check_root(uid_t *uid, gid_t *gid)
> >  	char buf[BUFSIZ];
> >  	int fd;
> > =20
> > -	if (getuid() && geteuid())
> > +	if (!*uid)
> > +		*uid =3D geteuid();
> > +
> > +	if (!*gid)
> > +		*gid =3D getegid();
> > +
> > +	if (*uid)
> >  		return;
> > =20
> >  	if ((fd =3D open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0)
> > @@ -524,11 +530,26 @@ void check_root(uid_t *uid, gid_t *gid)
> >  		*uid =3D *gid =3D 65534;
> >  #endif
> >  	}
> > +}
> > +
> > +/**
> > + * drop_root() - Switch to given UID and GID
>=20
> I would add the usual:
>=20
>  * @uid:	User ID to switch to
>  * @gid:	Group ID to switch to
>=20
> even though it's totally obvious here, in case somebody should ever need
> to parse this stuff to produce documentation.

Good point, added.

--=20
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

--===============0392317290992836258==
Content-Type: application/pgp-signature
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="signature.asc"
MIME-Version: 1.0

LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KCmlRSXpCQUVCQ0FBZEZpRUVvVUx4V3U0L1dz
MGRCK1h0Z3lwWTRnRXdZU0lGQW1NY09DTUFDZ2tRZ3lwWTRnRXcKWVNJQVlnLzhDdGJBZC9Sd09P
aFFPYzJkcUhGdlpqY2xhekg0VnZmQ1BGZldsdU90ckw1Tk9PVnM3UTZYQi9BZApDWnd5WkxkTldH
ajMrZ2lDRmZrNnhoMXY5S0w1TEwrT0RNVFRFK0xZV0dtcWRqdDZSbXEzMnJRM2ZqM05KR2VGCjlS
aDdPN0NOeWhPZ2R3RlR5TWtvR1lZUDA3NFJ5ZnVTUlZnTTEwbGlYQWh4bHBjQTFUUzM3cjBLQU5l
OUFOamIKejRaSVRPeC9MRFFuZzdKMC80U2NXNnByeTVibWVVTmhLdHM0Zzd5N3BMM3NhVEVjRjdL
YWdlcm5GRUJtUGo4MgpQNUFvMlhCWGg1RktDajVqZXVRMmU5TVp1ZnBQdGlmNmVkZ2V1cUxoVjZH
MzlmMkpWRUdjVExQMmNMMUlHTWJJClhEbXhjL2ZJaTZOWTBjbzdkcVk3M3dhZlpSWWVwSHNPbDlK
MzJueDFWS3BRMWN5MTh1QUZRaUkwbzJuK2I2Uk8KbUE3ZXR4VkJuNmd4cEJ0L09NVmJLUCtrNXVV
VEdpMnNYZmwvN0pJYmNEZ3hSMDhiUFp0djdqS1dGUjJXZzJ1ZQp3SmdlOTR0czd6SGJoZXNDV1hW
RVV2N3VrTkpLVkErazlSUlk3bDcycWkwZzIxSUtkSUg2QVY5Y01BcTlXS0lwCjh4cGpsWmxwR3Z2
bVo4clZxSFBUSFk1NGtub3pRdUU3YlpIalJtVk1ZVnNncWk3cVY3VmtJaXZGQVBkb1JMQlgKclZs
Y28zcUpBdlU0dlk0RFZ1RzR5eU1NdWtCNGg1Y2I1Z2xHNW8vcUNma3RFR0JyVVI3OUd2SWhOdDNX
d01pVwpGODJGQ01FbHhSeEU0MkJhUC9MZjBFZTRNbHJCWXlqQ1hydm9wSWlraGhDTDRDTHZNRG89
Cj1sZENsCi0tLS0tRU5EIFBHUCBTSUdOQVRVUkUtLS0tLQo=

--===============0392317290992836258==--