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@

diff --git a/usr.bin/ssh/auth2.c b/usr.bin/ssh/auth2.c
index d6cc3d7..ba7b357 100644 (file)
--- 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.