#3100 make challenge handlers more re-usable

also run them in a thread so we don't block the UI with any IO (loading password files) or remote requests (gss / kerberos)

tests/unittests/unit/client/auth_handlers_test.py

@@ -30,12 +30,10 @@ def do_test_handler(self, client, success, password, handler_class, **kwargs):
 		salt_digest = kwargs.pop("salt-digest", "xor")
 		packet = ("challenge", server_salt, "", digest, salt_digest)
 		r = h.handle(packet)
-		assert r==success, "expected %s(%s) to return %s but got %s (handler class=%s)" % (
-			h.handle, packet, success, r, handler_class)
-		if success:
-			passwords = h.client.challenge_reply_passwords
-			assert len(passwords)==1
-			assert passwords[0]==password
+		if not success:
+			assert not r, f"expected {h.handle}({packet}) to fail but it returned {r} (handler class={handler_class})"
+		else:
+			assert r==password, f"expected password value {password} but got {r}"
 			h.get_digest()
 		#client_salt = ""
 		#salt = gendigest(salt_digest, client_salt, server_salt)
@@ -46,10 +44,7 @@ def test_prompt(self):
 		from xpra.client.auth.prompt_handler import Handler
 		client = FakeClient()
 		password = "prompt-password"
-		def rec(packet, _prompt):
-			client.send_challenge_reply(packet, password)
-			return True
-		client.do_process_challenge_prompt = rec
+		client.do_process_challenge_prompt = lambda packet, prompt : password
 		self.do_test_handler(client, True, password, Handler, digest="gss:token-type")
 
 	def test_env_handler(self):

xpra/client/auth/env_handler.py

@@ -1,5 +1,5 @@
 # This file is part of Xpra.
-# Copyright (C) 2019 Antoine Martin <antoine@xpra.org>
+# Copyright (C) 2019-2022 Antoine Martin <antoine@xpra.org>
 # Xpra is released under the terms of the GNU GPL v2, or, at your option, any
 # later version. See the file COPYING for details.
 
@@ -19,8 +19,4 @@ def get_digest(self) -> str:
         return None
 
     def handle(self, packet) -> bool:
-        password = os.environ.get(self.var_name)
-        if not password:
-            return False
-        self.client.send_challenge_reply(packet, password)
-        return True
+        return os.environ.get(self.var_name)

xpra/client/auth/file_handler.py

@@ -1,5 +1,5 @@
 # This file is part of Xpra.
-# Copyright (C) 2019-2020 Antoine Martin <antoine@xpra.org>
+# Copyright (C) 2019-2022 Antoine Martin <antoine@xpra.org>
 # Xpra is released under the terms of the GNU GPL v2, or, at your option, any
 # later version. See the file COPYING for details.
 
@@ -10,6 +10,7 @@
 
 log = Logger("auth")
 
+
 class Handler:
 
     def __init__(self, client, **kwargs):
@@ -29,11 +30,8 @@ def get_digest(self) -> str:
     def handle(self, packet) -> bool:
         log("handle(..) password_file=%s", self.password_file)
         if not self.password_file:
-            return False
+            return None
         filename = os.path.expanduser(self.password_file)
         data = load_binary_file(filename)
         log("loaded password data from %s: %s", filename, bool(data))
-        if not data:
-            return False
-        self.client.send_challenge_reply(packet, data)
-        return True
+        return data

xpra/client/auth/gss_handler.py

@@ -1,5 +1,5 @@
 # This file is part of Xpra.
-# Copyright (C) 2019 Antoine Martin <antoine@xpra.org>
+# Copyright (C) 2019-2022 Antoine Martin <antoine@xpra.org>
 # Xpra is released under the terms of the GNU GPL v2, or, at your option, any
 # later version. See the file COPYING for details.
 
@@ -29,7 +29,7 @@ def handle(self, packet) -> bool:
         if not digest.startswith("gss:"):
             #not a gss challenge
             log("%s is not a gss challenge", digest)
-            return False
+            return None
         try:
             import gssapi       #@UnresolvedImport
             self.gssapi = gssapi
@@ -41,12 +41,12 @@ def handle(self, packet) -> bool:
         except ImportError as e:
             log.warn("Warning: cannot use gss authentication handler")
             log.warn(" %s", e)
-            return False
+            return None
         service = bytestostr(digest.split(b":", 1)[1])
         if service not in self.services and "*" not in self.services:
             log.warn("Warning: invalid GSS request for service '%s'", service)
             log.warn(" services supported: %s", csv(self.services))
-            return False
+            return None
         log("gss service=%s", service)
         service_name = self.gssapi.Name(service)
         try:
@@ -60,7 +60,6 @@ def handle(self, packet) -> bool:
                     log.error(" %s", x.lstrip(" "))
             except Exception:
                 log.error(" %s", e)
-            return False
+            return None
         log("gss token=%s", repr(token))
-        self.client.send_challenge_reply(packet, token)
-        return True
+        return token

