From 947bb3e060b4fd52f0a3bacd78420c8dff737afe Mon Sep 17 00:00:00 2001
From: djm <djm@openbsd.org>
Date: Sun, 26 Dec 2021 23:34:41 +0000
Subject: [PATCH] split method list search functionality from
 authmethod_lookup() into a separate authmethod_byname(), for cases where we
 don't need to check whether a method is enabled, etc.

use this to fix the "none" authentication method regression reported
by Nam Nguyen via bugs@

ok deraadt@
---
 usr.bin/ssh/auth2.c | 51 +++++++++++++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/usr.bin/ssh/auth2.c b/usr.bin/ssh/auth2.c
index d6cc3d78c8e..ba7b3573d75 100644
--- a/usr.bin/ssh/auth2.c
+++ b/usr.bin/ssh/auth2.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth2.c,v 1.162 2021/12/19 22:12:07 djm Exp $ */
+/* $OpenBSD: auth2.c,v 1.163 2021/12/26 23:34:41 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -89,6 +89,7 @@ static int input_service_request(int, u_int32_t, struct ssh *);
 static int input_userauth_request(int, u_int32_t, struct ssh *);
 
 /* helper */
+static Authmethod *authmethod_byname(const char *);
 static Authmethod *authmethod_lookup(Authctxt *, const char *);
 static char *authmethods_get(Authctxt *authctxt);
 
@@ -345,9 +346,10 @@ userauth_finish(struct ssh *ssh, int authenticated, const char *packet_method,
 		}
 		if (authctxt->postponed)
 			fatal("INTERNAL ERROR: authenticated and postponed");
-		if ((m = authmethod_lookup(authctxt, method)) == NULL)
+		/* prefer primary authmethod name to possible synonym */
+		if ((m = authmethod_byname(method)) == NULL)
 			fatal("INTERNAL ERROR: bad method %s", method);
-		method = m->name; /* prefer primary name to possible synonym */
+		method = m->name;
 	}
 
 	/* Special handling for root */
@@ -457,25 +459,42 @@ authmethods_get(Authctxt *authctxt)
 }
 
 static Authmethod *
-authmethod_lookup(Authctxt *authctxt, const char *name)
+authmethod_byname(const char *name)
 {
 	int i;
 
-	if (name != NULL)
-		for (i = 0; authmethods[i] != NULL; i++)
-			if (authmethods[i]->enabled != NULL &&
-			    *(authmethods[i]->enabled) != 0 &&
-			    (strcmp(name, authmethods[i]->name) == 0 ||
-			    (authmethods[i]->synonym != NULL &&
-			    strcmp(name, authmethods[i]->synonym) == 0)) &&
-			    auth2_method_allowed(authctxt,
-			    authmethods[i]->name, NULL))
-				return authmethods[i];
-	debug2("Unrecognized authentication method name: %s",
-	    name ? name : "NULL");
+	if (name == NULL)
+		fatal_f("NULL authentication method name");
+	for (i = 0; authmethods[i] != NULL; i++) {
+		if (strcmp(name, authmethods[i]->name) == 0 ||
+		    (authmethods[i]->synonym != NULL &&
+		    strcmp(name, authmethods[i]->synonym) == 0))
+			return authmethods[i];
+	}
+	debug_f("unrecognized authentication method name: %s", name);
 	return NULL;
 }
 
+static Authmethod *
+authmethod_lookup(Authctxt *authctxt, const char *name)
+{
+	Authmethod *method;
+
+	if ((method = authmethod_byname(name)) == NULL)
+		return NULL;
+
+	if (method->enabled == NULL || *(method->enabled) == 0) {
+		debug3_f("method %s not enabled", name);
+		return NULL;
+	}
+	if (!auth2_method_allowed(authctxt, method->name, NULL)) {
+		debug3_f("method %s not allowed "
+		    "by AuthenticationMethods", name);
+		return NULL;
+	}
+	return method;
+}
+
 /*
  * Check a comma-separated list of methods for validity. Is need_enable is
  * non-zero, then also require that the methods are enabled.
-- 
2.20.1