Merge ~alexmurray/ubuntu-cve-tracker:misc-cve-lib-and-check-syntax-cleanups into ubuntu-cve-tracker:master

Proposed by Alex Murray
Status: Merged
Merged at revision: b7100a0074408e315ffae2395875e69036d3044d
Proposed branch: ~alexmurray/ubuntu-cve-tracker:misc-cve-lib-and-check-syntax-cleanups
Merge into: ubuntu-cve-tracker:master
Diff against target: 434 lines (+94/-68)
3 files modified
scripts/check-syntax (+15/-1)
scripts/cve-mode.el (+2/-2)
scripts/cve_lib.py (+77/-65)
Reviewer Review Type Date Requested Status
Eduardo Barretto Approve
Review via email: mp+440925@code.launchpad.net

Description of the change

A bunch of miscellaneous fixups to get some bits of check-syntax working betterer

To post a comment you must log in.
Revision history for this message
Seth Arnold (seth-arnold) wrote :

I added one comment on the /proc/pid/fd/0 symlink-breaking -- and did a very quick skim on the rest; it seemed fine, but also like the sort of mechanical thing that would be easy to overlook a mistake.

Thanks

Revision history for this message
Alex Murray (alexmurray) wrote :

Thanks Seth - not sure I understand your comment so if you could help explain it for me that would be great.

Revision history for this message
Seth Arnold (seth-arnold) :
Revision history for this message
Alex Murray (alexmurray) :
Revision history for this message
Steve Beattie (sbeattie) :
Revision history for this message
Eduardo Barretto (ebarretto) wrote :

hey Alex, is this still needed?

review: Needs Information
Revision history for this message
Alex Murray (alexmurray) wrote :

Added --stdin as first class argument to check-syntax and fixed up other bits. Yes this is still needed. As can be seen the check-syntax lpci job fails as expected currently showing check-syntax still works for the regular use-case as well as this new --stdin one (which is used by the flymake integration for emacs in cve-mode.el)

Revision history for this message
Alex Murray (alexmurray) wrote :

Any chance to get a review on this? Otherwise I'll look to merge it tomorrow during CVE triage.

Revision history for this message
Eduardo Barretto (ebarretto) wrote :

