change parser.parse 'default_opts' to 'options'

Now, parser.parse specifies options that override any options found,
rather than just being default options.

There could still potentially be a user for default_options, but since we're
not using them anywhere, I've dropped it.  The difference is that in setting up
the root user, we're now insisting that all keys that go in there have the
key_prefix, even if the key content had other options.

I think this is actually the commit that fixes LP: #1136343.

3 files changed, 33 insertions, 26 deletions

diff --git a/cloudinit/config/cc_ssh.py b/cloudinit/config/cc_ssh.py
index b623d476..7ef20d9f 100644
--- a/cloudinit/config/cc_ssh.py
+++ b/cloudinit/config/cc_ssh.py
@@ -126,7 +126,7 @@ def apply_credentials(keys, user, disable_root, disable_root_opts):
 
     keys = set(keys)
     if user:
-        ssh_util.setup_user_keys(keys, user, '')
+        ssh_util.setup_user_keys(keys, user)
 
     if disable_root:
         if not user:
@@ -135,4 +135,4 @@ def apply_credentials(keys, user, disable_root, disable_root_opts):
     else:
         key_prefix = ''
 
-    ssh_util.setup_user_keys(keys, 'root', key_prefix)
+    ssh_util.setup_user_keys(keys, 'root', options=key_prefix)
diff --git a/cloudinit/ssh_util.py b/cloudinit/ssh_util.py
index 082c5bbd..44c7c15b 100644
--- a/cloudinit/ssh_util.py
+++ b/cloudinit/ssh_util.py
@@ -51,11 +51,8 @@ class AuthKeyLine(object):
         self.keytype = keytype
         self.source = source
 
-    def empty(self):
-        if (not self.base64 and
-            not self.comment and not self.keytype and not self.options):
-            return True
-        return False
+    def valid(self):
+        return (self.base64 and self.keytype)
 
     def __str__(self):
         toks = []
@@ -120,7 +117,7 @@ class AuthKeyLineParser(object):
         remain = ent[i:].lstrip()
         return (options, remain)
 
-    def parse(self, src_line, def_opt=None):
+    def parse(self, src_line, options=None):
         # modeled after opensshes auth2-pubkey.c:user_key_allowed2
         line = src_line.rstrip("\r\n")
         if line.startswith("#") or line.strip() == '':
@@ -141,13 +138,17 @@ class AuthKeyLineParser(object):
 
             return toks
 
+        if "badopt" in src_line:
+            import ipdb; ipdb.set_trace()
+
         ent = line.strip()
-        options = None
         try:
             (keytype, base64, comment) = parse_ssh_key(ent)
-            options = def_opt
         except TypeError as e:
-            (options, remain) = self._extract_options(ent)
+            (keyopts, remain) = self._extract_options(ent)
+            if options is None:
+                options = keyopts
+            
             try:
                 (keytype, base64, comment) = parse_ssh_key(remain)
             except TypeError as e:
@@ -178,11 +179,11 @@ def update_authorized_keys(old_entries, keys):
 
     for i in range(0, len(old_entries)):
         ent = old_entries[i]
-        if ent.empty() or not ent.base64:
+        if ent.valid():
             continue
         # Replace those with the same base64
         for k in keys:
-            if k.empty() or not k.base64:
+            if ent.valid():
                 continue
             if k.base64 == ent.base64:
                 # Replace it with our better one
@@ -241,7 +242,7 @@ def extract_authorized_keys(username):
     return (auth_key_fn, parse_authorized_keys(auth_key_fn))
 
 
-def setup_user_keys(keys, username, key_prefix):
+def setup_user_keys(keys, username, options=None):
     # Make sure the users .ssh dir is setup accordingly
     (ssh_dir, pwent) = users_ssh_info(username)
     if not os.path.isdir(ssh_dir):
@@ -252,7 +253,7 @@ def setup_user_keys(keys, username, key_prefix):
     parser = AuthKeyLineParser()
     key_entries = []
     for k in keys:
-        key_entries.append(parser.parse(str(k), def_opt=key_prefix))
+        key_entries.append(parser.parse(str(k), options=options))
 
     # Extract the old and make the new
     (auth_key_fn, auth_key_entries) = extract_authorized_keys(username)
diff --git a/tests/unittests/test_sshutil.py b/tests/unittests/test_sshutil.py
index 4564d9be..2415d06f 100644
--- a/tests/unittests/test_sshutil.py
+++ b/tests/unittests/test_sshutil.py
@@ -62,7 +62,7 @@ class TestAuthKeyLineParser(TestCase):
             self.assertFalse(key.comment)
             self.assertEqual(key.keytype, ktype)
     
-    def test_parse_with_options(self):
+    def test_parse_with_keyoptions(self):
         # test key line with options in it
         parser = ssh_util.AuthKeyLineParser()
         options = TEST_OPTIONS
@@ -77,18 +77,24 @@ class TestAuthKeyLineParser(TestCase):
             self.assertEqual(key.comment, comment)
             self.assertEqual(key.keytype, ktype)
 
-    def test_parse_with_defopt(self):
+    def test_parse_with_options_passed_in(self):
         # test key line with key type and base64 only
         parser = ssh_util.AuthKeyLineParser()
-        for ktype in ['rsa', 'ecdsa', 'dsa']:
-            content = VALID_CONTENT[ktype]
-            line = ' '.join((ktype, content,))
-            myopts = "no-port-forwarding,no-agent-forwarding"
-            key = parser.parse(line, myopts)
 
-            self.assertEqual(key.base64, content)
-            self.assertEqual(key.options, myopts)
-            self.assertFalse(key.comment)
-            self.assertEqual(key.keytype, ktype)
+        baseline = ' '.join(("rsa", VALID_CONTENT['rsa'], "user@host"))
+        myopts = "no-port-forwarding,no-agent-forwarding"
+
+        key = parser.parse("allowedopt" + " " + baseline)
+        self.assertEqual(key.options, "allowedopt")
+
+        key = parser.parse("overridden_opt " + baseline, options=myopts)
+        self.assertEqual(key.options, myopts)
+
+    def test_parse_invalid_keytype(self):
+        parser = ssh_util.AuthKeyLineParser()
+        key = parser.parse(' '.join(["badkeytype", VALID_CONTENT['rsa']]))
+
+        self.assertFalse(key.valid())
+
 
 # vi: ts=4 expandtab