Skip to content

Commit df81db9

Browse files
aerisaeris
committed
Better call to subprocess, avoid raw bash command
1 parent bdeddc9 commit df81db9

1 file changed

Lines changed: 83 additions & 83 deletions

File tree

‎ssh-ident‎

Lines changed: 83 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,14 @@ to solve the problem:
122122
rsync -e '/path/to/ssh-ident' ...
123123
scp -S '/path/to/ssh-ident' ...
124124
125-
4) Replace the real ssh on the system with ssh-ident, and set the
125+
4) Replace the real ssh on the system with ssh-ident, and set the
126126
BINARY_SSH configuration parameter to the original value.
127127
128128
On Debian based system, you can make this change in a way that
129129
will survive automated upgrades and audits by running:
130130
131131
dpkg-divert --divert /usr/bin/ssh.ssh-ident --rename /usr/bin/ssh
132-
132+
133133
After which, you will need to use:
134134
135135
BINARY_SSH="/usr/bin/ssh.ssh-ident"
@@ -177,10 +177,10 @@ To have multiple identities, all I have to do is:
177177
SSH_ADD_OPTIONS = {
178178
# Regardless, ask for confirmation before using any of the
179179
# work keys.
180-
"work": "-c",
180+
"work": ["-c"],
181181
# Forget about secret keys after ten minutes. ssh-ident will
182182
# automatically ask you your passphrase again if they are needed.
183-
"secret": "-t 600",
183+
"secret": ["-t", "600"],
184184
}
185185
186186
# This is option - dont' include any SSH_OPTIONS if you don't
@@ -190,21 +190,21 @@ To have multiple identities, all I have to do is:
190190
"SSH_OPTIONS": {
191191
# Disable forwarding of the agent, but enable X forwarding,
192192
# when using the work profile.
193-
"work": "-Xa",
193+
"work": ["-Xa"],
194194
195195
# Always forward the agent when using the secret identity.
196-
"secret": "-A"
196+
"secret": ["-A"]
197197
},
198198
199199
# Options to pass to ssh by default.
200200
# If you don't specify anything, UserRoaming=no is passed, due
201201
# to CVE-2016-0777. Leave it empty to disable this.
202-
"SSH_DEFAULT_OPTIONS": "-oUseRoaming=no",
202+
"SSH_DEFAULT_OPTIONS": ["-oUseRoaming=no"],
203203
204204
# Which options to use by default if no match with SSH_ADD_OPTIONS
205205
# was found. Note that ssh-ident hard codes -t 7200 to prevent your
206206
# keys from remaining in memory for too long.
207-
SSH_ADD_DEFAULT_OPTIONS = "-t 7200"
207+
SSH_ADD_DEFAULT_OPTIONS = ["-t", "7200"]
208208
209209
# Output verbosity
210210
# valid values are: LOG_ERROR, LOG_WARN, LOG_INFO, LOG_DEBUG
@@ -417,7 +417,7 @@ class Config(object):
417417
# the specified options to the ssh command run.
418418
"SSH_OPTIONS": {},
419419
# Additional options to append to ssh by default.
420-
"SSH_DEFAULT_OPTIONS": "-oUseRoaming=no",
420+
"SSH_DEFAULT_OPTIONS": ["-oUseRoaming=no"],
421421

422422
# Complete path of full ssh binary to use. If not set, ssh-ident will
423423
# try to find the correct binary in PATH.
@@ -438,7 +438,7 @@ class Config(object):
438438
"SSH_ADD_OPTIONS": {},
439439
# ssh-add default options. By default, don't keep a key longer
440440
# than 2 hours.
441-
"SSH_ADD_DEFAULT_OPTIONS": "-t 7200",
441+
"SSH_ADD_DEFAULT_OPTIONS": ["-t", "7200"],
442442

443443
# Like BatchMode in ssh, see man 5 ssh_config.
444444
# In BatchMode ssh-ident will not print any output and not ask for
@@ -653,7 +653,7 @@ class AgentManager(object):
653653
file=sys.stderr, loglevel=LOG_INFO)
654654
self.LoadKeyFiles(toload)
655655
else:
656-
print("All keys already loaded", file=sys.stderr, loglevel=LOG_INFO)
656+
print("All keys already loaded", file=sys.stderr, loglevel=LOG_DEBUG)
657657

658658
def FindUnloadedKeys(self, keys):
659659
"""Determines which keys have not been loaded yet.
@@ -685,40 +685,38 @@ class AgentManager(object):
685685
Args:
686686
keys: iterable of strings, each string a path to a key to load.
687687
"""
688-
keys = " ".join(keys)
689688
options = self.config.Get("SSH_ADD_OPTIONS").get(
690689
self.identity, self.config.Get("SSH_ADD_DEFAULT_OPTIONS"))
691-
self.RunShellCommandInAgent(
692-
self.agent_file, "ssh-add {0} {1}".format(options, keys))
690+
try:
691+
self.RunShellCommandInAgent(self.agent_file, ["ssh-add"] + options + list(keys), disable_io=True)
692+
except subprocess.CalledProcessError:
693+
print("Error occurs during keys loading, exiting...", file=sys.stderr)
694+
exit(-1)
693695

