From 292994dd204c316c5c545f00807183ba05d758cd Mon Sep 17 00:00:00 2001 From: Björn Persson Date: Sep 19 2019 01:31:07 +0000 Subject: Added a check for source file verification. --- diff --git a/plugins/generic_should.py b/plugins/generic_should.py index 7408e0b..d2cad34 100644 --- a/plugins/generic_should.py +++ b/plugins/generic_should.py @@ -546,6 +546,91 @@ class CheckSourceUrl(GenericShouldCheckBase): self.set_passed(self.FAIL, output) +class CheckSourceVerification(GenericShouldCheckBase): + """ + Packages should verify signatures on source files if the upstream project + publishes signatures. + """ + + def __init__(self, base): + GenericShouldCheckBase.__init__(self, base) + self.url = ( + "https://docs.fedoraproject.org/en-US/packaging-guidelines/" + "#_source_file_verification" + ) + self.text = ( + "Sources are verified with gpgverify first in %prep if upstream " + "publishes signatures." + ) + self.type = "SHOULD" + self.automatic = True + + def run(self): + # Search for occurrences of gpgverify in the whole spec except the + # changelog. This searches in the raw text, so it sees the SOURCEn + # macros. + hits_in_spec = self.spec.find_all_re(re.compile(r"%\{?gpgverify\}? "), + skip_changelog=True) + + # Count the occurrences of gpgverify in the prep section, and check + # whether anything other than comments occurs before them. This + # searches in the macro-expanded text, so there's no need to look for + # "%dnl". + count_in_prep = 0 + other_seen = False + other_before = False + command_pattern = re.compile(r"/gpgverify ") + comment_pattern = re.compile(r"^[ \t]*#") + for line in self.spec.get_section("prep"): + if comment_pattern.match(line) is None: + if command_pattern.search(line) is None: + other_seen = True + else: + count_in_prep += 1 + if other_seen: + other_before = True + found_in_prep = count_in_prep > 0 + found_elsewhere = len(hits_in_spec) > count_in_prep + + # Find any source files that aren't passed to gpgverify. + unverified_source_numbers = [] + if found_in_prep: + verified_source_numbers = set() + source_macro_pattern = re.compile(r"%\{?SOURCE(\d+)(\W|$)") + for line in hits_in_spec: + for match in source_macro_pattern.finditer(line): + verified_source_numbers.add(int(match.group(1))) + digits_pattern = re.compile(r"\d+") + # get_all apparently returns the sources in reverse order, so + # reverse them back. + for source_tag in reversed(self.sources.get_all()): + number = int(digits_pattern.search(source_tag).group(0)) + if not number in verified_source_numbers: + unverified_source_numbers.append(repr(number)) + + # Report. + if (found_in_prep and not found_elsewhere and not other_before and + len(unverified_source_numbers) == 0): + self.set_passed(self.PASS) + else: + notes = [] + if other_before: + notes.append("gpgverify is not the first command in %prep.") + if found_elsewhere: + notes.append("gpgverify occurs outside of %prep.") + elif not found_in_prep: + notes.append("gpgverify is not used.") + if len(unverified_source_numbers) == 1: + notes.append("Source " + unverified_source_numbers[0] + + " is not passed to gpgverify.") + elif len(unverified_source_numbers) > 1: + listtext = (", ".join(unverified_source_numbers[0:-1]) + + " and " + unverified_source_numbers[-1]) + notes.append("Sources " + listtext + + " are not passed to gpgverify.") + self.set_passed(self.PENDING, " ".join(notes)) + + class CheckSpecAsInSRPM(GenericShouldCheckBase): """ Not in guidelines, buth the spec in the spec URL should diff --git a/test/test_source_verification.py b/test/test_source_verification.py new file mode 100644 index 0000000..0246f5b --- /dev/null +++ b/test/test_source_verification.py @@ -0,0 +1,155 @@ +# -*- coding: utf-8 -*- + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, +# MA 02110-1301 USA. + +""" +unit tests of CheckSourceVerification +""" + +import os, sys, logging +import unittest2 as unittest +from glob import glob + +import srcpath +from FedoraReview.name_bug import NameBug + +from fr_testcase import FR_TestCase + + +class TestSourceVerification(FR_TestCase): + """ unit tests of CheckSourceVerification """ + + def run_with_spec(self, spec_name): + """ runs the source verification check on the given test spec file """ + + class DummyBug(NameBug): + """ allows the testcases to share a dummy source RPM package """ + + def find_srpm_url(self): + srpm = glob("dummy*.src.rpm")[0] + self.srpm_url = "file://" + os.path.abspath(srpm) + + self.init_test("test_source_verification", + argv=["--name", spec_name, "--prebuilt", + "--define", "DISTTAG=fc30"]) + return self.run_single_check(DummyBug(spec_name), + "CheckSourceVerification") + + def test_single_unverified(self): + """ + This tests that a package that doesn't call gpgverify leaves the source + verification check pending with an appropriate note. + """ + + check = self.run_with_spec("single_unverified") + self.assertTrue(check.is_pending) + self.assertEqual(check.result.output_extra, "gpgverify is not used.") + + def test_single_verified(self): + """ + This tests that a package with a single correctly verified tarball + passes the source verification check, and that a comment is not + mistaken for a command before the source verification. + """ + + check = self.run_with_spec("single_verified") + self.assertTrue(check.is_passed) + self.assertIsNone(check.result.output_extra) + + def test_two_verified_correctly(self): + """ + This tests that a package with two correctly verified tarballs passes + the source verification check, and that macros are recognized both with + and without braces. + """ + + check = self.run_with_spec("two_verified_correctly") + self.assertTrue(check.is_passed) + self.assertIsNone(check.result.output_extra) + + def test_one_verified_three_not(self): + """ + This tests that a package with several unverified sources leaves the + source verification check pending with a note that lists the unverified + sources. + """ + + check = self.run_with_spec("one_verified_three_not") + self.assertTrue(check.is_pending) + self.assertEqual(check.result.output_extra, + "Sources 2, 3 and 4 are not passed to gpgverify.") + + def test_late_verification(self): + """ + This tests that a package that calls gpgverify after setup leaves the + source verification check pending with an appropriate note. + """ + + check = self.run_with_spec("late_verification") + self.assertTrue(check.is_pending) + self.assertEqual(check.result.output_extra, + "gpgverify is not the first command in %prep.") + + def test_two_verified_one_late_one_not(self): + """ + This tests that a package with two separate problems leaves the source + verification check pending with a note that mentions both problems, + that one gpgverify call first in prep doesn't mask another that is too + late, and that the note for a single unverified source is correct. + """ + + check = self.run_with_spec("two_verified_one_late_one_not") + self.assertTrue(check.is_pending) + self.assertEqual(check.result.output_extra, + "gpgverify is not the first command in %prep. " + + "Source 3 is not passed to gpgverify.") + + def test_misplaced_verification(self): + """ + This tests that a package that calls gpgverify only from the wrong + section leaves the source verification check pending with an + appropriate note. + """ + + check = self.run_with_spec("misplaced_verification") + self.assertTrue(check.is_pending) + self.assertEqual(check.result.output_extra, + "gpgverify occurs outside of %prep.") + + def test_two_verified_one_misplaced(self): + """ + This tests that a package that calls gpgverify both from prep and from + another section leaves the source verification check pending with an + appropriate note. + """ + + check = self.run_with_spec("two_verified_one_misplaced") + self.assertTrue(check.is_pending) + self.assertEqual(check.result.output_extra, + "gpgverify occurs outside of %prep.") + + +if __name__ == "__main__": + if len(sys.argv) > 1: + suite = unittest.TestSuite() + for test in sys.argv[1:]: + suite.addTest(TestSourceVerification(test)) + else: + suite = unittest.TestLoader().loadTestsFromTestCase( + TestSourceVerification) + unittest.TextTestRunner(verbosity=2).run(suite) + +# vim: set expandtab ts=4 sw=4: diff --git a/test/test_source_verification/dummy-1-1.fc30.src.rpm b/test/test_source_verification/dummy-1-1.fc30.src.rpm new file mode 100644 index 0000000..6a8c265 Binary files /dev/null and b/test/test_source_verification/dummy-1-1.fc30.src.rpm differ diff --git a/test/test_source_verification/late_verification.spec b/test/test_source_verification/late_verification.spec new file mode 100644 index 0000000..48cd93d --- /dev/null +++ b/test/test_source_verification/late_verification.spec @@ -0,0 +1,27 @@ +Name: late_verification +Version: 1 +Release: 1%{?dist} +Summary: CheckSourceVerification test case late_verification + +License: GPLv2+ +Source: %{name}-%{version}.tar.gz +Source2: %{name}-%{version}.tar.gz.asc +Source3: keys.gpg + +BuildArch: noarch +BuildRequires: gnupg2 + +%description +This tests that a package that calls gpgverify after setup leaves the source +verification check pending with an appropriate note. + +%prep +%setup +%{gpgverify} --keyring='%{SOURCE3}' --signature='%{SOURCE2}' --data='%{SOURCE0}' + +%build + +%install + +%files + diff --git a/test/test_source_verification/misplaced_verification.spec b/test/test_source_verification/misplaced_verification.spec new file mode 100644 index 0000000..e7f43a6 --- /dev/null +++ b/test/test_source_verification/misplaced_verification.spec @@ -0,0 +1,27 @@ +Name: misplaced_verification +Version: 1 +Release: 1%{?dist} +Summary: CheckSourceVerification test case misplaced_verification + +License: GPLv2+ +Source: %{name}-%{version}.tar.gz +Source2: %{name}-%{version}.tar.gz.asc +Source3: keys.gpg + +BuildArch: noarch +BuildRequires: gnupg2 + +%description +This tests that a package that calls gpgverify only from the wrong section +leaves the source verification check pending with an appropriate note. + +%prep +%setup + +%build +%{gpgverify} --keyring='%{SOURCE3}' --signature='%{SOURCE2}' --data='%{SOURCE0}' + +%install + +%files + diff --git a/test/test_source_verification/one_verified_three_not.spec b/test/test_source_verification/one_verified_three_not.spec new file mode 100644 index 0000000..2892bb1 --- /dev/null +++ b/test/test_source_verification/one_verified_three_not.spec @@ -0,0 +1,30 @@ +Name: one_verified_three_not +Version: 1 +Release: 1%{?dist} +Summary: CheckSourceVerification test case one_verified_three_not + +License: GPLv2+ +Source: first.tar.gz +Source2: second.tar.gz +Source3: third.tar.gz +Source4: fourth.tar.gz +Source5: first.tar.gz.asc +Source6: keys.gpg + +BuildArch: noarch +BuildRequires: gnupg2 + +%description +This tests that a package with several unverified sources leaves the source +verification check pending with a note that lists the unverified sources. + +%prep +%{gpgverify} --keyring='%{SOURCE6}' --signature='%{SOURCE5}' --data='%{SOURCE0}' +%setup + +%build + +%install + +%files + diff --git a/test/test_source_verification/single_unverified.spec b/test/test_source_verification/single_unverified.spec new file mode 100644 index 0000000..761fab0 --- /dev/null +++ b/test/test_source_verification/single_unverified.spec @@ -0,0 +1,23 @@ +Name: single_unverified +Version: 1 +Release: 1%{?dist} +Summary: CheckSourceVerification test case single_unverified + +License: GPLv2+ +Source: %{name}-%{version}.tar.gz + +BuildArch: noarch + +%description +This tests that a package that doesn't call gpgverify leaves the source +verification check pending with an appropriate note. + +%prep +%setup + +%build + +%install + +%files + diff --git a/test/test_source_verification/single_verified.spec b/test/test_source_verification/single_verified.spec new file mode 100644 index 0000000..af6b09f --- /dev/null +++ b/test/test_source_verification/single_verified.spec @@ -0,0 +1,29 @@ +Name: single_verified +Version: 1 +Release: 1%{?dist} +Summary: CheckSourceVerification test case single_verified + +License: GPLv2+ +Source: %{name}-%{version}.tar.gz +Source2: %{name}-%{version}.tar.gz.asc +Source3: keys.gpg + +BuildArch: noarch +BuildRequires: gnupg2 + +%description +This tests that a package with a single correctly verified tarball passes the +source verification check, and that a comment is not mistaken for a command +before the source verification. + +%prep +# A comment here is not a problem. +%{gpgverify} --keyring='%{SOURCE3}' --signature='%{SOURCE2}' --data='%{SOURCE0}' +%setup + +%build + +%install + +%files + diff --git a/test/test_source_verification/two_verified_correctly.spec b/test/test_source_verification/two_verified_correctly.spec new file mode 100644 index 0000000..6708484 --- /dev/null +++ b/test/test_source_verification/two_verified_correctly.spec @@ -0,0 +1,30 @@ +Name: two_verified_correctly +Version: 1 +Release: 1%{?dist} +Summary: CheckSourceVerification test case two_verified_correctly + +License: GPLv2+ +Source: first.tar.gz +Source2: second.tar.gz +Source3: first.tar.gz.asc +Source4: second.tar.gz.asc +Source5: keys.gpg + +BuildArch: noarch +BuildRequires: gnupg2 + +%description +This tests that a package with two correctly verified tarballs passes the +source verification check, and that macros are recognized both with and without braces. + +%prep +%gpgverify --keyring='%SOURCE5' --signature='%SOURCE3' --data='%SOURCE0' +%{gpgverify} --keyring='%{SOURCE5}' --signature='%{SOURCE4}' --data='%{SOURCE2}' +%setup + +%build + +%install + +%files + diff --git a/test/test_source_verification/two_verified_one_late_one_not.spec b/test/test_source_verification/two_verified_one_late_one_not.spec new file mode 100644 index 0000000..e0e3d26 --- /dev/null +++ b/test/test_source_verification/two_verified_one_late_one_not.spec @@ -0,0 +1,33 @@ +Name: two_verified_one_late_one_not +Version: 1 +Release: 1%{?dist} +Summary: CheckSourceVerification test case two_verified_one_late_one_not + +License: GPLv2+ +Source: first.tar.gz +Source2: second.tar.gz +Source3: third.tar.gz +Source4: first.tar.gz.asc +Source5: second.tar.gz.asc +Source6: keys.gpg + +BuildArch: noarch +BuildRequires: gnupg2 + +%description +This tests that a package with two separate problems leaves the source +verification check pending with a note that mentions both problems, that one +gpgverify call first in prep doesn't mask another that is too late, and that +the note for a single unverified source is correct. + +%prep +%{gpgverify} --keyring='%{SOURCE6}' --signature='%{SOURCE4}' --data='%{SOURCE0}' +%setup +%{gpgverify} --keyring='%{SOURCE6}' --signature='%{SOURCE5}' --data='%{SOURCE2}' + +%build + +%install + +%files + diff --git a/test/test_source_verification/two_verified_one_misplaced.spec b/test/test_source_verification/two_verified_one_misplaced.spec new file mode 100644 index 0000000..6548c4c --- /dev/null +++ b/test/test_source_verification/two_verified_one_misplaced.spec @@ -0,0 +1,30 @@ +Name: two_verified_one_misplaced +Version: 1 +Release: 1%{?dist} +Summary: CheckSourceVerification test case two_verified_one_misplaced + +License: GPLv2+ +Source: first.tar.gz +Source2: second.tar.gz +Source3: first.tar.gz.asc +Source4: second.tar.gz.asc +Source5: keys.gpg + +BuildArch: noarch +BuildRequires: gnupg2 + +%description +This tests that a package that calls gpgverify both from prep and from another +section leaves the source verification check pending with an appropriate note. + +%prep +%{gpgverify} --keyring='%{SOURCE5}' --signature='%{SOURCE3}' --data='%{SOURCE0}' +%setup + +%build + +%install +%{gpgverify} --keyring='%{SOURCE5}' --signature='%{SOURCE4}' --data='%{SOURCE2}' + +%files +