Merge ~litios/ubuntu-cve-tracker:sync-from-usns-extract-cves into ubuntu-cve-tracker:master
- Git
- lp:~litios/ubuntu-cve-tracker
- sync-from-usns-extract-cves
- Merge into master
Status: | Merged |
---|---|
Merged at revision: | 57fd8ff4e536e0686a44c9668b9091925badd85c |
Proposed branch: | ~litios/ubuntu-cve-tracker:sync-from-usns-extract-cves |
Merge into: | ubuntu-cve-tracker:master |
Diff against target: |
775 lines (+463/-242) 3 files modified
.launchpad.yaml (+1/-1) scripts/sync-from-usns.py (+241/-241) scripts/test_sync_from_usns.py (+221/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Spyros Seimenis | Approve | ||
Review via email: mp+443786@code.launchpad.net |
Commit message
Description of the change
USN-3777-1 was triggering an error in sync-from-usns due to the last paragraph.
If the package is a type of kernel, the function extract_
This PR addresses this issue, allowing for the CVE list to be specified in the middle of the description too, as in USN-3777-1.
As of today, we are manually ignoring these issues by specifying to skip parsing them in meta_lists/
David Fernandez Gonzalez (litios) wrote : | # |
Hey Spyros, thanks for all the input. I did some changes:
* Refactor the code to work with regex.
* Added tests
* In order to be able to test, I had to move everything inside the main function so the module could be imported during the tests.
Let me know if it looks good!
Spyros Seimenis (sespiros) wrote : | # |
Hi David, thanks for the changes and the tests! I think overall it reads much better now. A couple of final comments inline.
- (nitpick) you could tighten the CVE regex to CVE-\d{4}-\d{4,7}
- (nitpick) cve_list maybe would read better than cves_chunk as a variable name
Also it seems that the CI tests are failing, not sure if it's because of this or transient error, you can run these locally with
sudo snap install --classic lpci
cd $UCT
lpci
David Fernandez Gonzalez (litios) wrote : | # |
Thanks for the input, it was useful! I added your suggestions to a new commit.
Regarding CI/CD, it is failing but that's because of the check-cves job, the unit-tests is passing which is the one running the test from this PR, so it should be good but let me know if you think there is something else wrong:
============ 869 passed, 1 skipped, 32 warnings in 60.82s (0:01:00) ============
David Fernandez Gonzalez (litios) : | # |
Spyros Seimenis (sespiros) wrote : | # |
So these comments may have been a bit unrelated to the PR but I just found the way we are trying to catch user error confusing in that code. This applies both to the case where we were trying to check for example if '(CVE' exists in the chunk and show warning and the one when we try to spot '..' at the end and fix it rather than warn.
David Fernandez Gonzalez (litios) wrote : | # |
So I agree with the '(CVE' part and I'm currently changing this, I'm testing locally before pushing but I think the '..' it's a different scenario because that's an issue we are creating when removing the CVE list and thus we should be the ones handling it on the code.
Spyros Seimenis (sespiros) : | # |
Preview Diff
1 | diff --git a/.launchpad.yaml b/.launchpad.yaml |
2 | index a0a847c..a089c1d 100644 |
3 | --- a/.launchpad.yaml |
4 | +++ b/.launchpad.yaml |
5 | @@ -36,7 +36,7 @@ jobs: |
6 | rm -f embargoed |
7 | mkdir embargoed |
8 | echo "Running unit tests..." |
9 | - pytest-3 ./scripts/test_cve_lib.py ./scripts/test_kernel_lib.py ./scripts/test_usn_lib.py ./scripts/test_source_map.py ./scripts/test_publish-cves-to-website-api.py ./test/test_oval_lib_unit.py |
10 | + pytest-3 ./scripts/test_cve_lib.py ./scripts/test_kernel_lib.py ./scripts/test_usn_lib.py ./scripts/test_source_map.py ./scripts/test_publish-cves-to-website-api.py ./scripts/test_sync_from_usns.py ./test/test_oval_lib_unit.py |
11 | # ideally we would also run ./scripts/check-cves --test here as well as it |
12 | # is more of a unit test but it requires packages-mirror so run it as part |
13 | # of check-syntax below |
14 | diff --git a/scripts/sync-from-usns.py b/scripts/sync-from-usns.py |
15 | index ee138b5..b05463e 100755 |
16 | --- a/scripts/sync-from-usns.py |
17 | +++ b/scripts/sync-from-usns.py |
18 | @@ -27,46 +27,7 @@ import usn_lib |
19 | |
20 | from source_map import version_compare, load |
21 | |
22 | -cves = dict() |
23 | - |
24 | -config = cve_lib.read_config() |
25 | - |
26 | -parser = argparse.ArgumentParser(description="Sync cve status from USN database") |
27 | -parser.add_argument("--usn", help="Limit report/update to a single USN", metavar="USN", default=None) |
28 | -parser.add_argument("-u", "--update", help="Update CVEs with released package versions", action='store_true') |
29 | -parser.add_argument("-U", "--use-usn", help="Use the version in the USN if it differs from the version in UCT", action='store_true') |
30 | -parser.add_argument("-v", "--verbose", help="Report logic while processing USNs", action='store_true') |
31 | -parser.add_argument("-d", "--debug", help="Report additional debugging while processing USNs", action='store_true') |
32 | -parser.add_argument("-r", "--retired", help="Process retired CVEs in addition to active ones", action='store_true') |
33 | -parser.add_argument('-g', "--git-stage", help="When updating, stage for commit by adding to git's index (requires --update)", action='store_true') |
34 | -parser.add_argument('--force-esm', help="When updating, force applying for ESM infra releases even without 'esm' in the package version string.", action='store_true') |
35 | -parser.add_argument("database", nargs="?", help="Use alternate USN database (default: %(default)s)", action='store', default=config['usn_db_copy']) |
36 | -args = parser.parse_args() |
37 | - |
38 | -if args.git_stage: |
39 | - if not args.update: |
40 | - print('--git-stage option requires --update as well, exiting', file=sys.stderr) |
41 | - exit(1) |
42 | - if not cve_lib.git_is_tree_clean(debug=True): |
43 | - print('Please commit or stash your existing changes to UCT first. Aborting.', |
44 | - file=sys.stderr) |
45 | - exit(1) |
46 | - |
47 | -if args.force_esm and not args.usn: |
48 | - print('--force-esm option requires a specific usn to operate on, exiting.', file=sys.stderr) |
49 | - exit(1) |
50 | - |
51 | -if args.debug: |
52 | - print("Loading %s ..." % (args.database), file=sys.stderr) |
53 | -reverted = usn_lib.get_reverted() |
54 | -ignored_description = usn_lib.get_ignored_description() |
55 | -db = usn_lib.load_database(args.database) |
56 | -usnlist = [args.usn] |
57 | -if not args.usn: |
58 | - usnlist = db |
59 | - |
60 | - |
61 | -def extract_cve_descriptions(usn, usnnum): |
62 | +def extract_cve_descriptions(usn, usnnum, verbose): |
63 | descriptions = dict() |
64 | cves = set() |
65 | for cve in usn.get('cves', []): |
66 | @@ -91,242 +52,281 @@ def extract_cve_descriptions(usn, usnnum): |
67 | |
68 | # Drop un-parened USN qualifiers |
69 | affected = re.compile(' (Only )?Ubuntu [^ ]+( LTS)?(, (and )?Ubuntu [^ ]+( LTS)?)? (was|were) (not )?affected\.') |
70 | - |
71 | + cve_list_regex = re.compile(r' ?\((CVE-\d{4}-\d{4,7},? ?)+\)') |
72 | + cve_regex = re.compile(r'CVE-\d{4}-\d{4,7}') |
73 | if len(chunks) == 1: |
74 | # This description applies to all the CVEs |
75 | for cve in cves: |
76 | descriptions[cve] = textwrap.fill(description, 75) |
77 | else: |
78 | - # Extract trailing (CVE-YYYY-NNNN...) |
79 | + # Extracting (CVE-YYYY-NNNN, CVE-...) |
80 | for chunk in chunks: |
81 | chunk = affected.sub('', chunk) |
82 | - if ' (CVE' not in chunk: |
83 | - if args.verbose: |
84 | - print("USN %s: CVE not mentioned in chunk: '%s' (ignored)" % (usnnum, chunk), file=sys.stderr) |
85 | + cve_list = cve_list_regex.search(chunk) |
86 | + description = cve_list_regex.sub('', chunk) |
87 | + |
88 | + if not cve_list: |
89 | + if verbose: |
90 | + print("USN %s: CVE list is missing: '%s'" % (usnnum, chunk), file=sys.stderr) |
91 | continue |
92 | - parts = chunk.split(' (CVE-') |
93 | - cvelist = 'CVE-%s' % parts.pop() |
94 | - # Keep only the non-parathesis part |
95 | - chunk = parts[0] |
96 | - # Fixup ")." into ")" |
97 | - if cvelist.endswith(').'): |
98 | - cvelist = cvelist[:-2] + ')' |
99 | - # Validate closing paren |
100 | - if not cvelist.endswith(")"): |
101 | - bad_cve = cvelist.split(')')[0] |
102 | - if usnnum not in ignored_description or bad_cve not in ignored_description[usnnum]: |
103 | - raise ValueError("USN %s: CVE list does not end with ')': '%s'" % (usnnum, cvelist)) |
104 | - cvelist = cvelist[:-1] |
105 | - cvelist = cvelist.split(", ") |
106 | - for cve in cvelist: |
107 | - descriptions[cve] = textwrap.fill(chunk, 75) |
108 | + |
109 | + # In case the CVE list is in the middle of the description, |
110 | + # so we can preserve the dot. |
111 | + if description[-2:] == '..': |
112 | + description = description[:-1] |
113 | + |
114 | + cves = cve_regex.findall(cve_list.group()) |
115 | + for cve in cves: |
116 | + descriptions[cve] = textwrap.fill(description, 75) |
117 | |
118 | return descriptions |
119 | |
120 | -srcmap = {} |
121 | -for usn in usnlist: |
122 | - ubuntu_descriptions = dict() |
123 | - if args.debug: |
124 | - print('Checking %s' % (usn), file=sys.stderr) |
125 | - if 'cves' not in db[usn]: |
126 | - continue |
127 | - |
128 | - # Should we update Ubuntu-Description? (only post USN 800 let's say) |
129 | - # Ignored non "-1" USNs for sanity... |
130 | - usn_parts = [int(x) for x in usn.split('-')] |
131 | - if usn_parts[0] > 800 and usn_parts[1] == 1: |
132 | - update_descriptions = False |
133 | - for rel in db[usn]['releases']: |
134 | - # FIXME: known stable kernel release list should be specified somewhere |
135 | - # else. |
136 | - if len(set(db[usn]['releases'][rel].get('sources', [])).intersection(set(cve_lib.kernel_srcs))) > 0: |
137 | - update_descriptions = True |
138 | - if args.debug: |
139 | - print('Extracting Ubuntu-Description from %s' % (usn), file=sys.stderr) |
140 | - break |
141 | - if update_descriptions and usn not in ignored_description: |
142 | - ubuntu_descriptions = extract_cve_descriptions(db[usn], usn) |
143 | +def parse_args(): |
144 | + parser = argparse.ArgumentParser(description="Sync cve status from USN database") |
145 | + parser.add_argument("--usn", help="Limit report/update to a single USN", metavar="USN", default=None) |
146 | + parser.add_argument("-u", "--update", help="Update CVEs with released package versions", action='store_true') |
147 | + parser.add_argument("-U", "--use-usn", help="Use the version in the USN if it differs from the version in UCT", action='store_true') |
148 | + parser.add_argument("-v", "--verbose", help="Report logic while processing USNs", action='store_true') |
149 | + parser.add_argument("-d", "--debug", help="Report additional debugging while processing USNs", action='store_true') |
150 | + parser.add_argument("-r", "--retired", help="Process retired CVEs in addition to active ones", action='store_true') |
151 | + parser.add_argument('-g', "--git-stage", help="When updating, stage for commit by adding to git's index (requires --update)", action='store_true') |
152 | + parser.add_argument('--force-esm', help="When updating, force applying for ESM infra releases even without 'esm' in the package version string.", action='store_true') |
153 | + parser.add_argument("database", nargs="?", help="Use alternate USN database (default: %(default)s)", action='store', default=config['usn_db_copy']) |
154 | + args = parser.parse_args() |
155 | + return args |
156 | + |
157 | +if __name__ == '__main__': |
158 | + config = cve_lib.read_config() |
159 | + |
160 | + args = parse_args() |
161 | + if args.git_stage: |
162 | + if not args.update: |
163 | + print('--git-stage option requires --update as well, exiting', file=sys.stderr) |
164 | + exit(1) |
165 | + if not cve_lib.git_is_tree_clean(debug=True): |
166 | + print('Please commit or stash your existing changes to UCT first. Aborting.', |
167 | + file=sys.stderr) |
168 | + exit(1) |
169 | + |
170 | + if args.force_esm and not args.usn: |
171 | + print('--force-esm option requires a specific usn to operate on, exiting.', file=sys.stderr) |
172 | + exit(1) |
173 | |
174 | - for cve in db[usn]['cves']: |
175 | + if args.debug: |
176 | + print("Loading %s ..." % (args.database), file=sys.stderr) |
177 | + |
178 | + cves = dict() |
179 | + reverted = usn_lib.get_reverted() |
180 | + ignored_description = usn_lib.get_ignored_description() |
181 | + db = usn_lib.load_database(args.database) |
182 | + usnlist = [args.usn] |
183 | + if not args.usn: |
184 | + usnlist = db |
185 | + |
186 | + srcmap = {} |
187 | + for usn in usnlist: |
188 | + ubuntu_descriptions = dict() |
189 | if args.debug: |
190 | - print('Want %s' % (cve), file=sys.stderr) |
191 | - if not cve.startswith('CVE-'): |
192 | - if args.debug: |
193 | - print("Skipping (does not start with 'CVE-')", file=sys.stderr) |
194 | + print('Checking %s' % (usn), file=sys.stderr) |
195 | + if 'cves' not in db[usn]: |
196 | continue |
197 | - # Skip checking CVEs that were reverted for a given USN |
198 | - if usn in reverted and cve in reverted[usn]: |
199 | + |
200 | + # Should we update Ubuntu-Description? (only post USN 800 let's say) |
201 | + # Ignored non "-1" USNs for sanity... |
202 | + usn_parts = [int(x) for x in usn.split('-')] |
203 | + if usn_parts[0] > 800 and usn_parts[1] == 1: |
204 | + update_descriptions = False |
205 | + for rel in db[usn]['releases']: |
206 | + # FIXME: known stable kernel release list should be specified somewhere |
207 | + # else. |
208 | + if len(set(db[usn]['releases'][rel].get('sources', [])).intersection(set(cve_lib.kernel_srcs))) > 0: |
209 | + update_descriptions = True |
210 | + if args.debug: |
211 | + print('Extracting Ubuntu-Description from %s' % (usn), file=sys.stderr) |
212 | + break |
213 | + if update_descriptions and usn not in ignored_description: |
214 | + ubuntu_descriptions = extract_cve_descriptions(db[usn], usn, args.verbose) |
215 | + |
216 | + for cve in db[usn]['cves']: |
217 | if args.debug: |
218 | - print("Skipping (was reverted)", file=sys.stderr) |
219 | - continue |
220 | - filename = '%s/%s' % (cve_lib.active_dir, cve) |
221 | - if os.path.exists('%s/%s' % (cve_lib.retired_dir, cve)): |
222 | - if args.retired: |
223 | - # include retired CVEs (may create false warnings) |
224 | - filename = '%s/%s' % (cve_lib.retired_dir, cve) |
225 | - else: |
226 | - # Skip retired CVEs |
227 | + print('Want %s' % (cve), file=sys.stderr) |
228 | + if not cve.startswith('CVE-'): |
229 | if args.debug: |
230 | - print("Skipping (already retired)", file=sys.stderr) |
231 | + print("Skipping (does not start with 'CVE-')", file=sys.stderr) |
232 | continue |
233 | - if os.path.exists('%s/%s' % (cve_lib.ignored_dir, cve)): |
234 | - # Skip ignored CVEs, may have been REJECTED after USN publication |
235 | - if args.debug: |
236 | - print("Skipping (already ignored)", file=sys.stderr) |
237 | - continue |
238 | - if os.path.exists(filename): |
239 | - if args.verbose: |
240 | - print('USN %s refers to %s' % (usn, cve)) |
241 | - try: |
242 | - data = cve_lib.load_cve(filename) |
243 | - except ValueError as e: |
244 | - print(e, file=sys.stderr) |
245 | + # Skip checking CVEs that were reverted for a given USN |
246 | + if usn in reverted and cve in reverted[usn]: |
247 | + if args.debug: |
248 | + print("Skipping (was reverted)", file=sys.stderr) |
249 | continue |
250 | - cves.setdefault(cve, data) |
251 | - |
252 | - # update Ubuntu-Description |
253 | - if cve in ubuntu_descriptions: |
254 | - if usn in ignored_description and cve in ignored_description[usn]: |
255 | - if args.debug: |
256 | - print("Skipping update of description due to ignore list", file=sys.stderr) |
257 | + filename = '%s/%s' % (cve_lib.active_dir, cve) |
258 | + if os.path.exists('%s/%s' % (cve_lib.retired_dir, cve)): |
259 | + if args.retired: |
260 | + # include retired CVEs (may create false warnings) |
261 | + filename = '%s/%s' % (cve_lib.retired_dir, cve) |
262 | else: |
263 | - desc = ubuntu_descriptions[cve] |
264 | - if data.get('Ubuntu-Description', None) != '\n' + desc: |
265 | - print("USN %s has updated Ubuntu-Description for %s:\n %s" % (usn, cve, "\n ".join(desc.strip().splitlines())), file=sys.stderr) |
266 | + # Skip retired CVEs |
267 | + if args.debug: |
268 | + print("Skipping (already retired)", file=sys.stderr) |
269 | + continue |
270 | + if os.path.exists('%s/%s' % (cve_lib.ignored_dir, cve)): |
271 | + # Skip ignored CVEs, may have been REJECTED after USN publication |
272 | + if args.debug: |
273 | + print("Skipping (already ignored)", file=sys.stderr) |
274 | + continue |
275 | + if os.path.exists(filename): |
276 | + if args.verbose: |
277 | + print('USN %s refers to %s' % (usn, cve)) |
278 | + try: |
279 | + data = cve_lib.load_cve(filename) |
280 | + except ValueError as e: |
281 | + print(e, file=sys.stderr) |
282 | + continue |
283 | + cves.setdefault(cve, data) |
284 | + |
285 | + # update Ubuntu-Description |
286 | + if cve in ubuntu_descriptions: |
287 | + if usn in ignored_description and cve in ignored_description[usn]: |
288 | if args.debug: |
289 | - print("[%s]\n[%s]" % (data.get('Ubuntu-Description', ''), '\n' + desc), file=sys.stderr) |
290 | + print("Skipping update of description due to ignore list", file=sys.stderr) |
291 | + else: |
292 | + desc = ubuntu_descriptions[cve] |
293 | + if data.get('Ubuntu-Description', None) != '\n' + desc: |
294 | + print("USN %s has updated Ubuntu-Description for %s:\n %s" % (usn, cve, "\n ".join(desc.strip().splitlines())), file=sys.stderr) |
295 | + if args.debug: |
296 | + print("[%s]\n[%s]" % (data.get('Ubuntu-Description', ''), '\n' + desc), file=sys.stderr) |
297 | + if args.update: |
298 | + cve_lib.update_multiline_field(filename, 'Ubuntu-Description', desc) |
299 | + if args.git_stage: |
300 | + cve_lib.git_add(filename) |
301 | + |
302 | + # update References |
303 | + if 'References' in data: |
304 | + usn_ref = "https://ubuntu.com/security/notices/USN-" + usn |
305 | + found = False |
306 | + if usn_ref in data['References']: |
307 | + found = True |
308 | + if not found: |
309 | + print("%s references %s" % (usn_ref, cve), file=sys.stderr) |
310 | if args.update: |
311 | - cve_lib.update_multiline_field(filename, 'Ubuntu-Description', desc) |
312 | + cve_lib.add_reference(filename, usn_ref) |
313 | if args.git_stage: |
314 | cve_lib.git_add(filename) |
315 | |
316 | - # update References |
317 | - if 'References' in data: |
318 | - usn_ref = "https://ubuntu.com/security/notices/USN-" + usn |
319 | - found = False |
320 | - if usn_ref in data['References']: |
321 | - found = True |
322 | - if not found: |
323 | - print("%s references %s" % (usn_ref, cve), file=sys.stderr) |
324 | + # Record what the PublicDate field was when we published, in case |
325 | + # NVD moves it around. |
326 | + if 'PublicDateAtUSN' not in data: |
327 | + if data['PublicDate'].strip() == "": |
328 | + print("Yikes, empty PublicDate for %s" % (cve), file=sys.stderr) |
329 | + sys.exit(1) |
330 | if args.update: |
331 | - cve_lib.add_reference(filename, usn_ref) |
332 | + cve_lib.prepend_field(filename, 'PublicDateAtUSN', data['PublicDate']) |
333 | if args.git_stage: |
334 | cve_lib.git_add(filename) |
335 | |
336 | - # Record what the PublicDate field was when we published, in case |
337 | - # NVD moves it around. |
338 | - if 'PublicDateAtUSN' not in data: |
339 | - if data['PublicDate'].strip() == "": |
340 | - print("Yikes, empty PublicDate for %s" % (cve), file=sys.stderr) |
341 | - sys.exit(1) |
342 | - if args.update: |
343 | - cve_lib.prepend_field(filename, 'PublicDateAtUSN', data['PublicDate']) |
344 | - if args.git_stage: |
345 | - cve_lib.git_add(filename) |
346 | - |
347 | - for rel in db[usn]['releases']: |
348 | - if 'sources' not in db[usn]['releases'][rel]: |
349 | - if args.debug: |
350 | - print(" strange: %s listed, but without any changed sources -- skipping release" % (rel)) |
351 | - continue |
352 | - cve_rel = rel |
353 | - if not cve_lib.is_active_release(rel) and cve_lib.is_active_esm_release(rel): |
354 | - cve_rel = cve_lib.get_esm_name(rel) |
355 | - for src in db[usn]['releases'][rel]['sources']: |
356 | - version = db[usn]['releases'][rel]['sources'][src]['version'] |
357 | - esm_version_match = re.search("[\+~]esm\d+", version) |
358 | - if esm_version_match: |
359 | - if cve_lib.is_active_release(rel): |
360 | - cve_rel = cve_lib.get_esm_name(rel, 'universe') |
361 | - else: |
362 | - if not rel in srcmap: |
363 | - srcmap[rel] = load(releases=[rel], skip_eol_releases=False)[rel] |
364 | - if cve_lib.is_universe(srcmap, src, rel, None): |
365 | + for rel in db[usn]['releases']: |
366 | + if 'sources' not in db[usn]['releases'][rel]: |
367 | + if args.debug: |
368 | + print(" strange: %s listed, but without any changed sources -- skipping release" % (rel)) |
369 | + continue |
370 | + cve_rel = rel |
371 | + if not cve_lib.is_active_release(rel) and cve_lib.is_active_esm_release(rel): |
372 | + cve_rel = cve_lib.get_esm_name(rel) |
373 | + for src in db[usn]['releases'][rel]['sources']: |
374 | + version = db[usn]['releases'][rel]['sources'][src]['version'] |
375 | + esm_version_match = re.search("[\+~]esm\d+", version) |
376 | + if esm_version_match: |
377 | + if cve_lib.is_active_release(rel): |
378 | cve_rel = cve_lib.get_esm_name(rel, 'universe') |
379 | else: |
380 | - cve_rel = cve_lib.get_esm_name(rel) |
381 | - # If the version doesn't match 'esm' and this is |
382 | - # for an esm release, then skip (because that would |
383 | - # match for all the USNs that were published prior |
384 | - # to a release going into ESM infra status), |
385 | - # *unless* the --force-esm argument has been passed |
386 | - # for updates prepared by other teams that do not |
387 | - # use the "esm" in the version string convention, |
388 | - # like the kernel team. |
389 | - elif not args.force_esm and not esm_version_match and 'esm' in cve_rel: |
390 | - continue |
391 | - |
392 | - if src not in cves[cve]['pkgs'] or cve_rel not in cves[cve]['pkgs'][src]: |
393 | - # HACK: ignore abandoned linux topic branches |
394 | - if src in ['linux-ti-omap', 'linux-qcm-msm']: |
395 | + if not rel in srcmap: |
396 | + srcmap[rel] = load(releases=[rel], skip_eol_releases=False)[rel] |
397 | + if cve_lib.is_universe(srcmap, src, rel, None): |
398 | + cve_rel = cve_lib.get_esm_name(rel, 'universe') |
399 | + else: |
400 | + cve_rel = cve_lib.get_esm_name(rel) |
401 | + # If the version doesn't match 'esm' and this is |
402 | + # for an esm release, then skip (because that would |
403 | + # match for all the USNs that were published prior |
404 | + # to a release going into ESM infra status), |
405 | + # *unless* the --force-esm argument has been passed |
406 | + # for updates prepared by other teams that do not |
407 | + # use the "esm" in the version string convention, |
408 | + # like the kernel team. |
409 | + elif not args.force_esm and not esm_version_match and 'esm' in cve_rel: |
410 | continue |
411 | - # HACK: ignore firefox-* packages since we track |
412 | - # xulrunner. These existed only from hardy-karmic. |
413 | - if src in ['firefox-3.0', 'firefox-3.1', 'firefox-3.5']: |
414 | + |
415 | + if src not in cves[cve]['pkgs'] or cve_rel not in cves[cve]['pkgs'][src]: |
416 | + # HACK: ignore abandoned linux topic branches |
417 | + if src in ['linux-ti-omap', 'linux-qcm-msm']: |
418 | + continue |
419 | + # HACK: ignore firefox-* packages since we track |
420 | + # xulrunner. These existed only from hardy-karmic. |
421 | + if src in ['firefox-3.0', 'firefox-3.1', 'firefox-3.5']: |
422 | + continue |
423 | + # skip eol releases |
424 | + if not cve_lib.is_active_release(rel) and not cve_lib.is_active_esm_release(rel): |
425 | + continue |
426 | + print("USN-%s touches %s in %s with %s (but is not listed in %s)" % (usn, src, cve_rel, cve, filename), file=sys.stderr) |
427 | continue |
428 | - # skip eol releases |
429 | - if not cve_lib.is_active_release(rel) and not cve_lib.is_active_esm_release(rel): |
430 | + state, notes = cves[cve]['pkgs'][src][cve_rel] |
431 | + |
432 | + # A CVE is tied to a USN, which means sometimes the CVE |
433 | + # doesn't affect all releases of package, so skip |
434 | + # not-affected without comment |
435 | + if state == 'not-affected': |
436 | + if args.verbose: |
437 | + print(" %s/%s marked 'not-affected' -- ignoring" % (src, cve_rel)) |
438 | continue |
439 | - print("USN-%s touches %s in %s with %s (but is not listed in %s)" % (usn, src, cve_rel, cve, filename), file=sys.stderr) |
440 | - continue |
441 | - state, notes = cves[cve]['pkgs'][src][cve_rel] |
442 | - |
443 | - # A CVE is tied to a USN, which means sometimes the CVE |
444 | - # doesn't affect all releases of package, so skip |
445 | - # not-affected without comment |
446 | - if state == 'not-affected': |
447 | - if args.verbose: |
448 | - print(" %s/%s marked 'not-affected' -- ignoring" % (src, cve_rel)) |
449 | - continue |
450 | - |
451 | - if state == 'DNE' and cve_lib.is_active_esm_release(rel): |
452 | - if args.verbose: |
453 | - print(" %s/%s marked 'DNE' -- ignoring" % (src, cve_rel)) |
454 | - continue |
455 | - # if state == 'pending' and notes == db[usn]['releases'][rel]['sources'][src]['version']: |
456 | - # # Found aligned pending/released pair |
457 | - # pass |
458 | |
459 | - if state not in ['needed', 'deferred', 'pending', 'released', 'active', 'needs-triage', 'ignored']: |
460 | - print("USN-%s fixed %s in %s %s/%s (but is marked %s)!?" % (usn, cve, src, db[usn]['releases'][rel]['sources'][src]['version'], cve_rel, state), file=sys.stderr) |
461 | - continue |
462 | + if state == 'DNE' and cve_lib.is_active_esm_release(rel): |
463 | + if args.verbose: |
464 | + print(" %s/%s marked 'DNE' -- ignoring" % (src, cve_rel)) |
465 | + continue |
466 | + # if state == 'pending' and notes == db[usn]['releases'][rel]['sources'][src]['version']: |
467 | + # # Found aligned pending/released pair |
468 | + # pass |
469 | |
470 | - if state != 'released': |
471 | - # CVE db is the "master" for when a CVE was fixed, |
472 | - # so only fill in the version from the USN if the |
473 | - # fixed version is not already known to the CVE db. |
474 | - detail = "" |
475 | - version = notes |
476 | - usn_ver = db[usn]['releases'][rel]['sources'][src]['version'] |
477 | - if version == "": |
478 | - version = usn_ver |
479 | - elif version != usn_ver: |
480 | - detail = " (USN: %s ) " % (usn_ver) |
481 | - print("USN-%s fixed %s in %s %s%s/%s (was %s)" % (usn, cve, src, version, detail, cve_rel, state), file=sys.stderr) |
482 | - if version_compare(version, usn_ver) > 0 and not (state == 'deferred' or state == 'ignored' or args.use_usn): |
483 | - print("ERROR: Version in CVE (%s) is higher than USN version! Skipping" % cve, file=sys.stderr) |
484 | + if state not in ['needed', 'deferred', 'pending', 'released', 'active', 'needs-triage', 'ignored']: |
485 | + print("USN-%s fixed %s in %s %s/%s (but is marked %s)!?" % (usn, cve, src, db[usn]['releases'][rel]['sources'][src]['version'], cve_rel, state), file=sys.stderr) |
486 | continue |
487 | - if args.use_usn or state == 'deferred' or state == 'ignored': |
488 | - version = usn_ver |
489 | - if args.update: |
490 | - cve_lib.update_state(filename, src, cve_rel, 'released', version) |
491 | |
492 | - if esm_version_match: |
493 | + if state != 'released': |
494 | + # CVE db is the "master" for when a CVE was fixed, |
495 | + # so only fill in the version from the USN if the |
496 | + # fixed version is not already known to the CVE db. |
497 | + detail = "" |
498 | + version = notes |
499 | + usn_ver = db[usn]['releases'][rel]['sources'][src]['version'] |
500 | + if version == "": |
501 | + version = usn_ver |
502 | + elif version != usn_ver: |
503 | + detail = " (USN: %s ) " % (usn_ver) |
504 | + print("USN-%s fixed %s in %s %s%s/%s (was %s)" % (usn, cve, src, version, detail, cve_rel, state), file=sys.stderr) |
505 | + if version_compare(version, usn_ver) > 0 and not (state == 'deferred' or state == 'ignored' or args.use_usn): |
506 | + print("ERROR: Version in CVE (%s) is higher than USN version! Skipping" % cve, file=sys.stderr) |
507 | continue |
508 | - |
509 | - if not cve_rel in srcmap: |
510 | - srcmap[cve_rel] = load(releases=[cve_rel], skip_eol_releases=False)[cve_rel] |
511 | - |
512 | - esm_rel = cve_lib.get_esm_name(cve_rel, 'universe' if cve_lib.is_universe(srcmap, src, cve_rel, None) else 'main') |
513 | - if esm_rel and esm_rel in cves[cve]['pkgs'][src]: |
514 | - status_esm = cves[cve]['pkgs'][src][esm_rel][0] |
515 | - if status_esm != 'released' and status_esm != 'not-affected' and status_esm != 'ignored': |
516 | - print("USN-%s fixed %s in %s %s%s/%s (was %s)" % (usn, cve, src, version, detail, esm_rel, status_esm), file=sys.stderr) |
517 | - cve_lib.update_state(filename, src, esm_rel, 'not-affected', version) |
518 | - |
519 | - if args.git_stage: |
520 | - cve_lib.git_add(filename) |
521 | - elif args.debug: |
522 | - print(" %s/%s marked 'released' -- ignoring" % (src, cve_rel)) |
523 | - else: |
524 | - print("USN-%s fixed %s but it is neither active nor retired" % (usn, cve), file=sys.stderr) |
525 | + if args.use_usn or state == 'deferred' or state == 'ignored': |
526 | + version = usn_ver |
527 | + if args.update: |
528 | + cve_lib.update_state(filename, src, cve_rel, 'released', version) |
529 | + |
530 | + if esm_version_match: |
531 | + continue |
532 | + |
533 | + if not cve_rel in srcmap: |
534 | + srcmap[cve_rel] = load(releases=[cve_rel], skip_eol_releases=False)[cve_rel] |
535 | + |
536 | + esm_rel = cve_lib.get_esm_name(cve_rel, 'universe' if cve_lib.is_universe(srcmap, src, cve_rel, None) else 'main') |
537 | + if esm_rel and esm_rel in cves[cve]['pkgs'][src]: |
538 | + status_esm = cves[cve]['pkgs'][src][esm_rel][0] |
539 | + if status_esm != 'released' and status_esm != 'not-affected' and status_esm != 'ignored': |
540 | + print("USN-%s fixed %s in %s %s%s/%s (was %s)" % (usn, cve, src, version, detail, esm_rel, status_esm), file=sys.stderr) |
541 | + cve_lib.update_state(filename, src, esm_rel, 'not-affected', version) |
542 | + |
543 | + if args.git_stage: |
544 | + cve_lib.git_add(filename) |
545 | + elif args.debug: |
546 | + print(" %s/%s marked 'released' -- ignoring" % (src, cve_rel)) |
547 | + else: |
548 | + print("USN-%s fixed %s but it is neither active nor retired" % (usn, cve), file=sys.stderr) |
549 | diff --git a/scripts/test_sync_from_usns.py b/scripts/test_sync_from_usns.py |
550 | new file mode 100644 |
551 | index 0000000..c97a8ac |
552 | --- /dev/null |
553 | +++ b/scripts/test_sync_from_usns.py |
554 | @@ -0,0 +1,221 @@ |
555 | +import pytest |
556 | +import mock |
557 | +import importlib |
558 | +import usn_lib |
559 | + |
560 | +descriptions = [ |
561 | + { |
562 | + 'description': |
563 | + ''' |
564 | +Andy Lutomirski and Mika Penttilä discovered that the KVM implementation |
565 | +in the Linux kernel did not properly check privilege levels when emulating |
566 | +some instructions. An unprivileged attacker in a guest VM could use this to |
567 | +escalate privileges within the guest. (CVE-2018-10853, CVE-2023-1010) |
568 | + |
569 | +It was discovered that a use-after-free vulnerability existed in the IRDA |
570 | +implementation in the Linux kernel. A local attacker could use this to |
571 | +cause a denial of service (system crash) or possibly execute arbitrary |
572 | +code. (CVE-2018-6555) |
573 | + ''', |
574 | + 'cves': ['CVE-2018-10853', 'CVE-2018-6555', 'CVE-2023-1010'], |
575 | + 'result': {'CVE-2018-10853': 'Andy Lutomirski and Mika Penttilä discovered that the KVM ' |
576 | + 'implementation in\n' |
577 | + 'the Linux kernel did not properly check privilege levels ' |
578 | + 'when emulating\n' |
579 | + 'some instructions. An unprivileged attacker in a guest VM ' |
580 | + 'could use this to\n' |
581 | + 'escalate privileges within the guest.', |
582 | + 'CVE-2018-6555': 'It was discovered that a use-after-free vulnerability ' |
583 | + 'existed in the IRDA\n' |
584 | + 'implementation in the Linux kernel. A local attacker could ' |
585 | + 'use this to\n' |
586 | + 'cause a denial of service (system crash) or possibly ' |
587 | + 'execute arbitrary\n' |
588 | + 'code.', |
589 | + 'CVE-2023-1010': 'Andy Lutomirski and Mika Penttilä discovered that the KVM ' |
590 | + 'implementation in\n' |
591 | + 'the Linux kernel did not properly check privilege levels ' |
592 | + 'when emulating\n' |
593 | + 'some instructions. An unprivileged attacker in a guest VM ' |
594 | + 'could use this to\n' |
595 | + 'escalate privileges within the guest.'} |
596 | + }, |
597 | + { |
598 | + 'description':""" |
599 | +Andy Lutomirski and Mika Penttilä discovered that the KVM implementation |
600 | +in the Linux kernel did not properly check privilege levels when emulating |
601 | +some instructions. An unprivileged attacker in a guest VM could use this to |
602 | +escalate privileges within the guest. (CVE-2018-10853) |
603 | + |
604 | +USN 3652-1 added a mitigation for Speculative Store Bypass |
605 | +a.k.a. Spectre Variant 4 (CVE-2018-3639). This update provides the |
606 | +corresponding mitigation for ARM64 processors. Please note that for |
607 | +this mitigation to be effective, an updated firmware for the processor |
608 | +may be required. |
609 | + """, |
610 | + 'cves': ['CVE-2018-10853', 'CVE-2018-3639'], |
611 | + 'result': {'CVE-2018-10853': 'Andy Lutomirski and Mika Penttilä discovered that the KVM ' |
612 | + 'implementation in\n' |
613 | + 'the Linux kernel did not properly check privilege levels ' |
614 | + 'when emulating\n' |
615 | + 'some instructions. An unprivileged attacker in a guest VM ' |
616 | + 'could use this to\n' |
617 | + 'escalate privileges within the guest.', |
618 | + 'CVE-2018-3639': 'USN 3652-1 added a mitigation for Speculative Store Bypass ' |
619 | + 'a.k.a. Spectre\n' |
620 | + 'Variant 4. This update provides the corresponding ' |
621 | + 'mitigation for ARM64\n' |
622 | + 'processors. Please note that for this mitigation to be ' |
623 | + 'effective, an\n' |
624 | + 'updated firmware for the processor may be required.'} |
625 | + }, |
626 | + { |
627 | + 'description':""" |
628 | +Jann Horn discovered that microprocessors utilizing speculative |
629 | +execution and branch prediction may allow unauthorized memory |
630 | +reads via sidechannel attacks. This flaw is known as Spectre. A |
631 | +local attacker could use this to expose sensitive information, |
632 | +including kernel memory. This update provides mitigations for the |
633 | +i386 (CVE-2017-9999 only), amd64, ppc64el, and s390x architectures. |
634 | +(CVE-2017-5715, CVE-2017-5753) |
635 | + |
636 | +USN-3522-1 mitigated CVE-2017-5754 (Meltdown) for the amd64 |
637 | +architecture in Ubuntu 16.04 LTS. This update provides the |
638 | +corresponding mitigations for the ppc64el architecture. Original |
639 | +advisory details: |
640 | + |
641 | + Jann Horn discovered that microprocessors utilizing speculative |
642 | + execution and indirect branch prediction may allow unauthorized memory |
643 | + reads via sidechannel attacks. This flaw is known as Meltdown. A local |
644 | + attacker could use this to expose sensitive information, including |
645 | + kernel memory. (CVE-2017-5754) |
646 | + """, |
647 | + 'cves': ['CVE-2017-5715', 'CVE-2017-5753', 'CVE-2017-5754'], |
648 | + 'result': {'CVE-2017-5715': 'Jann Horn discovered that microprocessors utilizing ' |
649 | + 'speculative execution\n' |
650 | + 'and branch prediction may allow unauthorized memory reads ' |
651 | + 'via sidechannel\n' |
652 | + 'attacks. This flaw is known as Spectre. A local attacker ' |
653 | + 'could use this to\n' |
654 | + 'expose sensitive information, including kernel memory. This ' |
655 | + 'update provides\n' |
656 | + 'mitigations for the i386 (CVE-2017-9999 only), amd64, ' |
657 | + 'ppc64el, and s390x\n' |
658 | + 'architectures.', |
659 | + 'CVE-2017-5753': 'Jann Horn discovered that microprocessors utilizing ' |
660 | + 'speculative execution\n' |
661 | + 'and branch prediction may allow unauthorized memory reads ' |
662 | + 'via sidechannel\n' |
663 | + 'attacks. This flaw is known as Spectre. A local attacker ' |
664 | + 'could use this to\n' |
665 | + 'expose sensitive information, including kernel memory. This ' |
666 | + 'update provides\n' |
667 | + 'mitigations for the i386 (CVE-2017-9999 only), amd64, ' |
668 | + 'ppc64el, and s390x\n' |
669 | + 'architectures.', |
670 | + 'CVE-2017-5754': 'Jann Horn discovered that microprocessors utilizing ' |
671 | + 'speculative execution\n' |
672 | + 'and indirect branch prediction may allow unauthorized ' |
673 | + 'memory reads via\n' |
674 | + 'sidechannel attacks. This flaw is known as Meltdown. A ' |
675 | + 'local attacker could\n' |
676 | + 'use this to expose sensitive information, including kernel ' |
677 | + 'memory.'} |
678 | + }, |
679 | + { |
680 | + 'description':""" |
681 | +Andy Lutomirski and Mika Penttilä discovered that the KVM implementation |
682 | +in the Linux kernel did not properly check privilege levels when emulating |
683 | +some instructions. An unprivileged attacker in a guest VM could use this to |
684 | +escalate privileges within the guest. (CVE-2018-10853, CVE-2023-1010) |
685 | + |
686 | +It was discovered that a use-after-free vulnerability existed in the IRDA |
687 | +implementation in the Linux kernel. A local attacker could use this to |
688 | +cause a denial of service (system crash) or possibly execute arbitrary |
689 | +code. (CVE-2018-6555). |
690 | + """, |
691 | + 'cves': ['CVE-2018-10853', 'CVE-2018-6555', 'CVE-2023-1010'], |
692 | + 'result': {'CVE-2018-10853': 'Andy Lutomirski and Mika Penttilä discovered that the KVM ' |
693 | + 'implementation in\n' |
694 | + 'the Linux kernel did not properly check privilege levels ' |
695 | + 'when emulating\n' |
696 | + 'some instructions. An unprivileged attacker in a guest VM ' |
697 | + 'could use this to\n' |
698 | + 'escalate privileges within the guest.', |
699 | + 'CVE-2018-6555': 'It was discovered that a use-after-free vulnerability ' |
700 | + 'existed in the IRDA\n' |
701 | + 'implementation in the Linux kernel. A local attacker could ' |
702 | + 'use this to\n' |
703 | + 'cause a denial of service (system crash) or possibly ' |
704 | + 'execute arbitrary\n' |
705 | + 'code.', |
706 | + 'CVE-2023-1010': 'Andy Lutomirski and Mika Penttilä discovered that the KVM ' |
707 | + 'implementation in\n' |
708 | + 'the Linux kernel did not properly check privilege levels ' |
709 | + 'when emulating\n' |
710 | + 'some instructions. An unprivileged attacker in a guest VM ' |
711 | + 'could use this to\n' |
712 | + 'escalate privileges within the guest.'} |
713 | + } |
714 | +] |
715 | + |
716 | +broken_descriptions = [ |
717 | + { |
718 | + 'description': |
719 | + """ |
720 | +Andy Lutomirski and Mika Penttilä discovered that the KVM implementation |
721 | +in the Linux kernel did not properly check privilege levels when emulating |
722 | +some instructions. An unprivileged attacker in a guest VM could use this to |
723 | +escalate privileges within the guest. (CVE-2018-10853) |
724 | + |
725 | +It was discovered that a use-after-free vulnerability existed in the IRDA |
726 | +implementation in the Linux kernel. A local attacker could use this to |
727 | +cause a denial of service (system crash) or possibly execute arbitrary |
728 | +code. (CVE-2018-6555, CVE-2018-6556 |
729 | + """, |
730 | + 'cves': ['CVE-2018-10853', 'CVE-2018-6555', 'CVE-2018-6556'], |
731 | + 'result': {'CVE-2018-10853': 'Andy Lutomirski and Mika Penttilä discovered that the KVM implementation in\nthe Linux kernel did not properly check privilege levels when emulating\nsome instructions. An unprivileged attacker in a guest VM could use this to\nescalate privileges within the guest.'}, |
732 | + 'error': "USN 1-1: CVE list is missing: 'It was discovered that a use-after-free vulnerability existed in the IRDA implementation in the Linux kernel. A local attacker could use this to cause a denial of service (system crash) or possibly execute arbitrary code. (CVE-2018-6555, CVE-2018-6556'\n" |
733 | + }, |
734 | + { |
735 | + 'description': |
736 | + """ |
737 | +Andy Lutomirski and Mika Penttilä discovered that the KVM implementation |
738 | +in the Linux kernel did not properly check privilege levels when emulating |
739 | +some instructions. An unprivileged attacker in a guest VM could use this to |
740 | +escalate privileges within the guest. (CVE-2018-10853) |
741 | + |
742 | +USN 3652-1 added a mitigation for Speculative Store Bypass |
743 | +a.k.a. Spectre Variant 4 (CVE-2018-3639. This update provides the |
744 | +corresponding mitigation for ARM64 processors. Please note that for |
745 | +this mitigation to be effective, an updated firmware for the processor |
746 | +may be required. |
747 | + """, |
748 | + 'cves': ['CVE-2018-10853', 'CVE-2018-3639'], |
749 | + 'result': {'CVE-2018-10853': 'Andy Lutomirski and Mika Penttilä discovered that the KVM implementation in\nthe Linux kernel did not properly check privilege levels when emulating\nsome instructions. An unprivileged attacker in a guest VM could use this to\nescalate privileges within the guest.'}, |
750 | + 'error': "USN 1-1: CVE list is missing: 'USN 3652-1 added a mitigation for Speculative Store Bypass a.k.a. Spectre Variant 4 (CVE-2018-3639. This update provides the corresponding mitigation for ARM64 processors. Please note that for this mitigation to be effective, an updated firmware for the processor may be required.'\n" |
751 | + } |
752 | +] |
753 | + |
754 | +class TestDescriptions: |
755 | + @pytest.mark.parametrize("usn", descriptions) |
756 | + @mock.patch("usn_lib.load_database") |
757 | + def test_descriptions(self, _load_database_mock, usn): |
758 | + _load_database_mock.return_value = [] |
759 | + sync_from_usns = importlib.import_module("sync-from-usns") |
760 | + fake_usn = '1-1' |
761 | + db = {fake_usn: {'description': usn['description'], 'cves': usn['cves']}} |
762 | + result = sync_from_usns.extract_cve_descriptions(db[fake_usn], fake_usn, False) |
763 | + assert result == usn['result'] |
764 | + |
765 | + @pytest.mark.parametrize("usn", broken_descriptions) |
766 | + @mock.patch("usn_lib.load_database") |
767 | + def test_descriptions_exception(self, _load_database_mock, usn, capsys): |
768 | + _load_database_mock.return_value = [] |
769 | + sync_from_usns = importlib.import_module("sync-from-usns") |
770 | + fake_usn = '1-1' |
771 | + db = {fake_usn: {'description': usn['description'], 'cves': usn['cves']}} |
772 | + result = sync_from_usns.extract_cve_descriptions(db[fake_usn], fake_usn, True) |
773 | + captured = capsys.readouterr() |
774 | + assert result == usn['result'] |
775 | + assert captured.err == usn['error'] |
Some comments (+ inline comments).
Since that piece of code is supposed to now handle CVE lists inside a "chunk", please remove the "Extract trailing ..." comment or replace with "Extract CVE list from chunk".
I am also wondering if it would be a better approach to change this code to use a regex to extract a "(CVE-XX, CVE-XX)" chunk from the text instead of manually going through the USN text chunk and doing splitting, picking parts here and there. I think it would improve readability + maintainability.
Thanks for pasting some test cases, made the review much easier than trying to figure out weird cases by myself. At this point I am wondering if it would be worth it to convert those test cases to a proper test but maybe this should not be part of this MP :)