694696
def GetLoadedKeys(self):
695697
"""Returns an iterable of strings, each the fingerprint of a loaded key."""
696-
retval, stdout = self.RunShellCommandInAgent(self.agent_file, "ssh-add -l")
697-
if retval != 0:
698+
try:
699+
stdout = self.RunShellCommandInAgent(self.agent_file, ["ssh-add", "-l"])
700+
fingerprints = []
701+
for line in stdout.decode("utf-8").split("\n"):
702+
try:
703+
_, fingerprint, _ = line.split(" ", 2)
704+
fingerprints.append(fingerprint)
705+
except ValueError:
706+
continue
707+
return fingerprints
708+
except subprocess.CalledProcessError:
698709
return []
699710

700-
fingerprints = []
701-
for line in stdout.decode("utf-8").split("\n"):
702-
try:
703-
_, fingerprint, _ = line.split(" ", 2)
704-
fingerprints.append(fingerprint)
705-
except ValueError:
706-
continue
707-
return fingerprints
708-
709711
@staticmethod
710712
def GetPublicKeyFingerprint(key):
711713
"""Returns the fingerprint of a public key as a string."""
712-
retval, stdout = AgentManager.RunShellCommand(
713-
"ssh-keygen -l -f {0} |tr -s ' '".format(key))
714-
if retval:
715-
return None
716-
717714
try:
715+
stdout = AgentManager.RunShellCommand(["ssh-keygen", "-l", "-f", key])
718716
_, fingerprint, _ = stdout.decode("utf-8").split(" ", 2)
719-
except ValueError:
717+
return fingerprint
718+
except subprocess.CalledProcessError, ValueError:
720719
return None
721-
return fingerprint
722720

723721
@staticmethod
724722
def GetAgentFile(path, identity):
@@ -750,66 +748,73 @@ class AgentManager(object):
750748

751749
print("Preparing new agent for identity {0}".format(identity), file=sys.stderr,
752750
loglevel=LOG_DEBUG)
753-
retval = subprocess.call(
754-
["/usr/bin/env", "-i", "/bin/sh", "-c", "ssh-agent > {0}".format(agentfile)])
751+
agentenv = AgentManager.RunShellCommand(["ssh-agent"])
752+
with open(agentfile, 'w') as env:
753+
env.write(agentenv)
755754
return agentfile
756755

757756
@staticmethod
758757
def IsAgentFileValid(agentfile):
759758
"""Returns true if the specified agentfile refers to a running agent."""
760-
retval, output = AgentManager.RunShellCommandInAgent(
761-
agentfile, "ssh-add -l >/dev/null 2>/dev/null")
762-
if retval & 0xff not in [0, 1]:
763-
print("Agent in {0} not running".format(agentfile), file=sys.stderr,
764-
loglevel=LOG_DEBUG)
759+
try:
760+
AgentManager.RunShellCommandInAgent(agentfile, ["ssh-add", "-l"])
761+
return True
762+
except subprocess.CalledProcessError:
765763
return False
766-
return True
767764

768765
@staticmethod
769-
def RunShellCommand(command):
770-
"""Runs a shell command, returns (status, stdout), (int, string)."""
771-
command = ["/bin/sh", "-c", command]
772-
process = subprocess.Popen(command, stdout=subprocess.PIPE)
773-
stdout, stderr = process.communicate()
774-
return process.wait(), stdout
766+
def RunShellCommand(command, disable_io=False, **kwargs):
767+
"""Runs a shell command, returns (stdout), (string)."""
768+
for io in ["stdin", "stdout", "stderr"]:
769+
kwargs[io] = subprocess.PIPE
770+
if disable_io:
771+
kwargs["preexec_fn"] = os.setsid
772+
subprocess.check_call(command, **kwargs)
773+
return None
774+
else:
775+
process = subprocess.Popen(command, **kwargs)
776+
stdout, stderr = process.communicate()
777+
retr = process.returncode
778+
if retr != 0:
779+
raise subprocess.CalledProcessError(retr, command, stderr)
780+
return stdout
781+
return stdout
775782

