Skip to content
This repository was archived by the owner on Oct 30, 2018. It is now read-only.

Commit 66d47c8

Browse files
authored
Only change to short IDs for delete (#5353)
* Only change to short IDs for delete If the user specifies long IDs, use them for all commands except for deleting a key. Need to use short IDs there because of an upstream apt_key bug. Fixed in apt_key 1.10 (fix is present in Ubuntu 16.04 but not Ubuntu 14.0 or some Debians). Fixes #5237 * Check that apt-key really erased the key When erasing a key, apt-key does not understand how to process subkeys. This update explicitly checks that the key_id is no longer present and throws an error if it is. It also hints at subkeys being a possible problem in the error message and the documentation. Fixes #5119 * Fix apt_key check mode with long ids apt-key can be given a key id longer than 16 chars to more accurately define what key to download. However, we can use a maximum of 16 chars to verify whether a key is installed or not. So we need to use different lengths for the id depending on what we're doing with it. Fixes #2622 Also: * Some style cleanups * Use get_bin_path to find the path to apt-key and then use that when invoking apt-key * Return a nice user error message if the key was not found on the keyserver * Make file and keyring parameters type='path' so envars and tilde are expanded
1 parent 952ad58 commit 66d47c8

File tree

1 file changed

+120
-67
lines changed

1 file changed

+120
-67
lines changed

packaging/os/apt_key.py

Lines changed: 120 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,17 @@
3737
default: none
3838
description:
3939
- identifier of key. Including this allows check mode to correctly report the changed state.
40+
- "If specifying a subkey's id be aware that apt-key does not understand how to remove keys via a subkey id. Specify the primary key's id instead."
4041
data:
4142
required: false
4243
default: none
4344
description:
44-
- keyfile contents
45+
- keyfile contents to add to the keyring
4546
file:
4647
required: false
4748
default: none
4849
description:
49-
- keyfile path
50+
- path to a keyfile to add to the keyring
5051
keyring:
5152
required: false
5253
default: none
@@ -106,29 +107,69 @@
106107

107108
# FIXME: standardize into module_common
108109
from traceback import format_exc
109-
from re import compile as re_compile
110-
# FIXME: standardize into module_common
111-
from distutils.spawn import find_executable
112-
from os import environ
113-
from sys import exc_info
114-
import traceback
115110

116-
match_key = re_compile("^gpg:.*key ([0-9a-fA-F]+):.*$")
111+
from ansible.module_utils.basic import AnsibleModule
112+
from ansible.module_utils._text import to_native
113+
from ansible.module_utils.urls import fetch_url
114+
115+
116+
apt_key_bin = None
117+
118+
119+
def find_needed_binaries(module):
120+
global apt_key_bin
121+
122+
apt_key_bin = module.get_bin_path('apt-key', required=True)
123+
124+
### FIXME: Is there a reason that gpg and grep are checked? Is it just
125+
# cruft or does the apt .deb package not require them (and if they're not
126+
# installed, /usr/bin/apt-key fails?)
127+
module.get_bin_path('gpg', required=True)
128+
module.get_bin_path('grep', required=True)
129+
130+
131+
def parse_key_id(key_id):
132+
"""validate the key_id and break it into segments
133+
134+
:arg key_id: The key_id as supplied by the user. A valid key_id will be
135+
8, 16, or more hexadecimal chars with an optional leading ``0x``.
136+
:returns: The portion of key_id suitable for apt-key del, the portion
137+
suitable for comparisons with --list-public-keys, and the portion that
138+
can be used with --recv-key. If key_id is long enough, these will be
139+
the last 8 characters of key_id, the last 16 characters, and all of
140+
key_id. If key_id is not long enough, some of the values will be the
141+
same.
142+
143+
* apt-key del <= 1.10 has a bug with key_id != 8 chars
144+
* apt-key adv --list-public-keys prints 16 chars
145+
* apt-key adv --recv-key can take more chars
146+
147+
"""
148+
# Make sure the key_id is valid hexadecimal
149+
int(key_id, 16)
150+
151+
key_id = key_id.upper()
152+
if key_id.startswith('0X'):
153+
key_id = key_id[2:]
154+
155+
key_id_len = len(key_id)
156+
if (key_id_len != 8 and key_id_len != 16) and key_id_len <= 16:
157+
raise ValueError('key_id must be 8, 16, or 16+ hexadecimal characters in length')
117158

118-
REQUIRED_EXECUTABLES=['gpg', 'grep', 'apt-key']
159+
short_key_id = key_id[-8:]
119160

161+
fingerprint = key_id
162+
if key_id_len > 16:
163+
fingerprint = key_id[-16:]
120164

121-
def check_missing_binaries(module):
122-
missing = [e for e in REQUIRED_EXECUTABLES if not find_executable(e)]
123-
if len(missing):
124-
module.fail_json(msg="binaries are missing", names=missing)
165+
return short_key_id, fingerprint, key_id
125166

126167

127168
def all_keys(module, keyring, short_format):
128169
if keyring:
129-
cmd = "apt-key --keyring %s adv --list-public-keys --keyid-format=long" % keyring
170+
cmd = "%s --keyring %s adv --list-public-keys --keyid-format=long" % (apt_key_bin, keyring)
130171
else:
131-
cmd = "apt-key adv --list-public-keys --keyid-format=long"
172+
cmd = "%s adv --list-public-keys --keyid-format=long" % apt_key_bin
132173
(rc, out, err) = module.run_command(cmd)
133174
results = []
134175
lines = to_native(out).split('\n')
@@ -172,42 +213,47 @@ def download_key(module, url):
172213

173214
def import_key(module, keyring, keyserver, key_id):
174215
if keyring:
175-
cmd = "apt-key --keyring %s adv --keyserver %s --recv %s" % (keyring, keyserver, key_id)
216+
cmd = "%s --keyring %s adv --keyserver %s --recv %s" % (apt_key_bin, keyring, keyserver, key_id)
176217
else:
177-
cmd = "apt-key adv --keyserver %s --recv %s" % (keyserver, key_id)
218+
cmd = "%s adv --keyserver %s --recv %s" % (apt_key_bin, keyserver, key_id)
178219
for retry in range(5):
179-
(rc, out, err) = module.run_command(cmd)
220+
lang_env = dict(LANG='C', LC_ALL='C', LC_MESSAGES='C')
221+
(rc, out, err) = module.run_command(cmd, environ_update=lang_env)
180222
if rc == 0:
181223
break
182224
else:
183225
# Out of retries
184-
module.fail_json(cmd=cmd, msg="error fetching key from keyserver: %s" % keyserver,
185-
rc=rc, stdout=out, stderr=err)
226+
if rc == 2 and 'not found on keyserver' in out:
227+
msg = 'Key %s not found on keyserver %s' % (key_id, keyserver)
228+
module.fail_json(cmd=cmd, msg=msg)
229+
else:
230+
msg = "Error fetching key %s from keyserver: %s" % (key_id, keyserver)
231+
module.fail_json(cmd=cmd, msg=msg, rc=rc, stdout=out, stderr=err)
186232
return True
187233

188234

189235
def add_key(module, keyfile, keyring, data=None):
190236
if data is not None:
191237
if keyring:
192-
cmd = "apt-key --keyring %s add -" % keyring
238+
cmd = "%s --keyring %s add -" % (apt_key_bin, keyring)
193239
else:
194-
cmd = "apt-key add -"
240+
cmd = "%s add -" % apt_key_bin
195241
(rc, out, err) = module.run_command(cmd, data=data, check_rc=True, binary_data=True)
196242
else:
197243
if keyring:
198-
cmd = "apt-key --keyring %s add %s" % (keyring, keyfile)
244+
cmd = "%s --keyring %s add %s" % (apt_key_bin, keyring, keyfile)
199245
else:
200-
cmd = "apt-key add %s" % (keyfile)
246+
cmd = "%s add %s" % (apt_key_bin, keyfile)
201247
(rc, out, err) = module.run_command(cmd, check_rc=True)
202248
return True
203249

204250

205251
def remove_key(module, key_id, keyring):
206252
# FIXME: use module.run_command, fail at point of error and don't discard useful stdin/stdout
207253
if keyring:
208-
cmd = 'apt-key --keyring %s del %s' % (keyring, key_id)
254+
cmd = '%s --keyring %s del %s' % (apt_key_bin, keyring, key_id)
209255
else:
210-
cmd = 'apt-key del %s' % key_id
256+
cmd = '%s del %s' % (apt_key_bin, key_id)
211257
(rc, out, err) = module.run_command(cmd, check_rc=True)
212258
return True
213259

@@ -218,14 +264,15 @@ def main():
218264
id=dict(required=False, default=None),
219265
url=dict(required=False),
220266
data=dict(required=False),
221-
file=dict(required=False),
267+
file=dict(required=False, type='path'),
222268
key=dict(required=False),
223-
keyring=dict(required=False),
269+
keyring=dict(required=False, type='path'),
224270
validate_certs=dict(default='yes', type='bool'),
225271
keyserver=dict(required=False),
226272
state=dict(required=False, choices=['present', 'absent'], default='present')
227273
),
228-
supports_check_mode=True
274+
supports_check_mode=True,
275+
mutually_exclusive=(('filename', 'keyserver', 'data', 'url'),),
229276
)
230277

