← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] ~andrey-fedoseev/launchpad:uct-import into launchpad:master

 


Diff comments:

> diff --git a/lib/lp/bugs/scripts/tests/test_uctimport.py b/lib/lp/bugs/scripts/tests/test_uctimport.py
> new file mode 100644
> index 0000000..743eea9
> --- /dev/null
> +++ b/lib/lp/bugs/scripts/tests/test_uctimport.py
> @@ -0,0 +1,357 @@
> +#  Copyright 2022 Canonical Ltd.  This software is licensed under the
> +#  GNU Affero General Public License version 3 (see the file LICENSE).
> +import datetime
> +from pathlib import Path
> +
> +from dateutil.tz import tzutc

I think that's mainly about its `localize` method and similar - if you're just using it to provide a `tzinfo` then it doesn't matter so much.

My understanding is that `datetime.timezone.utc` is basically equivalent in approach to `dateutil.tz.tzutc`, and there seems no particular reason to pull in a dependency for something that's in the standard library now.  (Also, Paul seems happy enough to recommend it in https://blog.ganssle.io/articles/2019/11/utcnow.html.)  The reason we use `pytz.UTC` all over the place is just that `datetime.timezone.utc` wasn't in the standard library until Python 3.2.

I don't want us to end up having three different versions of the UTC timezone object lying around, so my preference would be:

 * gradually migrate the large number of uses of `pytz.UTC` and similar to `datetime.timezone.utc`
 * carefully (!) replace the relatively small number of uses of `pytz.timezone` with something like `dateutil.tz.gettz`

