Ubuntu CVE Tracker

Merge ~pfsmorigo/ubuntu-cve-tracker:pfsmorigo/reason_in_notes into ubuntu-cve-tracker:master

  1. Git
  2. lp:~pfsmorigo/ubuntu-cve-tracker
  3. pfsmorigo/reason_in_notes
  4. Merge into master
Proposed by Paulo Flabiano Smorigo
Status: Merged
Merged at revision: 1a98a3ca6297b5152a92307a832b3c6f1a43af73
Proposed branch: ~pfsmorigo/ubuntu-cve-tracker:pfsmorigo/reason_in_notes
Merge into: ubuntu-cve-tracker:master
Diff against target: 14 lines (+4/-0)
1 file modified
scripts/publish-cves-to-website-api.py (+4/-0)
Reviewer Review Type Date Requested Status
Ubuntu Security Team Pending
Review via email: mp+448821@code.launchpad.net

Description of the change

Add the priority reason as the top comment in the notes session like this:

https://ubuntu.com/security/CVE-2018-7409

This is a temporary solution until the new field is added by the web team.

To post a comment you must log in.
Revision history for this message
Emilia Torino (emitorino) wrote :

Leaving one question just in case, otherwise LGTM!

Revision history for this message
Steve Beattie (sbeattie) wrote :

On Wed, Aug 09, 2023 at 06:39:15PM -0000, Paulo Flabiano Smorigo wrote:
> Paulo Flabiano Smorigo has proposed merging ~pfsmorigo/ubuntu-cve-tracker:pfsmorigo/reason_in_notes into ubuntu-cve-tracker:master.
>
> Requested reviews:
> Ubuntu Security Team (ubuntu-security)
>
> For more details, see:
> https://code.launchpad.net/~pfsmorigo/ubuntu-cve-tracker/+git/ubuntu-cve-tracker/+merge/448821
>
> Add the priority reason as the top comment in the notes session like this:
>
> https://ubuntu.com/security/CVE-2018-7409
>
> This is a temporary solution until the new field is added by the web team.

You could conceivably make the author of the priority reason be the
ubuntu-security user on launchpad, but I'm not sure it's worth the
bother for a temporary fix.

I would like to see the attached patch containing testcases for putting
the priority reason in the notes field, however.

Thanks.

--
Steve Beattie
<email address hidden>

