On Wed, Apr 04, 2012 at 09:43:35PM +0200, Bj?rn Ketelaars wrote:
> 2012/4/4 Todd C. Miller <[email protected]>:
> > Why do we care if the user exists? Ideally, you want the code to
> > behave more or less the same whether the user is real or not.
> > Otherwise, a remote attacker can guess valid usernames by timing a
> > login attempt.
> >
> > For safety's sake, it makes sense to reject a username with a '/'
> > in it since the yubikey database is just a directory where each
> > user has a file. But I don't see the need to bail early just because
> > the user is not in the passwd database since yubikey_login() will
> > only succeed if the user has an entry.
>
> You are right! It does not make sense to bail out early if the user
> does not exist in the passwd database. However, I still like the idea
> of using the same database for looking up purposes. So another
> attempt: This time every type of username is accepted (comparable - in
> behaviour - to login_passwd). The username and the password are then
> passed to yubikey_login() which first checks if the username is valid,
> i.e. exists in the passwd database. If not, then the user will receive
> AUTH_FAILED).
That sounds like timing bases attacks to guess a username still will work.
-Otto
>
> OK?
>
>
> Index: login_yubikey.c
> ===================================================================
> RCS file: /cvs/src/libexec/login_yubikey/login_yubikey.c,v
> retrieving revision 1.4
> diff -u -r1.4 login_yubikey.c
> --- login_yubikey.c 1 Feb 2012 16:07:28 -0000 1.4
> +++ login_yubikey.c 4 Apr 2012 19:04:23 -0000
> @@ -54,7 +54,6 @@
>
> static const char *path = "/var/db/yubikey";
>
> -static int clean_string(const char *);
> static int yubikey_login(const char *, const char *);
>
> int
> @@ -102,10 +101,6 @@
> /* passed by sshd(8) for non-existing users */
> if (!strcmp(username, "NOUSER"))
> exit(EXIT_FAILURE);
> - if (!clean_string(username)) {
> - syslog(LOG_ERR, "clean_string username");
> - exit(EXIT_FAILURE);
> - }
>
> if (f == NULL && (f = fdopen(3, "r+")) == NULL) {
> syslog(LOG_ERR, "user %s: fdopen: %m", username);
> @@ -163,17 +158,6 @@
> }
>
> static int
> -clean_string(const char *s)
> -{
> - while (*s) {
> - if (!isalnum(*s) && *s != '-' && *s != '_')
> - return (0);
> - ++s;
> - }
> - return (1);
> -}
> -
> -static int
> yubikey_login(const char *username, const char *password)
> {
> char fn[MAXPATHLEN];
> @@ -183,6 +167,10 @@
> yubikey_token_st tok;
> u_int32_t last_ctr = 0, ctr;
>
> + if (getpwnam(username) == NULL) {
> + syslog(LOG_INFO, "user %s: invalid username", username);
> + return (AUTH_FAILED);
> + }
> if (strlen(password) > 32)
> password = password + strlen(password) - 32;
> if (strlen(password) != 32) {