776783
@staticmethod
777-
def RunShellCommandInAgent(agentfile, command):
778-
"""Runs a shell command with an agent configured in the environment."""
779-
command = ["/bin/sh", "-c",
780-
". {0} >/dev/null 2>/dev/null; {1}".format(agentfile, command)]
781-
process = subprocess.Popen(command, stdout=subprocess.PIPE)
782-
stdout, stderr = process.communicate()
783-
return process.wait(), stdout
784+
def GetAgentEnv(agentfile):
785+
env = {}
786+
with open(agentfile, 'r') as agentenv:
787+
for line in agentenv:
788+
line = line.split(";")[0]
789+
line = line.split("=")
790+
if len(line) == 2:
791+
name, value = line
792+
env[name] = value
793+
for e in ["TERM"]:
794+
if e in os.environ:
795+
env[e] = os.environ[e]
796+
return env
784797

785798
@staticmethod
786-
def EscapeShellArguments(argv):
787-
"""Escapes all arguments to the shell, returns a string."""
788-
escaped = []
789-
for arg in argv:
790-
escaped.append("'{0}'".format(arg.replace("'", "'\"'\"'")))
791-
return " ".join(escaped)
792-
793-
def GetShellArgs(self):
794-
"""Returns the flags to be passed to the shell to run a command."""
795-
shell_args = "-c"
796-
if ShouldPrint(self.config, LOG_DEBUG):
797-
shell_args = "-xc"
798-
return shell_args
799+
def RunShellCommandInAgent(agentfile, command, disable_io=False, **kwargs):
800+
"""Runs a shell command with an agent configured in the environment."""
801+
env = AgentManager.GetAgentEnv(agentfile)
802+
for e in ["SSH_ASKPASS", "DISPLAY", "XDG_CURRENT_DESKTOP", "KDE_SESSION_VERSION", "HOME"]:
803+
if e in os.environ:
804+
env[e] = os.environ[e]
805+
kwargs["env"] = env
806+
return AgentManager.RunShellCommand(command, disable_io=disable_io, **kwargs)
799807

800808
def RunSSH(self, argv):
801809
"""Execs ssh with the specified arguments."""
802810
additional_flags = self.config.Get("SSH_OPTIONS").get(
803811
self.identity, self.config.Get("SSH_DEFAULT_OPTIONS"))
804812
if (self.ssh_config):
805-
additional_flags += " -F {0}".format(self.ssh_config)
806-
807-
command = [
808-
"/bin/sh", self.GetShellArgs(),
809-
". {0} >/dev/null 2>/dev/null; exec {1} {2} {3}".format(
810-
self.agent_file, self.config.Get("BINARY_SSH"),
811-
additional_flags, self.EscapeShellArguments(argv))]
812-
os.execv("/bin/sh", command)
813+
additional_flags += ["-F"] + self.ssh_config
814+
env = AgentManager.GetAgentEnv(self.agent_file)
815+
cmd = self.config.Get("BINARY_SSH")
816+
args = additional_flags + argv
817+
os.execve(cmd, args, env)
813818

814819
def AutodetectBinary(argv, config):
815820
"""Detects the correct binary to run and sets BINARY_SSH accordingly,
@@ -845,7 +850,7 @@ def AutodetectBinary(argv, config):
845850
# The logic here is pretty straightforward:
846851
# - Try to eliminate the path of ssh-ident from PATH.
847852
# - Search for a binary with the same name of ssh-ident to run.
848-
#
853+
#
849854
# If this fails, we may end up in some sort of loop, where ssh-ident
850855
# tries to run itself. This should normally be detected later on,
851856
# where the code checks for the next binary to run.
@@ -897,7 +902,7 @@ def AutodetectBinary(argv, config):
897902
ssh-ident was invoked in place of the binary {0} (determined from argv[0]).
898903
Neither this binary nor 'ssh' could be found in $PATH.
899904
900-
PATH="{1}"
905+
PATH="{1}"
901906
902907
You need to adjust your setup for ssh-ident to work: consider setting
903908
BINARY_SSH or BINARY_DIR in your config, or running ssh-ident some
@@ -931,11 +936,6 @@ def ParseCommandLine(argv, config):
931936
break
932937

933938
def main(argv):
934-
# Replace stdout and stderr with /dev/tty, so we don't mess up with scripts
935-
# that use ssh in case we error out or similar.
936-
sys.stdout = open("/dev/tty", "w")
937-
sys.stderr = open("/dev/tty", "w")
938-
939939
config = Config().Load()
940940
# overwrite python's print function with the wrapper SshIdentPrint
941941
global print
@@ -956,8 +956,8 @@ def main(argv):
956956
message = textwrap.dedent("""\
957957
ssh-ident found '{0}' as the next command to run.
958958
Based on argv[0] ({1}), it seems like this will create a
959-
loop.
960-
959+
loop.
960+
961961
Please use BINARY_SSH, BINARY_DIR, or change the way
962962
ssh-ident is invoked (eg, a different argv[0]) to make
963963
it work correctly.""")

0 commit comments

Comments
 (0)