Attachment 1334 Details for Bug 1348 – Patch to tidy keyfile opening slightly.

[patch] Patch to tidy keyfile opening slightly.

openssh-keyfile-duplication-tidy.patch (text/plain), 7.72 KB, created by Rob Holland on 2007-08-03 02:06:20 AEST

(hide)

Description:

Filename:

MIME Type:

Creator: Rob Holland

Created: 2007-08-03 02:06:20 AEST

Size: 7.72 KB

patch

obsolete

>=== modified file 'auth-rsa.c'
>--- auth-rsa.c	2007-07-30 09:54:36 +0000
>+++ auth-rsa.c	2007-08-02 12:17:32 +0000
>@@ -173,7 +173,6 @@
> 	u_int bits;
> 	FILE *f;
> 	u_long linenum = 0;
>-	struct stat st;
> 	Key *key;
> 
> 	/* Temporarily use the user's uid. */
>@@ -183,26 +182,9 @@
> 	file = authorized_keys_file(pw);
> 	debug("trying public RSA key file %s", file);
> 
>-	/* Fail quietly if file does not exist */
>-	if (stat(file, &st) < 0) {
>-		/* Restore the privileged uid. */
>-		restore_uid();
>-		xfree(file);
>-		return (0);
>-	}
>-	/* Open the file containing the authorized keys. */
>-	f = fopen(file, "r");
>+	f = open_keyfile(file, pw, options.strict_modes);
> 	if (!f) {
>-		/* Restore the privileged uid. */
>-		restore_uid();
>-		xfree(file);
>-		return (0);
>-	}
>-	if (options.strict_modes &&
>-	    secure_filename(f, file, pw, line, sizeof(line)) != 0) {
>-		xfree(file);
>-		fclose(f);
>-		logit("Authentication refused: %s", line);
>+		xfree(file);
> 		restore_uid();
> 		return (0);
> 	}
>
>=== modified file 'auth.c'
>--- auth.c	2007-07-30 09:54:36 +0000
>+++ auth.c	2007-08-02 12:03:02 +0000
>@@ -397,79 +397,6 @@
> 	return host_status;
> }
> 
>-
>-/*
>- * Check a given file for security. This is defined as all components
>- * of the path to the file must be owned by either the owner of
>- * of the file or root and no directories must be group or world writable.
>- *
>- * XXX Should any specific check be done for sym links ?
>- *
>- * Takes an open file descriptor, the file name, a uid and and
>- * error buffer plus max size as arguments.
>- *
>- * Returns 0 on success and -1 on failure
>- */
>-int
>-secure_filename(FILE *f, const char *file, struct passwd *pw,
>-    char *err, size_t errlen)
>-{
>-	uid_t uid = pw->pw_uid;
>-	char buf[MAXPATHLEN], homedir[MAXPATHLEN];
>-	char *cp;
>-	int comparehome = 0;
>-	struct stat st;
>-
>-	if (realpath(file, buf) == NULL) {
>-		snprintf(err, errlen, "realpath %s failed: %s", file,
>-		    strerror(errno));
>-		return -1;
>-	}
>-	if (realpath(pw->pw_dir, homedir) != NULL)
>-		comparehome = 1;
>-
>-	/* check the open file to avoid races */
>-	if (fstat(fileno(f), &st) < 0 ||
>-	    (st.st_uid != 0 && st.st_uid != uid) ||
>-	    (st.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));
>-
>-		debug3("secure_filename: checking '%s'", buf);
>-		if (stat(buf, &st) < 0 ||
>-		    (st.st_uid != 0 && st.st_uid != uid) ||
>-		    (st.st_mode & 022) != 0) {
>-			snprintf(err, errlen,
>-			    "bad ownership or modes for directory %s", buf);
>-			return -1;
>-		}
>-
>-		/* If are passed the homedir then we can stop */
>-		if (comparehome && strcmp(homedir, buf) == 0) {
>-			debug3("secure_filename: terminating check at '%s'",
>-			    buf);
>-			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;
>-}
>-
> struct passwd *
> getpwnamallow(const char *user)
> {
>
>=== modified file 'auth.h'
>--- auth.h	2007-07-30 09:54:36 +0000
>+++ auth.h	2007-08-02 12:02:24 +0000
>@@ -166,8 +166,6 @@
> char	*authorized_keys_file(struct passwd *);
> char	*authorized_keys_file2(struct passwd *);
> 
>-int
>-secure_filename(FILE *, const char *, struct passwd *, char *, size_t);
> 
> HostStatus
> check_key_in_hostfiles(struct passwd *, Key *, const char *,
>
>=== modified file 'auth2-pubkey.c'
>--- auth2-pubkey.c	2007-07-30 09:54:36 +0000
>+++ auth2-pubkey.c	2007-08-02 12:19:19 +0000
>@@ -183,34 +183,21 @@
> 	int found_key = 0;
> 	FILE *f;
> 	u_long linenum = 0;
>-	struct stat st;
> 	Key *found;
> 	char *fp;
> 
> 	/* Temporarily use the user's uid. */
> 	temporarily_use_uid(pw);
> 
>+	/* The authorized keys. */
>+	file = authorized_keys_file(pw);
> 	debug("trying public key file %s", file);
> 
>-	/* Fail quietly if file does not exist */
>-	if (stat(file, &st) < 0) {
>-		/* Restore the privileged uid. */
>-		restore_uid();
>-		return 0;
>-	}
>-	/* Open the file containing the authorized keys. */
>-	f = fopen(file, "r");
>+	f = open_keyfile(file, pw, options.strict_modes);
> 	if (!f) {
>-		/* Restore the privileged uid. */
>-		restore_uid();
>-		return 0;
>-	}
>-	if (options.strict_modes &&
>-	    secure_filename(f, file, pw, line, sizeof(line)) != 0) {
>-		fclose(f);
>-		logit("Authentication refused: %s", line);
>-		restore_uid();
>-		return 0;
>+		xfree(file);
>+		restore_uid();
>+		return (0);
> 	}
> 
> 	found_key = 0;
>
>=== modified file 'misc.c'
>--- misc.c	2007-07-30 09:54:36 +0000
>+++ misc.c	2007-08-02 12:47:54 +0000
>@@ -46,6 +46,9 @@
> # include <paths.h>
> #include <pwd.h>
> #endif
>+#ifdef HAVE_LIBGEN_H
>+#include <libgen.h>
>+#endif
> #ifdef SSH_TUN_OPENBSD
> #include <net/if.h>
> #endif
>@@ -608,6 +611,102 @@
> }
> 
> /*
>+ * Check a given file for security. This is defined as all components
>+ * of the path to the file must be owned by either the owner of
>+ * of the file or root and no directories must be group or world writable.
>+ *
>+ * XXX Should any specific check be done for sym links ?
>+ *
>+ * Takes an open file descriptor, the file name, a uid and and
>+ * error buffer plus max size as arguments.
>+ *
>+ * Returns 0 on success and -1 on failure
>+ */
>+static int
>+secure_filename(FILE *f, const char *file, struct passwd *pw,
>+    char *err, size_t errlen)
>+{
>+	uid_t uid = pw->pw_uid;
>+	char buf[MAXPATHLEN], homedir[MAXPATHLEN];
>+	char *cp;
>+	int comparehome = 0;
>+	struct stat st;
>+
>+	if (realpath(file, buf) == NULL) {
>+		snprintf(err, errlen, "realpath %s failed: %s", file,
>+		    strerror(errno));
>+		return -1;
>+	}
>+	if (realpath(pw->pw_dir, homedir) != NULL)
>+		comparehome = 1;
>+
>+	/* check the open file to avoid races */
>+	if (fstat(fileno(f), &st) < 0 ||
>+	    (st.st_uid != 0 && st.st_uid != uid) ||
>+	    (st.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));
>+
>+		debug3("secure_filename: checking '%s'", buf);
>+		if (stat(buf, &st) < 0 ||
>+		    (st.st_uid != 0 && st.st_uid != uid) ||
>+		    (st.st_mode & 022) != 0) {
>+			snprintf(err, errlen,
>+			    "bad ownership or modes for directory %s", buf);
>+			return -1;
>+		}
>+
>+		/* If are passed the homedir then we can stop */
>+		if (comparehome && strcmp(homedir, buf) == 0) {
>+			debug3("secure_filename: terminating check at '%s'",
>+			    buf);
>+			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;
>+}
>+
>+FILE *
>+open_keyfile(const char *filename, struct passwd *pw, int strict_modes)
>+{
>+	char err[1024];
>+	struct stat st;
>+	FILE *f;
>+
>+	if (stat(filename, &st) < 0)
>+		return NULL;
>+
>+	f = fopen(filename, "r");
>+	if (!f)
>+		return NULL;
>+
>+	if (strict_modes &&
>+	    secure_filename(f, filename, pw, err, sizeof(err)) != 0) {
>+		fclose(f);
>+		logit("Authentication refused: %s", err);
>+		return NULL;
>+	}
>+
>+	return f;
>+}
>+
>+/*
>  * Read an entire line from a public key file into a static buffer, discarding
>  * lines that exceed the buffer size.  Returns 0 on success, -1 on failure.
>  */
>
>=== modified file 'misc.h'
>--- misc.h	2007-07-30 09:54:36 +0000
>+++ misc.h	2007-08-02 12:12:51 +0000
>@@ -85,6 +85,7 @@
> 
> char	*read_passphrase(const char *, int);
> int	 ask_permission(const char *, ...) __attribute__((format(printf, 1, 2)));
>+FILE	*open_keyfile(const char *, struct passwd *, int);
> int	 read_keyfile_line(FILE *, const char *, char *, size_t, u_long *);
> 
> #endif /* _MISC_H */
>

Actions: View | Diff

Attachments on bug 1348: 1334 | 1526