diff --git a/src/python/jw/pkg/lib/ec/ssh/Exec.py b/src/python/jw/pkg/lib/ec/ssh/Exec.py index 4e169551..d9b74970 100644 --- a/src/python/jw/pkg/lib/ec/ssh/Exec.py +++ b/src/python/jw/pkg/lib/ec/ssh/Exec.py @@ -20,16 +20,24 @@ class Exec(Base): super().__init__(uri = uri, caps = self.Caps.ModEnv, **kwargs) def __del__(self) -> None: + # -- This method may be called more than once, so it must be + # idempotent and must never raise for key, val in self.__askpass_orig.items(): if val is None: - del os.environ[key] + os.environ.pop(key, None) else: os.environ[key] = val + self.__askpass_orig = dict() if self.__askpass is not None: - os.remove(self.__askpass) + try: + os.remove(self.__askpass) + except OSError: + pass + self.__askpass = None def __init_askpass(self) -> None: if self.__askpass is None and self.password is not None: + import base64 import sys import tempfile @@ -39,7 +47,11 @@ class Exec(Base): ) os.chmod(f.name, 0o0700) self.__askpass = f.name - f.write(f'#!/bin/bash\n\necho -n "{self.password}\n"') + # -- Embed the password as base64, so that quotes and shell + # metacharacters cannot break the script or inject commands + # into it, and no trailing newline is appended + b64 = base64.b64encode(self.password.encode()).decode() + f.write(f'#!/bin/bash\n\nprintf %s {b64} | base64 -d\n') f.close() for key, val in { 'SSH_ASKPASS': self.__askpass, diff --git a/test/unit/python/jw/pkg/lib/ec/Makefile b/test/unit/python/jw/pkg/lib/ec/Makefile new file mode 100644 index 00000000..c87020cb --- /dev/null +++ b/test/unit/python/jw/pkg/lib/ec/Makefile @@ -0,0 +1,4 @@ +TOPDIR = ../../../../../../.. + +include $(TOPDIR)/make/proj.mk +include $(JWBDIR)/make/dirs.mk diff --git a/test/unit/python/jw/pkg/lib/ec/ssh/Exec/Makefile b/test/unit/python/jw/pkg/lib/ec/ssh/Exec/Makefile new file mode 100644 index 00000000..d949ac97 --- /dev/null +++ b/test/unit/python/jw/pkg/lib/ec/ssh/Exec/Makefile @@ -0,0 +1,7 @@ +TOPDIR = ../../../../../../../../.. + +include $(TOPDIR)/make/proj.mk +include $(JWBDIR)/make/py-run.mk + +all: +test: run diff --git a/test/unit/python/jw/pkg/lib/ec/ssh/Exec/test.py b/test/unit/python/jw/pkg/lib/ec/ssh/Exec/test.py new file mode 100644 index 00000000..4594b002 --- /dev/null +++ b/test/unit/python/jw/pkg/lib/ec/ssh/Exec/test.py @@ -0,0 +1,32 @@ +import subprocess + +from jw.pkg.lib.ec.ssh.Exec import Exec +from jw.pkg.lib.Uri import Uri + +def askpass_output(password: str) -> bytes: + uri = Uri('ssh://host') + uri.set_password(password) + ec = Exec(uri = uri) + try: + ec._Exec__init_askpass() # type: ignore[attr-defined] + script = ec._Exec__askpass # type: ignore[attr-defined] + assert script is not None + out = subprocess.run(['bash', script], capture_output = True) + assert out.returncode == 0, out.stderr + return out.stdout + finally: + # -- Restore the environment and remove the script file + ec.__del__() + +# A plain password comes out byte-exact, without a trailing newline +assert askpass_output('secret') == b'secret' + +# Quotes, dollar signs, and backticks cannot break the script or +# inject commands into it +tricky = 'pa"ss$word `tick` $(dollar-paren) \'single\' \\backslash\\' +assert askpass_output(tricky) == tricky.encode() + +# A newline inside the password is preserved, and no extra one is added +assert askpass_output('line1\nline2') == b'line1\nline2' + +print('All ssh Exec askpass tests passed') diff --git a/test/unit/python/jw/pkg/lib/ec/ssh/Makefile b/test/unit/python/jw/pkg/lib/ec/ssh/Makefile new file mode 100644 index 00000000..3b96da8e --- /dev/null +++ b/test/unit/python/jw/pkg/lib/ec/ssh/Makefile @@ -0,0 +1,4 @@ +TOPDIR = ../../../../../../../.. + +include $(TOPDIR)/make/proj.mk +include $(JWBDIR)/make/dirs.mk