- djm@cvs.openbsd.org 2008/07/02 12:36:39

     [auth2-none.c auth2.c]
     Make protocol 2 MaxAuthTries behaviour a little more sensible:
     Check whether client has exceeded MaxAuthTries before running
     an authentication method and skip it if they have, previously it
     would always allow one try (for "none" auth).
     Preincrement failure count before post-auth test - previously this
     checked and postincremented, also to allow one "none" try.
     Together, these two changes always count the "none" auth method
     which could be skipped by a malicious client (e.g. an SSH worm)
     to get an extra attempt at a real auth method. They also make
     MaxAuthTries=0 a useful way to block users entirely (esp. in a
     sshd_config Match block).
     Also, move sending of any preauth banner from "none" auth method
     to the first call to input_userauth_request(), so worms that skip
     the "none" method get to see it too.

diff --git a/ChangeLog b/ChangeLog
index fd97e2a49fcf82ad6b5796ce77ad8c2afe7e9c40..318e79fc80879d001e0c8432b1d33e9606046fc4 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -33,6 +33,22 @@
      Merge duplicate host key file checks, based in part on a patch from Rob
      Holland via bz #1348 .  Also checks for non-regular files during protocol
      1 RSA auth.  ok djm@
+   - djm@cvs.openbsd.org 2008/07/02 12:36:39
+     [auth2-none.c auth2.c]
+     Make protocol 2 MaxAuthTries behaviour a little more sensible:
+     Check whether client has exceeded MaxAuthTries before running
+     an authentication method and skip it if they have, previously it
+     would always allow one try (for "none" auth).
+     Preincrement failure count before post-auth test - previously this
+     checked and postincremented, also to allow one "none" try.
+     Together, these two changes always count the "none" auth method
+     which could be skipped by a malicious client (e.g. an SSH worm)
+     to get an extra attempt at a real auth method. They also make
+     MaxAuthTries=0 a useful way to block users entirely (esp. in a
+     sshd_config Match block).
+     Also, move sending of any preauth banner from "none" auth method
+     to the first call to input_userauth_request(), so worms that skip
+     the "none" method get to see it too.
 
 20080630
  - (djm) OpenBSD CVS Sync

diff --git a/auth2-none.c b/auth2-none.c
index 28e593e6c9bdc7728a7be4b68689bb3b7def9c08..10accfe552dfb0657c5eeca5676d18097e0c07af 100644 (file)
--- a/auth2-none.c
+++ b/auth2-none.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth2-none.c,v 1.14 2007/08/23 03:22:16 djm Exp $ */
+/* $OpenBSD: auth2-none.c,v 1.15 2008/07/02 12:36:39 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -31,9 +31,10 @@
 
 #include <fcntl.h>
 #include <stdarg.h>
-#include <unistd.h>
 #include <string.h>
+#include <unistd.h>
 
+#include "atomicio.h"
 #include "xmalloc.h"
 #include "key.h"
 #include "hostfile.h"
@@ -42,7 +43,6 @@
 #include "log.h"
 #include "buffer.h"
 #include "servconf.h"
-#include "atomicio.h"
 #include "compat.h"
 #include "ssh2.h"
 #ifdef GSSAPI
@@ -56,77 +56,11 @@ extern ServerOptions options;
 /* "none" is allowed only one time */
 static int none_enabled = 1;
 
