diff options
author | Damien Miller <djm@mindrot.org> | 2016-02-23 16:12:13 +1100 |
---|---|---|
committer | Damien Miller <djm@mindrot.org> | 2016-02-23 17:40:16 +1100 |
commit | 1acc058d0a7913838c830ed998a1a1fb5b7864bf (patch) | |
tree | 14ece3b75d64372c430198b0f36a8c0a124a585a | |
parent | 39f303b1f36d934d8410b05625f25c7bcb75db4d (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.in | 5 | ||||
-rw-r--r-- | regress/check-perm.c | 205 | ||||
-rw-r--r-- | regress/keys-command.sh | 6 | ||||
-rw-r--r-- | regress/principals-command.sh | 7 | ||||
-rw-r--r-- | regress/setuid-allowed.c | 2 | ||||
-rw-r--r-- | regress/sftp-chroot.sh | 5 |
6 files changed, 229 insertions, 1 deletions
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 | |||
434 | $(CC) $(CFLAGS) $(CPPFLAGS) -o $@ $? \ | 434 | $(CC) $(CFLAGS) $(CPPFLAGS) -o $@ $? \ |
435 | $(LDFLAGS) -lssh -lopenbsd-compat -lssh -lopenbsd-compat $(LIBS) | 435 | $(LDFLAGS) -lssh -lopenbsd-compat -lssh -lopenbsd-compat $(LIBS) |
436 | 436 | ||
437 | regress/check-perm$(EXEEXT): $(srcdir)/regress/check-perm.c | ||
438 | $(CC) $(CFLAGS) $(CPPFLAGS) -o $@ $? \ | ||
439 | $(LDFLAGS) -lssh -lopenbsd-compat -lssh -lopenbsd-compat $(LIBS) | ||
440 | |||
437 | UNITTESTS_TEST_HELPER_OBJS=\ | 441 | UNITTESTS_TEST_HELPER_OBJS=\ |
438 | regress/unittests/test_helper/test_helper.o \ | 442 | regress/unittests/test_helper/test_helper.o \ |
439 | regress/unittests/test_helper/fuzz.o | 443 | regress/unittests/test_helper/fuzz.o |
@@ -505,6 +509,7 @@ REGRESS_BINARIES=\ | |||
505 | regress/modpipe$(EXEEXT) \ | 509 | regress/modpipe$(EXEEXT) \ |
506 | regress/setuid-allowed$(EXEEXT) \ | 510 | regress/setuid-allowed$(EXEEXT) \ |
507 | regress/netcat$(EXEEXT) \ | 511 | regress/netcat$(EXEEXT) \ |
512 | regress/check-perm$(EXEEXT) \ | ||
508 | regress/unittests/sshbuf/test_sshbuf$(EXEEXT) \ | 513 | regress/unittests/sshbuf/test_sshbuf$(EXEEXT) \ |
509 | regress/unittests/sshkey/test_sshkey$(EXEEXT) \ | 514 | regress/unittests/sshkey/test_sshkey$(EXEEXT) \ |
510 | regress/unittests/bitmap/test_bitmap$(EXEEXT) \ | 515 | 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 @@ | |||
1 | /* | ||
2 | * Placed in the public domain | ||
3 | */ | ||
4 | |||
5 | /* $OpenBSD: modpipe.c,v 1.6 2013/11/21 03:16:47 djm Exp $ */ | ||
6 | |||
7 | #include "includes.h" | ||
8 | |||
9 | #include <sys/types.h> | ||
10 | #include <sys/stat.h> | ||
11 | #include <unistd.h> | ||
12 | #include <stdio.h> | ||
13 | #include <string.h> | ||
14 | #include <stdarg.h> | ||
15 | #include <stdlib.h> | ||
16 | #include <errno.h> | ||
17 | #include <pwd.h> | ||
18 | #ifdef HAVE_LIBGEN_H | ||
19 | #include <libgen.h> | ||
20 | #endif | ||
21 | |||
22 | static void | ||
23 | fatal(const char *fmt, ...) | ||
24 | { | ||
25 | va_list args; | ||
26 | |||
27 | va_start(args, fmt); | ||
28 | vfprintf(stderr, fmt, args); | ||
29 | fputc('\n', stderr); | ||
30 | va_end(args); | ||
31 | exit(1); | ||
32 | } | ||
33 | /* Based on session.c. NB. keep tests in sync */ | ||
34 | static void | ||
35 | safely_chroot(const char *path, uid_t uid) | ||
36 | { | ||
37 | const char *cp; | ||
38 | char component[PATH_MAX]; | ||
39 | struct stat st; | ||
40 | |||
41 | if (*path != '/') | ||
42 | fatal("chroot path does not begin at root"); | ||
43 | if (strlen(path) >= sizeof(component)) | ||
44 | fatal("chroot path too long"); | ||
45 | |||
46 | /* | ||
47 | * Descend the path, checking that each component is a | ||
48 | * root-owned directory with strict permissions. | ||
49 | */ | ||
50 | for (cp = path; cp != NULL;) { | ||
51 | if ((cp = strchr(cp, '/')) == NULL) | ||
52 | strlcpy(component, path, sizeof(component)); | ||
53 | else { | ||
54 | cp++; | ||
55 | memcpy(component, path, cp - path); | ||
56 | component[cp - path] = '\0'; | ||
57 | } | ||
58 | |||
59 | /* debug3("%s: checking '%s'", __func__, component); */ | ||
60 | |||
61 | if (stat(component, &st) != 0) | ||
62 | fatal("%s: stat(\"%s\"): %s", __func__, | ||
63 | component, strerror(errno)); | ||
64 | if (st.st_uid != 0 || (st.st_mode & 022) != 0) | ||
65 | fatal("bad ownership or modes for chroot " | ||
66 | "directory %s\"%s\"", | ||
67 | cp == NULL ? "" : "component ", component); | ||
68 | if (!S_ISDIR(st.st_mode)) | ||
69 | fatal("chroot path %s\"%s\" is not a directory", | ||
70 | cp == NULL ? "" : "component ", component); | ||
71 | |||
72 | } | ||
73 | |||
74 | if (chdir(path) == -1) | ||
75 | fatal("Unable to chdir to chroot path \"%s\": " | ||
76 | "%s", path, strerror(errno)); | ||
77 | } | ||
78 | |||
79 | /* from platform.c */ | ||
80 | int | ||
81 | platform_sys_dir_uid(uid_t uid) | ||
82 | { | ||
83 | if (uid == 0) | ||
84 | return 1; | ||
85 | #ifdef PLATFORM_SYS_DIR_UID | ||
86 | if (uid == PLATFORM_SYS_DIR_UID) | ||
87 | return 1; | ||
88 | #endif | ||
89 | return 0; | ||
90 | } | ||
91 | |||
92 | /* from auth.c */ | ||
93 | int | ||
94 | auth_secure_path(const char *name, struct stat *stp, const char *pw_dir, | ||
95 | uid_t uid, char *err, size_t errlen) | ||
96 | { | ||
97 | char buf[PATH_MAX], homedir[PATH_MAX]; | ||
98 | char *cp; | ||
99 | int comparehome = 0; | ||
100 | struct stat st; | ||
101 | |||
102 | if (realpath(name, buf) == NULL) { | ||
103 | snprintf(err, errlen, "realpath %s failed: %s", name, | ||
104 | strerror(errno)); | ||
105 | return -1; | ||
106 | } | ||
107 | if (pw_dir != NULL && realpath(pw_dir, homedir) != NULL) | ||
108 | comparehome = 1; | ||
109 | |||
110 | if (!S_ISREG(stp->st_mode)) { | ||
111 | snprintf(err, errlen, "%s is not a regular file", buf); | ||
112 | return -1; | ||
113 | } | ||
114 | if ((!platform_sys_dir_uid(stp->st_uid) && stp->st_uid != uid) || | ||
115 | (stp->st_mode & 022) != 0) { | ||
116 | snprintf(err, errlen, "bad ownership or modes for file %s", | ||
117 | buf); | ||
118 | return -1; | ||
119 | } | ||
120 | |||
121 | /* for each component of the canonical path, walking upwards */ | ||
122 | for (;;) { | ||
123 | if ((cp = dirname(buf)) == NULL) { | ||
124 | snprintf(err, errlen, "dirname() failed"); | ||
125 | return -1; | ||
126 | } | ||
127 | strlcpy(buf, cp, sizeof(buf)); | ||
128 | |||
129 | if (stat(buf, &st) < 0 || | ||
130 | (!platform_sys_dir_uid(st.st_uid) && st.st_uid != uid) || | ||
131 | (st.st_mode & 022) != 0) { | ||
132 | snprintf(err, errlen, | ||
133 | "bad ownership or modes for directory %s", buf); | ||
134 | return -1; | ||
135 | } | ||
136 | |||
137 | /* If are past the homedir then we can stop */ | ||
138 | if (comparehome && strcmp(homedir, buf) == 0) | ||
139 | break; | ||
140 | |||
141 | /* | ||
142 | * dirname should always complete with a "/" path, | ||
143 | * but we can be paranoid and check for "." too | ||
144 | */ | ||
145 | if ((strcmp("/", buf) == 0) || (strcmp(".", buf) == 0)) | ||
146 | break; | ||
147 | } | ||
148 | return 0; | ||
149 | } | ||
150 | |||
151 | static void | ||
152 | usage(void) | ||
153 | { | ||
154 | fprintf(stderr, "check-perm -m [chroot | keys-command] [path]\n"); | ||
155 | exit(1); | ||
156 | } | ||
157 | |||
158 | int | ||
159 | main(int argc, char **argv) | ||
160 | { | ||
161 | const char *path = "."; | ||
162 | char errmsg[256]; | ||
163 | int ch, mode = -1; | ||
164 | extern char *optarg; | ||
165 | extern int optind; | ||
166 | struct stat st; | ||
167 | |||
168 | while ((ch = getopt(argc, argv, "hm:")) != -1) { | ||
169 | switch (ch) { | ||
170 | case 'm': | ||
171 | if (strcasecmp(optarg, "chroot") == 0) | ||
172 | mode = 1; | ||
173 | else if (strcasecmp(optarg, "keys-command") == 0) | ||
174 | mode = 2; | ||
175 | else { | ||
176 | fprintf(stderr, "Invalid -m option\n"), | ||
177 | usage(); | ||
178 | } | ||
179 | break; | ||
180 | default: | ||
181 | usage(); | ||
182 | } | ||
183 | } | ||
184 | argc -= optind; | ||
185 | argv += optind; | ||
186 | |||
187 | if (argc > 1) | ||
188 | usage(); | ||
189 | else if (argc == 1) | ||
190 | path = argv[0]; | ||
191 | |||
192 | if (mode == 1) | ||
193 | safely_chroot(path, getuid()); | ||
194 | else if (mode == 2) { | ||
195 | if (stat(path, &st) < 0) | ||
196 | fatal("Could not stat %s: %s", path, strerror(errno)); | ||
197 | if (auth_secure_path(path, &st, NULL, 0, | ||
198 | errmsg, sizeof(errmsg)) != 0) | ||
199 | fatal("Unsafe %s: %s", path, errmsg); | ||
200 | } else { | ||
201 | fprintf(stderr, "Invalid mode\n"); | ||
202 | usage(); | ||
203 | } | ||
204 | return 0; | ||
205 | } | ||
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}" | |||
36 | _EOF | 36 | _EOF |
37 | $SUDO chmod 0755 "$KEY_COMMAND" | 37 | $SUDO chmod 0755 "$KEY_COMMAND" |
38 | 38 | ||
39 | if ! $OBJ/check-perm -m keys-command $KEY_COMMAND ; then | ||
40 | echo "skipping: $KEY_COMMAND is unsuitable as AuthorizedKeysCommand" | ||
41 | $SUDO rm -f $KEY_COMMAND | ||
42 | exit 0 | ||
43 | fi | ||
44 | |||
39 | if [ -x $KEY_COMMAND ]; then | 45 | if [ -x $KEY_COMMAND ]; then |
40 | cp $OBJ/sshd_proxy $OBJ/sshd_proxy.bak | 46 | cp $OBJ/sshd_proxy $OBJ/sshd_proxy.bak |
41 | 47 | ||
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 | |||
24 | test $? -eq 0 || fatal "couldn't prepare principals command" | 24 | test $? -eq 0 || fatal "couldn't prepare principals command" |
25 | $SUDO chmod 0755 "$PRINCIPALS_CMD" | 25 | $SUDO chmod 0755 "$PRINCIPALS_CMD" |
26 | 26 | ||
27 | if ! $OBJ/check-perm -m keys-command $PRINCIPALS_CMD ; then | ||
28 | echo "skipping: $PRINCIPALS_CMD is unsuitable as " \ | ||
29 | "AuthorizedPrincipalsCommand" | ||
30 | $SUDO rm -f $PRINCIPALS_CMD | ||
31 | exit 0 | ||
32 | fi | ||
33 | |||
27 | # Create a CA key and a user certificate. | 34 | # Create a CA key and a user certificate. |
28 | ${SSHKEYGEN} -q -N '' -t ed25519 -f $OBJ/user_ca_key || \ | 35 | ${SSHKEYGEN} -q -N '' -t ed25519 -f $OBJ/user_ca_key || \ |
29 | fatal "ssh-keygen of user_ca_key failed" | 36 | 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 @@ | |||
26 | #include <string.h> | 26 | #include <string.h> |
27 | #include <errno.h> | 27 | #include <errno.h> |
28 | 28 | ||
29 | void | 29 | static void |
30 | usage(void) | 30 | usage(void) |
31 | { | 31 | { |
32 | fprintf(stderr, "check-setuid [path]\n"); | 32 | 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 | |||
12 | exit 0 | 12 | exit 0 |
13 | fi | 13 | fi |
14 | 14 | ||
15 | if ! $OBJ/check-perm -m chroot "$CHROOT" ; then | ||
16 | echo "skipped: $CHROOT is unsuitable as ChrootDirectory" | ||
17 | exit 0 | ||
18 | fi | ||
19 | |||
15 | $SUDO sh -c "echo mekmitastdigoat > $PRIVDATA" || \ | 20 | $SUDO sh -c "echo mekmitastdigoat > $PRIVDATA" || \ |
16 | fatal "create $PRIVDATA failed" | 21 | fatal "create $PRIVDATA failed" |
17 | 22 | ||