From 2684f0564ba408d87cd0f23d58dc35351a0b6997 Mon Sep 17 00:00:00 2001 From: Hritik Vijay Date: Tue, 30 Mar 2021 03:20:12 +0530 Subject: [PATCH 1/4] requests_with_5xx_retry: Retry on 5xx errors Introduced and used a helper function for retries on 5xx errors. This is important and some servers like bugzilla.redhat.com return 502 Proxy Error which was the cause of https://github.com/nexB/vulnerablecode/issues/398 A ticket has been raised in RedHat here https://redhat.service-now.com/help?id=rh_ticket&table=sc_req_item&sys_id=278239541b1ba010477e43fccd4bcb4a Signed-off-by: Hritik Vijay --- vulnerabilities/helpers.py | 19 ++++ vulnerabilities/importers/redhat.py | 162 +++++++++++++++------------- 2 files changed, 108 insertions(+), 73 deletions(-) diff --git a/vulnerabilities/helpers.py b/vulnerabilities/helpers.py index 36bf7c865..77376ee94 100644 --- a/vulnerabilities/helpers.py +++ b/vulnerabilities/helpers.py @@ -25,6 +25,7 @@ import requests import toml +import urllib3 import yaml # TODO add logging here @@ -79,3 +80,21 @@ def create_etag(data_src, url, etag_key): is_cve = re.compile(r"CVE-\d+-\d+", re.IGNORECASE).match + + +def requests_with_5xx_retry(max_retries=5, backoff_factor=0.5): + """ + Returns a requests sessions which retries on 5xx errors with + a backoff_factor + """ + retries = urllib3.util.Retry( + total=max_retries, + backoff_factor=backoff_factor, + raise_on_status=True, + status_forcelist=range(500, 600, 1), + ) + adapter = requests.adapters.HTTPAdapter(max_retries=retries) + session = requests.Session() + session.mount("https://", adapter) + session.mount("http://", adapter) + return session diff --git a/vulnerabilities/importers/redhat.py b/vulnerabilities/importers/redhat.py index d96036ae4..a591d3e72 100644 --- a/vulnerabilities/importers/redhat.py +++ b/vulnerabilities/importers/redhat.py @@ -20,17 +20,20 @@ # VulnerableCode is a free software code from nexB Inc. and others. # Visit https://github.com/nexB/vulnerablecode/ for support and download. -from packageurl import PackageURL import requests +from packageurl import PackageURL from vulnerabilities.data_source import Advisory from vulnerabilities.data_source import DataSource from vulnerabilities.data_source import DataSourceConfiguration from vulnerabilities.data_source import Reference from vulnerabilities.data_source import VulnerabilitySeverity +from vulnerabilities.helpers import requests_with_5xx_retry from vulnerabilities.severity_systems import scoring_systems +import pdb + class RedhatDataSource(DataSource): CONFIG_CLASS = DataSourceConfiguration @@ -43,6 +46,9 @@ def updated_advisories(self): return self.batch_advisories(processed_advisories) +requests_session = requests_with_5xx_retry(max_retries=5, backoff_factor=1) + + def fetch(): """ Return a list of CVE data mappings fetched from the RedHat API. @@ -58,7 +64,7 @@ def fetch(): current_url = url_template.format(page_no) try: print(f"Fetching: {current_url}") - response = requests.get(current_url) + response = requests_session.get(current_url) if response.status_code != requests.codes.ok: # TODO: log me print(f"Failed to fetch results from {current_url}") @@ -79,88 +85,98 @@ def fetch(): def to_advisory(advisory_data): - affected_purls = [] - if advisory_data.get("affected_packages"): - for rpm in advisory_data["affected_packages"]: - purl = rpm_to_purl(rpm) - if purl: - affected_purls.append(purl) - - references = [] - bugzilla = advisory_data.get("bugzilla") - if bugzilla: - url = "https://bugzilla.redhat.com/show_bug.cgi?id={}".format(bugzilla) - bugzilla_data = requests.get(f"https://bugzilla.redhat.com/rest/bug/{bugzilla}").json() - if ( - bugzilla_data.get("bugs") - and len(bugzilla_data["bugs"]) - and bugzilla_data["bugs"][0].get("severity") - ): - bugzilla_severity_val = bugzilla_data["bugs"][0]["severity"] - bugzilla_severity = VulnerabilitySeverity( - system=scoring_systems["rhbs"], - value=bugzilla_severity_val, - ) + try: + affected_purls = [] + if advisory_data.get("affected_packages"): + for rpm in advisory_data["affected_packages"]: + purl = rpm_to_purl(rpm) + if purl: + affected_purls.append(purl) + + references = [] + bugzilla = advisory_data.get("bugzilla") + if bugzilla: + url = "https://bugzilla.redhat.com/show_bug.cgi?id={}".format(bugzilla) + bugzilla_data = requests_session.get( + f"https://bugzilla.redhat.com/rest/bug/{bugzilla}" + ).json() + if ( + bugzilla_data.get("bugs") + and len(bugzilla_data["bugs"]) + and bugzilla_data["bugs"][0].get("severity") + ): + bugzilla_severity_val = bugzilla_data["bugs"][0]["severity"] + bugzilla_severity = VulnerabilitySeverity( + system=scoring_systems["rhbs"], + value=bugzilla_severity_val, + ) - references.append( - Reference( - severities=[bugzilla_severity], - url=url, - reference_id=bugzilla, + references.append( + Reference( + severities=[bugzilla_severity], + url=url, + reference_id=bugzilla, + ) ) - ) - for rh_adv in advisory_data["advisories"]: - # RH provides 3 types of advisories RHSA, RHBA, RHEA. Only RHSA's contain severity score. - # See https://access.redhat.com/articles/2130961 for more details. - - if "RHSA" in rh_adv.upper(): - rhsa_data = requests.get( - f"https://access.redhat.com/hydra/rest/securitydata/cvrf/{rh_adv}.json" - ).json() # nopep8 - value = rhsa_data["cvrfdoc"]["aggregate_severity"] - rhsa_aggregate_severity = VulnerabilitySeverity( - system=scoring_systems["rhas"], - value=value, - ) + for rh_adv in advisory_data["advisories"]: + # RH provides 3 types of advisories RHSA, RHBA, RHEA. Only RHSA's contain severity score. + # See https://access.redhat.com/articles/2130961 for more details. + + if "RHSA" in rh_adv.upper(): + rhsa_data = requests_session.get( + f"https://access.redhat.com/hydra/rest/securitydata/cvrf/{rh_adv}.json" + ).json() # nopep8 + value = rhsa_data["cvrfdoc"]["aggregate_severity"] + rhsa_aggregate_severity = VulnerabilitySeverity( + system=scoring_systems["rhas"], + value=value, + ) - references.append( - Reference( - severities=[rhsa_aggregate_severity], - url="https://access.redhat.com/errata/{}".format(rh_adv), - reference_id=rh_adv, + references.append( + Reference( + severities=[rhsa_aggregate_severity], + url="https://access.redhat.com/errata/{}".format(rh_adv), + reference_id=rh_adv, + ) ) - ) - else: - references.append(Reference(severities=[], url=url, reference_id=rh_adv)) + else: + references.append( + Reference(severities=[], url=url, reference_id=rh_adv) + ) - redhat_scores = [] - cvssv3_score = advisory_data.get("cvss3_score") - if cvssv3_score: - redhat_scores.append( - VulnerabilitySeverity( - system=scoring_systems["cvssv3"], - value=cvssv3_score, + redhat_scores = [] + cvssv3_score = advisory_data.get("cvss3_score") + if cvssv3_score: + redhat_scores.append( + VulnerabilitySeverity( + system=scoring_systems["cvssv3"], + value=cvssv3_score, + ) ) - ) - cvssv3_vector = advisory_data.get("cvss3_scoring_vector") - if cvssv3_vector: - redhat_scores.append( - VulnerabilitySeverity( - system=scoring_systems["cvssv3_vector"], - value=cvssv3_vector, + cvssv3_vector = advisory_data.get("cvss3_scoring_vector") + if cvssv3_vector: + redhat_scores.append( + VulnerabilitySeverity( + system=scoring_systems["cvssv3_vector"], + value=cvssv3_vector, + ) ) - ) - references.append(Reference(severities=redhat_scores, url=advisory_data["resource_url"])) - return Advisory( - vulnerability_id=advisory_data["CVE"], - summary=advisory_data["bugzilla_description"], - impacted_package_urls=affected_purls, - references=references, - ) + references.append( + Reference(severities=redhat_scores, url=advisory_data["resource_url"]) + ) + return Advisory( + vulnerability_id=advisory_data["CVE"], + summary=advisory_data["bugzilla_description"], + impacted_package_urls=affected_purls, + references=references, + ) + except Exception as e: + print(e) + pdb.set_trace() def rpm_to_purl(rpm_string): From aff0065ed4b7d3af2c5fbc1e575eab28327ef235 Mon Sep 17 00:00:00 2001 From: Hritik Vijay Date: Thu, 1 Apr 2021 00:53:53 +0530 Subject: [PATCH 2/4] Not all RHSA errata have a CVRF document This is mentioned in the NOTE of "2.1 List all CVRFs" of https://access.redhat.com/documentation/en-us/red_hat_security_data_api/1.0/html/red_hat_security_data_api/cvrf Such a case would lead to a crash before this commit. Eg: https://access.redhat.com/hydra/rest/securitydata/cvrf/RHSA-2005:835.json No cvrfdoc would be found in the statement value = rhsa_data["cvrfdoc"]["aggregate_severity"] Signed-off-by: Hritik Vijay --- vulnerabilities/importers/redhat.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/vulnerabilities/importers/redhat.py b/vulnerabilities/importers/redhat.py index a591d3e72..0fbc10b15 100644 --- a/vulnerabilities/importers/redhat.py +++ b/vulnerabilities/importers/redhat.py @@ -127,15 +127,19 @@ def to_advisory(advisory_data): rhsa_data = requests_session.get( f"https://access.redhat.com/hydra/rest/securitydata/cvrf/{rh_adv}.json" ).json() # nopep8 - value = rhsa_data["cvrfdoc"]["aggregate_severity"] - rhsa_aggregate_severity = VulnerabilitySeverity( - system=scoring_systems["rhas"], - value=value, - ) + + rhsa_aggregate_severities = [] + if rhsa_data.get("cvrfdoc"): + # not all RHSA errata have a corresponding CVRF document + value = rhsa_data["cvrfdoc"]["aggregate_severity"] + rhsa_aggregate_severities.append(VulnerabilitySeverity( + system=scoring_systems["rhas"], + value=value, + )) references.append( Reference( - severities=[rhsa_aggregate_severity], + severities=rhsa_aggregate_severities, url="https://access.redhat.com/errata/{}".format(rh_adv), reference_id=rh_adv, ) From b0329fae15bbb0504d90d12bd99bd2a701504443 Mon Sep 17 00:00:00 2001 From: Hritik Vijay Date: Thu, 1 Apr 2021 16:13:31 +0530 Subject: [PATCH 3/4] Remove debugging symbols and black -l 100 This finally fixes https://github.com/nexB/vulnerablecode/issues/398 Signed-off-by: Hritik Vijay --- vulnerabilities/importers/redhat.py | 160 +++++++++++++--------------- 1 file changed, 76 insertions(+), 84 deletions(-) diff --git a/vulnerabilities/importers/redhat.py b/vulnerabilities/importers/redhat.py index 0fbc10b15..9415cd94a 100644 --- a/vulnerabilities/importers/redhat.py +++ b/vulnerabilities/importers/redhat.py @@ -32,8 +32,6 @@ from vulnerabilities.severity_systems import scoring_systems -import pdb - class RedhatDataSource(DataSource): CONFIG_CLASS = DataSourceConfiguration @@ -85,102 +83,96 @@ def fetch(): def to_advisory(advisory_data): - try: - affected_purls = [] - if advisory_data.get("affected_packages"): - for rpm in advisory_data["affected_packages"]: - purl = rpm_to_purl(rpm) - if purl: - affected_purls.append(purl) - - references = [] - bugzilla = advisory_data.get("bugzilla") - if bugzilla: - url = "https://bugzilla.redhat.com/show_bug.cgi?id={}".format(bugzilla) - bugzilla_data = requests_session.get( - f"https://bugzilla.redhat.com/rest/bug/{bugzilla}" - ).json() - if ( - bugzilla_data.get("bugs") - and len(bugzilla_data["bugs"]) - and bugzilla_data["bugs"][0].get("severity") - ): - bugzilla_severity_val = bugzilla_data["bugs"][0]["severity"] - bugzilla_severity = VulnerabilitySeverity( - system=scoring_systems["rhbs"], - value=bugzilla_severity_val, - ) + affected_purls = [] + if advisory_data.get("affected_packages"): + for rpm in advisory_data["affected_packages"]: + purl = rpm_to_purl(rpm) + if purl: + affected_purls.append(purl) + + references = [] + bugzilla = advisory_data.get("bugzilla") + if bugzilla: + url = "https://bugzilla.redhat.com/show_bug.cgi?id={}".format(bugzilla) + bugzilla_data = requests_session.get( + f"https://bugzilla.redhat.com/rest/bug/{bugzilla}" + ).json() + if ( + bugzilla_data.get("bugs") + and len(bugzilla_data["bugs"]) + and bugzilla_data["bugs"][0].get("severity") + ): + bugzilla_severity_val = bugzilla_data["bugs"][0]["severity"] + bugzilla_severity = VulnerabilitySeverity( + system=scoring_systems["rhbs"], + value=bugzilla_severity_val, + ) - references.append( - Reference( - severities=[bugzilla_severity], - url=url, - reference_id=bugzilla, - ) + references.append( + Reference( + severities=[bugzilla_severity], + url=url, + reference_id=bugzilla, ) + ) - for rh_adv in advisory_data["advisories"]: - # RH provides 3 types of advisories RHSA, RHBA, RHEA. Only RHSA's contain severity score. - # See https://access.redhat.com/articles/2130961 for more details. - - if "RHSA" in rh_adv.upper(): - rhsa_data = requests_session.get( - f"https://access.redhat.com/hydra/rest/securitydata/cvrf/{rh_adv}.json" - ).json() # nopep8 - - rhsa_aggregate_severities = [] - if rhsa_data.get("cvrfdoc"): - # not all RHSA errata have a corresponding CVRF document - value = rhsa_data["cvrfdoc"]["aggregate_severity"] - rhsa_aggregate_severities.append(VulnerabilitySeverity( + for rh_adv in advisory_data["advisories"]: + # RH provides 3 types of advisories RHSA, RHBA, RHEA. Only RHSA's contain severity score. + # See https://access.redhat.com/articles/2130961 for more details. + + if "RHSA" in rh_adv.upper(): + rhsa_data = requests_session.get( + f"https://access.redhat.com/hydra/rest/securitydata/cvrf/{rh_adv}.json" + ).json() # nopep8 + + rhsa_aggregate_severities = [] + if rhsa_data.get("cvrfdoc"): + # not all RHSA errata have a corresponding CVRF document + value = rhsa_data["cvrfdoc"]["aggregate_severity"] + rhsa_aggregate_severities.append( + VulnerabilitySeverity( system=scoring_systems["rhas"], value=value, - )) - - references.append( - Reference( - severities=rhsa_aggregate_severities, - url="https://access.redhat.com/errata/{}".format(rh_adv), - reference_id=rh_adv, ) ) - else: - references.append( - Reference(severities=[], url=url, reference_id=rh_adv) - ) - - redhat_scores = [] - cvssv3_score = advisory_data.get("cvss3_score") - if cvssv3_score: - redhat_scores.append( - VulnerabilitySeverity( - system=scoring_systems["cvssv3"], - value=cvssv3_score, + references.append( + Reference( + severities=rhsa_aggregate_severities, + url="https://access.redhat.com/errata/{}".format(rh_adv), + reference_id=rh_adv, ) ) - cvssv3_vector = advisory_data.get("cvss3_scoring_vector") - if cvssv3_vector: - redhat_scores.append( - VulnerabilitySeverity( - system=scoring_systems["cvssv3_vector"], - value=cvssv3_vector, - ) - ) + else: + references.append(Reference(severities=[], url=url, reference_id=rh_adv)) - references.append( - Reference(severities=redhat_scores, url=advisory_data["resource_url"]) + redhat_scores = [] + cvssv3_score = advisory_data.get("cvss3_score") + if cvssv3_score: + redhat_scores.append( + VulnerabilitySeverity( + system=scoring_systems["cvssv3"], + value=cvssv3_score, + ) ) - return Advisory( - vulnerability_id=advisory_data["CVE"], - summary=advisory_data["bugzilla_description"], - impacted_package_urls=affected_purls, - references=references, + + cvssv3_vector = advisory_data.get("cvss3_scoring_vector") + if cvssv3_vector: + redhat_scores.append( + VulnerabilitySeverity( + system=scoring_systems["cvssv3_vector"], + value=cvssv3_vector, + ) ) - except Exception as e: - print(e) - pdb.set_trace() + + references.append(Reference(severities=redhat_scores, url=advisory_data["resource_url"])) + return Advisory( + vulnerability_id=advisory_data["CVE"], + summary=advisory_data["bugzilla_description"], + impacted_package_urls=affected_purls, + references=references, + ) def rpm_to_purl(rpm_string): From c90a8717b79591e7fe788c483bf353225cf01e6d Mon Sep 17 00:00:00 2001 From: Hritik Vijay Date: Fri, 2 Apr 2021 00:52:21 +0530 Subject: [PATCH 4/4] Mock requests_session in redhat importer Previous commits replace the usage of requests.get() altogether with a custom requests_session which provides better 5xx error handling. It is now required to mock that object in this test. IMHO it would make more sense to update this test altogether to use the real endpoints against some real data. Signed-off-by: Hritik Vijay --- vulnerabilities/tests/test_redhat_importer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vulnerabilities/tests/test_redhat_importer.py b/vulnerabilities/tests/test_redhat_importer.py index ba7631399..967b01684 100644 --- a/vulnerabilities/tests/test_redhat_importer.py +++ b/vulnerabilities/tests/test_redhat_importer.py @@ -139,7 +139,7 @@ def test_to_advisory(self): } for adv in data: with unittest.mock.patch( - "vulnerabilities.importers.redhat.requests.get", return_value=mock_resp + "vulnerabilities.importers.redhat.requests_session.get", return_value=mock_resp ): adv = redhat.to_advisory(adv) found_advisories.append(adv)