fix: run privileged commands on the exec channel, never typed into the shell
CI / test (3.10) (push) Successful in 56s
CI / test (3.11) (push) Successful in 50s
CI / test (3.12) (push) Successful in 44s
CI / test (3.10) (pull_request) Successful in 42s
CI / test (3.11) (pull_request) Successful in 44s
CI / test (3.12) (pull_request) Successful in 53s
CI / test (3.10) (push) Successful in 56s
CI / test (3.11) (push) Successful in 50s
CI / test (3.12) (push) Successful in 44s
CI / test (3.10) (pull_request) Successful in 42s
CI / test (3.11) (pull_request) Successful in 44s
CI / test (3.12) (pull_request) Successful in 53s
_sudo typed `echo '<password>' | sudo -S -p "" <command>` into netmiko's interactive shell. bash kept it in its history, so every poll wrote the sudo password into the login user's ~/.bash_history, and while the command ran it was in the process list for every local user (NetOrk/netork#1039). _sudo, _sudo_status and _run_privileged with a password now run on an exec channel like run_command(privileged=True): the password reaches sudo -S on stdin, there is no PTY to echo it, and the script starts with `exec </dev/null` so a non-prompting sudo leaves it unread. The whole command runs as root in one sh -c - before, only the part up to the first `;`, `&&` or `|` did. The exit status comes from the channel. The interactive session switches its history off before its first command. get_vpn_tunnels with a password reads `wg show all dump` from the exec channel instead of a temp file that held the private keys.
This commit is contained in:
1 parent
7b37366a44
commit
cf8ee38a90
5 files changed
+440
-51
No files matched your search
+98
-24
@@ -4,7 +4,7 @@ import re
|
||||
|
||||
import pytest
|
||||
from unittest.mock import MagicMock, patch
|
||||
from napalm_linux.linux import LinuxDriver, _arm_vendor_from_model
|
||||
from napalm_linux.linux import LinuxDriver, _arm_vendor_from_model, _split_status
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -31,9 +31,75 @@ def driver():
|
||||
d._sudo_password = None
|
||||
d.netmiko_optional_args = {}
|
||||
d._device = MagicMock()
|
||||
d._device.remote_conn_pre.get_transport.return_value = _ShellAnswers(d)
|
||||
return d
|
||||
|
||||
|
||||
_SCRIPTED_RC = re.compile(r"\n?^__NETORK_RC=(-?\d+)\s*$", re.MULTILINE)
|
||||
|
||||
|
||||
class _ShellAnswers:
|
||||
"""The SSH transport of the fixture: an exec channel answers what ``_send``
|
||||
answers for the same command line, so a test that scripts the shell's
|
||||
output scripts ``_sudo``'s too (it runs on an exec channel since
|
||||
NetOrk/netork#1039). It refuses a line holding the sudo password.
|
||||
|
||||
The exit status is 0, or what a scripted ``__NETORK_RC=<n>`` line says when
|
||||
the command did not echo that marker itself (``_with_rc``); that line is
|
||||
then no output. ``test_sudo_off_the_command_line`` covers the channel itself."""
|
||||
|
||||
def __init__(self, driver):
|
||||
self._driver = driver
|
||||
|
||||
def open_session(self):
|
||||
return _AnsweringChannel(self._driver)
|
||||
|
||||
|
||||
class _AnsweringChannel:
|
||||
def __init__(self, driver):
|
||||
self._driver = driver
|
||||
self._out = b""
|
||||
self._status = 0
|
||||
|
||||
def exec_command(self, command):
|
||||
password = self._driver._sudo_password
|
||||
assert not password or password not in command, f"sudo password in {command!r}"
|
||||
output = self._driver._send(command)
|
||||
marker = _SCRIPTED_RC.search(output)
|
||||
if marker and "echo __NETORK_RC" not in command:
|
||||
self._status = int(marker.group(1))
|
||||
output = _SCRIPTED_RC.sub("", output)
|
||||
self._out = output.encode()
|
||||
|
||||
def settimeout(self, timeout):
|
||||
pass
|
||||
|
||||
def sendall(self, data):
|
||||
pass
|
||||
|
||||
def shutdown_write(self):
|
||||
pass
|
||||
|
||||
def close(self):
|
||||
pass
|
||||
|
||||
def recv_ready(self):
|
||||
return bool(self._out)
|
||||
|
||||
def recv(self, size):
|
||||
chunk, self._out = self._out[:size], self._out[size:]
|
||||
return chunk
|
||||
|
||||
def recv_stderr_ready(self):
|
||||
return False
|
||||
|
||||
def exit_status_ready(self):
|
||||
return not self._out
|
||||
|
||||
def recv_exit_status(self):
|
||||
return self._status
|
||||
|
||||
|
||||
def _mock_send(driver_fixture, output: str):
|
||||
"""Patch _send to return *output* for any command."""
|
||||
driver_fixture._device.send_command.return_value = output
|
||||
@@ -915,9 +981,10 @@ class TestUninstallPackage:
|
||||
def test_a_half_configured_package_falls_back_to_dpkg(self, driver):
|
||||
"""`install ok unpacked` is the state apt cannot get out of. On one host
|
||||
only `dpkg --purge --force-all` removed it."""
|
||||
driver._root = False
|
||||
driver._device.send_command.side_effect = [
|
||||
"E: Sub-process /usr/bin/dpkg returned an error code (1)",
|
||||
"Removing wazuh-agent (4.14.7-1) ...",
|
||||
_with_rc("E: Sub-process /usr/bin/dpkg returned an error code (1)", 100),
|
||||
_with_rc("Removing wazuh-agent (4.14.7-1) ...", 0),
|
||||
]
|
||||
|
||||
result = driver.uninstall_package("wazuh-agent", purge=True)
|
||||
@@ -929,6 +996,7 @@ class TestUninstallPackage:
|
||||
def test_the_fallback_is_not_tried_when_the_first_pass_worked(self, driver):
|
||||
"""A forced dpkg purge is a bigger hammer than apt and must stay a last
|
||||
resort, not a routine second step."""
|
||||
driver._root = False
|
||||
_mock_send(driver, "Removing wazuh-agent ...")
|
||||
|
||||
driver.uninstall_package("wazuh-agent", purge=True)
|
||||
@@ -973,23 +1041,25 @@ class TestSudoStatus:
|
||||
|
||||
assert driver._sudo_status("apt-get remove -y nope")[1] == 100
|
||||
|
||||
def test_the_status_is_read_right_after_sudo_returns(self, driver):
|
||||
"""``$?`` must be read straight after the sudo pipeline — with an
|
||||
``|| true`` in between, every command would report 0."""
|
||||
driver._sudo_password = "pw" # noqa: S105
|
||||
def test_the_command_keeps_its_own_exit_status(self, driver):
|
||||
"""The exec channel reports the status (#1039): no ``|| true`` and no
|
||||
marker in between, and no password in the line."""
|
||||
driver._root = False
|
||||
driver._sudo_password = "s3cret" # noqa: S105
|
||||
_mock_send(driver, _with_rc("", 0))
|
||||
|
||||
driver._sudo_status("apt-get remove -y x 2>&1")
|
||||
|
||||
sent = driver._device.send_command.call_args[0][0]
|
||||
assert sent.startswith("echo pw | sudo -S")
|
||||
assert sent.endswith("apt-get remove -y x 2>&1; echo __NETORK_RC=$?")
|
||||
assert sent.startswith("sudo -S -p '' sh -c ")
|
||||
assert "apt-get remove -y x 2>&1" in sent
|
||||
assert "|| true" not in sent
|
||||
assert "__NETORK_RC" not in sent
|
||||
|
||||
def test_a_missing_marker_means_unknown_not_success(self, driver):
|
||||
"""Output cut short before the marker arrived says nothing about the
|
||||
exit status; ``None`` says so instead of guessing 0."""
|
||||
_mock_send(driver, "Removing wazuh-agent ...")
|
||||
def test_a_channel_closed_without_a_status_means_unknown_not_success(self, driver):
|
||||
"""paramiko's -1 says nothing about the exit status; ``None`` says so
|
||||
instead of guessing 0."""
|
||||
_mock_send(driver, _with_rc("Removing wazuh-agent ...", -1))
|
||||
|
||||
assert driver._sudo_status("apt-get remove -y wazuh-agent") == (
|
||||
"Removing wazuh-agent ...",
|
||||
@@ -1042,6 +1112,9 @@ _BADSIG_CHUNKS = [
|
||||
]
|
||||
|
||||
|
||||
_TRUE_WITH_STATUS = "true; echo __NETORK_RC=$?"
|
||||
|
||||
|
||||
class TestReadToTheEnd:
|
||||
"""A line the command prints can end in ``>``, ``#`` or ``$`` -- apt's
|
||||
``<ftpmaster@ubuntu.com>`` after a bad signature. Taken for the prompt, it
|
||||
@@ -1064,7 +1137,7 @@ class TestReadToTheEnd:
|
||||
driver._device = _Channel(_BADSIG_CHUNKS + ["true\n", "__NETORK_RC=0\nchris@vault-01:~$ "])
|
||||
|
||||
driver.refresh_available_updates()
|
||||
output, status = driver._sudo_status("true")
|
||||
output, status = _split_status(driver._send(_TRUE_WITH_STATUS))
|
||||
|
||||
assert status == 0
|
||||
assert "BADSIG" not in output
|
||||
@@ -1074,7 +1147,7 @@ class TestReadToTheEnd:
|
||||
channel = _Channel(["sudo true; echo __NETORK_RC=$?\n", "__NETORK_RC=0\nchris@vault-01:~$ "])
|
||||
driver._device = channel
|
||||
|
||||
assert driver._sudo_status("true")[1] == 0
|
||||
assert _split_status(driver._send(_TRUE_WITH_STATUS))[1] == 0
|
||||
assert channel.chunks == []
|
||||
|
||||
def test_the_marker_alone_is_not_the_end(self, driver):
|
||||
@@ -1082,7 +1155,7 @@ class TestReadToTheEnd:
|
||||
channel = _Channel(["out\n__NETORK_RC=0\n", "chris@vault-01:~$ "])
|
||||
driver._device = channel
|
||||
|
||||
driver._sudo_status("true")
|
||||
driver._send(_TRUE_WITH_STATUS)
|
||||
|
||||
assert channel.chunks == []
|
||||
|
||||
@@ -1158,10 +1231,10 @@ class TestUninstallExitStatus:
|
||||
driver.uninstall_package("wazuh-agent")
|
||||
|
||||
sent = driver._device.send_command.call_args[0][0]
|
||||
assert "|| true" not in sent
|
||||
assert sent.endswith("; echo __NETORK_RC=$?")
|
||||
assert "|| true" not in sent # the exec channel reports the status (#1039)
|
||||
|
||||
def test_apt_failing_by_exit_status_falls_back_to_dpkg(self, driver):
|
||||
driver._root = False
|
||||
driver._device.send_command.side_effect = [
|
||||
_with_rc("E: Sub-process /usr/bin/dpkg returned an error code (1)", 100),
|
||||
_with_rc("Removing wazuh-agent (4.14.7-1) ...", 0),
|
||||
@@ -1176,6 +1249,7 @@ class TestUninstallExitStatus:
|
||||
assert "__NETORK_RC" not in result["output"]
|
||||
|
||||
def test_the_dpkg_fallback_failing_is_a_failure(self, driver):
|
||||
driver._root = False
|
||||
driver._device.send_command.side_effect = [
|
||||
_with_rc("E: Sub-process /usr/bin/dpkg returned an error code (1)", 100),
|
||||
_with_rc("dpkg: error processing package wazuh-agent (--purge):", 1),
|
||||
@@ -1190,6 +1264,7 @@ class TestUninstallExitStatus:
|
||||
"""Even when the output contains words that used to mean failure: apt
|
||||
exits 0 for a package that is already gone, which is the state the
|
||||
caller asked for."""
|
||||
driver._root = False
|
||||
_mock_send(driver, _with_rc("Package 'x' is not installed, so not removed", 0))
|
||||
|
||||
result = driver.uninstall_package("x", purge=True)
|
||||
@@ -1198,10 +1273,10 @@ class TestUninstallExitStatus:
|
||||
assert driver._device.send_command.call_count == 1
|
||||
|
||||
def test_without_an_exit_status_the_output_is_read_as_before(self, driver):
|
||||
"""If the marker never arrived, the keyword check is still the best
|
||||
answer available — and it errs towards failure on apt's ``E:``."""
|
||||
"""If the channel closed without a status, the keyword check is still the
|
||||
best answer available — and it errs towards failure on apt's ``E:``."""
|
||||
driver._pkg_manager = "dnf"
|
||||
_mock_send(driver, "E: Sub-process /usr/bin/dpkg returned an error code (1)")
|
||||
_mock_send(driver, _with_rc("E: Sub-process /usr/bin/dpkg returned an error code (1)", -1))
|
||||
|
||||
result = driver.uninstall_package("wazuh-agent")
|
||||
|
||||
@@ -1295,7 +1370,7 @@ class TestManageService:
|
||||
|
||||
assert driver.manage_service("cron", "stop")["success"] is True
|
||||
action = self._sent(driver)[1]
|
||||
assert action.startswith("echo pw | sudo -S")
|
||||
assert action.startswith("sudo -S -p '' sh -c ") # the password on stdin (#1039)
|
||||
assert "timeout 45 systemctl --no-ask-password stop -- cron.service" in action
|
||||
|
||||
def test_without_one_sudo_never_waits_for_a_password(self, driver):
|
||||
@@ -1562,8 +1637,7 @@ class TestGetListeningSockets:
|
||||
driver.get_listening_sockets()
|
||||
|
||||
sent = self._sent(driver)[0]
|
||||
assert sent.startswith("echo pw | sudo -S")
|
||||
assert "sh -c '" in sent
|
||||
assert sent.startswith("sudo -S -p '' sh -c ") # the password on stdin (#1039)
|
||||
|
||||
def test_when_sudo_refuses_it_reads_what_the_user_may_see(self, driver):
|
||||
driver._root = False
|
||||
|
||||
Reference in new issue
Block a user