> +from zope.component import getUtility
> +
> +from lp.app.enums import InformationType
> +from lp.app.interfaces.launchpad import ILaunchpadCelebrities
> +from lp.bugs.enums import VulnerabilityStatus
> +from lp.bugs.interfaces.bugtask import (
> +    BugTaskImportance,
> +    BugTaskStatus,
> +    )
> +from lp.bugs.scripts.uctimport import (
> +    CVE,
> +    DistroSeriesPackageStatus,
> +    Note,
> +    Package,
> +    PackageStatus,
> +    Patch,
> +    Priority,
> +    UCTImporter,
> +    )
> +from lp.registry.interfaces.series import SeriesStatus
> +from lp.testing import TestCaseWithFactory
> +from lp.testing.layers import ZopelessDatabaseLayer
> +
> +
> +class TestUCTImporter(TestCaseWithFactory):
> +
> +    layer = ZopelessDatabaseLayer
> +
> +    def setUp(self, *args, **kwargs):
> +        super().setUp(*args, **kwargs)
> +        self.importer = UCTImporter()
> +
> +    def test_load_cve_from_file(self):
> +        cve_path = Path(__file__).parent / "sampledata" / "CVE-2022-23222"
> +        cve = self.importer.load_cve_from_file(cve_path)
> +        self.assertEqual(
> +            cve,
> +            CVE(
> +                assigned_to="",
> +                bugs=[
> +                    "https://github.com/mm2/Little-CMS/issues/29";,
> +                    "https://github.com/mm2/Little-CMS/issues/30";,
> +                    "https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=745471";,
> +                ],
> +                cvss=[
> +                    {
> +                        "source": "nvd",
> +                        "vector": (
> +                            "CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H"
> +                        ),
> +                        "baseScore": "7.8",
> +                        "baseSeverity": "HIGH",
> +                    }
> +                ],
> +                candidate="CVE-2022-23222",
> +                date_made_public=datetime.datetime(
> +                    2022, 1, 14, 8, 15, tzinfo=tzutc()
> +                ),
> +                description=(
> +                    "kernel/bpf/verifier.c in the Linux kernel through "
> +                    "5.15.14 allows local\nusers to gain privileges because "
> +                    "of the availability of pointer arithmetic\nvia certain "
> +                    "*_OR_NULL pointer types."
> +                ),
> +                discovered_by="tr3e wang",
> +                mitigation=(
> +                    "seth-arnold> set kernel.unprivileged_bpf_disabled to 1"
> +                ),
> +                notes=[
> +                    Note(
> +                        author="sbeattie",
> +                        text=(
> +                            "Ubuntu 21.10 / 5.13+ kernels disable "
> +                            "unprivileged BPF by default.\nkernels 5.8 and "
> +                            "older are not affected, priority high is "
> +                            "for\n5.10 and 5.11 based kernels only"
> +                        ),
> +                    ),
> +                ],
> +                priority=Priority.CRITICAL,
> +                references=[
> +                    "https://ubuntu.com/security/notices/USN-5368-1";
> +                ],
> +                ubuntu_description=(
> +                    "It was discovered that the BPF verifier in the Linux "
> +                    "kernel did not\nproperly restrict pointer types in "
> +                    "certain situations. A local attacker\ncould use this to "
> +                    "cause a denial of service (system crash) or possibly\n"
> +                    "execute arbitrary code."
> +                ),
> +                packages=[
> +                    Package(
> +                        name="linux",
> +                        statuses=[
> +                            DistroSeriesPackageStatus(
> +                                distroseries="upstream",
> +                                status=PackageStatus.RELEASED,
> +                                reason="5.17~rc1",
> +                                priority=None,
> +                            ),
> +                            DistroSeriesPackageStatus(
> +                                distroseries="impish",
> +                                status=PackageStatus.RELEASED,
> +                                reason="5.13.0-37.42",
> +                                priority=Priority.MEDIUM,
> +                            ),
> +                            DistroSeriesPackageStatus(
> +                                distroseries="devel",
> +                                status=PackageStatus.NOT_AFFECTED,
> +                                reason="5.15.0-25.25",
> +                                priority=Priority.MEDIUM,
> +                            ),
> +                        ],
> +                        priority=None,
> +                        tags={"not-ue"},
> +                        patches=[
> +                            Patch(
> +                                patch_type="break-fix",
> +                                entry=(
> +                                    "457f44363a8894135c85b7a9afd2bd8196db24ab "
> +                                    "c25b2ae136039ffa820c26138ed4a5e5f3ab3841|"
> +                                    "local-CVE-2022-23222-fix"
> +                                ),
> +                            )
> +                        ],
> +                    ),
> +                    Package(
> +                        name="linux-hwe",
> +                        statuses=[
> +                            DistroSeriesPackageStatus(
> +                                distroseries="upstream",
> +                                status=PackageStatus.RELEASED,
> +                                reason="5.17~rc1",
> +                                priority=None,
> +                            ),
> +                            DistroSeriesPackageStatus(
> +                                distroseries="impish",
> +                                status=PackageStatus.DOES_NOT_EXIST,
> +                                reason="",
> +                                priority=None,
> +                            ),
> +                            DistroSeriesPackageStatus(
> +                                distroseries="devel",
> +                                status=PackageStatus.DOES_NOT_EXIST,
> +                                reason="",
> +                                priority=None,
> +                            ),
> +                        ],
> +                        priority=Priority.HIGH,
> +                        tags=set(),
> +                        patches=[],
> +                    ),
> +                ],
> +            ),
> +        )
> +
> +    def test_create_bug(self):
> +        celebrities = getUtility(ILaunchpadCelebrities)
> +        ubuntu = celebrities.ubuntu
> +        owner = celebrities.bug_importer
> +        supported_series = self.factory.makeDistroSeries(
> +            distribution=ubuntu, status=SeriesStatus.SUPPORTED
> +        )
> +        current_series = self.factory.makeDistroSeries(
> +            distribution=ubuntu, status=SeriesStatus.CURRENT
> +        )
> +        devel_series = self.factory.makeDistroSeries(
> +            distribution=ubuntu, status=SeriesStatus.DEVELOPMENT
> +        )
> +        dsp1 = self.factory.makeDistributionSourcePackage(distribution=ubuntu)
> +        dsp2 = self.factory.makeDistributionSourcePackage(distribution=ubuntu)
> +        lp_cve = self.factory.makeCVE("2022-23222")
> +
> +        for package in (dsp1, dsp2):
> +            for series in (supported_series, current_series, devel_series):
> +                self.factory.makeSourcePackagePublishingHistory(
> +                    distroseries=series,
> +                    sourcepackagerelease=self.factory.makeSourcePackageRelease(
> +                        distroseries=series,
> +                        sourcepackagename=package.sourcepackagename,
> +                    ),
> +                )
> +
> +        now = datetime.datetime.now(tzutc())
> +        cve = CVE(
> +            assigned_to="",
> +            bugs=[
> +                "https://github.com/mm2/Little-CMS/issues/29";,
> +                "https://github.com/mm2/Little-CMS/issues/30";,
> +                "https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=745471";,
> +            ],
> +            cvss=[],
> +            candidate="CVE-2022-23222",
> +            date_made_public=now,
> +            description="description",
> +            discovered_by="tr3e wang",
> +            mitigation="mitigation",
> +            notes=[Note(author="author", text="text")],
> +            priority=Priority.MEDIUM,
> +            references=[
> +                "https://ubuntu.com/security/notices/USN-5368-1";
> +            ],
> +            ubuntu_description="ubuntu-description",
> +            packages=[
> +                Package(
> +                    name=dsp1.sourcepackagename.name,
> +                    statuses=[
> +                        DistroSeriesPackageStatus(
> +                            distroseries=supported_series.name,
> +                            status=PackageStatus.RELEASED,
> +                            reason="released",
> +                            priority=Priority.HIGH,
> +                        ),
> +                        DistroSeriesPackageStatus(
> +                            distroseries=current_series.name,
> +                            status=PackageStatus.DOES_NOT_EXIST,
> +                            reason="does not exist",
> +                            priority=None,
> +                        ),
> +                    ],
> +                    priority=Priority.LOW,
> +                    patches=[],
> +                    tags=set(),
> +                ),
> +                Package(
> +                    name=dsp2.sourcepackagename.name,
> +                    statuses=[
> +                        DistroSeriesPackageStatus(
> +                            distroseries=supported_series.name,
> +                            status=PackageStatus.NOT_AFFECTED,
> +                            reason="not affected",
> +                            priority=Priority.LOW,
> +                        ),
> +                        DistroSeriesPackageStatus(
> +                            distroseries=current_series.name,
> +                            status=PackageStatus.IGNORED,
> +                            reason="ignored",
> +                            priority=None,
> +                        ),
> +                        DistroSeriesPackageStatus(
> +                            distroseries="devel",
> +                            status=PackageStatus.NEEDS_TRIAGE,
> +                            reason="needs triage",
> +                            priority=None,
> +                        ),
> +                    ],
> +                    priority=None,
> +                    patches=[],
> +                    tags=set(),
> +                ),
> +            ],
> +        )
> +        bug, vulnerabilities = self.importer.create_bug(cve, lp_cve)
> +
> +        self.assertEqual(bug.title, "CVE-2022-23222")
> +        self.assertEqual(bug.description, "ubuntu-description")
> +        self.assertEqual(bug.owner, owner)
> +        self.assertEqual(bug.information_type, InformationType.PRIVATESECURITY)
> +
> +        messages = list(bug.messages)
> +        self.assertEqual(len(messages), 5)
> +
> +        message = messages.pop(0)
> +        self.assertEqual(message.owner, owner)
> +        self.assertEqual(message.text_contents, "description")
> +
> +        for external_bug_url in cve.bugs:
> +            message = messages.pop(0)
> +            self.assertEqual(message.text_contents, external_bug_url)
> +
> +        for reference in cve.references:
> +            message = messages.pop(0)
> +            self.assertEqual(message.text_contents, reference)
> +
> +        bug_tasks = bug.bugtasks
> +        # 7 bug tasks are supposed to be created:
> +        #   2 for distro packages
> +        #   5 for combinations of distroseries/package:
> +        #     2 for the first package (2 distro series)
> +        #     3 for the second package (3 distro series)
> +        self.assertEqual(len(bug_tasks), 7)
> +
> +        bug_tasks_by_target = {
> +            (t.distribution, t.distroseries, t.sourcepackagename): t
> +            for t in bug_tasks
> +        }
> +        t = bug_tasks_by_target.pop((ubuntu, None, dsp1.sourcepackagename))
> +        self.assertEqual(t.importance, BugTaskImportance.LOW)
> +        self.assertEqual(t.status, BugTaskStatus.NEW)
> +        self.assertEqual(t.status_explanation, None)
> +
> +        t = bug_tasks_by_target.pop((ubuntu, None, dsp2.sourcepackagename))
> +        self.assertEqual(t.importance, BugTaskImportance.MEDIUM)
> +        self.assertEqual(t.status, BugTaskStatus.UNKNOWN)
> +        self.assertEqual(t.status_explanation, None)
> +
> +        t = bug_tasks_by_target.pop(
> +            (None, supported_series, dsp1.sourcepackagename)
> +        )
> +        self.assertEqual(t.importance, BugTaskImportance.HIGH)
> +        self.assertEqual(t.status, BugTaskStatus.FIXRELEASED)
> +        self.assertEqual(t.status_explanation, "released")
> +
> +        t = bug_tasks_by_target.pop(
> +            (None, current_series, dsp1.sourcepackagename)
> +        )
> +        self.assertEqual(t.importance, BugTaskImportance.LOW)
> +        self.assertEqual(t.status, BugTaskStatus.DOESNOTEXIST)
> +        self.assertEqual(t.status_explanation, "does not exist")
> +
> +        t = bug_tasks_by_target.pop(
> +            (None, supported_series, dsp2.sourcepackagename)
> +        )
> +        self.assertEqual(t.importance, BugTaskImportance.LOW)
> +        self.assertEqual(t.status, BugTaskStatus.INVALID)
> +        self.assertEqual(t.status_explanation, "not affected")
> +
> +        t = bug_tasks_by_target.pop(
> +            (None, current_series, dsp2.sourcepackagename)
> +        )
> +        self.assertEqual(t.importance, BugTaskImportance.MEDIUM)
> +        self.assertEqual(t.status, BugTaskStatus.WONTFIX)
> +        self.assertEqual(t.status_explanation, "ignored")
> +
> +        t = bug_tasks_by_target.pop(
> +            (None, devel_series, dsp2.sourcepackagename)
> +        )
> +        self.assertEqual(t.importance, BugTaskImportance.MEDIUM)
> +        self.assertEqual(t.status, BugTaskStatus.UNKNOWN)
> +        self.assertEqual(t.status_explanation, "needs triage")
> +
> +        self.assertEqual(bug.cves, [lp_cve])
> +
> +        self.assertEqual(len(vulnerabilities), 1)
> +
> +        vulnerability = vulnerabilities[0]
> +        self.assertEqual(vulnerability.distribution, ubuntu)
> +        self.assertEqual(vulnerability.creator, owner)
> +        self.assertEqual(vulnerability.cve, lp_cve)
> +        self.assertEqual(
> +            vulnerability.status, VulnerabilityStatus.NEEDS_TRIAGE
> +        )
> +        self.assertEqual(vulnerability.description, "description")
> +        self.assertEqual(vulnerability.notes, "author> text")
> +        self.assertEqual(vulnerability.mitigation, "mitigation")
> +        self.assertEqual(vulnerability.importance, BugTaskImportance.MEDIUM)
> +        self.assertEqual(
> +            vulnerability.information_type, InformationType.PRIVATESECURITY
> +        )
> +        self.assertEqual(vulnerability.date_made_public, now)
> +        self.assertEqual(vulnerability.bugs, [bug])


-- 
https://code.launchpad.net/~andrey-fedoseev/launchpad/+git/launchpad/+merge/425142
Your team Launchpad code reviewers is requested to review the proposed merge of ~andrey-fedoseev/launchpad:uct-import into launchpad:master.



References