- 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.

1 files changed, 3 insertions, 69 deletions

diff --git a/auth2-none.c b/auth2-none.c
index 28e593e6..10accfe5 100644
--- 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);