0001-publish-cves-to-website-api.py-add-priority-reason-n.patch
1From 1849959a109b7b9d57af562932f0fe32f7d00e0d Mon Sep 17 00:00:00 2001
2From: Steve Beattie <steve.beattie@canonical.com>
3Date: Thu, 10 Aug 2023 15:34:15 -0700
4Subject: [PATCH] publish-cves-to-website-api.py: add priority reason notes
5 tests
6
7Add a couple of tests to validate putting the priority reason in the
8notes field as a temporary fix. These can also be used as a basis
9for tests when the web team adds a separate priority reason field.
10
11MR: https://code.launchpad.net/~pfsmorigo/ubuntu-cve-tracker/+git/ubuntu-cve-tracker/+merge/448821
12Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
13---
14 test/website_api/use_priority_reason | 26 ++++++++
15 test/website_api/use_priority_reason.json | 56 ++++++++++++++++
16 .../use_priority_reason_plus_notes | 29 +++++++++
17 .../use_priority_reason_plus_notes.json | 64 +++++++++++++++++++
18 4 files changed, 175 insertions(+)
19 create mode 100644 test/website_api/use_priority_reason
20 create mode 100644 test/website_api/use_priority_reason.json
21 create mode 100644 test/website_api/use_priority_reason_plus_notes
22 create mode 100644 test/website_api/use_priority_reason_plus_notes.json
23
24diff --git a/test/website_api/use_priority_reason b/test/website_api/use_priority_reason
25new file mode 100644
26index 00000000000..1d542f4e061
27--- /dev/null
28+++ b/test/website_api/use_priority_reason
29@@ -0,0 +1,26 @@
30+PublicDateAtUSN: 2020-08-04 17:00:00 UTC
31+Candidate: CVE-2020-1234
32+CRD: 2020-08-04 17:00:00 UTC
33+PublicDate: 2020-08-04 17:00:00 UTC
34+References:
35+ https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1234
36+Description:
37+ Publish CVEs to Website API tests
38+Ubuntu-Description:
39+Notes:
40+Mitigation:
41+Bugs:
42+Priority: low
43+ This is the reason the priority of this CVE is low. This cve is
44+ priority low for a very good reason.
45+Discovered-by:
46+Assigned-to:
47+CVSS:
48+
49+
50+Patches_package:
51+upstream_package: needs-triage
52+trusty_package: released (1.2.3)
53+trusty/esm_package: not-affected (1.2.3)
54+jammy_package: released (4.5.6)
55+esm-apps/jammy_package: not-affected (4.5.6)
56diff --git a/test/website_api/use_priority_reason.json b/test/website_api/use_priority_reason.json
57new file mode 100644
58index 00000000000..f0fdf4b9f76
59--- /dev/null
60+++ b/test/website_api/use_priority_reason.json
61@@ -0,0 +1,56 @@
62+[
63+ {
64+ "id": "CVE-2020-1234",
65+ "description": "\nPublish CVEs to Website API tests",
66+ "ubuntu_description": "",
67+ "mitigation": "",
68+ "notes": [
69+ {
70+ "author": "",
71+ "note": "Priority reason:\nThis is the reason the priority of this CVE is low. This cve is priority low for a very good reason."
72+ }
73+ ],
74+ "priority": "low",
75+ "cvss3": null,
76+ "references": [
77+ "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1234"
78+ ],
79+ "bugs": [
80+ ""
81+ ],
82+ "packages": [
83+ {
84+ "name": "package",
85+ "source": "https://launchpad.net/ubuntu/+source/package",
86+ "ubuntu": "https://packages.ubuntu.com/search?suite=all&section=all&arch=any&searchon=sourcenames&keywords=package",
87+ "debian": "https://tracker.debian.org/pkg/package",
88+ "statuses": [
89+ {
90+ "release_codename": "trusty",
91+ "status": "released",
92+ "description": "1.2.3",
93+ "pocket": "security"
94+ },
95+ {
96+ "release_codename": "jammy",
97+ "status": "released",
98+ "description": "4.5.6",
99+ "pocket": "security"
100+ },
101+ {
102+ "release_codename": "upstream",
103+ "status": "needs-triage",
104+ "description": "",
105+ "pocket": "security"
106+ }
107+ ]
108+ }
109+ ],
110+ "status": "active",
111+ "tags": {},
112+ "patches": {
113+ "package": []
114+ },
115+ "published": "2020-08-04 17:00:00 UTC"
116+ }
117+]
118diff --git a/test/website_api/use_priority_reason_plus_notes b/test/website_api/use_priority_reason_plus_notes
119new file mode 100644
120index 00000000000..41e6cc83b7c
121--- /dev/null
122+++ b/test/website_api/use_priority_reason_plus_notes
123@@ -0,0 +1,29 @@
124+PublicDateAtUSN: 2020-08-04 17:00:00 UTC
125+Candidate: CVE-2020-1234
126+CRD: 2020-08-04 17:00:00 UTC
127+PublicDate: 2020-08-04 17:00:00 UTC
128+References:
129+ https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1234
130+Description:
131+ Publish CVEs to Website API tests
132+Ubuntu-Description:
133+Notes:
134+ pfsmorigo> this is a note
135+ sbeattie> this is another note. It contains for too many words for a
136+ note. Why are there so many words in this note?
137+Mitigation:
138+Bugs:
139+Priority: low
140+ This is the reason the priority of this CVE is low. This cve is
141+ priority low for a very good reason.
142+Discovered-by:
143+Assigned-to:
144+CVSS:
145+
146+
147+Patches_package:
148+upstream_package: needs-triage
149+trusty_package: released (1.2.3)
150+trusty/esm_package: not-affected (1.2.3)
151+jammy_package: released (4.5.6)
152+esm-apps/jammy_package: not-affected (4.5.6)
153diff --git a/test/website_api/use_priority_reason_plus_notes.json b/test/website_api/use_priority_reason_plus_notes.json
154new file mode 100644
155index 00000000000..6e4a5d06d46
156--- /dev/null
157+++ b/test/website_api/use_priority_reason_plus_notes.json
158@@ -0,0 +1,64 @@
159+[
160+ {
161+ "id": "CVE-2020-1234",
162+ "description": "\nPublish CVEs to Website API tests",
163+ "ubuntu_description": "",
164+ "mitigation": "",
165+ "notes": [
166+ {
167+ "author": "",
168+ "note": "Priority reason:\nThis is the reason the priority of this CVE is low. This cve is priority low for a very good reason."
169+ },
170+ {
171+ "author": "pfsmorigo",
172+ "note": "this is a note"
173+ },
174+ {
175+ "author": "sbeattie",
176+ "note": "this is another note. It contains for too many words for a\nnote. Why are there so many words in this note?"
177+ }
178+ ],
179+ "priority": "low",
180+ "cvss3": null,
181+ "references": [
182+ "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1234"
183+ ],
184+ "bugs": [
185+ ""
186+ ],
187+ "packages": [
188+ {
189+ "name": "package",
190+ "source": "https://launchpad.net/ubuntu/+source/package",
191+ "ubuntu": "https://packages.ubuntu.com/search?suite=all&section=all&arch=any&searchon=sourcenames&keywords=package",
192+ "debian": "https://tracker.debian.org/pkg/package",
193+ "statuses": [
194+ {
195+ "release_codename": "trusty",
196+ "status": "released",
197+ "description": "1.2.3",
198+ "pocket": "security"
199+ },
200+ {
201+ "release_codename": "jammy",
202+ "status": "released",
203+ "description": "4.5.6",
204+ "pocket": "security"
205+ },
206+ {
207+ "release_codename": "upstream",
208+ "status": "needs-triage",
209+ "description": "",
210+ "pocket": "security"
211+ }
212+ ]
213+ }
214+ ],
215+ "status": "active",
216+ "tags": {},
217+ "patches": {
218+ "package": []
219+ },
220+ "published": "2020-08-04 17:00:00 UTC"
221+ }
222+]
223--
2242.40.1
Revision history for this message
Steve Beattie (sbeattie) wrote :

