[Reportbug-maint] Bug#878088: Bug#878088: reportbug: please inform security and lts teams about security update regressions
Markus Koschany
apo at debian.org
Sun Jan 28 23:11:03 UTC 2018
Am 28.01.2018 um 01:01 schrieb Nis Martensen:
> 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.
Ok, I have merged your patch and mine. Let's use that as basis for the
discussion to move forward. I have briefly tested the result but will do
more testing soon.
I noticed that you had to import apt but reportbug does not depend on
python3-apt. After I had installed this package it worked. I also
believe you don't need to check for the upstream changelog.gz file, the
Debian changelog should be sufficient.
> 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 assume there are more normal updates that go via a stable/oldstable
point update. Sometimes security updates and normal updates are
combined. Checking for the CVE string in the changelog file is probably
the only way to be sure in this case. Of course we will never catch the
situation when someone wants to report a security update regression but
in the meantime the security update was superseded by a normal update. I
don't know how common that is but it is hopefully negligible.
>> 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.
This works in unstable for me. secversion[2] is equivalent to
secversion.group(2). But I presume you tested that with an older Python
version hence it didn't work for you. Since we want to backport the
patch to older releases I have changed this to secversion.group(2) as I
have already done it in Wheezy.
> Using an else clause may be more pythonic than my previous suggestion of
> moving more stuff into the try block.
I have moved most of the code inside the try block now.
> Reportbug has an "ewrite()" function that you can use for the warning
> message.
Thanks. I am using this one now.
> Reportbug has a concept of user expertise levels. Can the question be
> skipped in novice mode?
Should it be skipped? Even novice users could detect such regressions.
We expect from them to choose severity levels, so why not expect that
they can answer a simple yes/no question? Though to keep it as simple as
possible for them I now check for self.options.mode == 'novice' and skip
the question completely in this case.
> 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.
I believe it is the other way around. From my experience reportbug is
less frequently updated and it is much simpler for us (and faster) to
change a value in distributions.json. You would also volunteer to keep
this file updated forever. Especially when Jessie will become
oldoldstable we need an update as soon as possible, otherwise reports
would be sent to the wrong address. I surely wouldn't mind to keep those
information like "supported" and "email address" in reportbug which
meant we could drop the requests code but judging from the feedback so
far, everyone seems to prefer the current distributions.json approach.
Markus
-------------- next part --------------
diff -Nru reportbug-7.1.8/bin/reportbug reportbug-7.1.8+nmu1/bin/reportbug
--- reportbug-7.1.8/bin/reportbug 2017-12-29 05:25:43.000000000 +0100
+++ reportbug-7.1.8+nmu1/bin/reportbug 2018-01-23 20:43:14.000000000 +0100
@@ -32,6 +32,8 @@
import optparse
import re
import locale
+import requests
+import json
import subprocess
import shlex
import email
@@ -1926,6 +1928,37 @@
listcc += ui.get_multiline(
'Enter any additional addresses this report should be sent to; press ENTER after each address.')
+ # If the bug is reported against a package with a version that
+ # indicates a security update add the security or lts team to CC
+ # after user confirmation
+ if pkgversion and package and not self.options.offline and not self.options.mode == 'novice':
+ if utils.is_security_update(package, pkgversion):
+ if ui.yes_no('Do you want to report a regression because of a security update? ',
+ 'Yes, please inform the LTS and security teams.',
+ 'No or I am not sure.', True):
+ regex = re.compile('(\+|~)deb(\d+)u(\d+)')
+ secversion = regex.search(pkgversion)
+ distnumber = secversion.group(2)
+ support = 'none'
+ email_address = []
+ try:
+ r = requests.get('https://security-tracker.debian.org/tracker/distributions.json',
+ timeout=self.options.timeout)
+ data = r.json()
+ for key, value in data.items():
+ if distnumber == value['major-version']:
+ if value['support']:
+ support = value['support']
+ if value['contact']:
+ email_address = value['contact']
+
+ if support != 'none':
+ listcc += [email_address]
+
+ except requests.exceptions.RequestException:
+ ewrite('Unable to connect to security-tracker.debian.org.\n'
+ 'Please try again later or contact the LTS or security team via email directly.\n')
+
if severity and rtype:
severity = debbugs.convert_severity(severity, rtype)
diff -Nru reportbug-7.1.8/debian/changelog reportbug-7.1.8+nmu1/debian/changelog
--- reportbug-7.1.8/debian/changelog 2017-12-29 05:25:43.000000000 +0100
+++ reportbug-7.1.8+nmu1/debian/changelog 2018-01-23 20:43:14.000000000 +0100
@@ -1,3 +1,10 @@
+reportbug (7.1.8+nmu1) UNRELEASED; urgency=medium
+
+ * Non-maintainer upload.
+ *
+
+ -- Markus Koschany <apo at debian.org> Tue, 23 Jan 2018 20:43:14 +0100
+
reportbug (7.1.8) unstable; urgency=medium
* reportbug/debbugs.py
diff -Nru reportbug-7.1.8/reportbug/utils.py reportbug-7.1.8+nmu1/reportbug/utils.py
--- reportbug-7.1.8/reportbug/utils.py 2017-12-29 05:25:43.000000000 +0100
+++ reportbug-7.1.8+nmu1/reportbug/utils.py 2018-01-23 20:43:14.000000000 +0100
@@ -39,6 +39,8 @@
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 @@
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 963 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/reportbug-maint/attachments/20180129/c2e2ca57/attachment.sig>
More information about the Reportbug-maint
mailing list