Commit 0232b768 authored by Michael Hanselmann's avatar Michael Hanselmann
Browse files

Compare significant fields only for simple SSH keys



For simple SSH keys, that is those without options such as
“command="…"”, only the first two parts need to be compared. The third
field is a free-form comment.

This patch changes the comparison used in
AddAuthorizedKey/RemoveAuthorizedKey to take this into account. Lines
with options are still compared in full.
Signed-off-by: default avatarMichael Hanselmann <hansmi@google.com>
Reviewed-by: default avatarIustin Pop <iustin@google.com>
parent 9805aa82
......@@ -828,6 +828,32 @@ def ReadLockedPidFile(path):
return None
_SSH_KEYS_WITH_TWO_PARTS = frozenset(["ssh-dss", "ssh-rsa"])
def _SplitSshKey(key):
"""Splits a line for SSH's C{authorized_keys} file.
If the line has no options (e.g. no C{command="..."}), only the significant
parts, the key type and its hash, are used. Otherwise the whole line is used
(split at whitespace).
@type key: string
@param key: Key line
@rtype: tuple
"""
parts = key.split()
if parts and parts[0] in _SSH_KEYS_WITH_TWO_PARTS:
# If the key has no options in front of it, we only want the significant
# fields
return (False, parts[:2])
else:
# Can't properly split the line, so use everything
return (True, parts)
def AddAuthorizedKey(file_obj, key):
"""Adds an SSH public key to an authorized_keys file.
......@@ -837,7 +863,7 @@ def AddAuthorizedKey(file_obj, key):
@param key: string containing key
"""
key_fields = key.split()
key_fields = _SplitSshKey(key)
if isinstance(file_obj, basestring):
f = open(file_obj, "a+")
......@@ -848,7 +874,7 @@ def AddAuthorizedKey(file_obj, key):
nl = True
for line in f:
# Ignore whitespace changes
if line.split() == key_fields:
if _SplitSshKey(line) == key_fields:
break
nl = line.endswith("\n")
else:
......@@ -870,7 +896,7 @@ def RemoveAuthorizedKey(file_name, key):
@param key: string containing key
"""
key_fields = key.split()
key_fields = _SplitSshKey(key)
fd, tmpname = tempfile.mkstemp(dir=os.path.dirname(file_name))
try:
......@@ -880,7 +906,7 @@ def RemoveAuthorizedKey(file_name, key):
try:
for line in f:
# Ignore whitespace changes while comparing lines
if line.split() != key_fields:
if _SplitSshKey(line) != key_fields:
out.write(line)
out.flush()
......
......@@ -842,20 +842,24 @@ class TestSshKeys(testutils.GanetiTestCase):
utils.AddAuthorizedKey(self.tmpname,
'ssh-dss AAAAB3NzaC1w5256closdj32mZaQU root@test')
# Only significant fields are compared, therefore the key won't be
# updated/added
self.assertFileContent(self.tmpname,
"ssh-dss AAAAB3NzaC1w5256closdj32mZaQU root@key-a\n"
'command="/usr/bin/fooserver -t --verbose",from="198.51.100.4"'
" ssh-dss AAAAB3NzaC1w520smc01ms0jfJs22 root@key-b\n"
"ssh-dss AAAAB3NzaC1w5256closdj32mZaQU root@test\n")
" ssh-dss AAAAB3NzaC1w520smc01ms0jfJs22 root@key-b\n")
def testAddingExistingKeyWithSomeMoreSpaces(self):
utils.AddAuthorizedKey(self.tmpname,
'ssh-dss AAAAB3NzaC1w5256closdj32mZaQU root@key-a')
"ssh-dss AAAAB3NzaC1w5256closdj32mZaQU root@key-a")
utils.AddAuthorizedKey(self.tmpname,
"ssh-dss AAAAB3NzaC1w520smc01ms0jfJs22")
self.assertFileContent(self.tmpname,
"ssh-dss AAAAB3NzaC1w5256closdj32mZaQU root@key-a\n"
'command="/usr/bin/fooserver -t --verbose",from="198.51.100.4"'
" ssh-dss AAAAB3NzaC1w520smc01ms0jfJs22 root@key-b\n")
" ssh-dss AAAAB3NzaC1w520smc01ms0jfJs22 root@key-b\n"
"ssh-dss AAAAB3NzaC1w520smc01ms0jfJs22\n")
def testRemovingExistingKeyWithSomeMoreSpaces(self):
utils.RemoveAuthorizedKey(self.tmpname,
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment