On Tue, 17 May 2016 17:03:06 +0200, Theo Buehler wrote:
> > @@ -151,6 +157,11 @@ main(int argc, char **argv)
> > } else if (strcmp(pp->pw_name, me) != 0 && getuid() != 0) {
> > /* Only root can change other's S/Keys. */
> > errx(1, "Permission denied.");
> > + } else {
> > + /* So the file ends up owned by the proper ID. */
> > + if (setresuid(-1, pp->pw_uid, -1) != 0)
> > + errx(1, "unable to change user ID to %u",
> > + pp->pw_uid);
> > }
> > }
> >
>
> I think it would be nice to drop the "id" promise here. I'd prefer to
> run as little code with "proc exec id" as possible.
We can do more than drop id, we can use the same minimal pledge
that will be enforced later since root doesn't need to call
auth_userokay().
In fact, root doesn't need proc or exec. Is this going too far?
- todd
Index: usr.bin/skeyinit/skeyinit.c
===================================================================
RCS file: /cvs/src/usr.bin/skeyinit/skeyinit.c,v
retrieving revision 1.69
diff -u -p -u -r1.69 skeyinit.c
--- usr.bin/skeyinit/skeyinit.c 21 Feb 2016 22:53:40 -0000 1.69
+++ usr.bin/skeyinit/skeyinit.c 17 May 2016 17:15:33 -0000
@@ -117,9 +117,19 @@ main(int argc, char **argv)
exit(0);
}
- if (pledge("stdio rpath wpath cpath fattr flock tty proc exec getpw",
- NULL) == -1)
- err(1, "pledge");
+ if (getuid() != 0) {
+ if (pledge("stdio rpath wpath cpath fattr flock tty proc exec "
+ "getpw", NULL) == -1)
+ err(1, "pledge");
+ } else if (argc == 1) {
+ if (pledge("stdio rpath wpath cpath fattr flock tty getpw id",
+ NULL) == -1)
+ err(1, "pledge");
+ } else {
+ if (pledge("stdio rpath wpath cpath fattr flock tty getpw",
+ NULL) == -1)
+ err(1, "pledge");
+ }
/* Build up a default seed based on the hostname and some randomness */
if (gethostname(hostname, sizeof(hostname)) < 0)
@@ -148,9 +158,17 @@ main(int argc, char **argv)
} else {
errx(1, "User unknown: %s", argv[0]);
}
- } else if (strcmp(pp->pw_name, me) != 0 && getuid() != 0) {
- /* Only root can change other's S/Keys. */
- errx(1, "Permission denied.");
+ } else if (getuid() == 0) {
+ /* So the file ends up owned by the proper ID. */
+ if (setresuid(-1, pp->pw_uid, -1) != 0)
+ errx(1, "unable to change user ID to %u",
+ pp->pw_uid);
+ if (pledge("stdio rpath wpath cpath fattr flock tty",
+ NULL) == -1)
+ err(1, "pledge");
+ } else {
+ if (strcmp(pp->pw_name, me) != 0)
+ errx(1, "Permission denied.");
}
}