xpra/client/auth/kerberos_handler.py

@@ -1,5 +1,5 @@
 # This file is part of Xpra.
-# Copyright (C) 2019 Antoine Martin <antoine@xpra.org>
+# Copyright (C) 2019-2022 Antoine Martin <antoine@xpra.org>
 # Xpra is released under the terms of the GNU GPL v2, or, at your option, any
 # later version. See the file COPYING for details.
 
@@ -42,7 +42,7 @@ def handle(self, packet) -> bool:
         if not digest.startswith("kerberos:"):
             log("%s is not a kerberos challenge", digest)
             #not a kerberos challenge
-            return False
+            return None
         try:
             if WIN32:
                 import winkerberos as kerberos
@@ -56,7 +56,7 @@ def handle(self, packet) -> bool:
         if service not in self.services and "*" not in self.services:
             log.warn("Warning: invalid kerberos request for service '%s'", service)
             log.warn(" services supported: %s", csv(self.services))
-            return False
+            return None
         log("kerberos service=%s", service)
         try:
             r, ctx = kerberos.authGSSClientInit(service)
@@ -65,15 +65,14 @@ def handle(self, packet) -> bool:
             log("kerberos.authGSSClientInit(%s)", service, exc_info=True)
             log.error("Error: cannot initialize kerberos client:")
             log_kerberos_exception(e)
-            return False
+            return None
         try:
             kerberos.authGSSClientStep(ctx, "")
         except Exception as e:
             log("kerberos.authGSSClientStep", exc_info=True)
             log.error("Error: kerberos client authentication failure:")
             log_kerberos_exception(e)
-            return False
+            return None
         token = kerberos.authGSSClientResponse(ctx)
         log("kerberos token=%s", token)
-        self.client.send_challenge_reply(packet, token)
-        return True
+        return token

xpra/client/auth/prompt_handler.py

@@ -1,5 +1,5 @@
 # This file is part of Xpra.
-# Copyright (C) 2019 Antoine Martin <antoine@xpra.org>
+# Copyright (C) 2019-2022 Antoine Martin <antoine@xpra.org>
 # Xpra is released under the terms of the GNU GPL v2, or, at your option, any
 # later version. See the file COPYING for details.
 

xpra/client/auth/u2f_handler.py

@@ -1,5 +1,5 @@
 # This file is part of Xpra.
-# Copyright (C) 2019 Antoine Martin <antoine@xpra.org>
+# Copyright (C) 2019-2022 Antoine Martin <antoine@xpra.org>
 # Xpra is released under the terms of the GNU GPL v2, or, at your option, any
 # later version. See the file COPYING for details.
 
@@ -28,22 +28,22 @@ def handle(self, packet) -> bool:
         digest = bytestostr(packet[3])
         if not digest.startswith("u2f:"):
             log("%s is not a u2f challenge", digest)
-            return False
+            return None
         try:
             from pyu2f import model                     #@UnresolvedImport
             from pyu2f.u2f import GetLocalU2FInterface  #@UnresolvedImport
         except ImportError as e:
             log.warn("Warning: cannot use u2f authentication handler")
             log.warn(" %s", e)
-            return False
+            return None
         if not is_debug_enabled("auth"):
             logging.getLogger("pyu2f.hardware").setLevel(logging.INFO)
             logging.getLogger("pyu2f.hidtransport").setLevel(logging.INFO)
         dev = GetLocalU2FInterface()
         APP_ID = os.environ.get("XPRA_U2F_APP_ID", "Xpra")
         key_handle = self.get_key_handle()
         if not key_handle:
-            return False
+            return None
         key = model.RegisteredKey(key_handle)
         #use server salt as challenge directly
         challenge = packet[1]
@@ -52,8 +52,7 @@ def handle(self, packet) -> bool:
         sig = response.signature_data
         client_data = response.client_data
         log("process_challenge_u2f client data=%s, signature=%s", client_data, binascii.hexlify(sig))
-        self.client.do_send_challenge_reply(bytes(sig), client_data.origin)
-        return True
+        return bytes(sig), client_data.origin
 
     def get_key_handle(self) -> bytes:
         key_handle_str = os.environ.get("XPRA_U2F_KEY_HANDLE")

xpra/client/auth/uri_handler.py

@@ -1,5 +1,5 @@
 # This file is part of Xpra.
-# Copyright (C) 2019 Antoine Martin <antoine@xpra.org>
+# Copyright (C) 2019-2022 Antoine Martin <antoine@xpra.org>
 # Xpra is released under the terms of the GNU GPL v2, or, at your option, any
 # later version. See the file COPYING for details.
 
@@ -16,7 +16,4 @@ def get_digest(self) -> str:
         return None
 
     def handle(self, packet) -> bool:
-        if not self.client.password:
-            return False
-        self.client.send_challenge_reply(packet, self.client.password)
-        return True
+        return self.client.password

xpra/client/client_base.py

