Merge ubuntu-cve-tracker:making_this_only_opt into ubuntu-cve-tracker:master

[Leonidas S. Barbosa]

Making this_only_affected feature opt and fixing minor issues

kernel updates are one example of usn generating that does not require this kind of feature. So, making it opt is the better approach. This patch also fixes some small issues.

[Steve Beattie (sbeattie)]

On Thu, Apr 21, 2022 at 02:25:44AM -0000, Leonidas S. Barbosa wrote:
> Leonidas S. Barbosa has proposed merging ubuntu-cve-tracker:making_this_only_opt into ubuntu-cve-tracker:master.
>
> Commit message:
> Making this_only_affected feature opt and fixing minor issues
>
> For more details, see:
> https://code.launchpad.net/~ubuntu-security/ubuntu-cve-tracker/+git/ubuntu-cve-tracker/+merge/419989
>
> kernel updates are one example of usn generating that does not require this kind of feature. So, making it opt is the better approach. This patch also fixes some small issues.

Thanks, I'm likely going to merge this as-is because I need it to finish
publishing kernels, but some comments and things that need fixing:

  - I am okay with making the "only affected" check being the default,
    have a --skip-only-affects-check or some such flag for opting-out;
    I can add that flag to the scripts/prepare-kernel-usn.py script
    that wraps sis-generate-usn (amongst other things).

  - When I generate a USN for one kernel in focal and another in bionic
    (different sources, same set of CVEs fixed), the resulting calls to
    to pull-usn-desc.py ed up getting passed the following releases to
    check: ['bionic', 'bionic', 'bionic', 'focal', 'focal', 'focal']
    This is because there are three source packages associated with each
    (signed) kernel that need to have binary packages they generate
    incorporated into the USN, but no de-duplication of the releases
    occurs.

  - For similar reasons, when I enable the "affected check" on the
    same set of two kernel source packages, the pull-usn-desc.py
    throws a traceback, because it can't find source packages like
    'linux-meta' and 'linux-signed' in the CVE file. This probably
    should be handled a little more gracefully, though for non-kernel
    situations where someone is trying to publish an emergency update
    where the cve entry doesn't exist or doesn't include the source
    package being updated, the disabling option can be given.

As a general rule, if someone is publishing updates that the touch
multiple source packages and have distinct sets of CVEs for the
same release, then for simplicity of data handling, they should be
separate USNs.

There's some work that could be done to make this work when applied
against kernel updates.

Thanks.

> --
> Your team Ubuntu Security Team is requested to review the proposed merge of ubuntu-cve-tracker:making_this_only_opt into ubuntu-cve-tracker:master.

