summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDamien Miller <djm@mindrot.org>2016-02-23 16:12:13 +1100
committerDamien Miller <djm@mindrot.org>2016-02-23 17:40:16 +1100
commit1acc058d0a7913838c830ed998a1a1fb5b7864bf (patch)
tree14ece3b75d64372c430198b0f36a8c0a124a585a
parent39f303b1f36d934d8410b05625f25c7bcb75db4d (diff)
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@
-rw-r--r--Makefile.in5
-rw-r--r--regress/check-perm.c205
-rw-r--r--regress/keys-command.sh6
-rw-r--r--regress/principals-command.sh7
-rw-r--r--regress/setuid-allowed.c2
-rw-r--r--regress/sftp-chroot.sh5
6 files changed, 229 insertions, 1 deletions
diff --git a/Makefile.in b/Makefile.in
index a8984c8f..d401787d 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 00000000..dac307d2
--- /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 <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdarg.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <pwd.h>
+#ifdef HAVE_LIBGEN_H
+#include <libgen.h>
+#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 700273b6..af68cf15 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 b90a8cf2..c0be7e74 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 676d2661..7a0527fd 100644
--- a/regress/setuid-allowed.c
+++ b/regress/setuid-allowed.c
@@ -26,7 +26,7 @@
#include <string.h>
#include <errno.h>
-void
+static void
usage(void)
{
fprintf(stderr, "check-setuid [path]\n");
diff --git a/regress/sftp-chroot.sh b/regress/sftp-chroot.sh
index 23f7456e..9c26eb68 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"