@@ -18,6 +18,7 @@
 from xpra.child_reaper import getChildReaper, reaper_cleanup
 from xpra.net import compression
 from xpra.net.common import may_log_packet, PACKET_TYPES
+from xpra.make_thread import start_thread
 from xpra.net.protocol_classes import get_client_protocol_class
 from xpra.net.protocol import CONNECTION_LOST, GIBBERISH, INVALID
 from xpra.net.net_util import get_network_caps
@@ -709,16 +710,20 @@ def _process_challenge(self, packet):
         authlog("processing challenge: %s", packet[1:])
         if not self.validate_challenge_packet(packet):
             return
-        authlog("challenge handlers: %s", self.challenge_handlers)
+        start_thread(self.do_process_challenge, "call-challenge-handlers", True, (packet, ))
+
+    def do_process_challenge(self, packet):
         digest = bytestostr(packet[3])
+        authlog(f"challenge handlers: {self.challenge_handlers}, digest: {digest}")
         while self.challenge_handlers:
             handler = self.pop_challenge_handler(digest)
             try:
                 authlog("calling challenge handler %s", handler)
-                r = handler.handle(packet)
-                authlog("%s(%s)=%s", handler.handle, packet, r)
-                if r:
-                    #the challenge handler claims to have handled authentication
+                value = handler.handle(packet)
+                authlog("%s(%s)=%s", handler.handle, packet, value)
+                if value:
+                    self.send_challenge_reply(packet, value)
+                    #stop since we have sent the reply
                     return
             except Exception as e:
                 authlog("%s(%s)", handler.handle, packet, exc_info=True)
@@ -749,23 +754,20 @@ def do_process_challenge_prompt(self, packet, prompt="password"):
             authlog("stdin isatty, using password prompt")
             password = getpass.getpass("%s :" % self.get_challenge_prompt(prompt))
             authlog("password read from tty via getpass: %s", obsc(password))
-            self.send_challenge_reply(packet, password)
-            return True
-        else:
-            from xpra.platform.paths import get_nodock_command
-            cmd = get_nodock_command()+["_pass", prompt]
-            try:
-                from subprocess import Popen, PIPE
-                proc = Popen(cmd, stdout=PIPE)
-                getChildReaper().add_process(proc, "password-prompt", cmd, True, True)
-                out, err = proc.communicate(None, 60)
-                authlog("err(%s)=%s", cmd, err)
-                password = out.decode()
-                self.send_challenge_reply(packet, password)
-                return True
-            except Exception:
-                log("Error: failed to show GUi for password prompt", exc_info=True)
-        return False
+            return password
+        from xpra.platform.paths import get_nodock_command
+        cmd = get_nodock_command()+["_pass", prompt]
+        try:
+            from subprocess import Popen, PIPE
+            proc = Popen(cmd, stdout=PIPE)
+            getChildReaper().add_process(proc, "password-prompt", cmd, True, True)
+            out, err = proc.communicate(None, 60)
+            authlog("err(%s)=%s", cmd, err)
+            password = out.decode()
+            return password
+        except Exception:
+            log("Error: failed to show GUi for password prompt", exc_info=True)
+            return None
 
     def auth_error(self, code, message, server_message="authentication failed"):
         authlog.error("Error: authentication failed:")
@@ -811,15 +813,10 @@ def get_challenge_prompt(self, prompt="password"):
             pass
         return text
 
-    def send_challenge_reply(self, packet, password):
-        if not password:
-            if self.password_file:
-                self.auth_error(EXIT_PASSWORD_FILE_ERROR,
-                                "failed to load password from file%s %s" % (engs(self.password_file), csv(self.password_file)),
-                                "no password available")
-            else:
-                self.auth_error(EXIT_PASSWORD_REQUIRED,
-                                "this server requires authentication and no password is available")
+    def send_challenge_reply(self, packet, value):
+        if not value:
+            self.auth_error(EXIT_PASSWORD_REQUIRED,
+                            "this server requires authentication and no password is available")
             return
         encryption = self.get_encryption()
         if encryption:
@@ -828,6 +825,13 @@ def send_challenge_reply(self, packet, password):
             key = self.get_encryption_key()
             if not self.set_server_encryption(server_cipher, key):
                 return
+        #some authentication handlers give us the response and salt,
+        #ready to use without needing to use the digest
+        #(ie: u2f handler)
+        if isinstance(value, (tuple, list)) and len(value)==2:
+            self.do_send_challenge_reply(*value)
+            return
+        password = value
         #all server versions support a client salt,
         #they also tell us which digest to use:
         server_salt = bytestostr(packet[1])
@@ -864,8 +868,9 @@ def do_send_challenge_reply(self, challenge_response, client_salt):
         self.password_sent = True
         if self._protocol.TYPE=="rfb":
             self._protocol.send_challenge_reply(challenge_response)
-        else:
-            self.send_hello(challenge_response, client_salt)
+            return
+        #call send_hello from the UI thread:
+        self.idle_add(self.send_hello, challenge_response, client_salt)
 
     ########################################
     # Encryption