231278
key_id = module.params['id']
@@ -237,64 +284,70 @@ def main():
237284
keyserver = module.params['keyserver']
238285
changed = False
239286

240-
# we use the "short" id: key_id[-8:], short_format=True
241-
# it's a workaround for https://bugs.launchpad.net/ubuntu/+source/apt/+bug/1481871
242-
287+
fingerprint = short_key_id = key_id
288+
short_format = False
243289
if key_id:
244290
try:
245-
_ = int(key_id, 16)
246-
if key_id.startswith('0x'):
247-
key_id = key_id[2:]
248-
key_id = key_id.upper()[-8:]
291+
key_id, fingerprint, short_key_id = parse_key_id(key_id)
249292
except ValueError:
250-
module.fail_json(msg="Invalid key_id", id=key_id)
293+
module.fail_json(msg='Invalid key_id', id=key_id)
251294

252-
# FIXME: I think we have a common facility for this, if not, want
253-
check_missing_binaries(module)
295+
if len(fingerprint) == 8:
296+
short_format = True
297+
298+
find_needed_binaries(module)
254299

255-
short_format = True
256300
keys = all_keys(module, keyring, short_format)
257301
return_values = {}
258302

259303
if state == 'present':
260-
if key_id and key_id in keys:
304+
if fingerprint and fingerprint in keys:
261305
module.exit_json(changed=False)
306+
elif fingerprint and fingerprint not in keys and module.check_mode:
307+
### TODO: Someday we could go further -- write keys out to
308+
# a temporary file and then extract the key id from there via gpg
309+
# to decide if the key is installed or not.
310+
module.exit_json(changed=True)
262311
else:
263312
if not filename and not data and not keyserver:
264313
data = download_key(module, url)
265-
if key_id and key_id in keys:
266-
module.exit_json(changed=False)
314+
315+
if filename:
316+
add_key(module, filename, keyring)
317+
elif keyserver:
318+
import_key(module, keyring, keyserver, key_id)
267319
else:
268-
if module.check_mode:
269-
module.exit_json(changed=True)
270-
if filename:
271-
add_key(module, filename, keyring)
272-
elif keyserver:
273-
import_key(module, keyring, keyserver, key_id)
274-
else:
275-
add_key(module, "-", keyring, data)
276-
changed=False
277-
keys2 = all_keys(module, keyring, short_format)
278-
if len(keys) != len(keys2):
279-
changed=True
280-
if key_id and not key_id in keys2:
281-
module.fail_json(msg="key does not seem to have been added", id=key_id)
282-
module.exit_json(changed=changed)
320+
add_key(module, "-", keyring, data)
321+
322+
changed = False
323+
keys2 = all_keys(module, keyring, short_format)
324+
if len(keys) != len(keys2):
325+
changed=True
326+
327+
if fingerprint and fingerprint not in keys2:
328+
module.fail_json(msg="key does not seem to have been added", id=key_id)
329+
module.exit_json(changed=changed)
330+
283331
elif state == 'absent':
284332
if not key_id:
285333
module.fail_json(msg="key is required")
286-
if key_id in keys:
334+
if fingerprint in keys:
287335
if module.check_mode:
288336
module.exit_json(changed=True)
289-
if remove_key(module, key_id, keyring):
290-
changed=True
337+
338+
# we use the "short" id: key_id[-8:], short_format=True
339+
# it's a workaround for https://bugs.launchpad.net/ubuntu/+source/apt/+bug/1481871
340+
if remove_key(module, short_key_id, keyring):
341+
keys = all_keys(module, keyring, short_format)
342+
if fingerprint in keys:
343+
module.fail_json(msg="apt-key del did not return an error but the key was not removed (check that the id is correct and *not* a subkey)", id=key_id)
344+
changed = True
291345
else:
292-
# FIXME: module.fail_json or exit-json immediately at point of failure
346+
# FIXME: module.fail_json or exit-json immediately at point of failure
293347
module.fail_json(msg="error removing key_id", **return_values)
294348

295349
module.exit_json(changed=changed, **return_values)
296350

297-
# import module snippets
298-
from ansible.module_utils.basic import *
299-
from ansible.module_utils.urls import *
300-
main()
351+
352+
if __name__ == '__main__':
353+
main()

0 commit comments

Comments
 (0)