From 1acc058d0a7913838c830ed998a1a1fb5b7864bf Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Tue, 23 Feb 2016 16:12:13 +1100 Subject: Disable tests where fs perms are incorrect Some tests have strict requirements on the filesystem permissions for certain files and directories. This adds a regress/check-perm tool that copies the relevant logic from sshd to exactly test the paths in question. This lets us skip tests when the local filesystem doesn't conform to our expectations rather than continuing and failing the test run. ok dtucker@ --- Makefile.in | 5 ++ regress/check-perm.c | 205 ++++++++++++++++++++++++++++++++++++++++++ regress/keys-command.sh | 6 ++ regress/principals-command.sh | 7 ++ regress/setuid-allowed.c | 2 +- regress/sftp-chroot.sh | 5 ++ 6 files changed, 229 insertions(+), 1 deletion(-) create mode 100644 regress/check-perm.c diff --git a/Makefile.in b/Makefile.in index a8984c8fb..d401787db 100644 --- a/Makefile.in +++ b/Makefile.in @@ -434,6 +434,10 @@ regress/netcat$(EXEEXT): $(srcdir)/regress/netcat.c $(CC) $(CFLAGS) $(CPPFLAGS) -o $@ $? \ $(LDFLAGS) -lssh -lopenbsd-compat -lssh -lopenbsd-compat $(LIBS) +regress/check-perm$(EXEEXT): $(srcdir)/regress/check-perm.c + $(CC) $(CFLAGS) $(CPPFLAGS) -o $@ $? \ + $(LDFLAGS) -lssh -lopenbsd-compat -lssh -lopenbsd-compat $(LIBS) + UNITTESTS_TEST_HELPER_OBJS=\ regress/unittests/test_helper/test_helper.o \ regress/unittests/test_helper/fuzz.o @@ -505,6 +509,7 @@ REGRESS_BINARIES=\ regress/modpipe$(EXEEXT) \ regress/setuid-allowed$(EXEEXT) \ regress/netcat$(EXEEXT) \ + regress/check-perm$(EXEEXT) \ regress/unittests/sshbuf/test_sshbuf$(EXEEXT) \ regress/unittests/sshkey/test_sshkey$(EXEEXT) \ regress/unittests/bitmap/test_bitmap$(EXEEXT) \ diff --git a/regress/check-perm.c b/regress/check-perm.c new file mode 100644 index 000000000..dac307d24 --- /dev/null +++ b/regress/check-perm.c @@ -0,0 +1,205 @@ +/* + * Placed in the public domain + */ + +/* $OpenBSD: modpipe.c,v 1.6 2013/11/21 03:16:47 djm Exp $ */ + +#include "includes.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#ifdef HAVE_LIBGEN_H +#include +#endif + +static void +fatal(const char *fmt, ...) +{ + va_list args; + + va_start(args, fmt); + vfprintf(stderr, fmt, args); + fputc('\n', stderr); + va_end(args); + exit(1); +} +/* Based on session.c. NB. keep tests in sync */ +static void +safely_chroot(const char *path, uid_t uid) +{ + const char *cp; + char component[PATH_MAX]; + struct stat st; + + if (*path != '/') + fatal("chroot path does not begin at root"); + if (strlen(path) >= sizeof(component)) + fatal("chroot path too long"); + + /* + * Descend the path, checking that each component is a + * root-owned directory with strict permissions. + */ + for (cp = path; cp != NULL;) { + if ((cp = strchr(cp, '/')) == NULL) + strlcpy(component, path, sizeof(component)); + else { + cp++; + memcpy(component, path, cp - path); + component[cp - path] = '\0'; + } + + /* debug3("%s: checking '%s'", __func__, component); */ + + if (stat(component, &st) != 0) + fatal("%s: stat(\"%s\"): %s", __func__, + component, strerror(errno)); + if (st.st_uid != 0 || (st.st_mode & 022) != 0) + fatal("bad ownership or modes for chroot " + "directory %s\"%s\"", + cp == NULL ? "" : "component ", component); + if (!S_ISDIR(st.st_mode)) + fatal("chroot path %s\"%s\" is not a directory", + cp == NULL ? "" : "component ", component); + + } + + if (chdir(path) == -1) + fatal("Unable to chdir to chroot path \"%s\": " + "%s", path, strerror(errno)); +} + +/* from platform.c */ +int +platform_sys_dir_uid(uid_t uid) +{ + if (uid == 0) + return 1; +#ifdef PLATFORM_SYS_DIR_UID + if (uid == PLATFORM_SYS_DIR_UID) + return 1; +#endif + return 0; +} + +/* from auth.c */ +int +auth_secure_path(const char *name, struct stat *stp, const char *pw_dir, + uid_t uid, char *err, size_t errlen) +{ + char buf[PATH_MAX], homedir[PATH_MAX]; + char *cp; + int comparehome = 0; + struct stat st; + + if (realpath(name, buf) == NULL) { + snprintf(err, errlen, "realpath %s failed: %s", name, + strerror(errno)); + return -1; + } + if (pw_dir != NULL && realpath(pw_dir, homedir) != NULL) + comparehome = 1; + + if (!S_ISREG(stp->st_mode)) { + snprintf(err, errlen, "%s is not a regular file", buf); + return -1; + } + if ((!platform_sys_dir_uid(stp->st_uid) && stp->st_uid != uid) || + (stp->st_mode & 022) != 0) { + snprintf(err, errlen, "bad ownership or modes for file %s", + buf); + return -1; + } + + /* for each component of the canonical path, walking upwards */ + for (;;) { + if ((cp = dirname(buf)) == NULL) { + snprintf(err, errlen, "dirname() failed"); + return -1; + } + strlcpy(buf, cp, sizeof(buf)); + + if (stat(buf, &st) < 0 || + (!platform_sys_dir_uid(st.st_uid) && st.st_uid != uid) || + (st.st_mode & 022) != 0) { + snprintf(err, errlen, + "bad ownership or modes for directory %s", buf); + return -1; + } + + /* If are past the homedir then we can stop */ + if (comparehome && strcmp(homedir, buf) == 0) + break; + + /* + * dirname should always complete with a "/" path, + * but we can be paranoid and check for "." too + */ + if ((strcmp("/", buf) == 0) || (strcmp(".", buf) == 0)) + break; + } + return 0; +} + +static void +usage(void) +{ + fprintf(stderr, "check-perm -m [chroot | keys-command] [path]\n"); + exit(1); +} + +int +main(int argc, char **argv) +{ + const char *path = "."; + char errmsg[256]; + int ch, mode = -1; + extern char *optarg; + extern int optind; + struct stat st; + + while ((ch = getopt(argc, argv, "hm:")) != -1) { + switch (ch) { + case 'm': + if (strcasecmp(optarg, "chroot") == 0) + mode = 1; + else if (strcasecmp(optarg, "keys-command") == 0) + mode = 2; + else { + fprintf(stderr, "Invalid -m option\n"), + usage(); + } + break; + default: + usage(); + } + } + argc -= optind; + argv += optind; + + if (argc > 1) + usage(); + else if (argc == 1) + path = argv[0]; + + if (mode == 1) + safely_chroot(path, getuid()); + else if (mode == 2) { + if (stat(path, &st) < 0) + fatal("Could not stat %s: %s", path, strerror(errno)); + if (auth_secure_path(path, &st, NULL, 0, + errmsg, sizeof(errmsg)) != 0) + fatal("Unsafe %s: %s", path, errmsg); + } else { + fprintf(stderr, "Invalid mode\n"); + usage(); + } + return 0; +} diff --git a/regress/keys-command.sh b/regress/keys-command.sh index 700273b66..af68cf15c 100644 --- a/regress/keys-command.sh +++ b/regress/keys-command.sh @@ -36,6 +36,12 @@ exec cat "$OBJ/authorized_keys_${LOGNAME}" _EOF $SUDO chmod 0755 "$KEY_COMMAND" +if ! $OBJ/check-perm -m keys-command $KEY_COMMAND ; then + echo "skipping: $KEY_COMMAND is unsuitable as AuthorizedKeysCommand" + $SUDO rm -f $KEY_COMMAND + exit 0 +fi + if [ -x $KEY_COMMAND ]; then cp $OBJ/sshd_proxy $OBJ/sshd_proxy.bak diff --git a/regress/principals-command.sh b/regress/principals-command.sh index b90a8cf2c..c0be7e747 100644 --- a/regress/principals-command.sh +++ b/regress/principals-command.sh @@ -24,6 +24,13 @@ _EOF test $? -eq 0 || fatal "couldn't prepare principals command" $SUDO chmod 0755 "$PRINCIPALS_CMD" +if ! $OBJ/check-perm -m keys-command $PRINCIPALS_CMD ; then + echo "skipping: $PRINCIPALS_CMD is unsuitable as " \ + "AuthorizedPrincipalsCommand" + $SUDO rm -f $PRINCIPALS_CMD + exit 0 +fi + # Create a CA key and a user certificate. ${SSHKEYGEN} -q -N '' -t ed25519 -f $OBJ/user_ca_key || \ fatal "ssh-keygen of user_ca_key failed" diff --git a/regress/setuid-allowed.c b/regress/setuid-allowed.c index 676d2661c..7a0527fd0 100644 --- a/regress/setuid-allowed.c +++ b/regress/setuid-allowed.c @@ -26,7 +26,7 @@ #include #include -void +static void usage(void) { fprintf(stderr, "check-setuid [path]\n"); diff --git a/regress/sftp-chroot.sh b/regress/sftp-chroot.sh index 23f7456e8..9c26eb680 100644 --- a/regress/sftp-chroot.sh +++ b/regress/sftp-chroot.sh @@ -12,6 +12,11 @@ if [ -z "$SUDO" ]; then exit 0 fi +if ! $OBJ/check-perm -m chroot "$CHROOT" ; then + echo "skipped: $CHROOT is unsuitable as ChrootDirectory" + exit 0 +fi + $SUDO sh -c "echo mekmitastdigoat > $PRIVDATA" || \ fatal "create $PRIVDATA failed" -- cgit v1.2.3