case sensitivity, "Match User" and "AllowUsers"

[Corinna Vinschen]

Hi Damien,

On Feb 28 03:39, Damien Miller wrote:
> Unfortunately, that patch only deals with SSHv2 connections. How about
> this?
> 
> Index: auth.c
> ===================================================================
> RCS file: /var/cvs/openssh/auth.c,v
> retrieving revision 1.136
> diff -u -r1.136 auth.c
> --- auth.c	11 Feb 2010 22:25:29 -0000	1.136
> +++ auth.c	27 Feb 2010 16:36:25 -0000
> @@ -535,6 +535,13 @@
>  	    get_canonical_hostname(options.use_dns), get_remote_ipaddr());
>  
>  	pw = getpwnam(user);
> +#if HAVE_CYGWIN
> +	if (strcmp(user, pw->pw_name) != 0) {
> +		logit("Login name %.100s does not match stored username %.100s",
> +		    user, pw->pw_name);
> +		pw = NULL;
> +	}
> +#endif
>  	if (pw == NULL) {
>  		logit("Invalid user %.100s from %.100s",
>  		    user, get_remote_ipaddr());

Yes, that's better.  There are just a few glitches.  The test for
pw == NULL should come first and the #if should be an #ifdef.  And
I think it wouldn't hurt to have a comment which explains why this is
done.  What about this?

Index: auth.c
===================================================================
RCS file: /cvs/openssh/auth.c,v
retrieving revision 1.136
diff -u -p -r1.136 auth.c
--- auth.c	11 Feb 2010 22:25:29 -0000	1.136
+++ auth.c	28 Feb 2010 12:52:25 -0000
@@ -547,6 +547,18 @@ getpwnamallow(const char *user)
 #endif /* SSH_AUDIT_EVENTS */
 		return (NULL);
 	}
+#ifdef HAVE_CYGWIN
+	/* Windows usernames are case-insensitive.  To avoid later problems
+	 * when trying to match the username, the user is only allowed to
+	 * login if the username is given in the same case as stored in the
+	 * user database.
+	 */
+	if (strcmp(user, pw->pw_name) != 0) {
+		logit("Login name %.100s does not match stored username %.100s",
+		      user, pw->pw_name);
+		pw = NULL;
+	}
+#endif
 	if (!allowed_user(pw))
 		return (NULL);
 #ifdef HAVE_LOGIN_CAP

> I'm a little worried about enabling this outside of Cygwin, since
> I'm not sure whether multiple UID-sharing accounts are guaranteed to
> deterministically return the username that was used to look them up.

This would affect Cygwin as well since nothing keeps an administrator to
add two accounts using different usernames to /etc/passwd.  However,
since you're not searching by uid, but by name, it's incredibly unlikely
that the returned entry is an entry not matching the name.

Anyway, if you're happy to keep this code Cygwin-only, I'm happy as well.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat