From 4acf7e8d3bc6db18fd725f8980a897f16f049591 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Fri, 19 Apr 2024 10:37:19 +0200 Subject: [PATCH 1/5] Remove unused SignatureAuthHandler.get_fingerprint() --- osc/connection.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/osc/connection.py b/osc/connection.py index b4f3496f..a7660a4e 100644 --- a/osc/connection.py +++ b/osc/connection.py @@ -591,13 +591,6 @@ class SignatureAuthHandler(AuthHandlerBase): else: return [] - @staticmethod - def get_fingerprint(line): - parts = line.strip().split(b" ") - if len(parts) < 2: - raise ValueError(f"Unable to retrieve ssh key fingerprint from line: {line}") - return parts[1] - def guess_keyfile(self): # `ssh-keygen -Y sign` requires a file with a key which is not available during ssh agent forwarding # that's why we need to list ssh-agent's keys and store the first one into a temp file From 9b08b3c45b54eb69d750de3dedf2f755cf8da74f Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Fri, 19 Apr 2024 10:40:30 +0200 Subject: [PATCH 2/5] Use strings instead of bytes in SignatureAuthHandler --- osc/connection.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/osc/connection.py b/osc/connection.py index a7660a4e..8d2f9d1c 100644 --- a/osc/connection.py +++ b/osc/connection.py @@ -584,7 +584,7 @@ class SignatureAuthHandler(AuthHandlerBase): if not self.ssh_add_path: return [] cmd = [self.ssh_add_path, '-L'] - proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8") stdout, _ = proc.communicate() if proc.returncode == 0 and stdout.strip(): return stdout.strip().splitlines() @@ -596,7 +596,7 @@ class SignatureAuthHandler(AuthHandlerBase): # that's why we need to list ssh-agent's keys and store the first one into a temp file keys_in_agent = self.list_ssh_agent_keys() if keys_in_agent: - self.temp_pubkey = tempfile.NamedTemporaryFile() + self.temp_pubkey = tempfile.NamedTemporaryFile(mode="w+") self.temp_pubkey.write(keys_in_agent[0]) self.temp_pubkey.flush() return self.temp_pubkey.name @@ -610,10 +610,6 @@ class SignatureAuthHandler(AuthHandlerBase): return None def ssh_sign(self, data, namespace, keyfile=None): - try: - data = bytes(data, 'utf-8') - except: - pass if not keyfile: keyfile = self.guess_keyfile() else: @@ -622,8 +618,8 @@ class SignatureAuthHandler(AuthHandlerBase): keyfile = os.path.expanduser(keyfile) cmd = [self.ssh_keygen_path, '-Y', 'sign', '-f', keyfile, '-n', namespace, '-q'] - proc = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE) - stdout, _ = proc.communicate(data) + proc = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, encoding="utf-8") + signature, _ = proc.communicate(data) if self.temp_pubkey: self.temp_pubkey.close() @@ -632,7 +628,6 @@ class SignatureAuthHandler(AuthHandlerBase): if proc.returncode: raise oscerr.OscIOError(None, 'ssh-keygen signature creation failed: %d' % proc.returncode) - signature = decode_it(stdout) match = re.match(r"\A-----BEGIN SSH SIGNATURE-----\n(.*)\n-----END SSH SIGNATURE-----", signature, re.S) if not match: raise oscerr.OscIOError(None, 'could not extract ssh signature') From 2cd69aa400c922aa26ee92e680913f7386e356d2 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Fri, 19 Apr 2024 11:03:05 +0200 Subject: [PATCH 3/5] Move prepending '~/.ssh' to the ssh key path from SignatureAuthHandler.ssh_sign() to __init__() --- osc/connection.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/osc/connection.py b/osc/connection.py index 8d2f9d1c..6a844b4c 100644 --- a/osc/connection.py +++ b/osc/connection.py @@ -568,6 +568,12 @@ class SignatureAuthHandler(AuthHandlerBase): self.user = user self.sshkey = sshkey + if self.sshkey: + # if only a file name is provided, prepend ~/.ssh + if "/" not in self.sshkey: + self.sshkey = os.path.join("~", ".ssh", self.sshkey) + self.sshkey = os.path.expanduser(self.sshkey) + self.ssh_keygen_path = shutil.which("ssh-keygen") self.ssh_add_path = shutil.which("ssh-add") @@ -612,11 +618,6 @@ class SignatureAuthHandler(AuthHandlerBase): def ssh_sign(self, data, namespace, keyfile=None): if not keyfile: keyfile = self.guess_keyfile() - else: - if '/' not in keyfile: - keyfile = f"~/.ssh/{keyfile}" - keyfile = os.path.expanduser(keyfile) - cmd = [self.ssh_keygen_path, '-Y', 'sign', '-f', keyfile, '-n', namespace, '-q'] proc = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, encoding="utf-8") signature, _ = proc.communicate(data) From 57311a5664bfc71273bf70631f4d756512eb201a Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Fri, 19 Apr 2024 11:06:39 +0200 Subject: [PATCH 4/5] Use ssh key from ssh agent that contains comment 'obs=' --- osc/conf.py | 9 +++++++++ osc/connection.py | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/osc/conf.py b/osc/conf.py index f77d183d..5c0ee5b7 100644 --- a/osc/conf.py +++ b/osc/conf.py @@ -303,10 +303,19 @@ class HostOptions(OscOptions): """ A pointer to public SSH key that corresponds with a private SSH used for authentication: + - keep empty for auto detection - path to the public SSH key - public SSH key filename (must be placed in ~/.ssh) NOTE: The private key may not be available on disk because it could be in a GPG keyring, on YubiKey or forwarded through SSH agent. + + TIP: To give osc a hint which ssh key from the agent to use during auto detection, + append ``obs=`` to the **private** key's comment. + This will also work nicely during SSH agent forwarding, because the comments get forwarded too. + + - To edit the key, run: ``ssh-keygen -c -f ~/.ssh/`` + - To query the key, run: ``ssh-keygen -y -f ~/.ssh/`` + - Example comment: `` obs=api.example.com obs=api-test.example.com`` """ ), ) # type: ignore[assignment] diff --git a/osc/connection.py b/osc/connection.py index 6a844b4c..906af246 100644 --- a/osc/connection.py +++ b/osc/connection.py @@ -573,6 +573,7 @@ class SignatureAuthHandler(AuthHandlerBase): if "/" not in self.sshkey: self.sshkey = os.path.join("~", ".ssh", self.sshkey) self.sshkey = os.path.expanduser(self.sshkey) + output.print_msg(f"Using ssh key file configured in oscrc: {self.sshkey}", print_to="debug") self.ssh_keygen_path = shutil.which("ssh-keygen") self.ssh_add_path = shutil.which("ssh-add") @@ -602,6 +603,21 @@ class SignatureAuthHandler(AuthHandlerBase): # that's why we need to list ssh-agent's keys and store the first one into a temp file keys_in_agent = self.list_ssh_agent_keys() if keys_in_agent: + selected_key = None + + apiurl_hostname = urllib.parse.urlparse(self.apiurl).hostname + for key in keys_in_agent: + comments = key.strip().split(" ")[2:] + pattern = f"obs={apiurl_hostname}" + if pattern in comments: + selected_key = key + output.print_msg(f"Using ssh key from ssh agent that has comment '{pattern}' which matches apiurl '{self.apiurl}': {selected_key}", print_to="debug") + break + + if selected_key is None: + selected_key = keys_in_agent[0] + output.print_msg(f"Using the first ssh key from ssh agent (see `ssh-add -L`): {selected_key}", print_to="debug") + self.temp_pubkey = tempfile.NamedTemporaryFile(mode="w+") self.temp_pubkey.write(keys_in_agent[0]) self.temp_pubkey.flush() @@ -609,9 +625,11 @@ class SignatureAuthHandler(AuthHandlerBase): sshdir = os.path.expanduser('~/.ssh') keyfiles = ('id_ed25519', 'id_ed25519_sk', 'id_rsa', 'id_ecdsa', 'id_ecdsa_sk', 'id_dsa') + output.print_msg(f"Searching ssh keys in '{sshdir}' in the following order: {', '.join(keyfiles)}", print_to="debug") for keyfile in keyfiles: keyfile_path = os.path.join(sshdir, keyfile) if os.path.isfile(keyfile_path): + output.print_msg(f"Using ssh key from file: {keyfile_path}", print_to="debug") return keyfile_path return None From 04566f8e55f5780106df61b681d5ee238bacbf87 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Fri, 19 Apr 2024 13:39:37 +0200 Subject: [PATCH 5/5] Update SignatureAuthHandler to support specifying ssh key by its fingerprint --- osc/conf.py | 1 + osc/connection.py | 50 ++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/osc/conf.py b/osc/conf.py index 5c0ee5b7..31c5c9ec 100644 --- a/osc/conf.py +++ b/osc/conf.py @@ -306,6 +306,7 @@ class HostOptions(OscOptions): - keep empty for auto detection - path to the public SSH key - public SSH key filename (must be placed in ~/.ssh) + - fingerprint of a SSH key (2nd column of ``ssh-add -l``) NOTE: The private key may not be available on disk because it could be in a GPG keyring, on YubiKey or forwarded through SSH agent. diff --git a/osc/connection.py b/osc/connection.py index 906af246..7a8b37de 100644 --- a/osc/connection.py +++ b/osc/connection.py @@ -566,7 +566,14 @@ class SignatureAuthHandler(AuthHandlerBase): def __init__(self, apiurl, user, sshkey, basic_auth_password=None): super().__init__(apiurl) self.user = user - self.sshkey = sshkey + self.sshkey = None + self.sshkey_fingerprint = None + + if sshkey and re.match("^[A-Z0-9]+:.*", sshkey): + # if it starts with a prefix such as 'SHA256:' then it's a fingerprint + self.sshkey_fingerprint = sshkey + else: + self.sshkey = sshkey if self.sshkey: # if only a file name is provided, prepend ~/.ssh @@ -598,6 +605,18 @@ class SignatureAuthHandler(AuthHandlerBase): else: return [] + def list_ssh_agent_fingerprints(self): + if not self.ssh_add_path: + return [] + cmd = [self.ssh_add_path, '-l'] + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8") + stdout, _ = proc.communicate() + if proc.returncode == 0 and stdout.strip(): + lines = stdout.strip().splitlines() + return [i.split(" ")[1] for i in lines] + else: + return [] + def guess_keyfile(self): # `ssh-keygen -Y sign` requires a file with a key which is not available during ssh agent forwarding # that's why we need to list ssh-agent's keys and store the first one into a temp file @@ -605,15 +624,28 @@ class SignatureAuthHandler(AuthHandlerBase): if keys_in_agent: selected_key = None - apiurl_hostname = urllib.parse.urlparse(self.apiurl).hostname - for key in keys_in_agent: - comments = key.strip().split(" ")[2:] - pattern = f"obs={apiurl_hostname}" - if pattern in comments: - selected_key = key - output.print_msg(f"Using ssh key from ssh agent that has comment '{pattern}' which matches apiurl '{self.apiurl}': {selected_key}", print_to="debug") - break + # use ssh key from ssh agent by the specified fingerprint + if self.sshkey_fingerprint: + fingerprints_in_agent = self.list_ssh_agent_fingerprints() + try: + indx = fingerprints_in_agent.index(self.sshkey_fingerprint) + selected_key = keys_in_agent[indx] + output.print_msg(f"Using ssh key from ssh agent that matches fingerprint '{self.sshkey_fingerprint}': {selected_key}", print_to="debug") + except ValueError: + pass + # use ssh key from ssh agent by key's comment obs= matching the hostname of apiurl + if selected_key is None: + apiurl_hostname = urllib.parse.urlparse(self.apiurl).hostname + for key in keys_in_agent: + comments = key.strip().split(" ")[2:] + pattern = f"obs={apiurl_hostname}" + if pattern in comments: + selected_key = key + output.print_msg(f"Using ssh key from ssh agent that has comment '{pattern}' which matches apiurl '{self.apiurl}': {selected_key}", print_to="debug") + break + + # use the first ssh key from ssh agent if selected_key is None: selected_key = keys_in_agent[0] output.print_msg(f"Using the first ssh key from ssh agent (see `ssh-add -L`): {selected_key}", print_to="debug")