From 0232b76857346d814fdb1e2a423ef038b2d02967 Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Thu, 18 Oct 2012 19:59:22 +0200
Subject: [PATCH] Compare significant fields only for simple SSH keys
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
---
 lib/utils/io.py                  | 34 ++++++++++++++++++++++++++++----
 test/ganeti.utils.io_unittest.py | 12 +++++++----
 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/lib/utils/io.py b/lib/utils/io.py
index cf3e45324..a10ba840d 100644
--- a/lib/utils/io.py
+++ b/lib/utils/io.py
@@ -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()
diff --git a/test/ganeti.utils.io_unittest.py b/test/ganeti.utils.io_unittest.py
index bb9dc5e51..458ecabf1 100755
--- a/test/ganeti.utils.io_unittest.py
+++ b/test/ganeti.utils.io_unittest.py
@@ -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,
-- 
GitLab