> diff --git a/scripts/pull-usn-desc.py b/scripts/pull-usn-desc.py
> index c38824b..6bb6ce1 100755
> --- a/scripts/pull-usn-desc.py
> +++ b/scripts/pull-usn-desc.py
> @@ -39,6 +39,8 @@ opter = optparse.OptionParser()
> opter.add_option("--prioritize", help="Display 'critical' and 'high' first, negligible last", action='store_true')
> opter.add_option("--cve", metavar="CVE-YYYY-NNNN", help="Request a given CVE's description or template", action='append', default=[])
> opter.add_option("--releases", help="List of releases CVEs affect to be filter used in description", action='append', default=[])
> +opter.add_option("--this-only-affected", help="Makes this only affected feature option...

diff --git a/scripts/pull-usn-desc.py b/scripts/pull-usn-desc.py
index c38824b..6bb6ce1 100755
--- a/scripts/pull-usn-desc.py
+++ b/scripts/pull-usn-desc.py
@@ -39,6 +39,8 @@ opter = optparse.OptionParser()
 opter.add_option("--prioritize", help="Display 'critical' and 'high' first, negligible last", action='store_true')
 opter.add_option("--cve", metavar="CVE-YYYY-NNNN", help="Request a given CVE's description or template", action='append', default=[])
 opter.add_option("--releases", help="List of releases CVEs affect to be filter used in description", action='append', default=[])
+opter.add_option("--this-only-affected", help="Makes this only affected feature optional", action='store_true')
+opter.add_option("--src", help="The package source to be check through if --this-only-affected is used", action='append', default=[])
 opter.add_option("--embargoed", help="Use the embargoed tree to look for desctiptions in addition", action='store_true')
 (opt, args) = opter.parse_args()
 
@@ -49,7 +51,7 @@ found = []
 # This function cross the provided releases info with CVE files info
 # in order to find 'This only affected <releases>' info and returns it
 # to creates the CVE description.
-def only_affected(cve_data, cve_number, releases):
+def only_affected(cve_data, cve_number, releases, srcs):
     pkgs = cve_data[cve_number]['pkgs']
     affects = {}
 
@@ -58,7 +60,7 @@ def only_affected(cve_data, cve_number, releases):
         releases.remove('esm-infra/trusty')
         releases.append('trusty/esm')
 
-    for pkg in pkgs:
+    for pkg in srcs:
         has_releases = pkgs[pkg].keys() & set(releases)
         for release in has_releases:
             pkg_release = pkgs[pkg][release][0]
@@ -76,15 +78,16 @@ def only_affected(cve_data, cve_number, releases):
                 affects_values.sort()
                 txt = ", ".join(affects_values[:-1])
                 txt += ', and {}'.format(affects_values[-1:][0])
-                return ". This issue only affected {}.".format(txt)
+                return " This issue only affected {}.".format(txt)
             else:
-                return ". This issue only affected {}.".format(list(affects.values())[0])
+                return " This issue only affected {}.".format(list(affects.values())[0])
 
     return ""
 
 
 rc = 0
 empty = set()
+affected_txt = ""
 for cve in opt.cve + args:
     if cve.endswith(','):
         cve = cve[:-1]
@@ -98,9 +101,8 @@ for cve in opt.cve + args:
     filename = get_filename(cve, use_embargoed=(opt.embargoed == True))
     if os.path.exists(filename):
         cves[cve] = cve_lib.load_cve(filename)
-        affected_txt = ""
-        if opt.releases:
-            affected_txt = only_affected(cves, cve, opt.releases)
+        if opt.this_only_affected and opt.releases and opt.src:
+            affected_txt = only_affected(cves, cve, opt.releases, opt.src)
         chunks = cves[cve]
 
         desc = chunks['Ubuntu-Description'].strip()
@@ -118,6 +120,10 @@ for cve in opt.cve + args:
         rc = 1
         desc = "XXX-FIXME-%s-NOT-KNOWN-TO-TRACKER-XXX" % (cve)
 
+    # some descriptions has a period others don't, so, check for it.
+    if not desc.endswith('.') and affected_txt:
+        affected_txt = '.' + affected_txt
+
     descriptions[cve] = desc + affected_txt
     found.append(cve)
 
diff --git a/scripts/sis-generate-usn b/scripts/sis-generate-usn
index 864f25f..dd3c1b4 100755
--- a/scripts/sis-generate-usn
+++ b/scripts/sis-generate-usn
@@ -36,6 +36,7 @@ opter.add_option("--ignore-cves", metavar="CVES", help="Comma separated list of 
 opter.add_option("--embargoed", help="Include embargoed directory when looking for CVE descriptions", action='store_true')
 opter.add_option("--include-eol", help="Include EoL releases", action='store_true')
 opter.add_option("--binaries-json", help="Path to JSON mapping of binary packages to versions (can repeat, default: binaries.json)", action='append', default=[])
+opter.add_option("--this-only-affected", help="Makes this only affected feature optional", action='store_true')
 (opt, args) = opter.parse_args()
 
 if len(args) < 2:
@@ -669,18 +670,22 @@ releases.sort()
 # Building a release filter list with the current info regarding if it is
 # esm or not so we can properly filter in it in pull-usn-desc.py --releases
 releases_filter = []
-for pkg in binaries.keys():
-    for release in  binaries[pkg].keys():
-        for _pkg in binaries[pkg][release].keys():
-            pocket = binaries[pkg][release][_pkg]['pocket']
-            if 'esm' in pocket:
-                releases_filter.append(pocket + '/' + release)
-            else:
-                releases_filter.append(release)
-            # We just need one info as it reflects the release so skip all
-            # pkgs into binaries
-            break
+srcs_list =[]
+if opt.this_only_affected:
+    for pkg in binaries.keys():
+        for release in  binaries[pkg].keys():
+            for _pkg in binaries[pkg][release].keys():
+                pocket = binaries[pkg][release][_pkg]['pocket']
+                if 'esm' in pocket:
+                    releases_filter.append(pocket + '/' + release)
+                else:
+                    releases_filter.append(release)
+                # We just need one info as it reflects the release so skip all
+                # pkgs into binaries
+                break
 
+    for src in srcs:
+        srcs_list.append(src)
 
 print('# title: used for Email Subject, Web title. XXX-EXPAND-TO-UPSTREAM-NAME-XXX')
 print('# summary: used inside USN, should be package names')
@@ -702,10 +707,18 @@ if addition and opt.kernel_mode and not ([x for x in srcs if is_lts_kernel(x)] =
 if len(CVEs):
     pull_usn_cmd = ['%s/scripts/pull-usn-desc.py' % os.environ['UCT'], '--prioritize']
 
-    # We need to pass all releases we are publishing in order to filter it
-    # properly in the __this_only_affected__ feature.
-    for release in releases_filter:
+    if opt.this_only_affected:
+        # We need to pass all releases we are publishing in order to filter it
+        # properly in the __this_only_affected__ feature.
+        for release in releases_filter:
+            pull_usn_cmd.append('--releases=%s' % release)
+        for src in srcs_list:
+            pull_usn_cmd.append('--src=%s' % src)
+
+        pull_usn_cmd.append('--this-only-affected')
+    else:
         pull_usn_cmd.append('--releases=%s' % release)
+
     if opt.embargoed:
         pull_usn_cmd.append('--embargoed')
     print(subprocess.Popen(pull_usn_cmd + sorted(CVEs), stdout=subprocess.PIPE).communicate()[0].decode("utf-8"))