-char *
-auth2_read_banner(void)
-{
-       struct stat st;
-       char *banner = NULL;
-       size_t len, n;
-       int fd;
-
-       if ((fd = open(options.banner, O_RDONLY)) == -1)
-               return (NULL);
-       if (fstat(fd, &st) == -1) {
-               close(fd);
-               return (NULL);
-       }
-       if (st.st_size > 1*1024*1024) {
-               close(fd);
-               return (NULL);
-       }
-
-       len = (size_t)st.st_size;               /* truncate */
-       banner = xmalloc(len + 1);
-       n = atomicio(read, fd, banner, len);
-       close(fd);
-
-       if (n != len) {
-               xfree(banner);
-               return (NULL);
-       }
-       banner[n] = '\0';
-
-       return (banner);
-}
-
-void
-userauth_send_banner(const char *msg)
-{
-       if (datafellows & SSH_BUG_BANNER)
-               return;
-
-       packet_start(SSH2_MSG_USERAUTH_BANNER);
-       packet_put_cstring(msg);
-       packet_put_cstring("");         /* language, unused */
-       packet_send();
-       debug("%s: sent", __func__);
-}
-
-static void
-userauth_banner(void)
-{
-       char *banner = NULL;
-
-       if (options.banner == NULL ||
-           strcasecmp(options.banner, "none") == 0 ||
-           (datafellows & SSH_BUG_BANNER) != 0)
-               return;
-
-       if ((banner = PRIVSEP(auth2_read_banner())) == NULL)
-               goto done;
-       userauth_send_banner(banner);
-
-done:
-       if (banner)
-               xfree(banner);
-}
-
 static int
 userauth_none(Authctxt *authctxt)
 {
        none_enabled = 0;
        packet_check_eom();
-       userauth_banner();
 #ifdef HAVE_CYGWIN
        if (check_nt_auth(1, authctxt->pw) == 0)
                return (0);

diff --git a/auth2.c b/auth2.c
index 03d7f09dc680c8720803190479300a29aacf67d3..31f01f9fbecd38b93d3b0732456e2e42ca19288b 100644 (file)
--- a/auth2.c
+++ b/auth2.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth2.c,v 1.116 2007/09/29 00:25:51 dtucker Exp $ */
+/* $OpenBSD: auth2.c,v 1.117 2008/07/02 12:36:39 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -26,10 +26,14 @@
 #include "includes.h"
 
 #include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/uio.h>
 
+#include <fcntl.h>
 #include <pwd.h>
 #include <stdarg.h>
 #include <string.h>
+#include <unistd.h>
 
 #include "xmalloc.h"
 #include "ssh2.h"
@@ -88,10 +92,74 @@ static void input_userauth_request(int, u_int32_t, void *);
 static Authmethod *authmethod_lookup(const char *);
 static char *authmethods_get(void);
 
+char *
+auth2_read_banner(void)
+{
+       struct stat st;
+       char *banner = NULL;
+       size_t len, n;
+       int fd;
+
+       if ((fd = open(options.banner, O_RDONLY)) == -1)
+               return (NULL);
+       if (fstat(fd, &st) == -1) {
+               close(fd);
+               return (NULL);
+       }
+       if (st.st_size > 1*1024*1024) {
+               close(fd);
+               return (NULL);
+       }
+
+       len = (size_t)st.st_size;               /* truncate */
+       banner = xmalloc(len + 1);
+       n = atomicio(read, fd, banner, len);
+       close(fd);
+
+       if (n != len) {
+               xfree(banner);
+               return (NULL);
+       }
+       banner[n] = '\0';
+
+       return (banner);
+}
+
+void
+userauth_send_banner(const char *msg)
+{
+       if (datafellows & SSH_BUG_BANNER)
+               return;
+
+       packet_start(SSH2_MSG_USERAUTH_BANNER);
+       packet_put_cstring(msg);
+       packet_put_cstring("");         /* language, unused */
+       packet_send();
+       debug("%s: sent", __func__);
+}
+
+static void
+userauth_banner(void)
+{
+       char *banner = NULL;
+
+       if (options.banner == NULL ||
+           strcasecmp(options.banner, "none") == 0 ||
+           (datafellows & SSH_BUG_BANNER) != 0)
+               return;
+
+       if ((banner = PRIVSEP(auth2_read_banner())) == NULL)
+               goto done;
+       userauth_send_banner(banner);
+
+done:
+       if (banner)
+               xfree(banner);
+}
+
 /*
  * loop until authctxt->success == TRUE
  */
-
 void
 do_authentication2(Authctxt *authctxt)
 {
@@ -179,6 +247,7 @@ input_userauth_request(int type, u_int32_t seq, void *ctxt)
                authctxt->style = style ? xstrdup(style) : NULL;
                if (use_privsep)
                        mm_inform_authserv(service, style);
+               userauth_banner();
        } else if (strcmp(user, authctxt->user) != 0 ||
            strcmp(service, authctxt->service) != 0) {
                packet_disconnect("Change of username or service not allowed: "
@@ -197,7 +266,7 @@ input_userauth_request(int type, u_int32_t seq, void *ctxt)
 
        /* try to authenticate user */
        m = authmethod_lookup(method);
-       if (m != NULL) {
+       if (m != NULL && authctxt->failures < options.max_authtries) {
                debug2("input_userauth_request: try method %s", method);
                authenticated = m->userauth(authctxt);
        }
@@ -264,7 +333,7 @@ userauth_finish(Authctxt *authctxt, int authenticated, char *method)
                /* now we can break out */
                authctxt->success = 1;
        } else {
-               if (authctxt->failures++ > options.max_authtries) {
+               if (++authctxt->failures > options.max_authtries) {
 #ifdef SSH_AUDIT_EVENTS
                        PRIVSEP(audit_event(SSH_LOGIN_EXCEED_MAXTRIES));
 #endif
@@ -320,3 +389,4 @@ authmethod_lookup(const char *name)
            name ? name : "NULL");
        return NULL;
 }
+