#292 Convert security data api to a bugzilla api.
Merged a year ago by ralph. Opened a year ago by ralph.
ralph/freshmaker no-more-sec-data  into  master

freshmaker/bugzilla.py freshmaker/security_data.py
file renamed
+55 -19

@@ -20,13 +20,15 @@ 

  # SOFTWARE.

  #

  # Written by Jan Kaluza <jkaluza@redhat.com>

+ #            Ralph Bean <rbean@redhat.com

  

  import requests

  

+ import lxml.etree

  from freshmaker import log, conf

  

  

- class SecurityDataAPI(object):

+ class BugzillaAPI(object):

  

      # Ordered Threat severities.

      THREAT_SEVERITIES = [

@@ -38,33 +40,53 @@ 

  

      def __init__(self, server_url=None):

          """

-         Creates new SecurityDataAPI instance.

+         Creates new BugzillaAPI instance.

  

-         :param str server_url: SecurityDataAPI base URL.

+         :param str server_url: BugzillaAPI base URL.

          """

          if server_url is not None:

              self.server_url = server_url.rstrip('/')

          else:

-             self.server_url = conf.security_data_server_url.rstrip('/')

+             self.server_url = conf.bugzilla_server_url.rstrip('/')

  

-     def _get_cve(self, cve):

+     def _get_cve_whiteboard(self, cve):

          """

-         Returns the JSON with metadata about `cve` obtained from

-         /cve/$cve.json endpoint.

+         Returns the whiteboard dict about `cve` obtained from

+         show_bug.cgi?ctype=xml&id=$cve endpoint

  

          :param str cve: CVE, for example "CVE-2017-10268".

          :rtype: dict

-         :return: Dict with metadata about CVE.

+         :return: the status whiteboard of the bugzilla CVE.

          """

-         log.debug("Querying SecurityDataAPI for %s", cve)

-         r = requests.get("%s/cve/%s.json" % (self.server_url, cve))

+         log.debug("Querying bugzilla for %s", cve)

+         r = requests.get(

+             "%s/show_bug.cgi" % self.server_url,

+             params={"ctype": "xml", "id": cve})

          r.raise_for_status()

-         return r.json()

  

-     def get_highest_threat_severity(self, cve_list):

+         # Parse

+         root = lxml.etree.fromstring(r.text.encode('utf-8'))

+ 

+         # List the major xml elements

+         elements = root.getchildren()[0].getchildren()

+ 

+         # Extract the whiteboard string

+         whiteboard = [e.text for e in elements if e.tag == 'status_whiteboard']

+ 

+         # Handle missing and/or empty whiteboards

+         if not whiteboard:

+             return dict()

+         whiteboard = whiteboard[0]

+         if not whiteboard:

+             return dict()

+ 

+         # Convert the whiteboard to a dict, and return

+         return dict(entry.split('=') for entry in whiteboard.split(','))

+ 

+     def get_highest_impact(self, cve_list):

          """

          Fetches metadata about each CVE in `cve_list` and returns the name of

-         highest severity rate. See `SecurityDataAPI.THREAT_SEVERITIES` for

+         highest severity rate. See `BugzillaAPI.THREAT_SEVERITIES` for

          list of possible severity rates.

  

          :param list cve_list: List of strings with CVE names.

@@ -74,23 +96,37 @@ 

          max_rating = -1

          for cve in cve_list:

              try:

-                 data = self._get_cve(cve)

+                 data = self._get_cve_whiteboard(cve)

              except requests.exceptions.HTTPError as e:

                  if e.response.status_code == 404:

                      log.warn(

-                         "CVE %s cannot be found in SecurityDataAPI, "

-                         "threat_severity unknown.", cve)

+                         "CVE %s cannot be found in bugzilla, "

+                         "threat_severity unknown.  %s", cve, e.response.request.url)

                      continue

                  raise

-             severity = data["threat_severity"].lower()

+             except IndexError:

+                 log.warn(

+                     "CVE %s XML appears malformed.  No children?  "

+                     "threat_severity unknown.", cve)

+                 continue

+ 

              try:

-                 rating = SecurityDataAPI.THREAT_SEVERITIES.index(severity)

+                 severity = data["impact"].lower()

+             except KeyError:

+                 log.warn(

+                     "CVE %s has no 'impact' in bugzilla whiteboard, "

+                     "threat_severity unknown.", cve)

+                 continue

+ 

+             try:

+                 rating = BugzillaAPI.THREAT_SEVERITIES.index(severity)

              except ValueError:

                  log.error("Unknown threat_severity '%s' for CVE %s",

                            severity, cve)

                  continue

+ 

              max_rating = max(max_rating, rating)

  

          if max_rating == -1:

              return None

-         return SecurityDataAPI.THREAT_SEVERITIES[max_rating]

+         return BugzillaAPI.THREAT_SEVERITIES[max_rating]

file modified
+3 -3

@@ -229,10 +229,10 @@ 

              'desc': 'Dict with base container "name-version" as key and URL '

                      'to extra .repo file to include in a rebuild',

          },

-         'security_data_server_url': {

+         'bugzilla_server_url': {

              'type': str,

-             'default': 'https://access.redhat.com/labs/securitydataapi',

-             'desc': 'Server URL of SecurityDataAPI.'},

+             'default': 'https://bugzilla.redhat.com',

+             'desc': 'Server URL of BugzillaAPI.'},

          'lightblue_server_url': {

              'type': str,

              'default': '',

file modified
+3 -4

@@ -30,7 +30,7 @@ 

      BrewSignRPMEvent, ErrataBaseEvent,

      FreshmakerManualRebuildEvent)

  from freshmaker import conf, log

- from freshmaker.security_data import SecurityDataAPI

+ from freshmaker.bugzilla import BugzillaAPI

  

  

  class ErrataAdvisory(object):

@@ -53,9 +53,8 @@ 

          self.cve_list = cve_list or []

          self.has_hightouch_bug = has_hightouch_bug

  

-         sec_data = SecurityDataAPI()

-         self.highest_cve_severity = sec_data.get_highest_threat_severity(

-             self.cve_list)

+         bugzilla = BugzillaAPI()

+         self.highest_cve_severity = bugzilla.get_highest_impact(self.cve_list)

  

      @classmethod

      def from_advisory_id(cls, errata, errata_id):

file modified
+1

@@ -25,3 +25,4 @@ 

  pyldap

  koji

  tabulate

+ lxml

file added
+101

@@ -0,0 +1,101 @@ 

+ # -*- coding: utf-8 -*-

+ #

+ # Copyright (c) 2017  Red Hat, Inc.

+ #

+ # Permission is hereby granted, free of charge, to any person obtaining a copy

+ # of this software and associated documentation files (the "Software"), to deal

+ # in the Software without restriction, including without limitation the rights

+ # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell

+ # copies of the Software, and to permit persons to whom the Software is

+ # furnished to do so, subject to the following conditions:

+ #

+ # The above copyright notice and this permission notice shall be included in all

+ # copies or substantial portions of the Software.

+ #

+ # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR

+ # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,

+ # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE

+ # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER

+ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,

+ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE

+ # SOFTWARE.

+ 

+ from mock import patch

+ 

+ from freshmaker.bugzilla import BugzillaAPI

+ from tests import helpers

+ 

+ 

+ class MockResponse(object):

+     def __init__(self, text):

+         self.text = text

+ 

+     def raise_for_status(self):

+         pass

+ 

+ 

+ xml_with_status = """

+ <bugzilla><bug>

+ <status_whiteboard>impact={impact}</status_whiteboard>

+ </bug></bugzilla>

+ """

+ 

+ xml_with_empty_status = """

+ <bugzilla><bug>

+ <status_whiteboard></status_whiteboard>

+ </bug></bugzilla>

+ """

+ xml_without_status = """<bugzilla><bug></bug></bugzilla>"""

+ xml_with_empty_bug = """<bugzilla></bugzilla>"""

+ 

+ 

+ class TestBugzillaAPI(helpers.FreshmakerTestCase):

+ 

+     @patch("freshmaker.bugzilla.requests.get")

+     def test_get_highest_impact(self, requests_get):

+         impacts = ["Low", "Moderate", "Important", "Critical"]

+         bugzilla = BugzillaAPI()

+         for num_of_cves in range(1, 4):

+             requests_get.side_effect = [

+                 MockResponse(xml_with_status.format(impact=impact))

+                 for impact in impacts]

+             ret = bugzilla.get_highest_impact(["CVE-1"] * num_of_cves)

+             self.assertEqual(ret, impacts[num_of_cves - 1].lower())

+ 

+     @patch("freshmaker.bugzilla.requests.get")

+     def test_get_highest_impact_empty_list(self, requests_get):

+         bugzilla = BugzillaAPI()

+         ret = bugzilla.get_highest_impact([])

+         self.assertEqual(ret, None)

+         requests_get.assert_not_called()

+ 

+     @patch("freshmaker.bugzilla.requests.get")

+     def test_get_highest_impact_no_status(self, requests_get):

+         bugzilla = BugzillaAPI()

+         requests_get.return_value = MockResponse(xml_without_status)

+         ret = bugzilla.get_highest_impact(["CVE-1"])

+         self.assertEqual(ret, None)

+ 

+     @patch("freshmaker.bugzilla.requests.get")

+     def test_get_highest_impact_empty_status(self, requests_get):

+         bugzilla = BugzillaAPI()

+         requests_get.return_value = MockResponse(xml_with_empty_status)

+         ret = bugzilla.get_highest_impact(["CVE-1"])

+         self.assertEqual(ret, None)

+ 

+     @patch("freshmaker.bugzilla.requests.get")

+     def test_get_highest_impact_empty_bug(self, requests_get):

+         bugzilla = BugzillaAPI()

+         requests_get.return_value = MockResponse(xml_with_empty_bug)

+         ret = bugzilla.get_highest_impact(["CVE-1"])

+         self.assertEqual(ret, None)

+ 

+     @patch("freshmaker.bugzilla.requests.get")

+     def test_get_highest_impact_unknown_impact(self, requests_get):

+         impacts = ["Low", "unknown"]

+         requests_get.side_effect = [

+             MockResponse(xml_with_status.format(impact=impact))

+             for impact in impacts]

+         bugzilla = BugzillaAPI()

+         ret = bugzilla.get_highest_impact(["CVE-1", "CVE-2"])

+         self.assertEqual(ret, "low")

file modified
+2 -2

@@ -160,8 +160,8 @@ 

          self.errata = Errata("https://localhost/")

  

          self.patcher = helpers.Patcher(

-             'freshmaker.errata.SecurityDataAPI.')

-         self.patcher.patch("get_highest_threat_severity",

+             'freshmaker.errata.BugzillaAPI.')

+         self.patcher.patch("get_highest_impact",

                             return_value="moderate")

  

      def tearDown(self):

@@ -1,55 +0,0 @@ 

- # -*- coding: utf-8 -*-

- #

- # Copyright (c) 2017  Red Hat, Inc.

- #

- # Permission is hereby granted, free of charge, to any person obtaining a copy

- # of this software and associated documentation files (the "Software"), to deal

- # in the Software without restriction, including without limitation the rights

- # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell

- # copies of the Software, and to permit persons to whom the Software is

- # furnished to do so, subject to the following conditions:

- #

- # The above copyright notice and this permission notice shall be included in all

- # copies or substantial portions of the Software.

- #

- # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR

- # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,

- # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE

- # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER

- # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,

- # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE

- # SOFTWARE.

- 

- from mock import patch

- 

- from freshmaker.security_data import SecurityDataAPI

- from tests import helpers

- 

- 

- class TestSecurityDataAPI(helpers.FreshmakerTestCase):

- 

-     @patch("freshmaker.security_data.requests.get")

-     def test_get_highest_threat_severity(self, requests_get):

-         severities = ["Low", "Moderate", "Important", "Critical"]

-         sec_data = SecurityDataAPI()

-         for num_of_cves in range(1, 4):

-             requests_get.return_value.json.side_effect = [

-                 {"threat_severity": severity} for severity in severities]

-             ret = sec_data.get_highest_threat_severity(["CVE-1"] * num_of_cves)

-             self.assertEqual(ret, severities[num_of_cves - 1].lower())

- 

-     @patch("freshmaker.security_data.requests.get")

-     def test_get_highest_threat_severity_empty_list(self, requests_get):

-         sec_data = SecurityDataAPI()

-         ret = sec_data.get_highest_threat_severity([])

-         self.assertEqual(ret, None)

-         requests_get.assert_not_called()

- 

-     @patch("freshmaker.security_data.requests.get")

-     def test_get_highest_threat_severity_unknown_severity(self, requests_get):

-         severities = ["Low", "unknown"]

-         requests_get.return_value.json.side_effect = [

-             {"threat_severity": severity} for severity in severities]

-         sec_data = SecurityDataAPI()

-         ret = sec_data.get_highest_threat_severity(["CVE-1", "CVE-2"])

-         self.assertEqual(ret, "low")

Turns out we have a race condition with the security data api. It only gets
updated an hour or so after the advisory ships and we get triggered. This
means that in practice, freshmaker would never trigger since it would always
come up with "unknown" for the severity on any event.

This moves the code to query bugzilla instead, which is the source of the
severity information.

Note that we don't need to make an authenticated query to bugzilla since PST assures us that the bugs will be public before the advisory which triggers us can go SHIPPED_LIVE.

Is it possible for the root to not have children? If yes, then probably this can be put in a try?

Yeah, or catch it outside this call in the other method. Good idea.

I think it should always have children... but, better to be safe.

rebased onto 92e2c17

a year ago

This code looks good to me. I haven't had a chance to get the unit tests fully running (still working on getting them set up correctly on my end), I'll assume you've run them.

Pull-Request has been merged by ralph

a year ago