On 26-01-2018 15:45, Markus Koschany wrote:
> I am not convinced that the apt-cache method is more efficient than
> parsing the version string. I believe my method is simpler and it would
> catch the same potential candidates as your apt-cache idea. Manual
> intervention (answering a question) cannot be avoided unless the
> security team agrees to receive all bug reports against a version with a
> security update. I am absolutely sure that is not desired.

I agree that the question should be asked when the package version is a
security update.  What I am trying to achieve using the apt-cache method
(on top of the version string method) is to avoid asking the question
for "normal" package updates in stable.

Attached a new version of the is_security_update function. This could be
further refined by fetching the changelog from the package tracker if
the package version is not the installed one, but this is probably going
too far...

No idea how many of the stable package updates are usually normal bug
fix updates compared to the number of security updates. If updates are
almost all security updates, then we should definitely not do such
micro-optimization and go with your original approach.


> I favor my current patch because of the reasons I mentioned before. I
> can remove the sys.exit call? What else should be done?

"secversion[2]" should be "secversion.group(2)", right?  The former
variant did not work for me in a quick test.

Using an else clause may be more pythonic than my previous suggestion of
moving more stuff into the try block.

Reportbug has an "ewrite()" function that you can use for the warning
message.

Reportbug has a concept of user expertise levels.  Can the question be
skipped in novice mode?

Should reportbug incorporate a default version of the json file to fall
back to if the lookup fails? Reportbug is probably going to be updated
more often than the online version of the json file. An internal version
could also be updated regularly.
>From 56d6dbffd3075bb9f06aa1b31fd8f688b6b16d4b Mon Sep 17 00:00:00 2001
From: Nis Martensen <nis.marten...@web.de>
Date: Sat, 27 Jan 2018 22:22:31 +0100
Subject: [PATCH] utils: add new is_security_update() function

The security and LTS teams want to ask the reporting user for
notification if one of their updates caused a regression.  On the other
hand, reportbug should not ask users unnecessary questions.  So let's
try to collect evidence that this report is really about a package
version from a security update.
---
 reportbug/utils.py | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/reportbug/utils.py b/reportbug/utils.py
index b376c6c..c372727 100644
--- a/reportbug/utils.py
+++ b/reportbug/utils.py
@@ -39,6 +39,8 @@ import email
 import socket
 import subprocess
 import pipes
+import apt
+import gzip
 
 from .urlutils import open_url
 from string import ascii_letters, digits
@@ -1333,3 +1335,80 @@ def get_lsm_info():
                     break
 
     return lsminfo
+
+
+def is_security_update(pkgname, pkgversion):
+    """Determine whether a given package is a security update.
+
+    Detection of security update versions works most reliably if the
+    package version under investigation is the currently installed
+    version.  If this is not the case, the probability of false
+    negatives increases.
+
+    Parameters
+    ----------
+    pkgname : str
+        package name
+    pkgversion : str
+        package version
+
+    Returns
+    -------
+    bool
+        True if there is evidence that this version is a security
+        update, otherwise False
+    """
+    # Check 1:
+    # If it does not follow the debXuY version number pattern, it is
+    # definitely no security update.
+    #
+    # This check is not sufficient to detect security updates reliably,
+    # since other stable updates also use the same version pattern.
+    regex = re.compile('(\+|~)deb(\d+)u(\d+)')
+    secversion = regex.search(pkgversion)
+    if not secversion:
+        return False
+
+    # Check 2:
+    # If the package comes from the Debian-Security package source, it
+    # is definitely a security update.
+    #
+    # This check does not identify all security updates, since some of
+    # them are distributed through the normal channels as part of a
+    # stable release update.
+    try:
+        p = apt.Cache()[pkgname]
+        if 'Debian-Security' in [o.label for o in
+                        p.versions[pkgversion].origins]:
+            return True
+    except:
+        pass
+
+    # Check 3:
+    # Inspect the package changelog if it mentions any vulnerability,
+    # identified by a CVE number, in the section of the latest version.
+
+    cl = None
+    for cl in ['/usr/share/doc/{}/changelog.Debian.gz'.format(pkgname),
+               '/usr/share/doc/{}/changelog.gz'.format(pkgname)]:
+        if os.path.exists(cl):
+            break
+
+    try:
+        with gzip.open(cl, 'rt') as f:
+            ln = f.readline()
+            if pkgversion not in ln:
+                raise KeyError
+
+            for ln in f.readlines():
+                # stop reading at the end of the first section
+                if ln.rstrip() != '' and (ln.startswith(' -- ') or not ln.startswith(' ')):
+                    break
+
+                if 'CVE-20' in ln.upper():
+                    return True
+    except:
+        pass
+
+    # guess 'no security update, but normal stable update' by default
+    return False
-- 
2.11.0

Reply via email to