Skip to content

Commit

Permalink
#3100 cleanup the authentication handler interface
Browse files Browse the repository at this point in the history
the modules don't need to know about the packet,
which is a lower layer concept
  • Loading branch information
totaam committed Aug 26, 2022
1 parent 74d2be6 commit eb78aa8
Show file tree
Hide file tree
Showing 11 changed files with 40 additions and 41 deletions.
13 changes: 8 additions & 5 deletions tests/unittests/unit/client/auth_handlers_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,14 @@ def do_test_handler(self, client, success, password, handler_class, **kwargs):
assert repr(h)
server_salt = kwargs.pop("server-salt", "0"*32)
digest = kwargs.pop("digest", "xor")
salt_digest = kwargs.pop("salt-digest", "xor")
packet = ("challenge", server_salt, "", digest, salt_digest)
r = h.handle(packet)
kwargs = {
"challenge" : server_salt,
"digest" : digest,
"prompt" : "test",
}
r = h.handle(**kwargs)
if not success:
assert not r, f"expected {h.handle}({packet}) to fail but it returned {r} (handler class={handler_class})"
assert not r, f"expected {h.handle}({kwargs}) 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()
Expand All @@ -44,7 +47,7 @@ def test_prompt(self):
from xpra.client.auth.prompt_handler import Handler
client = FakeClient()
password = "prompt-password"
client.do_process_challenge_prompt = lambda packet, prompt : password
client.do_process_challenge_prompt = lambda *_args : password
self.do_test_handler(client, True, password, Handler, digest="gss:token-type")

def test_env_handler(self):
Expand Down
2 changes: 1 addition & 1 deletion xpra/client/auth/env_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ def __repr__(self):
def get_digest(self) -> str:
return None

def handle(self, packet) -> bool:
def handle(self, challenge, digest, prompt) -> bool:
return os.environ.get(self.var_name)
2 changes: 1 addition & 1 deletion xpra/client/auth/file_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def __repr__(self):
def get_digest(self) -> str:
return None

def handle(self, packet) -> bool:
def handle(self, challenge, digest, prompt) -> bool:
log("handle(..) password_file=%s", self.password_file)
if not self.password_file:
return None
Expand Down
3 changes: 1 addition & 2 deletions xpra/client/auth/gss_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ def __repr__(self):
def get_digest(self) -> str:
return "gss"

def handle(self, packet) -> bool:
digest = bytestostr(packet[3])
def handle(self, challenge, digest, prompt) -> bool:
if not digest.startswith("gss:"):
#not a gss challenge
log("%s is not a gss challenge", digest)
Expand Down
3 changes: 1 addition & 2 deletions xpra/client/auth/kerberos_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ def __repr__(self):
def get_digest(self) -> str:
return "kerberos"

def handle(self, packet) -> bool:
digest = bytestostr(packet[3])
def handle(self, challenge, digest, prompt) -> bool:
if not digest.startswith("kerberos:"):
log("%s is not a kerberos challenge", digest)
#not a kerberos challenge
Expand Down
10 changes: 3 additions & 7 deletions xpra/client/auth/prompt_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ def __repr__(self):
def get_digest(self) -> str:
return None

def handle(self, packet) -> bool:
prompt = "password"
digest = bytestostr(packet[3])
if digest.startswith("gss:") or digest.startswith("kerberos:"):
def handle(self, challenge, digest, prompt : str = "password") -> bool:
if not prompt and (digest.startswith("gss:") or digest.startswith("kerberos:")):
prompt = "%s token" % (digest.split(":", 1)[0])
if len(packet)>=6:
prompt = std(bytestostr(packet[5]), extras="-,./: '")
return self.client.do_process_challenge_prompt(packet, prompt)
return self.client.do_process_challenge_prompt(prompt)
6 changes: 2 additions & 4 deletions xpra/client/auth/u2f_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import logging
import binascii

