> 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?
That's a nice observation. Your diff looks fine to me as it is.
In the patch below I tried to refactor the code a little, so there is a
bit less branching depending on getuid() == 0 and argv == 1. The result
seems a bit easier to follow this way.
Index: skeyinit.c
===================================================================
RCS file: /var/cvs/src/usr.bin/skeyinit/skeyinit.c,v
retrieving revision 1.69
diff -u -p -r1.69 skeyinit.c
--- skeyinit.c 21 Feb 2016 22:53:40 -0000 1.69
+++ skeyinit.c 17 May 2016 18:46:44 -0000
@@ -39,6 +39,7 @@
#endif
void usage(void);
+void isskeyenabled(const char *, const char *, int);
void secure_mode(int *, char *, char *, size_t, char *, size_t);
void normal_mode(char *, int, char *, char *);
void enable_db(int);
@@ -50,7 +51,7 @@ main(int argc, char **argv)
char hostname[HOST_NAME_MAX+1];
char seed[SKEY_MAX_SEED_LEN + 1];
char buf[256], key[SKEY_BINKEY_SIZE], filename[PATH_MAX], *ht;
- char lastc, me[UT_NAMESIZE + 1], *p, *auth_type;
+ char lastc, *p, *auth_type;
const char *errstr;
struct skey skey;
struct passwd *pp;
@@ -117,41 +118,47 @@ 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");
- /* Build up a default seed based on the hostname and some randomness */
- if (gethostname(hostname, sizeof(hostname)) < 0)
- err(1, "gethostname");
- for (i = 0, p = seed; hostname[i] && i < SKEY_NAMELEN; i++) {
- if (isalnum((unsigned char)hostname[i]))
- *p++ = tolower((unsigned char)hostname[i]);
- }
- for (i = 0; i < 5; i++)
- *p++ = arc4random_uniform(10) + '0';
- *p = '\0';
+ if ((pp = getpwuid(getuid())) == NULL)
+ err(1, "no user with uid %u", getuid());
+
+ if (argc == 1) {
+ char me[UT_NAMESIZE + 1];
+
+ (void)strlcpy(me, pp->pw_name, sizeof me);
+ if ((pp = getpwnam(argv[0])) == NULL)
+ errx(1, "User unknown: %s", argv[0]);
+ if (strcmp(pp->pw_name, me) != 0)
+ errx(1, "Permission denied.");
+ }
+ } else {
+ if (pledge("stdio rpath wpath cpath fattr flock tty getpw id",
+ NULL) == -1)
+ err(1, "pledge");
- if ((pp = getpwuid(getuid())) == NULL)
- err(1, "no user with uid %u", getuid());
- (void)strlcpy(me, pp->pw_name, sizeof me);
-
- /* Check for optional user string. */
- if (argc == 1) {
- if ((pp = getpwnam(argv[0])) == NULL) {
- if (getuid() == 0) {
+ if (argc == 1) {
+ if ((pp = getpwnam(argv[0])) == NULL) {
static struct passwd _pp;
_pp.pw_name = argv[0];
pp = &_pp;
warnx("Warning, user unknown: %s", argv[0]);
} else {
- errx(1, "User unknown: %s", argv[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);
}
- } else if (strcmp(pp->pw_name, me) != 0 && getuid() != 0) {
- /* Only root can change other's S/Keys. */
- errx(1, "Permission denied.");
- }
+ } else if ((pp = getpwuid(0)) == NULL)
+ err(1, "no user with uid %u", getuid());
+
+ if (pledge("stdio rpath wpath cpath fattr flock tty", NULL)
+ == -1)
+ err(1, "pledge");
}
switch (skey_haskey(pp->pw_name)) {
@@ -188,6 +195,17 @@ main(int argc, char **argv)
if (pledge("stdio rpath wpath cpath fattr flock tty", NULL) == -1)
err(1, "pledge");
+
+ /* Build up a default seed based on the hostname and some randomness */
+ if (gethostname(hostname, sizeof(hostname)) < 0)
+ err(1, "gethostname");
+ for (i = 0, p = seed; hostname[i] && i < SKEY_NAMELEN; i++) {
+ if (isalnum((unsigned char)hostname[i]))
+ *p++ = tolower((unsigned char)hostname[i]);
+ }
+ for (i = 0; i < 5; i++)
+ *p++ = arc4random_uniform(10) + '0';
+ *p = '\0';
/*
* Lookup and lock the record we are about to modify.