cve_lib and check-syntax changes lgtm!
thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/scripts/check-syntax b/scripts/check-syntax
2index f89a0e4..7d401f6 100755
3--- a/scripts/check-syntax
4+++ b/scripts/check-syntax
5@@ -261,6 +261,13 @@ parser.add_option(
6 help="Number of jobs to run simultaneously (default: %d)" % multiprocessing.cpu_count(),
7 default=multiprocessing.cpu_count(),
8 )
9+parser.add_option(
10+ "--stdin",
11+ help="Check the contents of stdin instead of a filelist or modified files etc",
12+ action="store_true",
13+ default=False,
14+)
15+
16 # parser.add_option("-c", "--cna", help="Ensure every CVE assigned by Canonical's CNA has an entry", action='store_true')
17 (opt, args) = parser.parse_args()
18
19@@ -304,7 +311,11 @@ if os.path.islink(cve_lib.embargoed_dir):
20 debug("check_dirs %s" % check_dirs)
21
22 all_files = True
23-if len(args) == 0:
24+if opt.stdin:
25+ debug("Using /dev/stdin as input")
26+ args = ["/dev/stdin"]
27+ all_files = False
28+elif len(args) == 0:
29 if opt.filelist:
30 debug("Using filelist %s" % opt.filelist)
31
32@@ -525,6 +536,9 @@ def check_cve(cve):
33 except ValueError as e:
34 print(e, file=sys.stderr)
35 return False
36+ # get the real CVE if we are using stdin
37+ if opt.stdin:
38+ cve = data["Candidate"]
39 if cve in ignored:
40 print(
41 "%s: %d: duplicate CVE found in not-for-us.txt" % (cvepath, 1),
42diff --git a/scripts/cve-mode.el b/scripts/cve-mode.el
43index b8d8230..4f5797e 100644
44--- a/scripts/cve-mode.el
45+++ b/scripts/cve-mode.el
46@@ -683,7 +683,7 @@ cross boundaries of block literals."
47 :name "cve-mode-flymake" :noquery t :connection-type 'pipe
48 ;; Make output go to a temporary buffer.
49 :buffer (generate-new-buffer " *cve-mode-flymake*")
50- :command `(,cve-mode--check-syntax-executable "/dev/stdin")
51+ :command `(,cve-mode--check-syntax-executable "--stdin")
52 :sentinel
53 (lambda (proc _event)
54 (when (memq (process-status proc) '(exit signal))
55@@ -723,7 +723,7 @@ cross boundaries of block literals."
56
57 ;;;###autoload
58 (defun cve-mode-setup-flymake-backend ()
59- "Setuo the `flymake' backend for `cve-mode'."
60+ "Setup the `flymake' backend for `cve-mode'."
61 (add-hook 'flymake-diagnostic-functions 'cve-mode-flymake nil t))
62
63 ;;;###autoload
64diff --git a/scripts/cve_lib.py b/scripts/cve_lib.py
65index 416d5f7..811ea55 100755
66--- a/scripts/cve_lib.py
67+++ b/scripts/cve_lib.py
68@@ -980,8 +980,8 @@ def find_external_subproject_cves(cve):
69 # top-level project directory even though this is preferred
70 for d in [get_external_subproject_cve_dir(rel),
71 get_external_subproject_dir(rel)]:
72- path = os.path.join(d, cve)
73- if os.path.exists(path):
74+ path = os.path.realpath(os.path.join(d, cve))
75+ if os.path.exists(path) and path not in cves:
76 cves.append(path)
77 return cves
78
79@@ -1076,24 +1076,26 @@ def load_external_subprojects(strict=False):
80 if project:
81 subprojects[subproject].setdefault("customer", project)
82
83-load_external_subprojects()
84+ # now ensure they are consistent
85+ global devel_release
86+ for release in subprojects:
87+ details = subprojects[release]
88+ rel = release_alias(release)
89+ # prefer the alias name
90+ all_releases.append(rel)
91+ if details["eol"]:
92+ eol_releases.append(rel)
93+ if "devel" in details and details["devel"]:
94+ if devel_release != "":
95+ raise ValueError("there can be only one ⚔ devel")
96+ devel_release = rel
97+ # ubuntu specific releases
98+ product, _ = product_series(release)
99+ if product == PRODUCT_UBUNTU:
100+ releases.append(rel)
101
102-for release in subprojects:
103- details = subprojects[release]
104- rel = release_alias(release)
105- # prefer the alias name
106- all_releases.append(rel)
107- if details["eol"]:
108- eol_releases.append(rel)
109- if "devel" in details and details["devel"]:
110- if devel_release != "":
111- raise ValueError("there can be only one ⚔ devel")
112- devel_release = rel
113- # ubuntu specific releases
114- product, series = product_series(release)
115- if product == PRODUCT_UBUNTU:
116- releases.append(rel)
117
118+load_external_subprojects()
119
120 def release_sort(release_list):
121 '''takes a list of release names and sorts them in release order
122@@ -2099,7 +2101,7 @@ def find_cve(cve):
123 # e.g.: git/github.com/gogo/protobuf_gogoprotobuf: needs-triage
124 # This method should keep supporting existing current format:
125 # e.g.: bionic_jackson-databind: needs-triage
126-def parse_cve_release_package_field(cve, field, data, value, code, msg, linenum):
127+def parse_cve_release_package_field(cvefile, field, data, value, code, msg, linenum):
128 package = ""
129 release = ""
130 state = ""
131@@ -2107,14 +2109,14 @@ def parse_cve_release_package_field(cve, field, data, value, code, msg, linenum)
132 try:
133 release, package = field.split('_', 1)
134 except ValueError:
135- msg += "%s: %d: bad field with '_': '%s'\n" % (cve, linenum, field)
136+ msg += "%s: %d: bad field with '_': '%s'\n" % (cvefile, linenum, field)
137 code = EXIT_FAIL
138 return False, package, release, state, details, code, msg
139
140 try:
141 info = value.split(' ', 1)
142 except ValueError:
143- msg += "%s: %d: missing state for '%s': '%s'\n" % (cve, linenum, field, value)
144+ msg += "%s: %d: missing state for '%s': '%s'\n" % (cvefile, linenum, field, value)
145 code = EXIT_FAIL
146 return False, package, release, state, details, code, msg
147
148@@ -2128,7 +2130,7 @@ def parse_cve_release_package_field(cve, field, data, value, code, msg, linenum)
149 details = info[1].strip()
150
151 if details.startswith("["):
152- msg += "%s: %d: %s has details that starts with a bracket: '%s'\n" % (cve, linenum, field, details)
153+ msg += "%s: %d: %s has details that starts with a bracket: '%s'\n" % (cvefile, linenum, field, details)
154 code = EXIT_FAIL
155 return False, package, release, state, details, code, msg
156
157@@ -2144,19 +2146,19 @@ def parse_cve_release_package_field(cve, field, data, value, code, msg, linenum)
158
159 valid_states = ['needs-triage', 'needed', 'active', 'pending', 'released', 'deferred', 'DNE', 'ignored', 'not-affected']
160 if state not in valid_states:
161- msg += "%s: %d: %s has unknown state: '%s' (valid states are: %s)\n" % (cve, linenum, field, state,
162+ msg += "%s: %d: %s has unknown state: '%s' (valid states are: %s)\n" % (cvefile, linenum, field, state,
163 ' '.join(valid_states))
164 code = EXIT_FAIL
165 return False, package, release, state, details, code, msg
166
167 # Verify "released" kernels have version details
168 #if state == 'released' and package in kernel_srcs and details == '':
169- # msg += "%s: %s_%s has state '%s' but lacks version note\n" % (cve, package, release, state)
170+ # msg += "%s: %s_%s has state '%s' but lacks version note\n" % (cvefile, package, release, state)
171 # code = EXIT_FAIL
172
173 # Verify "active" states have an Assignee
174 if state == 'active' and data['Assigned-to'].strip() == "":
175- msg += "%s: %d: %s has state '%s' but lacks 'Assigned-to'\n" % (cve, linenum, field, state)
176+ msg += "%s: %d: %s has state '%s' but lacks 'Assigned-to'\n" % (cvefile, linenum, field, state)
177 code = EXIT_FAIL
178 return False, package, release, state, details, code, msg
179
180@@ -2247,16 +2249,25 @@ def amend_external_subproject_pkg(cve, data, srcmap, amendments, code, msg):
181 return code, msg
182
183 if '_' in field:
184- success, pkg, release, state, details, code, msg = parse_cve_release_package_field(cve, field, data, value, code, msg, linenum)
185+ success, pkg, rel, state, details, code, msg = parse_cve_release_package_field(cve, field, data, value, code, msg, linenum)
186 if not success:
187 return code, msg
188
189+ canon, _, _, _ = get_subproject_details(rel)
190+ if canon is None and rel not in ['upstream', 'devel']:
191+ msg += "%s: %d: unknown entry '%s'\n" % (cve, linenum, rel)
192+ code = EXIT_FAIL
193+ return code, msg
194 data.setdefault("pkgs", dict())
195 data["pkgs"].setdefault(pkg, dict())
196 srcmap["pkgs"].setdefault(pkg, dict())
197- # override existing release info if it exists
198- data["pkgs"][pkg][release] = [state, details]
199- srcmap["pkgs"][pkg][release] = (cve, linenum)
200+ if rel in data["pkgs"][pkg]:
201+ msg += ("%s: %d: duplicate entry for '%s': original at %s line %d (%s)\n"
202+ % (cve, linenum, rel, srcmap['pkgs'][pkg][rel][0], srcmap['pkgs'][pkg][rel][1], data["pkgs"][pkg][rel]))
203+ code = EXIT_FAIL
204+ return code, msg
205+ data["pkgs"][pkg][rel] = [state, details]
206+ srcmap["pkgs"][pkg][rel] = (cve, linenum)
207
208 return code, msg
209
210@@ -2271,7 +2282,7 @@ def load_external_subproject_cve_data(cve, data, srcmap, code, msg):
211
212 return code, msg
213
214-def load_cve(cve, strict=False, srcmap=None):
215+def load_cve(cvefile, strict=False, srcmap=None):
216 '''Loads a given CVE into:
217 dict( fields...
218 'pkgs' -> dict( pkg -> dict( release -> (state, details) ) )
219@@ -2298,13 +2309,13 @@ def load_cve(cve, strict=False, srcmap=None):
220 affected = dict()
221 lastfield = ""
222 fields_seen = []
223- if not os.path.exists(cve):
224- raise ValueError("File does not exist: '%s'" % (cve))
225+ if not os.path.exists(cvefile):
226+ raise ValueError("File does not exist: '%s'" % cvefile)
227 linenum = 0
228 notes_parser = NotesParser()
229 priority_reason = {}
230 cvss_entries = []
231- with codecs.open(cve, encoding="utf-8") as inF:
232+ with codecs.open(cvefile, encoding="utf-8") as inF:
233 lines = inF.readlines()
234 for line in lines:
235 line = line.rstrip()
236@@ -2317,7 +2328,7 @@ def load_cve(cve, strict=False, srcmap=None):
237 try:
238 # parse Notes properly
239 if lastfield == 'Notes':
240- code, newmsg = notes_parser.parse_line(cve, line, linenum, code)
241+ code, newmsg = notes_parser.parse_line(cvefile, line, linenum, code)
242 if code != EXIT_OKAY:
243 msg += newmsg
244 elif lastfield.startswith('Priority'):
245@@ -2333,9 +2344,9 @@ def load_cve(cve, strict=False, srcmap=None):
246 patch_type = patch_type.strip()
247 entry = entry.strip()
248 data['patches'][pkg].append((patch_type, entry))
249- srcmap['patches'][pkg].append((cve, linenum))
250+ srcmap['patches'][pkg].append((cvefile, linenum))
251 except Exception as e:
252- msg += "%s: %d: Failed to parse '%s' entry %s: %s\n" % (cve, linenum, lastfield, line, e)
253+ msg += "%s: %d: Failed to parse '%s' entry %s: %s\n" % (cvefile, linenum, lastfield, line, e)
254 code = EXIT_FAIL
255 elif lastfield == 'CVSS':
256 try:
257@@ -2358,36 +2369,36 @@ def load_cve(cve, strict=False, srcmap=None):
258 # to a dict first if needed
259 if type(srcmap["CVSS"]) is tuple:
260 srcmap["CVSS"] = dict()
261- srcmap["CVSS"].setdefault(cvss['source'], (cve, linenum))
262+ srcmap["CVSS"].setdefault(cvss['source'], (cvefile, linenum))
263 except Exception as e:
264- msg += "%s: %d: Failed to parse CVSS: %s\n" % (cve, linenum, e)
265+ msg += "%s: %d: Failed to parse CVSS: %s\n" % (cvefile, linenum, e)
266 code = EXIT_FAIL
267 else:
268 data[lastfield] += '\n%s' % (line[1:])
269 except KeyError as e:
270- msg += "%s: %d: bad line '%s' (%s)\n" % (cve, linenum, line, e)
271+ msg += "%s: %d: bad line '%s' (%s)\n" % (cvefile, linenum, line, e)
272 code = EXIT_FAIL
273 continue
274
275 try:
276 field, value = line.split(':', 1)
277 except ValueError as e:
278- msg += "%s: %d: bad line '%s' (%s)\n" % (cve, linenum, line, e)
279+ msg += "%s: %d: bad line '%s' (%s)\n" % (cvefile, linenum, line, e)
280 code = EXIT_FAIL
281 continue
282
283 lastfield = field = field.strip()
284 if field in fields_seen:
285- msg += "%s: %d: repeated field '%s'\n" % (cve, linenum, field)
286+ msg += "%s: %d: repeated field '%s'\n" % (cvefile, linenum, field)
287 code = EXIT_FAIL
288 else:
289 fields_seen.append(field)
290 value = value.strip()
291 if field == 'Candidate':
292 data.setdefault(field, value)
293- srcmap.setdefault(field, (cve, linenum))
294+ srcmap.setdefault(field, (cvefile, linenum))
295 if value != "" and not value.startswith('CVE-') and not value.startswith('UEM-') and not value.startswith('EMB-'):
296- msg += "%s: %d: unknown Candidate '%s' (must be /(CVE|UEM|EMB)-/)\n" % (cve, linenum, value)
297+ msg += "%s: %d: unknown Candidate '%s' (must be /(CVE|UEM|EMB)-/)\n" % (cvefile, linenum, value)
298 code = EXIT_FAIL
299 elif 'Priority' in field:
300 # For now, throw away comments on Priority fields
301@@ -2397,26 +2408,26 @@ def load_cve(cve, strict=False, srcmap=None):
302 try:
303 _, pkg = field.split('_', 1)
304 except ValueError:
305- msg += "%s: %d: bad field with 'Priority_': '%s'\n" % (cve, linenum, field)
306+ msg += "%s: %d: bad field with 'Priority_': '%s'\n" % (cvefile, linenum, field)
307 code = EXIT_FAIL
308 continue
309 # initially set the priority reason as an empty string - this will
310 # be fixed up later with a real value if one is found
311 data.setdefault(field, [value, ""])
312- srcmap.setdefault(field, (cve, linenum))
313+ srcmap.setdefault(field, (cvefile, linenum))
314 if value not in ['untriaged', 'not-for-us'] + priorities:
315- msg += "%s: %d: unknown Priority '%s'\n" % (cve, linenum, value)
316+ msg += "%s: %d: unknown Priority '%s'\n" % (cvefile, linenum, value)
317 code = EXIT_FAIL
318 elif 'Patches_' in field:
319 try:
320 _, pkg = field.split('_', 1)
321 except ValueError:
322- msg += "%s: %d: bad field with 'Patches_': '%s'\n" % (cve, linenum, field)
323+ msg += "%s: %d: bad field with 'Patches_': '%s'\n" % (cvefile, linenum, field)
324 code = EXIT_FAIL
325 continue
326 # value should be empty
327 if len(value) > 0:
328- msg += "%s: %d: '%s' field should have no value\n" % (cve, linenum, field)
329+ msg += "%s: %d: '%s' field should have no value\n" % (cvefile, linenum, field)
330 code = EXIT_FAIL
331 continue
332 data['patches'].setdefault(pkg, list())
333@@ -2426,41 +2437,42 @@ def load_cve(cve, strict=False, srcmap=None):
334 try:
335 _, pkg = field.split('_', 1)
336 except ValueError:
337- msg += "%s: %d: bad field with 'Tags_': '%s'\n" % (cve, linenum, field)
338+ msg += "%s: %d: bad field with 'Tags_': '%s'\n" % (cvefile, linenum, field)
339 code = EXIT_FAIL
340 continue
341 data['tags'].setdefault(pkg, set())
342- srcmap['tags'].setdefault(pkg, (cve, linenum))
343+ srcmap['tags'].setdefault(pkg, (cvefile, linenum))
344 for word in value.strip().split(' '):
345 if word not in valid_tags:
346- msg += "%s: %d: invalid tag '%s': '%s'\n" % (cve, linenum, word, field)
347+ msg += "%s: %d: invalid tag '%s': '%s'\n" % (cvefile, linenum, word, field)
348 code = EXIT_FAIL
349 continue
350 data['tags'][pkg].add(word)
351 elif '_' in field:
352- success, pkg, rel, state, details, code, msg = parse_cve_release_package_field(cve, field, data, value, code, msg, linenum)
353+ success, pkg, rel, state, details, code, msg = parse_cve_release_package_field(cvefile, field, data, value, code, msg, linenum)
354 if not success:
355 assert(code == EXIT_FAIL)
356 continue
357 canon, _, _, _ = get_subproject_details(rel)
358 if canon is None and rel not in ['upstream', 'devel']:
359- msg += "%s: %d: unknown entry '%s'\n" % (cve, linenum, rel)
360+ msg += "%s: %d: unknown entry '%s'\n" % (cvefile, linenum, rel)
361 code = EXIT_FAIL
362 continue
363 affected.setdefault(pkg, dict())
364 if rel in affected[pkg]:
365- msg += "%s: %d: duplicate entry for '%s': original at line %d\n" % (cve, linenum, rel, srcmap['pkgs'][pkg][rel][1])
366+ msg += ("%s: %d: duplicate entry for '%s': original at %s line %d\n"
367+ % (cvefile, linenum, rel, srcmap['pkgs'][pkg][rel][0], srcmap['pkgs'][pkg][rel][1]))
368 code = EXIT_FAIL
369 continue
370 affected[pkg].setdefault(rel, [state, details])
371 srcmap['pkgs'].setdefault(pkg, dict())
372- srcmap['pkgs'][pkg].setdefault(rel, (cve, linenum))
373+ srcmap['pkgs'][pkg].setdefault(rel, (cvefile, linenum))
374 elif field not in required_fields + extra_fields:
375- msg += "%s: %d: unknown field '%s'\n" % (cve, linenum, field)
376+ msg += "%s: %d: unknown field '%s'\n" % (cvefile, linenum, field)
377 code = EXIT_FAIL
378 else:
379 data.setdefault(field, value)
380- srcmap.setdefault(field, (cve, linenum))
381+ srcmap.setdefault(field, (cvefile, linenum))
382
383 data['Notes'] = notes_parser.finalize()
384 data['CVSS'] = cvss_entries
385@@ -2468,28 +2480,28 @@ def load_cve(cve, strict=False, srcmap=None):
386 # Check for required fields
387 for field in required_fields:
388 # boilerplate files are special and can (should?) be empty
389- nonempty = [] if "boilerplate" in cve else ['Candidate']
390+ nonempty = [] if "boilerplate" in cvefile else ['Candidate']
391 if strict:
392 nonempty += ['PublicDate']
393
394 if field not in data or field not in fields_seen:
395- msg += "%s: %d: missing field '%s'\n" % (cve, linenum, field)
396+ msg += "%s: %d: missing field '%s'\n" % (cvefile, linenum, field)
397 code = EXIT_FAIL
398 elif field in nonempty and data[field].strip() == "":
399- msg += "%s: %d: required field '%s' is empty\n" % (cve, linenum, field)
400+ msg += "%s: %d: required field '%s' is empty\n" % (cvefile, linenum, field)
401 code = EXIT_FAIL
402
403 # Fill in defaults for missing fields
404 if 'Priority' not in data:
405 data.setdefault('Priority', ['untriaged'])
406- srcmap.setdefault('Priority', (cve, 1))
407+ srcmap.setdefault('Priority', (cvefile, 1))
408 # Perform override fields
409 if 'PublicDateAtUSN' in data:
410 data['PublicDate'] = data['PublicDateAtUSN']
411 srcmap['PublicDate'] = srcmap['PublicDateAtUSN']
412 if 'CRD' in data and data['CRD'].strip() != '' and data['PublicDate'] != data['CRD']:
413- if cve.startswith("embargoed"):
414- print("%s: %d: adjusting PublicDate to use CRD: %s" % (cve, linenum, data['CRD']), file=sys.stderr)
415+ if cvefile.startswith("embargoed"):
416+ print("%s: %d: adjusting PublicDate to use CRD: %s" % (cvefile, linenum, data['CRD']), file=sys.stderr)
417 data['PublicDate'] = data['CRD']
418 srcmap['PublicDate'] = srcmap['CRD']
419
420@@ -2510,12 +2522,12 @@ def load_cve(cve, strict=False, srcmap=None):
421 if rel not in external_releases:
422 needs_upstream = True
423 if needs_upstream and 'upstream' not in affected[pkg]:
424- msg += "%s: %d: missing upstream '%s'\n" % (cve, linenum, pkg)
425+ msg += "%s: %d: missing upstream '%s'\n" % (cvefile, linenum, pkg)
426 code = EXIT_FAIL
427
428 data['pkgs'] = affected
429
430- code, msg = load_external_subproject_cve_data(cve, data, srcmap, code, msg)
431+ code, msg = load_external_subproject_cve_data(cvefile, data, srcmap, code, msg)
432
433 if code != EXIT_OKAY:
434 raise ValueError(msg.strip())

Subscribers

People subscribed via source and target branches