On Thu, Aug 10, 2023 at 08:35:58PM -0000, Emilia Torino wrote:
> Leaving one question just in case, otherwise LGTM!
>
> Diff comments:
>
> > diff --git a/scripts/publish-cves-to-website-api.py b/scripts/publish-cves-to-website-api.py
> > index d6af0e7..d5a0042 100755
> > --- a/scripts/publish-cves-to-website-api.py
> > +++ b/scripts/publish-cves-to-website-api.py
> > @@ -163,6 +163,10 @@ def post_single_cve(cve_filename):
> >
> > notes = []
> >
> > + # TODO Remove this when we have the proper field ir ready
>
> s/ir/is? Seems a typo :), also if thats the case maybe "we have" should be removed? So either remove everything after field, or remove "we have" I guess
>
> > + if cve_data["Priority"][1]:
>
> Should we check cve_data["Priority"] length before accessing the reason at index 1?

In hindsight, it would have been useful to have hidden the accessors behind an
api in cve_lib, something like

  cve_lib.cve_get_priority(cve)
  cve_lib.cve_get_priority_reason(cve)

so that we can move to a future where the internal structures of cves
are hidden in a class. That said, I don't know how to make the api
export the case where we have a different priority for a specific
package or release.

> > + notes.append({"author": "", "note": "Priority reason:\n" + cve_data["Priority"][1]})
> > +
> > for [author, note] in cve_data["Notes"]:
> > notes.append({"author": author, "note": note})
> >
>
>
> --
> https://code.launchpad.net/~pfsmorigo/ubuntu-cve-tracker/+git/ubuntu-cve-tracker/+merge/448821
> You are subscribed to branch ubuntu-cve-tracker:master.
>

--
Steve Beattie
<email address hidden>

Revision history for this message
Steve Beattie (sbeattie) wrote :

Wow, emailing a patch attachment really confused the merge request interface.

Anyway, my attempts to create testcases resulted in the following merge request:
https://code.launchpad.net/~sbeattie/ubuntu-cve-tracker/+git/ubuntu-cve-tracker/+merge/448958

Revision history for this message
Paulo Flabiano Smorigo (pfsmorigo) wrote :

> Leaving one question just in case, otherwise LGTM!

Fixed the mistypo and added a check as you suggested. I added it anyway but, to be honest, the check is not really necessary since the element is always a tuplet with two elements:

>>> cveinfo["CVE-2019-11840"]["Priority"]
['medium', '']
>>> cveinfo["CVE-2019-11840"]["Priority_snapd"]
['negligible', '']
>>> cveinfo["CVE-2007-6752"]["Priority"]
['low', '']

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/scripts/publish-cves-to-website-api.py b/scripts/publish-cves-to-website-api.py
2index d6af0e7..4bb95c7 100755
3--- a/scripts/publish-cves-to-website-api.py
4+++ b/scripts/publish-cves-to-website-api.py
5@@ -163,6 +163,10 @@ def post_single_cve(cve_filename):
6
7 notes = []
8
9+ # TODO Remove this when we have the proper field is ready
10+ if len(cve_data["Priority"]) > 1 and cve_data["Priority"][1]:
11+ notes.append({"author": "", "note": "Priority reason:\n" + cve_data["Priority"][1]})
12+
13 for [author, note] in cve_data["Notes"]:
14 notes.append({"author": author, "note": note})
15

Subscribers

People subscribed via source and target branches