Skip to content

Commit ff0a348

Browse files
smosersmoser
committed
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.
1 parent ceec672 commit ff0a348

3 files changed

Lines changed: 33 additions & 26 deletions

File tree

‎cloudinit/config/cc_ssh.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ def apply_credentials(keys, user, disable_root, disable_root_opts):
126126

127127
keys = set(keys)
128128
if user:
129-
ssh_util.setup_user_keys(keys, user, '')
129+
ssh_util.setup_user_keys(keys, user)
130130

131131
if disable_root:
132132
if not user:
@@ -135,4 +135,4 @@ def apply_credentials(keys, user, disable_root, disable_root_opts):
135135
else:
136136
key_prefix = ''
137137

138-
ssh_util.setup_user_keys(keys, 'root', key_prefix)
138+
ssh_util.setup_user_keys(keys, 'root', options=key_prefix)

‎cloudinit/ssh_util.py‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,8 @@ def __init__(self, source, keytype=None, base64=None,
5151
self.keytype = keytype
5252
self.source = source
5353

54-
def empty(self):
55-
if (not self.base64 and
56-
not self.comment and not self.keytype and not self.options):
57-
return True
58-
return False
54+
def valid(self):
55+
return (self.base64 and self.keytype)
5956

6057
def __str__(self):
6158
toks = []
@@ -120,7 +117,7 @@ def _extract_options(self, ent):
120117
remain = ent[i:].lstrip()
121118
return (options, remain)
122119

123-
def parse(self, src_line, def_opt=None):
120+
def parse(self, src_line, options=None):
124121
# modeled after opensshes auth2-pubkey.c:user_key_allowed2
125122
line = src_line.rstrip("\r\n")
126123
if line.startswith("#") or line.strip() == '':
@@ -141,13 +138,17 @@ def parse_ssh_key(ent):
141138

142139
return toks
143140

141+
if "badopt" in src_line:
142+
import ipdb; ipdb.set_trace()
143+
144144
ent = line.strip()
145-
options = None
146145
try:
147146
(keytype, base64, comment) = parse_ssh_key(ent)
148-
options = def_opt
149147
except TypeError as e:
150-
(options, remain) = self._extract_options(ent)
148+
(keyopts, remain) = self._extract_options(ent)
149+
if options is None:
150+
options = keyopts
151+
151152
try:
152153
(keytype, base64, comment) = parse_ssh_key(remain)
153154
except TypeError as e:
@@ -178,11 +179,11 @@ def update_authorized_keys(old_entries, keys):
178179

179180
for i in range(0, len(old_entries)):
180181
ent = old_entries[i]
181-
if ent.empty() or not ent.base64:
182+
if ent.valid():
182183
continue
183184
# Replace those with the same base64
184185
for k in keys:
185-
if k.empty() or not k.base64:
186+
if ent.valid():
186187
continue
187188
if k.base64 == ent.base64:
188189
# Replace it with our better one
@@ -241,7 +242,7 @@ def extract_authorized_keys(username):
241242
return (auth_key_fn, parse_authorized_keys(auth_key_fn))
242243

243244

244-
def setup_user_keys(keys, username, key_prefix):
245+
def setup_user_keys(keys, username, options=None):
245246
# Make sure the users .ssh dir is setup accordingly
246247
(ssh_dir, pwent) = users_ssh_info(username)
247248
if not os.path.isdir(ssh_dir):
@@ -252,7 +253,7 @@ def setup_user_keys(keys, username, key_prefix):
252253
parser = AuthKeyLineParser()
253254
key_entries = []
254255
for k in keys:
255-
key_entries.append(parser.parse(str(k), def_opt=key_prefix))
256+
key_entries.append(parser.parse(str(k), options=options))
256257

257258
# Extract the old and make the new
258259
(auth_key_fn, auth_key_entries) = extract_authorized_keys(username)

‎tests/unittests/test_sshutil.py‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ def test_parse_no_comment(self):
6262
self.assertFalse(key.comment)
6363
self.assertEqual(key.keytype, ktype)
6464

65-
def test_parse_with_options(self):
65+
def test_parse_with_keyoptions(self):
6666
# test key line with options in it
6767
parser = ssh_util.AuthKeyLineParser()
6868
options = TEST_OPTIONS
@@ -77,18 +77,24 @@ def test_parse_with_options(self):
7777
self.assertEqual(key.comment, comment)
7878
self.assertEqual(key.keytype, ktype)
7979

80-
def test_parse_with_defopt(self):
80+
def test_parse_with_options_passed_in(self):
8181
# test key line with key type and base64 only
8282
parser = ssh_util.AuthKeyLineParser()
83-
for ktype in ['rsa', 'ecdsa', 'dsa']:
84-
content = VALID_CONTENT[ktype]
85-
line = ' '.join((ktype, content,))
86-
myopts = "no-port-forwarding,no-agent-forwarding"
87-
key = parser.parse(line, myopts)
8883

89-
self.assertEqual(key.base64, content)
90-
self.assertEqual(key.options, myopts)
91-
self.assertFalse(key.comment)
92-
self.assertEqual(key.keytype, ktype)
84+
baseline = ' '.join(("rsa", VALID_CONTENT['rsa'], "user@host"))
85+
myopts = "no-port-forwarding,no-agent-forwarding"
86+
87+
key = parser.parse("allowedopt" + " " + baseline)
88+
self.assertEqual(key.options, "allowedopt")
89+
90+
key = parser.parse("overridden_opt " + baseline, options=myopts)
91+
self.assertEqual(key.options, myopts)
92+
93+
def test_parse_invalid_keytype(self):
94+
parser = ssh_util.AuthKeyLineParser()
95+
key = parser.parse(' '.join(["badkeytype", VALID_CONTENT['rsa']]))
96+
97+
self.assertFalse(key.valid())
98+
9399

94100
# vi: ts=4 expandtab

0 commit comments

Comments
 (0)