From 558e1d828fc01934f6a9b493ba0dff1779d6204a Mon Sep 17 00:00:00 2001 From: Fede Luzzi Date: Tue, 7 Jul 2026 14:03:14 -0300 Subject: [PATCH] bug fix jump over jump --- connpy/_version.py | 2 +- connpy/cli/node_handler.py | 2 +- connpy/core.py | 205 ++++++++++++++++++----------- connpy/tests/test_core.py | 261 +++++++++++++++++++++++++++++++++++++ 4 files changed, 393 insertions(+), 77 deletions(-) diff --git a/connpy/_version.py b/connpy/_version.py index 58e5740..bb78858 100644 --- a/connpy/_version.py +++ b/connpy/_version.py @@ -1 +1 @@ -__version__ = "6.0.3" +__version__ = "6.0.4" diff --git a/connpy/cli/node_handler.py b/connpy/cli/node_handler.py index d45ab7f..e9c0847 100644 --- a/connpy/cli/node_handler.py +++ b/connpy/cli/node_handler.py @@ -76,7 +76,7 @@ class NodeHandler: debug=args.debug, logger=self.app._service_logger ) - except ConnpyError as e: + except (ConnpyError, ValueError) as e: printer.error(str(e)) sys.exit(1) diff --git a/connpy/core.py b/connpy/core.py index e19d9a1..2f6ca09 100755 --- a/connpy/core.py +++ b/connpy/core.py @@ -3,6 +3,7 @@ import os import re import pexpect +import shlex from Crypto.PublicKey import RSA from Crypto.Cipher import PKCS1_OAEP import ast @@ -131,80 +132,12 @@ class node: else: self.password = [password] if self.jumphost != "" and config != '': - self.jumphost = config.getitem(self.jumphost) - for key in self.jumphost: - profile = re.search("^@(.*)", str(self.jumphost[key])) - if profile: - try: - self.jumphost[key] = config.profiles[profile.group(1)][key] - except KeyError: - self.jumphost[key] = "" - elif self.jumphost[key] == '' and key == "protocol": - try: - self.jumphost[key] = config.profiles["default"][key] - except KeyError: - self.jumphost[key] = "ssh" - if isinstance(self.jumphost["password"],list): - jumphost_password = [] - for i, s in enumerate(self.jumphost["password"]): - profile = re.search("^@(.*)", self.jumphost["password"][i]) - if profile: - jumphost_password.append(config.profiles[profile.group(1)]["password"]) - else: - jumphost_password.append(self.jumphost["password"][i]) - self.jumphost["password"] = jumphost_password - else: - self.jumphost["password"] = [self.jumphost["password"]] - if self.jumphost["password"] != [""]: - self.password = self.jumphost["password"] + self.password - - if self.jumphost["protocol"] == "ssh": - jumphost_cmd = self.jumphost["protocol"] + " -W %h:%p" - if self.jumphost["port"] != '': - jumphost_cmd = jumphost_cmd + " -p " + self.jumphost["port"] - if self.jumphost["options"] != '': - jumphost_cmd = jumphost_cmd + " " + self.jumphost["options"] - if self.jumphost["user"] == '': - jumphost_cmd = jumphost_cmd + " {}".format(self.jumphost["host"]) - else: - jumphost_cmd = jumphost_cmd + " {}".format("@".join([self.jumphost["user"],self.jumphost["host"]])) - self.jumphost = f"-o ProxyCommand=\"{jumphost_cmd}\"" - elif self.jumphost["protocol"] == "ssm": - ssm_target = self.jumphost["host"] - ssm_cmd = f"aws ssm start-session --target {ssm_target} --document-name AWS-StartSSHSession --parameters 'portNumber=22'" - if isinstance(self.jumphost.get("tags"), dict): - if "profile" in self.jumphost["tags"]: - ssm_cmd += f" --profile {self.jumphost['tags']['profile']}" - if "region" in self.jumphost["tags"]: - ssm_cmd += f" --region {self.jumphost['tags']['region']}" - if self.jumphost["options"] != '': - ssm_cmd += f" {self.jumphost['options']}" - - bastion_user_part = f"{self.jumphost['user']}@{ssm_target}" if self.jumphost['user'] else ssm_target - - ssh_opts = "" - if isinstance(self.jumphost.get("tags"), dict) and "ssh_options" in self.jumphost["tags"]: - ssh_opts = f" {self.jumphost['tags']['ssh_options']}" - - inner_ssh = f"ssh{ssh_opts} -o ProxyCommand='{ssm_cmd}' -W %h:%p {bastion_user_part}" - self.jumphost = f"-o ProxyCommand=\"{inner_ssh}\"" - elif self.jumphost["protocol"] in ["kubectl", "docker"]: - nc_cmd = "nc" - if isinstance(self.jumphost.get("tags"), dict) and "nc_command" in self.jumphost["tags"]: - nc_cmd = self.jumphost["tags"]["nc_command"] - - if self.jumphost["protocol"] == "kubectl": - proxy_cmd = f"kubectl exec " - if self.jumphost["options"] != '': - proxy_cmd += f"{self.jumphost['options']} " - proxy_cmd += f"{self.jumphost['host']} -i -- {nc_cmd} %h %p" - else: - proxy_cmd = f"docker " - if self.jumphost["options"] != '': - proxy_cmd += f"{self.jumphost['options']} " - proxy_cmd += f"exec -i {self.jumphost['host']} {nc_cmd} %h %p" - - self.jumphost = f"-o ProxyCommand=\"{proxy_cmd}\"" + raw_cmd, jh_passwords = self._build_jumphost_chain(self.jumphost, config) + if jh_passwords: + self.password = jh_passwords + self.password + if raw_cmd: + escaped = raw_cmd.replace('\\', '\\\\').replace('"', '\\"') + self.jumphost = f'-o ProxyCommand="{escaped}"' else: self.jumphost = "" @@ -213,6 +146,127 @@ class node: self.result = {} self.cmd_byte_positions = [(0, None)] + @staticmethod + def _resolve_jumphost_data(jh_dict, config): + '''Resolve @profile references and normalize passwords in a jumphost dict.''' + for key in jh_dict: + profile = re.search("^@(.*)", str(jh_dict[key])) + if profile: + try: + jh_dict[key] = config.profiles[profile.group(1)][key] + except KeyError: + jh_dict[key] = "" + elif jh_dict[key] == '' and key == "protocol": + try: + jh_dict[key] = config.profiles["default"][key] + except KeyError: + jh_dict[key] = "ssh" + if isinstance(jh_dict["password"], list): + resolved = [] + for p in jh_dict["password"]: + profile = re.search("^@(.*)", p) + if profile: + resolved.append(config.profiles[profile.group(1)]["password"]) + else: + resolved.append(p) + jh_dict["password"] = resolved + else: + jh_dict["password"] = [jh_dict["password"]] + return jh_dict + + def _build_jumphost_chain(self, jumphost_name, config, visited=None, depth=0, target_host="%h", target_port="%p"): + '''Recursively build ProxyCommand for chained jumphosts. + + Returns: + tuple: (raw_proxy_command, passwords_list) + - raw_proxy_command: Command string to embed in ProxyCommand + - passwords_list: Ordered passwords (innermost first) + + Raises: + ValueError: On circular references or exceeding max depth (5). + ''' + if depth >= 5: + raise ValueError("Jumphost chain exceeds maximum depth of 5 hops") + if visited is None: + visited = [] + if jumphost_name in visited: + cycle = " -> ".join(visited + [jumphost_name]) + raise ValueError(f"Circular jumphost reference detected: {cycle}") + visited = visited + [jumphost_name] + + jh = config.getitem(jumphost_name) + jh = self._resolve_jumphost_data(jh, config) + + passwords = [] + inner_proxy_opt = "" + + # Recursively resolve inner jumphost + if jh.get("jumphost", "") != "": + if jh["protocol"] not in ["ssh"]: + raise ValueError( + f"Jumphost '{jumphost_name}' uses protocol '{jh['protocol']}' " + f"which does not support chained jumphosts. " + f"Only SSH jumphosts can have their own jumphosts." + ) + parent_port = jh["port"] if jh["port"] != "" else "22" + inner_raw_cmd, inner_passwords = self._build_jumphost_chain( + jh["jumphost"], config, visited, depth + 1, target_host=jh["host"], target_port=parent_port + ) + passwords = inner_passwords + escaped = inner_raw_cmd.replace('\\', '\\\\').replace('"', '\\"') + inner_proxy_opt = f'-o ProxyCommand="{escaped}"' + + # Collect this hop's passwords + if jh["password"] != [""]: + passwords = passwords + jh["password"] + + t_port = target_port if target_port != "" else "22" + + # Build raw command based on protocol + if jh["protocol"] == "ssh": + cmd = f"ssh -W {target_host}:{t_port}" + if inner_proxy_opt: + cmd += f" {inner_proxy_opt}" + if jh["port"] != '': + cmd += f" -p {jh['port']}" + if jh["options"] != '': + cmd += f" {jh['options']}" + user_host = f"{jh['user']}@{jh['host']}" if jh['user'] != '' else jh['host'] + cmd += f" {user_host}" + elif jh["protocol"] == "ssm": + ssm_target = jh["host"] + ssm_cmd = f"aws ssm start-session --target {ssm_target} --document-name AWS-StartSSHSession --parameters 'portNumber=22'" + if isinstance(jh.get("tags"), dict): + if "profile" in jh["tags"]: + ssm_cmd += f" --profile {jh['tags']['profile']}" + if "region" in jh["tags"]: + ssm_cmd += f" --region {jh['tags']['region']}" + if jh["options"] != '': + ssm_cmd += f" {jh['options']}" + bastion_user_part = f"{jh['user']}@{ssm_target}" if jh['user'] else ssm_target + ssh_opts = "" + if isinstance(jh.get("tags"), dict) and "ssh_options" in jh["tags"]: + ssh_opts = f" {jh['tags']['ssh_options']}" + cmd = f"ssh{ssh_opts} -o ProxyCommand='{ssm_cmd}' -W {target_host}:{t_port} {bastion_user_part}" + elif jh["protocol"] in ["kubectl", "docker"]: + nc_cmd = "nc" + if isinstance(jh.get("tags"), dict) and "nc_command" in jh["tags"]: + nc_cmd = jh["tags"]["nc_command"] + if jh["protocol"] == "kubectl": + cmd = "kubectl exec " + if jh["options"] != '': + cmd += f"{jh['options']} " + cmd += f"{jh['host']} -i -- {nc_cmd} {target_host} {t_port}" + else: + cmd = "docker " + if jh["options"] != '': + cmd += f"{jh['options']} " + cmd += f"exec -i {jh['host']} {nc_cmd} {target_host} {t_port}" + else: + return "", passwords + + return cmd, passwords + @MethodHook def _passtx(self, passwords, *, keyfile=None): # decrypts passwords, used by other methdos. @@ -1104,7 +1158,8 @@ class node: attempts = 1 while attempts <= max_attempts: - child = pexpect.spawn(cmd) + args = shlex.split(cmd) + child = pexpect.spawn(args[0], args[1:]) if isinstance(self.tags, dict) and self.tags.get("console"): child.sendline() if debug: diff --git a/connpy/tests/test_core.py b/connpy/tests/test_core.py index 1dad836..8913e21 100644 --- a/connpy/tests/test_core.py +++ b/connpy/tests/test_core.py @@ -487,3 +487,264 @@ class TestNodes: mynodes.run(["show version"], on_complete=on_done) assert "r1" in completed + + +# ========================================================================= +# Jumphost chain tests +# ========================================================================= + +class TestJumphostChain: + """Tests for chained jumphost (multi-hop ProxyCommand) support.""" + + def _make_config_with_nodes(self, tmp_config_dir, nodes, profiles=None): + """Helper to create a config with custom nodes.""" + import yaml + from connpy.configfile import configfile + if profiles is None: + profiles = { + "default": { + "host": "", "protocol": "ssh", "port": "", "user": "", + "password": "", "options": "", "logs": "", "tags": "", "jumphost": "" + } + } + data = { + "config": {"case": False, "idletime": 30, "fzf": False}, + "connections": nodes, + "profiles": profiles + } + config_file = tmp_config_dir / "config.yaml" + config_file.write_text(yaml.dump(data, default_flow_style=False, sort_keys=False)) + import os + os.chmod(str(config_file), 0o600) + return configfile(conf=str(config_file), key=str(tmp_config_dir / ".osk")) + + def test_single_jumphost(self, tmp_config_dir): + """Regression: single jumphost produces correct ProxyCommand.""" + from connpy.core import node + nodes = { + "bastion": { + "host": "10.0.0.1", "protocol": "ssh", "port": "2222", + "user": "admin", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "", "type": "connection" + }, + "dest": { + "host": "10.0.1.1", "protocol": "ssh", "port": "", + "user": "root", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "bastion", "type": "connection" + } + } + config = self._make_config_with_nodes(tmp_config_dir, nodes) + n = node("dest", "10.0.1.1", user="root", jumphost="bastion", config=config) + assert 'ProxyCommand=' in n.jumphost + assert '-W %h:%p' in n.jumphost + assert '-p 2222' in n.jumphost + assert 'admin@10.0.0.1' in n.jumphost + + def test_two_hop_chain(self, tmp_config_dir): + """Two-hop SSH chain: dest -> bastionA -> bastionB.""" + from connpy.core import node + nodes = { + "bastionB": { + "host": "10.0.0.1", "protocol": "ssh", "port": "", + "user": "userB", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "", "type": "connection" + }, + "bastionA": { + "host": "10.0.0.2", "protocol": "ssh", "port": "", + "user": "userA", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "bastionB", "type": "connection" + }, + "dest": { + "host": "10.0.1.1", "protocol": "ssh", "port": "", + "user": "root", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "bastionA", "type": "connection" + } + } + config = self._make_config_with_nodes(tmp_config_dir, nodes) + n = node("dest", "10.0.1.1", user="root", jumphost="bastionA", config=config) + # Should contain nested ProxyCommand + assert 'ProxyCommand=' in n.jumphost + assert 'userA@10.0.0.2' in n.jumphost + assert 'userB@10.0.0.1' in n.jumphost + # Inner proxy should be escaped + assert 'ProxyCommand=\\"' in n.jumphost or 'ProxyCommand=\\\\' in n.jumphost + + def test_three_hop_chain(self, tmp_config_dir): + """Three-hop SSH chain: dest -> A -> B -> C.""" + from connpy.core import node + nodes = { + "hopC": { + "host": "10.0.0.3", "protocol": "ssh", "port": "", + "user": "uc", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "", "type": "connection" + }, + "hopB": { + "host": "10.0.0.2", "protocol": "ssh", "port": "", + "user": "ub", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "hopC", "type": "connection" + }, + "hopA": { + "host": "10.0.0.1", "protocol": "ssh", "port": "", + "user": "ua", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "hopB", "type": "connection" + }, + "dest": { + "host": "10.0.1.1", "protocol": "ssh", "port": "", + "user": "root", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "hopA", "type": "connection" + } + } + config = self._make_config_with_nodes(tmp_config_dir, nodes) + n = node("dest", "10.0.1.1", user="root", jumphost="hopA", config=config) + # All three hosts should appear in the command + assert 'uc@10.0.0.3' in n.jumphost + assert 'ub@10.0.0.2' in n.jumphost + assert 'ua@10.0.0.1' in n.jumphost + + def test_circular_detection(self, tmp_config_dir): + """Circular jumphost reference raises ValueError.""" + from connpy.core import node + nodes = { + "hopA": { + "host": "10.0.0.1", "protocol": "ssh", "port": "", + "user": "", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "hopB", "type": "connection" + }, + "hopB": { + "host": "10.0.0.2", "protocol": "ssh", "port": "", + "user": "", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "hopA", "type": "connection" + }, + "dest": { + "host": "10.0.1.1", "protocol": "ssh", "port": "", + "user": "", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "hopA", "type": "connection" + } + } + config = self._make_config_with_nodes(tmp_config_dir, nodes) + with pytest.raises(ValueError, match="Circular jumphost reference"): + node("dest", "10.0.1.1", jumphost="hopA", config=config) + + def test_max_depth(self, tmp_config_dir): + """Chain exceeding 5 hops raises ValueError.""" + from connpy.core import node + nodes = {} + # Build chain of 6 hops: hop0 -> hop1 -> ... -> hop5 + for i in range(6): + jh = f"hop{i+1}" if i < 5 else "" + nodes[f"hop{i}"] = { + "host": f"10.0.0.{i}", "protocol": "ssh", "port": "", + "user": "", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": jh, "type": "connection" + } + nodes["dest"] = { + "host": "10.0.1.1", "protocol": "ssh", "port": "", + "user": "", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "hop0", "type": "connection" + } + config = self._make_config_with_nodes(tmp_config_dir, nodes) + with pytest.raises(ValueError, match="maximum depth of 5"): + node("dest", "10.0.1.1", jumphost="hop0", config=config) + + def test_kubectl_with_jumphost_error(self, tmp_config_dir): + """kubectl jumphost with its own jumphost raises ValueError.""" + from connpy.core import node + nodes = { + "sshhost": { + "host": "10.0.0.1", "protocol": "ssh", "port": "", + "user": "", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "", "type": "connection" + }, + "kubejump": { + "host": "my-pod", "protocol": "kubectl", "port": "", + "user": "", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "sshhost", "type": "connection" + }, + "dest": { + "host": "10.0.1.1", "protocol": "ssh", "port": "", + "user": "", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "kubejump", "type": "connection" + } + } + config = self._make_config_with_nodes(tmp_config_dir, nodes) + with pytest.raises(ValueError, match="does not support chained jumphosts"): + node("dest", "10.0.1.1", jumphost="kubejump", config=config) + + def test_password_chain_order(self, tmp_config_dir): + """Passwords are collected innermost-first: B, A, dest.""" + from connpy.core import node + nodes = { + "bastionB": { + "host": "10.0.0.1", "protocol": "ssh", "port": "", + "user": "ub", "password": "passB", "options": "", + "logs": "", "tags": "", "jumphost": "", "type": "connection" + }, + "bastionA": { + "host": "10.0.0.2", "protocol": "ssh", "port": "", + "user": "ua", "password": "passA", "options": "", + "logs": "", "tags": "", "jumphost": "bastionB", "type": "connection" + }, + "dest": { + "host": "10.0.1.1", "protocol": "ssh", "port": "", + "user": "root", "password": "passDest", "options": "", + "logs": "", "tags": "", "jumphost": "bastionA", "type": "connection" + } + } + config = self._make_config_with_nodes(tmp_config_dir, nodes) + n = node("dest", "10.0.1.1", user="root", password="passDest", + jumphost="bastionA", config=config) + # Order: innermost (B) -> outer (A) -> destination + assert n.password == ["passB", "passA", "passDest"] + + def test_chain_with_options_and_port(self, tmp_config_dir): + """Options and ports are preserved for each hop in the chain.""" + from connpy.core import node + nodes = { + "bastionB": { + "host": "10.0.0.1", "protocol": "ssh", "port": "2222", + "user": "ub", "password": "", "options": "-o StrictHostKeyChecking=no", + "logs": "", "tags": "", "jumphost": "", "type": "connection" + }, + "bastionA": { + "host": "10.0.0.2", "protocol": "ssh", "port": "3333", + "user": "ua", "password": "", "options": "-i /tmp/key.pem", + "logs": "", "tags": "", "jumphost": "bastionB", "type": "connection" + }, + "dest": { + "host": "10.0.1.1", "protocol": "ssh", "port": "", + "user": "root", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "bastionA", "type": "connection" + } + } + config = self._make_config_with_nodes(tmp_config_dir, nodes) + n = node("dest", "10.0.1.1", user="root", jumphost="bastionA", config=config) + assert '-p 2222' in n.jumphost + assert '-p 3333' in n.jumphost + assert 'StrictHostKeyChecking=no' in n.jumphost + assert '/tmp/key.pem' in n.jumphost + + def test_chain_with_ssm_inner(self, tmp_config_dir): + """SSH outer jumphost with SSM inner jumphost works.""" + from connpy.core import node + nodes = { + "ssm_bastion": { + "host": "i-12345", "protocol": "ssm", "port": "", + "user": "ec2-user", "password": "", "options": "", + "logs": "", "tags": {"region": "us-east-1"}, "jumphost": "", "type": "connection" + }, + "ssh_jump": { + "host": "10.0.0.2", "protocol": "ssh", "port": "", + "user": "admin", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "ssm_bastion", "type": "connection" + }, + "dest": { + "host": "10.0.1.1", "protocol": "ssh", "port": "", + "user": "root", "password": "", "options": "", + "logs": "", "tags": "", "jumphost": "ssh_jump", "type": "connection" + } + } + config = self._make_config_with_nodes(tmp_config_dir, nodes) + n = node("dest", "10.0.1.1", user="root", jumphost="ssh_jump", config=config) + assert 'aws ssm start-session' in n.jumphost + assert 'admin@10.0.0.2' in n.jumphost + assert '--region us-east-1' in n.jumphost