from xpra.os_util import bytestostr, load_binary_file, osexpand
from xpra.os_util import load_binary_file, osexpand
from xpra.log import Logger, is_debug_enabled

log = Logger("auth")
Expand All @@ -24,8 +24,7 @@ def __repr__(self):
def get_digest(self) -> str:
return "u2f"

def handle(self, packet) -> bool:
digest = bytestostr(packet[3])
def handle(self, challenge, digest, prompt) -> bool:
if not digest.startswith("u2f:"):
log("%s is not a u2f challenge", digest)
return None
Expand All @@ -46,7 +45,6 @@ def handle(self, packet) -> bool:
return None
key = model.RegisteredKey(key_handle)
#use server salt as challenge directly
challenge = packet[1]
log.info("activate your U2F device for authentication")
response = dev.Authenticate(APP_ID, challenge, [key])
sig = response.signature_data
Expand Down
2 changes: 1 addition & 1 deletion xpra/client/auth/uri_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ def __repr__(self):
def get_digest(self) -> str:
return None

def handle(self, packet) -> bool:
def handle(self, challenge, digest, prompt) -> bool:
return self.client.password
17 changes: 11 additions & 6 deletions xpra/client/client_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@
parse_encoded_bin_data,
)
from xpra.util import (
flatten_dict, typedict, updict, parse_simple_dict, noerr,
flatten_dict, typedict, updict, parse_simple_dict, noerr, std,
repr_ellipsized, ellipsizer, nonl,
envbool, envint, disconnect_is_an_error, dump_all_frames, engs, csv, obsc,
envbool, envint, disconnect_is_an_error, dump_all_frames, csv, obsc,
SERVER_UPGRADE, CONNECTION_ERROR,
)
from xpra.client.mixins.serverinfo_mixin import ServerInfoMixin
from xpra.client.mixins.fileprint_mixin import FilePrintMixin
from xpra.exit_codes import (EXIT_OK, EXIT_CONNECTION_LOST, EXIT_TIMEOUT, EXIT_UNSUPPORTED,
EXIT_PASSWORD_REQUIRED, EXIT_PASSWORD_FILE_ERROR, EXIT_INCOMPATIBLE_VERSION,
EXIT_PASSWORD_REQUIRED, EXIT_INCOMPATIBLE_VERSION,
EXIT_ENCRYPTION, EXIT_FAILURE, EXIT_PACKET_FAILURE, EXIT_CONNECTION_FAILED,
EXIT_NO_AUTHENTICATION, EXIT_INTERNAL_ERROR, EXIT_UPGRADE,
EXIT_STR,
Expand Down Expand Up @@ -718,8 +718,13 @@ def do_process_challenge(self, packet):
while self.challenge_handlers:
handler = self.pop_challenge_handler(digest)
try:
challenge = packet[1]
digest = bytestostr(packet[3])
prompt = "password"
if len(packet)>=6:
prompt = std(bytestostr(packet[5]), extras="-,./: '")
authlog("calling challenge handler %s", handler)
value = handler.handle(packet)
value = handler.handle(challenge=challenge, digest=digest, prompt=prompt)
authlog("%s(%s)=%s", handler.handle, packet, value)
if value:
self.send_challenge_reply(packet, value)
Expand All @@ -733,10 +738,10 @@ def do_process_challenge(self, packet):
authlog.warn("Warning: failed to connect, authentication required")
self.quit(EXIT_PASSWORD_REQUIRED)

def pop_challenge_handler(self, digest):
def pop_challenge_handler(self, digest=None):
#find the challenge handler most suitable for this digest type,
#otherwise take the first one
digest_type = digest.split(":")[0] #ie: "kerberos:value" -> "kerberos"
digest_type = None if digest is None else digest.split(":")[0] #ie: "kerberos:value" -> "kerberos"
index = 0
for i, handler in enumerate(self.challenge_handlers):
if handler.get_digest()==digest_type:
Expand Down
2 changes: 1 addition & 1 deletion xpra/client/gtk_base/client_launcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ def do_connect_builtin(self, display_desc):
self.set_sensitive(False)
try:
log("calling %s%s", connect_to, (display_desc, repr_ellipsized(str(self.config)), self.set_info_text, self.ssh_failed))
conn = connect_to(display_desc, opts=self.config, debug_cb=self.set_info_text, ssh_fail_cb=self.ssh_failed)
conn = connect_to(display_desc, opts=self.config, client=self.client, debug_cb=self.set_info_text, ssh_fail_cb=self.ssh_failed)
except Exception as e:
log("do_connect_builtin(%s) failed to connect", display_desc, exc_info=True)
self.handle_exception(e)
Expand Down
21 changes: 10 additions & 11 deletions xpra/client/gtk_base/gtk_client_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,17 +273,16 @@ def _process_startup_complete(self, packet):
Gdk.notify_startup_complete()


def do_process_challenge_prompt(self, packet, prompt="password"):
def do_process_challenge_prompt(self, prompt="password"):
authlog = Logger("auth")
self.show_progress(100, "authentication")
PINENTRY = os.environ.get("XPRA_PINENTRY", "")
from xpra.scripts.pinentry_wrapper import get_pinentry_command
pinentry_cmd = get_pinentry_command(PINENTRY)
authlog(f"do_process_challenge_prompt%s get_pinentry_command({PINENTRY})={pinentry_cmd}",
(packet, prompt))
authlog(f"do_process_challenge_prompt({prompt}) get_pinentry_command({PINENTRY})={pinentry_cmd}")
if pinentry_cmd:
return self.handle_challenge_with_pinentry(packet, prompt, pinentry_cmd)
return self.process_challenge_prompt_dialog(packet, prompt)
return self.handle_challenge_with_pinentry(prompt, pinentry_cmd)
return self.process_challenge_prompt_dialog(prompt)

def stop_pinentry(self):
pp = self.pinentry_proc
Expand All @@ -302,14 +301,14 @@ def get_server_authentication_string(self):
server_type = ""
return "%sServer Authentication:" % server_type

def handle_challenge_with_pinentry(self, packet, prompt="password", cmd="pinentry"):
def handle_challenge_with_pinentry(self, prompt="password", cmd="pinentry"):
authlog = Logger("auth")
authlog("handle_challenge_with_pinentry%s", (packet, prompt, cmd))
authlog("handle_challenge_with_pinentry%s", (prompt, cmd))
try:
proc = Popen([cmd], stdin=PIPE, stdout=PIPE, stderr=PIPE)
except OSError:
authlog("pinentry failed", exc_info=True)
return self.process_challenge_prompt_dialog(packet, prompt)
return self.process_challenge_prompt_dialog(prompt)
self.pinentry_proc = proc
q = "Enter %s" % prompt
p = self._protocol
Expand All @@ -330,19 +329,19 @@ def rec(value=None):
return None
return values[0]

def process_challenge_prompt_dialog(self, packet, prompt="password"):
def process_challenge_prompt_dialog(self, prompt="password"):
#challenge handlers run in a separate 'challenge' thread
#but we need to run in the UI thread to access the GUI with Gtk
#so we block the current thread using an event:
wait = Event()
values = []
self.idle_add(self.do_process_challenge_prompt_dialog, packet, values, wait, prompt)
self.idle_add(self.do_process_challenge_prompt_dialog, values, wait, prompt)
wait.wait()
if not values:
return None
return values[0]

def do_process_challenge_prompt_dialog(self, packet, values : list, wait : Event, prompt="password"):
def do_process_challenge_prompt_dialog(self, values : list, wait : Event, prompt="password"):
title = self.get_server_authentication_string()
dialog = Gtk.Dialog(title,
None,
Expand Down

0 comments on commit eb78aa8

Please sign in to comment.