From f51b70dd3b9d60b77bf4ef6be8d74e71a8729f76 Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Mon, 22 Jun 2020 15:14:58 +0200 Subject: [PATCH 01/20] pam-unauthorized-module: add link explaining how to fix the lint --- rpmlint/descriptions/PAMModulesCheck.toml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rpmlint/descriptions/PAMModulesCheck.toml b/rpmlint/descriptions/PAMModulesCheck.toml index 879878204..fe882272c 100644 --- a/rpmlint/descriptions/PAMModulesCheck.toml +++ b/rpmlint/descriptions/PAMModulesCheck.toml @@ -1,5 +1,7 @@ pam-unauthorized-module=""" The package installs a PAM module. If the package -is intended for inclusion the PAM module name must -be included in the white list. +is intended for inclusion in any SUSE product please open a bug +report to request review of the service by the security team. +Please refer to https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs +for more information. """ From f9e9468b48b135acb4ecaa968ea3229de2cc1a32 Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Mon, 22 Jun 2020 14:23:56 +0200 Subject: [PATCH 02/20] port over generic allow list classes --- rpmlint/checks/Allowlisting.py | 428 +++++++++++++++++++++++++++++++++ 1 file changed, 428 insertions(+) create mode 100644 rpmlint/checks/Allowlisting.py diff --git a/rpmlint/checks/Allowlisting.py b/rpmlint/checks/Allowlisting.py new file mode 100644 index 000000000..f2c0a1ee8 --- /dev/null +++ b/rpmlint/checks/Allowlisting.py @@ -0,0 +1,428 @@ +# vim: sw=4 ts=4 sts=4 et : +############################################################################# +# Author : Matthias Gerstner +# Purpose : reusable code for dealing with security allow lists +############################################################################# + +import abc +import hashlib +import json +import os +import os.path +import sys +import traceback + +from rpmlint.checks.AbstractCheck import AbstractCheck + + +class DigestVerificationResult(object): + """This type represents the result of a digest verification as returned + from AuditEntry.compareDigests().""" + + def __init__(self, path, alg, expected, encountered): + + self.m_path = path + self.m_alg = alg + self.m_expected = expected + self.m_encountered = encountered + + def path(self): + return self.m_path + + def algorithm(self): + return self.m_alg + + def matches(self): + """Returns a boolean whether the encountered digest matches the + expected digest.""" + return self.m_expected == self.m_encountered + + def expected(self): + return self.m_expected + + def encountered(self): + return self.m_encountered + + +class AuditEntry(object): + """This object represents a single audit entry as found in an allow entry like: + + "bsc#1234": { + "comment": "some comment", + "digests": { + "/some/file": ":", + ... + } + } + + """ + + def __init__(self, bug): + + self.m_bug = bug + self._verifyBugNr() + self.m_comment = '' + self.m_digests = {} + + def bug(self): + return self.m_bug + + def setComment(self, comment): + self.m_comment = comment + + def comment(self): + return self.m_comment + + def setDigests(self, digests): + for path, digest in digests.items(): + self._verifyPath(path) + self._verifyDigestSyntax(digest) + + self.m_digests = digests + + def digests(self): + """Returns a dictionary specifying file paths and their allow listed + digests. The digests are suitable for the + Python hashlib module. They're of the form ':'. As a + special case the digest entry can be 'skip:' which indicates + that no digest verification should be performed and the file is + acceptable regardless of its contents.""" + return self.m_digests + + def isSkipDigest(self, digest): + """Returns whether the given digest entry denotes the special 'skip + digest' case which means not to check the file digest at all.""" + return digest == 'skip:' + + def coversAllFiles(self, file_paths): + """Returns a boolean indicating whether all files from the set + 'file_paths' are covered by this audit.""" + return file_paths.issubset(self.digests().keys()) + + def compareDigests(self, pkg): + """Compares the digests recorded in this AuditEntry against the actual + files coming from the given rpmlint @pkg. Returns a tuple of + (boolean, [DigestVerificationResult, ...]). The boolean indicates the + overall verification result, while the list of + DigestVerificationResult entries provides detailed information about + the encountered data. Any 'skip digest' entries will be ignored and + not be included in the result list.""" + + results = [] + + # NOTE: syntax and algorithm validity of stored digests was already + # checked in setDigests() so we can skip the respective error handling + # here. + + fileinfos = pkg.files + + for path, digest in self.digests().items(): + alg, digest = digest.split(':', 1) + + if self.isSkipDigest(f'{alg}:{digest}'): + results.append(DigestVerificationResult(path, alg, digest, digest)) + continue + + try: + h = hashlib.new(alg) + + src_info = fileinfos.get(path, None) + + if not src_info: + raise Exception('expected file {} is not part of the RPM'.format(path)) + + # resolve potential symbolic links + # + # this function handles both absolute and relative symlinks + # and does not access paths outside the RPM. + # + # it is not safe against symlink loops, however, it will + # result in an infinite loop it such cases. But there are + # probably a lot of other possibilities to DoS the RPM build + # process or rpmlint. + dst_info = pkg.readlink(src_info) + + if not dst_info: + raise Exception('symlink {} -> {} is broken or pointing outside this RPM'.format(src_info.path, src_info.linkto)) + + # NOTE: this path is dynamic, rpmlint unpacks the RPM + # contents into a temporary directory even when outside the + # build environment i.e. the file content should always be + # available to us. + with open(dst_info.path, 'rb') as fd: + while True: + chunk = fd.read(4096) + if not chunk: + break + + h.update(chunk) + + encountered = h.hexdigest() + except IOError as e: + encountered = 'error:' + str(e) + except Exception as e: + encountered = 'error:' + str(e) + + dig_res = DigestVerificationResult(path, alg, digest, encountered) + results.append(dig_res) + + return (all([res.matches() for res in results]), results) + + def _verifyBugNr(self): + """Perform some sanity checks on the bug nr associated with this audit + entry.""" + + parts = self.m_bug.split('#') + + if len(parts) != 2 or \ + parts[0] not in ('bsc', 'boo', 'bnc') or \ + not parts[1].isdigit(): + raise Exception('Bad bug nr# "{}"'.format(self.m_bug)) + + def _verifyDigestSyntax(self, digest): + if self.isSkipDigest(digest): + return + + parts = digest.split(':') + if len(parts) != 2: + raise Exception('Bad digest specification ' + digest) + + alg, hexdigest = parts + + try: + hashlib.new(alg) + except ValueError: + raise Exception('Bad digest algorithm in ' + digest) + + def _verifyPath(self, path): + if not path.startswith(os.path.sep): + raise Exception('Bad allow listing path ' + path) + + +def allowlist_for_package(allowlist_path, pkg_name): + class AllowlistParser(object): + """This type knows how to parse the JSON allow listing format. The format + is documented in [1]. + + [1]: https://github.com/openSUSE/rpmlint-security-whitelistings/blob/master/README.md + """ + + def __init__(self, wl_path): + """Creates a new instance of AllowlistParser that operates on + @wl_path.""" + + self.m_path = wl_path + + def parse(self, package): + """Parses the allow list file for the current package and returns a list of AuditEntry objects.""" + + try: + with open(self.m_path, 'r') as fd: + data = json.load(fd) + + try: + config = data[package] + except KeyError: + return [] + + return self._parseAllowlistEntry(package, config) + except Exception as e: + _, _, tb = sys.exc_info() + fn, ln, _, _ = traceback.extract_tb(tb)[-1] + raise Exception(self._getErrorPrefix() + 'Failed to parse JSON file: {}:{}: {}'.format( + fn, ln, str(e) + )) + + def _parseAllowlistEntry(self, package, config): + """Parses a single JSON allow entry and returns a AuditEntry() + object for it. On non-critical error conditions None is returned, + otherwise an exception is raised.""" + + ret = [] + + audits = config.get('audits') + + if not audits: + raise Exception(self._getErrorPrefix() + f"no 'audits' entries for package {package}") + + for bug, data in audits.items(): + try: + audit = self._parseAuditEntry(bug, data) + except Exception as e: + raise Exception(self._getErrorPrefix() + 'Failed to parse audit entries: ' + str(e)) + + # missing audit is soft error, continue parsing + if audit: + ret.append(audit) + + return ret + + def _parseAuditEntry(self, bug, data): + """Parses a single JSON audit sub-entry returns an AuditEntry() object + for it. On non-critical error conditions None is returned, otherwise + an exception is raised""" + + ret = AuditEntry(bug) + + comment = data.get('comment') + if comment: + ret.setComment(comment) + + digests = data.get('digests') + + if not digests: + raise Exception(self._getErrorPrefix() + f"no 'digests' entry for '{bug}'") + + ret.setDigests(digests) + + return ret + + def _getErrorPrefix(self): + return self.m_path + ': ERROR: ' + + def _getWarnPrefix(self): + return self.m_path + ': WARN: ' + + parser = AllowlistParser(allowlist_path) + return parser.parse(pkg_name) + + +class AbstractAllowlistCheck(AbstractCheck, metaclass=abc.ABCMeta): + """An abstract base class for comparing files found in an RPM against an allow list with hashed file contents.""" + + @property + @abc.abstractmethod + def allowlist_filenames(self): + """ The file names (an iterable of strings) which contain the allow lists to be used. """ + pass + + @property + @abc.abstractmethod + def restricted_paths(self): + """ An iterable of file paths (strings) children of which must be checked against + the allow list. + Note: use paths ending with / in order to only catch descendants of directories but not these + dirs themselves.""" + pass + + @property + @abc.abstractmethod + def error_map(self): + """ A dictionary with keys 'unauthorized', 'changed', 'ghost' that maps these to the + desired rpmlint error identifiers for this check. """ + pass + + def __init__(self, config, output): + for req_key in ('unauthorized', 'changed', 'ghost'): + assert req_key in self.error_map, f'Missing error mapping in class {type(self)}' + super().__init__(config, output) + + def read_allowlist(self, pkg): + """ Retrieves the allow list for a package. + + The special return value 'None' means no list is configured and all files should pass.""" + # this option is found in config files in /opt/testing/share/rpmlint/mini, + # installed there by the rpmlint-mini package. + allowlist_dir = self.config.configuration.get('WhitelistDataDir', []) + + if not self.allowlist_filenames: + return None + + rules_entries = [] + for filename in self.allowlist_filenames: + for wd in allowlist_dir: + candidate = os.path.join(wd, filename) + if os.path.exists(candidate): + rules_entries += allowlist_for_package(candidate, pkg.name) + break + + return rules_entries + + def collect_restricted_files(self, pkg): + """ Collects the set of files that are restricted and need to be in the allowed list. """ + + def is_restricted(f): + return any(f.startswith(restricted) for restricted in self.restricted_paths) + return set(f for f in pkg.files.keys() if is_restricted(f)) + + def check_binary(self, pkg): + restricted_files = self.collect_restricted_files(pkg) + + for f in restricted_files & set(pkg.ghost_files): + self.output.add_info('E', pkg, self.error_map['ghost'], f) + restricted_files.remove(f) + + if not restricted_files: + return + + results = [] + + # don't ruin the whole run if this check is not configured, this + # was hopefully intended by the user. + allow_list = self.read_allowlist(pkg) + if allow_list is None: + return + + for audit in allow_list: + digest_matches, results = audit.compareDigests(pkg) + + if digest_matches and audit.coversAllFiles(restricted_files): + break + else: + # print the encountered and expected digests and paths for diagnostic purposes + for result in results: + restricted_files -= {result.path()} + + if result.matches(): + continue + + print(f'{result.path()}: expected {result.algorithm()} digest {result.expected()} but encountered {result.encountered()}', file=sys.stderr) + self.output.add_info('E', pkg, self.error_map['changed'], result.path()) + + for f in restricted_files: + self.output.add_info('E', pkg, self.error_map['unauthorized'], f) + + +class AbstractSimpleAllowlistCheck(AbstractCheck, metaclass=abc.ABCMeta): + """An abstract base class for comparing files found in an RPM against a + simple list from the configuration with allowed file names in a directory.""" + + @property + @abc.abstractmethod + def allowlist_config_key(self): + """ The config key (a string) under which all allowed file names are stored. """ + pass + + @property + @abc.abstractmethod + def restricted_paths(self): + """ An iterable of file paths (strings) children of which must be checked against + the allow list. + Note: use paths ending with / in order to only catch descendants of directories but not these + dirs themselves.""" + pass + + @property + @abc.abstractmethod + def error_map(self): + """ A dictionary with keys 'unauthorized', 'ghost' that maps these to the + desired rpmlint error identifiers for this check. """ + pass + + def check_binary(self, pkg): + """Checks that files in polkit-default-privs.d are on an allow list.""" + allowed_filenames = self.config.configuration.get(self.allowlist_config_key, ()) + + for filename, fileinfo in pkg.files.items(): + for dir_name in self.restricted_paths: + if filename.startswith(dir_name): + bn = filename[len(dir_name):] + break + else: + continue + print(repr(filename), file=sys.stderr) + + if fileinfo.is_ghost: + self.output.add_info('E', pkg, self.error_map['ghost'], filename) + elif bn not in allowed_filenames: + self.output.add_info('E', pkg, self.error_map['unauthorized'], filename) From a77bde39b93c2693c7f6d354bc64feba5b8bfc9f Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Mon, 22 Jun 2020 14:25:42 +0200 Subject: [PATCH 03/20] port over lint to enforce a cron job allow list --- rpmlint/checks/CronJobsCheck.py | 20 ++++++++++++++++ rpmlint/descriptions/CronJobs.toml | 22 +++++++++++++++++ test/binary/cronjobs-1-0.x86_64.rpm | Bin 0 -> 6440 bytes test/test_cron_jobs.py | 35 ++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+) create mode 100644 rpmlint/checks/CronJobsCheck.py create mode 100644 rpmlint/descriptions/CronJobs.toml create mode 100644 test/binary/cronjobs-1-0.x86_64.rpm create mode 100644 test/test_cron_jobs.py diff --git a/rpmlint/checks/CronJobsCheck.py b/rpmlint/checks/CronJobsCheck.py new file mode 100644 index 000000000..1f67f2162 --- /dev/null +++ b/rpmlint/checks/CronJobsCheck.py @@ -0,0 +1,20 @@ +# vim: sw=4 ts=4 sts=4 et : +############################################################################# +# Author : Matthias Gerstner +# Purpose : Enforce cron jobs in /etc/cron.* directories to be on a allow list +############################################################################# + +from .Allowlisting import AbstractAllowlistCheck + + +class CronCheck(AbstractAllowlistCheck): + allowlist_filenames = ('cron-whitelist.json',) + restricted_paths = ( + '/etc/cron.d/', '/etc/cron.hourly/', '/etc/cron.daily/', + '/etc/cron.weekly/', '/etc/cron.monthly/' + ) + error_map = { + 'unauthorized': 'cronjob-unauthorized-file', + 'changed': 'cronjob-changed-file', + 'ghost': 'cronjob-ghost-file' + } diff --git a/rpmlint/descriptions/CronJobs.toml b/rpmlint/descriptions/CronJobs.toml new file mode 100644 index 000000000..9f0205988 --- /dev/null +++ b/rpmlint/descriptions/CronJobs.toml @@ -0,0 +1,22 @@ +cronjob-unauthorized-file=""" +A cron job file is installed by this package. If the package is +intended for inclusion in any SUSE product please open a bug report to request +review of the package by the security team. Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for more +information. +""" + +cronjob-changed-file=""" +A cron job or cron job related file installed by this package changed +in content. Please open a bug report to request follow-up review of the +introduced changes by the security team. Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for more +information. +""" + +cronjob-ghost-file=""" +A cron job path has been marked as %ghost file by this package. +This is not allowed as it is impossible to review. Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for more +information. +""" diff --git a/test/binary/cronjobs-1-0.x86_64.rpm b/test/binary/cronjobs-1-0.x86_64.rpm new file mode 100644 index 0000000000000000000000000000000000000000..a80e5dedc5047a782e4e3b4993f428bd16a860fb GIT binary patch literal 6440 zcmeI0ZHN_B7{||j(abartu%{9DA>hj?!3Q9ma@9Ki@A%ruq)|yX6DS^aqqoznVGw9 zNGjO32&1olQ1U}rK|~lvkx^(teJCX9Ie2tR8we5NCJ!{s-zBU8r{^xg|^PKaX z=bW8|=hEw!PZkI~IWLZC+afm+6;Z~H|M29v>)eulQYhX@%+j8_ZAm%ZfxiW_9FU1sR~|O zjB_LN{T62f!}uKf;CS3OT@78i5-KF$xcIFIEwn5WnV3iIb52E};oR(~G*ksVN&KmQo# z<(yY>UJDBI|!3tk3AJI}|0qrm7d zbNf%AINs!Z78J*iA$O4JdM8ncOg6D373$O*rbA2^4b!qfg&?djp&YW9#$I{6)msh6 zm@hOE7AM4Ki5G|UG>mFO6bNaV-I^gS(^Lo(p%T@GSPUjwYub}WwMyf0A&G=A70NV$ zw<^PxDmaXdVeZM+Um7c_!!d0nVl7IU8%4uX5Qpi6f3qBYm}`&b%-P znNP}TT2GdjN~5ErQdDQP1Ok=8T`ail(v4R^NMw3DTzHP zu2%_xZ`;(g`&dNc%PV9-Ts_#?vvE^b=fL38J$)NH)~tZx zL6X9i73(qF5)Xoao)pN8_ML0uK1o}>b~JnMT1JY@0zlyI+SUFZYiD@I6h;hDI3hP z43+u{l+QK%0yO_7TmQ?ZrY7G4o`2Jt=!cK$>@zp!?9Y>>x%;3qEuDSkG`T^#w~$41 zzb|@w<0T{ekF;GI>6hQ$RXXwNjjzA>=!)_|_iqSbHv+;v8_~?f-4zJm^bn(%{uN{g`uG}{H i<>Q?Phxb|ElH7BXcxZj@jAMt+pL!5Jj1bwt=Dz?YtiQSd literal 0 HcmV?d00001 diff --git a/test/test_cron_jobs.py b/test/test_cron_jobs.py new file mode 100644 index 000000000..d7090931a --- /dev/null +++ b/test/test_cron_jobs.py @@ -0,0 +1,35 @@ +import json +import os +import os.path + +import pytest +from rpmlint.checks.CronJobsCheck import CronCheck +from rpmlint.filter import Filter + +from Testing import CONFIG, get_tested_package + + +@pytest.fixture(scope='function', autouse=True) +def cronjobscheck(tmpdir): + CONFIG.info = True + + # make the cron check search for its allow list in 'tmpdir' + CONFIG.configuration['WhitelistDataDir'] = [tmpdir] + + output = Filter(CONFIG) + test = CronCheck(CONFIG, output) + + # make an empty allow list + with open(os.path.join(tmpdir, test.allowlist_filenames[0]), 'w') as fd: + json.dump({}, fd) + + return output, test + + +@pytest.mark.parametrize('package', ['binary/cronjobs']) +def test_cron_jobs(tmpdir, package, cronjobscheck): + output, test = cronjobscheck + test.check(get_tested_package(package, tmpdir)) + out = output.print_results(output.results) + assert 'noproblem' not in out + assert 'cronjob-unauthorized-file /etc/cron.d/job' in out From 8a97102f5bf1bae61ddf1ec8c7e870f16a3c4204 Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Mon, 22 Jun 2020 14:44:57 +0200 Subject: [PATCH 04/20] port over lint to enforce a D-Bus service allow list --- rpmlint/checks/CheckDBUSServices.py | 15 ++++++++++++ rpmlint/descriptions/DBUSServices.toml | 14 +++++++++++ test/binary/dbus_svc-1-0.x86_64.rpm | Bin 0 -> 7116 bytes test/test_dbus_services.py | 32 +++++++++++++++++++++++++ 4 files changed, 61 insertions(+) create mode 100644 rpmlint/checks/CheckDBUSServices.py create mode 100644 rpmlint/descriptions/DBUSServices.toml create mode 100644 test/binary/dbus_svc-1-0.x86_64.rpm create mode 100644 test/test_dbus_services.py diff --git a/rpmlint/checks/CheckDBUSServices.py b/rpmlint/checks/CheckDBUSServices.py new file mode 100644 index 000000000..f5e647136 --- /dev/null +++ b/rpmlint/checks/CheckDBUSServices.py @@ -0,0 +1,15 @@ +from .Allowlisting import AbstractSimpleAllowlistCheck + + +class DBUSServiceCheck(AbstractSimpleAllowlistCheck): + """ Verifies that installed D-Bus services are on an allow list. """ + restricted_paths = ( + '/usr/share/dbus-1/system-services/', + '/usr/share/dbus-1/system.d/', + '/etc/dbus-1/system.d/' + ) + error_map = { + 'ghost': 'suse-dbus-ghost-service', + 'unauthorized': 'suse-dbus-unauthorized-service', + } + allowlist_config_key = 'DBUSServices.WhiteList' diff --git a/rpmlint/descriptions/DBUSServices.toml b/rpmlint/descriptions/DBUSServices.toml new file mode 100644 index 000000000..7c8e4876d --- /dev/null +++ b/rpmlint/descriptions/DBUSServices.toml @@ -0,0 +1,14 @@ +suse-dbus-unauthorized-service=""" +The package installs a DBUS system service file. If the package +is intended for inclusion in any SUSE product please open a bug +report to request review of the service by the security team. Please +refer to https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs +for more information. +""" + +suse-dbus-ghost-service=""" +This package installs a DBUS system service marked as %ghost. +This is not allowed, since it is impossible to review. Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for more +information. +""" diff --git a/test/binary/dbus_svc-1-0.x86_64.rpm b/test/binary/dbus_svc-1-0.x86_64.rpm new file mode 100644 index 0000000000000000000000000000000000000000..1d26abedb020bd9ac44719be68a80d7c1af97093 GIT binary patch literal 7116 zcmeI1Yitx%6vyv&rxhy15(E)R9DJpXncbP)ncW#)BE|9up$&zkRH4k{PIv6?&Sqw| z4-y58Mhr$V7*PY!Mxx=NA`uf!OlnZ#1LGsY2Sfq21k|EDq8KRkoZZ_}5Wf(=xzpVJ z-`_pwKIY8ayUp3Y?I*tt6SzW_k@s8tc2kf95gV@I3X$?Fdp-5~@epgfj*zOip*{l& z*0+FG!Nj9ME1=*7zM(Oo$TdX{9|z^2q#6GT3jIkA3iJ>E28#C8P@sSKcTluH3TEEW zL`gPfTTzvSp424K(vnh=%1NrlWGNm~^_ZQo44tS+Gfr(&Q$#IJ4LPoAqO55N#R9Z# zT~T9}rkRRi>4q5Bsiw%bl@M(`uF94y>IO_mNTqDvdFyQlrmoyqF}7}9^W&$M!g}@< zOr?)f1WFMoMW7UcQUpp7C`F(Yfl>rY5hz8V6oFC%N)hm9>m9&w}GW$@ml~+BY%&0~GBqGd>53<8Nhr0Tk`K8TT{(g7HPh`-{xs*jNq~ z+d1rusj9P#acp3?mBP4OB`ElZMlr_whWLc;V2tg+W!o5I9KgS9Cu58Q+COHDF-AVd z_{O4!aZNBOHx3l);YrMX6DatHWl(qq+yrKCC@Oa|v#(%0k?|hJ)u3>X;lrRfzFKTA zJXd557{|xwhCHRn9MlUu2Nc%B@t`mcHz;oaX)_xQe*Jl;Eypb_wjPs%0W_&;6G-I3x?Rij`zdToL z=h{JGzVfw2``8W4zMk=3P*_*_vy6WNMgJEW_k!Yh_&$Yd$zZ;dNJJ!ysUPq;-E7la zDc|7)DSq~>C5^mKy>`c>KC!58dQL8I+$`_fe2}6;-a$$ac*p0{Znl+rVA8V%yPwZw zbgzr|UET@!lSz!)7w4VSx~fhfxFMRd9Me?WluSw1blb3HQ-!psZ9w`omei>%Q8^)Nh8CAiUDgy$lZ>RQ#0*tY zNNIroAO^#q)IaN<>jvZ+KL&RIX8a?*XGXkS24{fpp{rhl{zF@*8eTXdA=f}3*sztu z3k;QZjOto9lk=$WQ>)HN(}jA5`Za{vuw}N>3OnyoTZ7(}cJ+d@KUZ(34IZ_f&Y>>M zJkM`hSwrFxF(L~Rkt2zSSWx^_BC$eUQX&$pYx(5vo{XLjXr!>oW)`-L>1K$)o3K-1 zx2GIGaABXaZH>DPgHX67ysfJi{Eme)56_drZys>&3|(;MkP-MJ$`wDP4333;%i_d@ zgGVPZVvr~e%qRv|kfMH+Cl@~GznHOw` zyAMa3kfzzzAT?d&h1o52^@|qIt6SRASifXZ?S0e1-Sz`G0)?ChCy@z3ftGx>!^v7g zpc`rG1M7ZQ)=YVB)`jI>W+Z0LaY^S&qOfyFz*q~#lW8b8S5{~y7jzXHN8|%rP)Sl3 z6A6pPG)t9aDjG^WX_%TE*K8YpyAi39j3s2StG2DgHA9jUNlIgiC?)k|(oEvFOyS%w z{=YOZaA7BKRey42Q!~7yN6(MyE+dmetX-O}FOQ+=wv zN!+z2dhpFvcU3&S>XVlq+O9S#r?VTsZ|r;P*zM;R^na?JYppo6!8-Wrw0CdN69b2j zj9+-BcSie;wjV=h=Z`$LL8~0d`5RyBowjAij(aD(ptNk->`qO$oVi$gnyY_)YeV(e z55E5Vo1O~~&d<(@#TxPo*L{EszOr{dJQJ2hp6S}NO8M~5-f=(ld69L;pWC(Vjh?Cd mFOKn=t2Y1j$iy#6=>5<>_)oC^z1{jdCk{`-7m8FaZT=f}nS^}+ literal 0 HcmV?d00001 diff --git a/test/test_dbus_services.py b/test/test_dbus_services.py new file mode 100644 index 000000000..9d7c8b456 --- /dev/null +++ b/test/test_dbus_services.py @@ -0,0 +1,32 @@ +import pytest +from rpmlint.checks.CheckDBUSServices import DBUSServiceCheck +from rpmlint.filter import Filter + +from Testing import CONFIG, get_tested_package + + +@pytest.fixture(scope='function', autouse=True) +def dbusservicescheck(tmpdir): + CONFIG.info = True + CONFIG.configuration['DBUSServices.WhiteList'] = ('1', '2', '3') + + output = Filter(CONFIG) + test = DBUSServiceCheck(CONFIG, output) + + return output, test + + +@pytest.mark.parametrize('package', ['binary/dbus_svc']) +def test_dbus_services(tmpdir, package, dbusservicescheck): + output, test = dbusservicescheck + test.check(get_tested_package(package, tmpdir)) + out = output.print_results(output.results) + assert 'noproblem' not in out + # these are allowed and should NOT produce errors + assert 'suse-dbus-unauthorized-service /usr/share/dbus-1/system-services/1' not in out + assert 'suse-dbus-unauthorized-service /usr/share/dbus-1/system.d/2' not in out + assert 'suse-dbus-unauthorized-service /etc/dbus-1/system.d/3' not in out + # these are not allowed and should produce errors + assert 'suse-dbus-unauthorized-service /usr/share/dbus-1/system-services/a' in out + assert 'suse-dbus-unauthorized-service /usr/share/dbus-1/system.d/b' in out + assert 'suse-dbus-unauthorized-service /etc/dbus-1/system.d/c' in out From 1e15751632a73e4567a2cc85e243805905647e62 Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Mon, 22 Jun 2020 14:46:28 +0200 Subject: [PATCH 05/20] port over lint to enforce a polkit rule allow list --- rpmlint/checks/PolkitRulesAllowedCheck.py | 19 +++++++++ .../descriptions/PolkitRulesAllowedCheck.toml | 25 +++++++++++ test/binary/polkit-rules-test-0-0.noarch.rpm | Bin 0 -> 7112 bytes test/test_polkit_allowlist.py | 40 ++++++++++++++++++ 4 files changed, 84 insertions(+) create mode 100644 rpmlint/checks/PolkitRulesAllowedCheck.py create mode 100644 rpmlint/descriptions/PolkitRulesAllowedCheck.toml create mode 100644 test/binary/polkit-rules-test-0-0.noarch.rpm create mode 100644 test/test_polkit_allowlist.py diff --git a/rpmlint/checks/PolkitRulesAllowedCheck.py b/rpmlint/checks/PolkitRulesAllowedCheck.py new file mode 100644 index 000000000..564980d04 --- /dev/null +++ b/rpmlint/checks/PolkitRulesAllowedCheck.py @@ -0,0 +1,19 @@ +from .Allowlisting import AbstractAllowlistCheck + + +class PolkitRulesAllowedCheck(AbstractAllowlistCheck): + """ Checks that polkit rules files are on an allow list. """ + + restricted_paths = ( + '/etc/polkit-1/rules.d/', + '/usr/share/polkit-1/rules.d/', + ) + error_map = { + 'unauthorized': 'polkit-unauthorized-rules', + 'changed': 'polkit-changed-rules', + 'ghost': 'polkit-ghost-file', + } + + @property + def allowlist_filenames(self): + return self.config.configuration.get('PolkitRulesWhitelist', ()) diff --git a/rpmlint/descriptions/PolkitRulesAllowedCheck.toml b/rpmlint/descriptions/PolkitRulesAllowedCheck.toml new file mode 100644 index 000000000..11778a738 --- /dev/null +++ b/rpmlint/descriptions/PolkitRulesAllowedCheck.toml @@ -0,0 +1,25 @@ +polkit-unauthorized-rules=""" +A polkit rules file installed by this package is not whitelisted in the +polkit-whitelisting package. If the package is intended for inclusion in any +SUSE product please open a bug report to request review of the package by the +security team. Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for +more information. +""" + + +polkit-changed-rules=""" +A polkit rules file installed by this package changed in content. Please open a +bug report to request follow-up review of the introduced changes by the +security team. Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for +more information. +""" + + +polkit-ghost-file=""" +This package installs a polkit rule or policy as %ghost file. This is not +allowed as it is impossible to review. For more information please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for +more information. +""" diff --git a/test/binary/polkit-rules-test-0-0.noarch.rpm b/test/binary/polkit-rules-test-0-0.noarch.rpm new file mode 100644 index 0000000000000000000000000000000000000000..41e204af284a00bf8fa460b7e9169841ce60e0d4 GIT binary patch literal 7112 zcmeHMd#D^&89#e>uZ?SJQu;{xP>xdYrp>%|XJ&VT)zI9gi4D0iO(ZEz(law>c8_;n zoyX1IDAI@`f`&k#6;e!NgVENa)zZ>G#2~bP6s%Ab0;R2%MpMbtM~!;Z>-Wu`Yf_^k z_@{f8Gr#%G_nq&2=QrofEN4zXdFItRfl^D8a9R{fR)#!R3Z55=uIOm_?@Nu0|FiSH zr%u-7wW|nO`V5?Z0xF�bB|lUk&&^s93P>3D|%dN`49u+LIJ2w6C89 zMEgTfp?&=vAlknKX6oxcv)#67xm_M~8FxI(@VV{Se%I7}+v;>|OK-P*Vi}IDyN2#^ zZniC(bIajf+iV%O7g%ti!whJko0jPtF7Mc`)3$?-=@^D(u)s5+LqeMR=e~Ms`7fV* z>gD_AlC!UO&ApJ)W0h%i7%4DPV5GoEfsq0u1x5;t6c{NmQedRONP&?8BL)876{z+^ z=I7`4gAlN%LC9tMCKhamEQU*;gbMo_<8y%M<3n8p6$|cr;64fNn~X{Rw&d4I{yWKU zkR10>-Vb%T8a2wPMQ92?-+q~w@8@=rV&g_> z$9$k&<0i>}BKZU$#(xzM+hZJzkNF{Y05Sea$yZ5^I->n^WG;R0p-D;BZI4^UqCRtA-p@_=?i4&H2{VLj} z!`>qgk-bNT9Q$6VyzT0qX||kh;2EA_IxKJl)9X5p9k@=*Y;_nn4Q|?|1G~tk$4tj^ z9K-E&t(M!h;CcVQN4S(FS&ipek`({`aK&1{y?-^&JT*%r0`Da($MJB5C&Y%u*Sx$! zn^3qDYmx}26U-QYtqA!>SU33!B5inOQuN>kUb?o4&4vl9obz>ThHuJvAO`o%aS&u6rk9$S`4xK8KyVg!^-m-q}_FeZ*ZQZ=)t~E#Q$= z%o2~qu!<3Pa>`j7ncV2(8YD5-MMxBhRMxSQ(G2>_uh|3>Y!`yvd#abLM7wJ z=)}eZmcQ4h0n7^u-X*S{Ljwt8$N!p{rt}ky|ng;dltXkn?CyO-#)VH_YWU@ z_@=1+t%LU9!gmkccH;Bi+}G;MzVY;rdVf8-?ooR+A&)G(%6|Ss?`;3g;Wt0iS#t9e uKfjSaccJ#p+n4P7{woJQe9<7aL$$|Xb{>6qV#l*@zi|srR?^&l-`@f3E4FU{ literal 0 HcmV?d00001 diff --git a/test/test_polkit_allowlist.py b/test/test_polkit_allowlist.py new file mode 100644 index 000000000..7e7a18706 --- /dev/null +++ b/test/test_polkit_allowlist.py @@ -0,0 +1,40 @@ +import json +import os +import os.path + +import pytest +from rpmlint.checks.PolkitRulesAllowedCheck import PolkitRulesAllowedCheck +from rpmlint.filter import Filter + +from Testing import CONFIG, get_tested_package + + +@pytest.fixture(scope='function', autouse=True) +def polkit_rules_allowed_check(tmpdir): + CONFIG.info = True + + # make the check search for its allow list in 'tmpdir' + CONFIG.configuration['WhitelistDataDir'] = [tmpdir] + CONFIG.configuration['PolkitRulesWhitelist'] = ('allowed-polkit-rules.json',) + + output = Filter(CONFIG) + test = PolkitRulesAllowedCheck(CONFIG, output) + + # make an empty allow list + with open(os.path.join(tmpdir, 'allowed-polkit-rules.json'), 'w') as fd: + json.dump({'polkit-rules-test': {'audits': {'bsc#0': {'comment': '', 'digests': { + '/usr/share/polkit-1/rules.d/regular': 'sha256:invalid' + }}}}}, fd) + + return output, test + + +@pytest.mark.parametrize('package', ['binary/polkit-rules-test']) +def test_polkit_privs(tmpdir, package, polkit_rules_allowed_check): + output, test = polkit_rules_allowed_check + test.check(get_tested_package(package, tmpdir)) + out = output.print_results(output.results) + assert 'noproblem' not in out + assert 'polkit-changed-rules /usr/share/polkit-1/rules.d/regular' in out + assert 'polkit-unauthorized-rules /etc/polkit-1/rules.d/regular' in out + assert 'polkit-ghost-file /usr/share/polkit-1/rules.d/ghost' in out From b188c7bf473436d28d37bf631d0d1b351b7d4ddb Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Mon, 22 Jun 2020 14:47:29 +0200 Subject: [PATCH 06/20] port over lints to enforce polkit-default-privs allow lists --- rpmlint/checks/PolkitDefaultPrivsAllowlist.py | 140 ++++++++++++++++++ .../PolkitDefaultPrivsAllowlist.toml | 37 +++++ test/binary/polkit-0-0.noarch.rpm | Bin 0 -> 7240 bytes .../polkit-def-privs-test-0-0.noarch.rpm | Bin 0 -> 7012 bytes test/test_polkit_default_privs_allowlist.py | 44 ++++++ 5 files changed, 221 insertions(+) create mode 100644 rpmlint/checks/PolkitDefaultPrivsAllowlist.py create mode 100644 rpmlint/descriptions/PolkitDefaultPrivsAllowlist.toml create mode 100644 test/binary/polkit-0-0.noarch.rpm create mode 100644 test/binary/polkit-def-privs-test-0-0.noarch.rpm create mode 100644 test/test_polkit_default_privs_allowlist.py diff --git a/rpmlint/checks/PolkitDefaultPrivsAllowlist.py b/rpmlint/checks/PolkitDefaultPrivsAllowlist.py new file mode 100644 index 000000000..7db560bb9 --- /dev/null +++ b/rpmlint/checks/PolkitDefaultPrivsAllowlist.py @@ -0,0 +1,140 @@ +""" Lints to enforce that polkit rules and actions are allowed in the +(open)SUSE specific 'polkit-default-privs' mechanism. """ + +from xml.dom.minidom import parse + +from .AbstractCheck import AbstractFilesCheck +from .Allowlisting import AbstractSimpleAllowlistCheck + + +POLKIT_DEFAULT_PRIVS_D = '/etc/polkit-default-privs.d/' + + +class PolkitDefaultPrivsOverrideCheck(AbstractSimpleAllowlistCheck): + """ Checks that files that override polkit-default-privs configuration are on an allow list. """ + restricted_paths = (POLKIT_DEFAULT_PRIVS_D,) + error_map = { + 'unauthorized': 'polkit-unauthorized-file', + 'ghost': 'polkit-ghost-file' + } + allowlist_config_key = 'PolkitRulesWhitelist' + + +class PolkitActionsCheck(AbstractFilesCheck): + """ Checks that polkit actions are allowed through polkit-default-privs. """ + def __init__(self, config, output): + super().__init__(config, output, r'^/usr/share/polkit-1/actions/') + self._privs = None + + def check_binary(self, pkg): + self._collect_privs(pkg) + + # `check_file` is not called for ghost files, so handle them here separately + for f in pkg.ghost_files: + if f.startswith('/usr/share/polkit-1/actions/'): + self.output.add_info('E', pkg, 'polkit-ghost-file', f) + + super().check_binary(pkg) + + def check_file(self, pkg, filename): + """Checks files in the actions directory.""" + f = pkg.files[filename] + + try: + # minidom is secure against XML retrieval attacks, but malicious XML + # can cause unbounded memory or CPU usage - which we can live with + xml = parse(f.path) + except Exception as x: + self.output.add_info('E', pkg, 'rpmlint-exception', f'{filename} raised an exception: {x}') + return + for a in xml.getElementsByTagName('action'): + self._check_action(pkg, a) + + def _collect_privs(self, pkg): + self._privs = {} + for filename in self.config.configuration.get('PolkitPrivsFiles', ('/etc/polkit-default-privs.standard',)): + try: + self._parse_privs_file(filename) + except FileNotFoundError: + pass + + permfiles = [] + profiles = ('restrictive', 'standard', 'relaxed') + # first pass: find additional files + for path, f in pkg.files.items(): + if path.startswith(POLKIT_DEFAULT_PRIVS_D) and not f.is_ghost: + bn = path[len(POLKIT_DEFAULT_PRIVS_D):] + parts = bn.rsplit('.', 1) + + if len(parts) == 2 and parts[1] in profiles: + bn = parts[0] + + if bn not in permfiles: + permfiles.append(bn) + + # second pass: parse these files + for f in sorted(permfiles): + f = pkg.dirName() + POLKIT_DEFAULT_PRIVS_D + f + + # prefer parsing profile-specific files (in most-to-least restrictive order) + for profile in profiles: + try: + self._parse_privs_file(f'{f}.{profile}') + break + except FileNotFoundError: + pass + else: + # fall-back to generic file if no profile-specific one was found + self._parse_privs_file(f) + + def _parse_privs_file(self, filename): + with open(filename) as inputfile: + for line in inputfile: + line = line.split('#')[0].rstrip('\n') + if line: + priv, value, *_ = line.split() + self._privs[priv] = value + + def _check_action(self, pkg, action): + """Inspect a single polkit action used by an application.""" + action_id = action.getAttribute('id') + + if action_id in self._privs: + # the action is explicitly whitelisted, nothing else to do + return + + allow_types = ('allow_any', 'allow_inactive', 'allow_active') + foundunauthorized = False + foundno = False + foundundef = False + settings = {} + try: + defaults = action.getElementsByTagName('defaults')[0] + for i in defaults.childNodes: + if not i.nodeType == i.ELEMENT_NODE: + continue + + if i.nodeName in allow_types: + settings[i.nodeName] = i.firstChild.data + except KeyError: + foundunauthorized = True + + for i in allow_types: + if i not in settings: + foundundef = True + settings[i] = '??' + elif settings[i].find('auth_admin') != 0: + if settings[i] == 'no': + foundno = True + else: + foundunauthorized = True + + action_settings = '{} ({}:{}:{})'.format(action_id, *(settings[t] for t in allow_types)) + + if foundunauthorized: + self.output.add_info('E', pkg, 'polkit-unauthorized-privilege', action_settings) + else: + self.output.add_info('E', pkg, 'polkit-untracked-privilege', action_settings) + + if foundno or foundundef: + self.output.add_info('E', pkg, 'polkit-cant-acquire-privilege', action_settings) diff --git a/rpmlint/descriptions/PolkitDefaultPrivsAllowlist.toml b/rpmlint/descriptions/PolkitDefaultPrivsAllowlist.toml new file mode 100644 index 000000000..40e914ed1 --- /dev/null +++ b/rpmlint/descriptions/PolkitDefaultPrivsAllowlist.toml @@ -0,0 +1,37 @@ +polkit-unauthorized-file=""" +A custom polkit rule file is installed by this package. If the package is +intended for inclusion in any SUSE product please open a bug report to request +review of the package by the security team. Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for +more information. +""" + + +polkit-unauthorized-privilege=""" +The package allows unprivileged users to carry out privileged operations +without authentication. This could cause security problems if not done +carefully. If the package is intended for inclusion in any SUSE product please +open a bug report to request review of the package by the security team. Please +refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for +more information. +""" + + +polkit-untracked-privilege=""" +The privilege is not listed in /etc/polkit-default-privs.* which makes it +harder for admins to find. Furthermore polkit authorization checks can easily +introduce security issues. If the package is intended for inclusion in any +SUSE product please open a bug report to request review of the package by the +security team. Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for +more information. +""" + + +polkit-cant-acquire-privilege=""" +Usability can be improved by allowing users to acquire privileges via +authentication. Use e.g. 'auth_admin' instead of 'no' and make sure to define +'allow_any'. This is an issue only if the privilege is not listed in +/etc/polkit-default-privs.* +""" diff --git a/test/binary/polkit-0-0.noarch.rpm b/test/binary/polkit-0-0.noarch.rpm new file mode 100644 index 0000000000000000000000000000000000000000..5dd45f2dd35dffa40776dd2d4cb7fc48b7c6489c GIT binary patch literal 7240 zcmeHMdvF!i89z6V0)b$dC>W@^YHL8a@4NRABqaoq&>$$N5W#SE_iVDmzSsu@93Vij zj%^V~5g4cs$N*BpP+BM!?4SeIwp19AinUUpgo5%44r*1T-*@jpu+#q2e|tA`?(h8e zdz|n5_S{Y8%h{iw`=F4(6|gORy5Ts;s2NjF=Y1jI1amo|4qGsERQumdvOU%_LZiNn%VCQ%Q-@ zL?SB2l(?Fq2{o#yaVAIN(O4!KNh-;t3Rz)6LQ1wwY^`yRzc)=29$7kfYsJsL52d8D zru;`i|bw|)Qv`WnUe0HTiqqdN?|u${;vQbz5sG&`DM;;;rwgP@67Qc z^n+0_1Q1$OMYuP>0_>+C$J&vjcF0Qr(SIc8xKGfhU_9r&IDeLN+&B8K;QR(|Z|58} z#`s@yej^~({{|rJr|2d?j6cln{Q%Mb1h?PJ?Wi;O6``(ZM;(y&&+)=W&IbZQ{=&xr zF+c7T{0iGR&j3Pv;gg(KagOz)|9yby{}LeTF`DyDoYw(j{0`11ar-VnjGxTyyE&f% zi23#aV!o+>7=M8C$2iCRp*s=E>6Rn7nkP_$c|!!*GF>Kn%<~En0;5o}0TpSv(qSntBs_pAsqZmC^Pwb% z*$!)FrotT9sAiVTv~=HwMHmzTTN9Y;GC|gK-8Zfl&wN4kn>6YPCYB-C)B)l1mZwx(d|uo+XZ9ZOS~iL#+SnhAE@rnuV@OI1NtDk#il4q6RM z$1*6&g8Op0n!XVtquv&Pc zW$A8sgsqD+1haB6$IQSj%&BaeR9{!T+2e|5*G_ck(;K{?o zRYqw#yC06YJBV-_-WjPif!5dVji)c6m29bCu5=sQ4 zBoRu6B3Z;wF%-+rqhcruwT|6Xxm!1Vs6Zc{6<=W_C@8*R%*L%i$=@3Idkx+**{l0? z|E55{0SHjl$(>V?>b?(nK~?d{rqakf4<4{gh_B*Ob9j!K_i(Ef+~pQ z(k2$vnc3tu4~q!Fk&UCqJvgy$)a1qq;~sjj_TFLOuDTwTrw8Gfd+@%1S3ywq%~_hM z1U)M0%mq%}*{0m=Sf&L>_gh9H+nPmM<`9t|!61qg%pFR;e}sUdIoa&9$)vIMLNpYC zucz$eNgpEAOsK7^gBMYYmyPJ|dulLEqN=J&>12$iSw@bhnV6Q7DvQQrQaYBDWih2H ziAW?F&rmffNhw8&r9?@RVo@=Zk)krbIkWH7+}|Qsu6(rxc-iIDoJkFE#*Tg2>*WG+ zd!R4e^unEMk9JE1Hcllsmws3zuXYA_RZ@NZZO8xKbMfJi<_;Y5mmj@a+VW2HQv1?w zTMzc_sQB@hT32TL<~Q|Y!{RSCR=bNT`mfkpws>zpYo$Zp(-$zzm znELQBeRE%nWpDyQt&eW>fbRxAU+;TNc02_kOl!@T21X1IxD0t-Rgs-8Ht)9nYO#alYqki_W#R?!T#Tx@E(Rqm2~< z8$VgT@5Y98PX{)WyKgH!&|dwUM^e`1`cMA!{%>Q+-ot;Q6!zb2w62Ojzirdd?X~8@ zvp-!m_QTZkUBesJZkaIumBdB2|D-=JTOPe-ed(IWkx}2fG;g_3b=(mb4%MHof9-yG z!4F?Ld{3Wg`+o<|?b9#jndJ>9-#FBo>3XKeu$qp3HS?GBYCAI5?(xL>w_deL;7H+) W-h>2Ktem&~^xwXVUrJIkx#3^nYZiR~ literal 0 HcmV?d00001 diff --git a/test/binary/polkit-def-privs-test-0-0.noarch.rpm b/test/binary/polkit-def-privs-test-0-0.noarch.rpm new file mode 100644 index 0000000000000000000000000000000000000000..fe820c9aec83fff1fa8029c5ed7078829e221c26 GIT binary patch literal 7012 zcmeHMZ;V|<6`!~JwzMn6nhLEfiQ^}?*n8i-@4b89d*~u`m$qw~?ozhcR<>pC%-pw= z-haJ+w!7O9i*{@~op{RCPmiGw{VH-z+!BvrGSk;^PMEW<+U|JF)m z;L?HjJ@IBqKf8jEl~2R{jnMJ_5x|w;_$t6<(6PbT(nkQ1ukG^wHv!Ag1I^C>!hDiI zhxz?~0Yv)&=rF(kEkLv%1T*DLZZ%ud@|op3qG@}zdaYWm3y(FKXEm9_>rS)bS%k5M zbXXMzam%-D->zEB6P|D_n={Xq7I)oN&9xeqYdcK(Ue#~%s*s-7sCzEA!66|lUO0E0 zp4xrqU)RkKP2cz*hiqjDWrzIyS6(V4Vc(CViTp)_jfT zf71L~&9RQM0{V5DpVfR=^K*buS1Fy;9P2Ewe(86bpVxL=2mSv;+uzds0wAnkdPnnj zyY{}1YW|+)hUOPFuXTADe53!q&j3PBWy}pqRefWcW9-N`X+8*u<2P&mAwbZlZ%Xqk zHNRhTtUKX&)E3wOu(m&>IckjaAJ-h?h8X&=h6VYSuLi{Vs1MFx4T$4k1w{KAK(wO{ zfaQ+?g1sMlhvPmV-=q03Agtg2Wk8(Y>^|>5qSmg1p5H$1pZh391#2u;CVs*2S6A< zuwL^w0CD_5Y#v7osO6PTzXu0#>#w8&||Ja9r~-d$AEkhLR5S)le7%%t0LDkBvR zGC@NHH*^*#3)j)Th016YXL(u(D(7U%RnBr1N3;k67V@~eG7f@hz|&x7Nri){WDs%) z5vgwI;3_xit+*sBa!FMIA*C`&<&2DkOd(Mf4bH|vk$@8kK|pF$W|^d11wj$MAG|E6 zU$hm=X@p_Wgrz`=)EvBJj>{yMAW)_t^Vs7|au~;pLb1oj|d znC)Qf`=BbLLS`~WYZwO@218jeuS~01geGk4t}}moJlvDAA~T{mmtGv>>qV~JIJw!V zn%1Ytn1nRqBm{SrsW=X@%KAbDLPc#dljq3|mCC+-`^-3zQC4KqjMH|71S%@#NEEY_ z&){mX9$xN#9wuR@5mTk9cwVG`?b=UV=wg)9;Pl%S~F}EgK zwT9zVJ>HO2*=*rCe!tz6#&J$Axm@ZX`i;VmV>8QHBv>j)dj{SVQjs~Ye4qy_g1q~M!xR-_OjN1Srbg)2so_J~8EZ=; zkWo9I8L_A_wtK_G)~WFgJ9ghTv2E+7{zN15W0q<2Rsf%(IhR0`+lwJFxRn}Rc+R7?)VO`HT}9@vmC!!uZo6jFqgUYYTc4G zn>lsi_^xnW2_Ij&Yr(bif6e0JyGP)D<$H~V?K|O&zV^>65BHHzl&*n?dF73VUoBP0 z!ZcZP)hlKGU^+B=`1tVo+3nU-cUS)S_MulEeEs9UJSKkowT)jrTYLVV>Fn024PNoi zUHgA>)V}!KmXY76HTQk))6WdO_}c2}k^4{W|L*=Hlj)CNx_HeuvU~D74*dS5gU?o% zotR#=Z0NM^}%Y{=!{9+xBva{*T4Dp>)(5H wD9%ssSe~4pd_H;X>4}%G-*n;Zk4Wjs(h)e7SFAqqvu949x~_9T2Y2lJ7uGeY3;+NC literal 0 HcmV?d00001 diff --git a/test/test_polkit_default_privs_allowlist.py b/test/test_polkit_default_privs_allowlist.py new file mode 100644 index 000000000..67f04ef7f --- /dev/null +++ b/test/test_polkit_default_privs_allowlist.py @@ -0,0 +1,44 @@ +import pytest +from rpmlint.checks.PolkitDefaultPrivsAllowlist import PolkitActionsCheck, PolkitDefaultPrivsOverrideCheck +from rpmlint.filter import Filter + +from Testing import CONFIG, get_tested_package + + +@pytest.fixture(scope='function', autouse=True) +def polkit_default_privs_override_check(tmpdir): + CONFIG.info = True + output = Filter(CONFIG) + test = PolkitDefaultPrivsOverrideCheck(CONFIG, output) + return output, test + + +@pytest.mark.parametrize('package', ['binary/polkit-def-privs-test']) +def test_polkit_perm_files(tmpdir, package, polkit_default_privs_override_check): + output, test = polkit_default_privs_override_check + test.check(get_tested_package(package, tmpdir)) + out = output.print_results(output.results) + assert 'noproblem' not in out + assert 'polkit-unauthorized-file /etc/polkit-default-privs.d/foo.standard' in out + assert 'polkit-ghost-file /etc/polkit-default-privs.d/ghost' in out + + +@pytest.fixture(scope='function', autouse=True) +def polkit_actions_check(tmpdir): + CONFIG.info = True + output = Filter(CONFIG) + test = PolkitActionsCheck(CONFIG, output) + return output, test + + +@pytest.mark.parametrize('package', ['binary/polkit']) +def test_polkit_actions(tmpdir, package, polkit_actions_check): + output, test = polkit_actions_check + test.check(get_tested_package(package, tmpdir)) + out = output.print_results(output.results) + assert 'noproblem' not in out + assert 'polkit-cant-acquire-privilege org.opensuse.test.bar (no:auth_admin:auth_admin)' in out + assert 'polkit-unauthorized-privilege org.opensuse.test.baz (auth_admin:auth_admin:auth_self)' in out + assert 'polkit-unauthorized-privilege org.opensuse.test.baz2 (auth_admin:auth_admin:yes)' in out + assert 'polkit-untracked-privilege org.opensuse.test.bar (no:auth_admin:auth_admin)' in out + assert 'polkit-untracked-privilege org.opensuse.test.foo (auth_admin:auth_admin:auth_admin)' in out From 5b9449b624357ba41d380e2c827ab374c05995b5 Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Tue, 23 Jun 2020 17:35:22 +0200 Subject: [PATCH 07/20] port over lints to enforce sXid/capability allow lists --- rpmlint/checks/SUIDPermissionsCheck.py | 223 ++++++++++++++++++ .../descriptions/SUIDPermissionsCheck.toml | 104 ++++++++ test/binary/suid-no-scripts-0-0.x86_64.rpm | Bin 0 -> 6284 bytes test/binary/suid-with-scripts-0-0.x86_64.rpm | Bin 0 -> 6596 bytes test/suid_permissions_check.py | 150 ++++++++++++ 5 files changed, 477 insertions(+) create mode 100644 rpmlint/checks/SUIDPermissionsCheck.py create mode 100644 rpmlint/descriptions/SUIDPermissionsCheck.toml create mode 100644 test/binary/suid-no-scripts-0-0.x86_64.rpm create mode 100644 test/binary/suid-with-scripts-0-0.x86_64.rpm create mode 100644 test/suid_permissions_check.py diff --git a/rpmlint/checks/SUIDPermissionsCheck.py b/rpmlint/checks/SUIDPermissionsCheck.py new file mode 100644 index 000000000..6c0c0fe54 --- /dev/null +++ b/rpmlint/checks/SUIDPermissionsCheck.py @@ -0,0 +1,223 @@ +# vim: sw=4 et sts=4 ts=4 : +############################################################################# +# File : CheckSUIDPermissions.py +# Package : rpmlint +# Author : Ludwig Nussel +# Purpose : Check for /usr/share/permissions violations +############################################################################# + +import os +import stat +import sys + +import rpm +from ..pkg import FakePkg +from .AbstractCheck import AbstractCheck + +_permissions_d_allowed = ( + 'lprng', + 'lprng.paranoid', + 'mail-server', + 'mail-server.paranoid', + 'postfix', + 'postfix.paranoid', + 'sendmail', + 'sendmail.paranoid', + 'squid', + 'texlive', + 'texlive.texlive', + 'otrs', # bsc#1118049 +) + + +class SUIDCheck(AbstractCheck): + def __init__(self, config, output): + super().__init__(config, output) + + self.perms = {} + + for fname in self._paths_to('permissions', 'permissions.secure'): + if os.path.exists(fname): + self._parsefile(fname) + + @staticmethod + def _paths_to(*file_names): + # we used to store the permissions data in /etc even though they aren't configuration files + # the allow listing should check both paths (old /etc and new /usr/share) until all + # distributions using the old one (SLE15) are retired + for name in file_names: + # return the new path first. + # chkstat prefers the new paths over the old ones, so callers that only care about the + # first matching file must mimic that. + yield '/usr/share/permissions/' + name + yield '/etc/' + name + + def _parsefile(self, fname): + lastfn = None + with open(fname) as inputfile: + for lnr, line in enumerate(inputfile, start=1): + line = line.split('#')[0].strip() + if not line: + continue + + if line.startswith('+capabilities '): + line = line[len('+capabilities '):] + if lastfn: + self.perms[lastfn]['fscaps'] = line + continue + + line = line.split() + if len(line) == 3: + fn = line[0] + owner = line[1].replace('.', ':') + mode = line[2] + + self.perms[fn] = {'owner': owner, 'mode': stat.S_IMODE(int(mode, 8))} + # for permissions that don't change and therefore + # don't need special handling + if fname in self._paths_to('permissions'): + self.perms[fn]['static'] = True + else: + print(f'{fname}: Malformatted line {lnr}: {" ".join(line)}...', file=sys.stderr) + + def check_binary(self, pkg): + permfiles = set() + # first pass, find permissions.d files and check against allow list + for f, finfo in pkg.files.items(): + for prefix in self._paths_to('permissions.d/'): + if f.startswith(prefix): + if finfo.is_ghost: + self.output.add_info('E', pkg, 'permissions-ghostfile', f) + continue + + bn = f[len(prefix):] + if bn not in _permissions_d_allowed: + self.output.add_info('E', pkg, 'permissions-unauthorized-file', f) + + bn = 'permissions.d/' + bn.split('.')[0] + permfiles.add(bn) + + # parse found permissions.d files + for f in permfiles: + # check for a .secure file first, falling back to the plain file + for path in self._paths_to(f + '.secure', f): + if path in pkg.files: + self._parsefile(pkg.dirName() + path) + break + + need_set_permissions = False + found_suseconfig = False + # second pass, find permissions violations + for f, pkgfile in pkg.files.items(): + + if pkgfile.filecaps: + self.output.add_info('E', pkg, 'permissions-fscaps', f'{f} has fscaps "{pkgfile.filecaps}"') + + mode = pkgfile.mode + owner = pkgfile.user + ':' + pkgfile.group + + need_verifyscript = False + if f in self.perms or (stat.S_ISDIR(mode) and f + '/' in self.perms): + if stat.S_ISLNK(mode): + self.output.add_info('W', pkg, 'permissions-symlink', f) + continue + + need_verifyscript = True + + m = 0 + o = 'invalid' + if stat.S_ISDIR(mode): + if f in self.perms: + self.output.add_info('W', pkg, 'permissions-dir-without-slash', f) + else: + f += '/' + + if stat.S_ISREG(mode) and mode & (stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH): + # pie binaries have 'shared object' here + if (pkgfile.magic.startswith('ELF ') and + ('shared object' not in pkgfile.magic) and + ('pie executable' not in pkgfile.magic)): + # TODO: not sure how much sense it makes to duplicate this check here + # (there's also many other compilation options we *could* check) + self.output.add_info('E', pkg, 'non-position-independent-executable', f) + + m = self.perms[f]['mode'] + o = self.perms[f]['owner'] + + if stat.S_IMODE(mode) != m: + self.output.add_info('E', pkg, 'permissions-incorrect', + f'{f} has mode {oct(stat.S_IMODE(mode))} but should be {oct(m)}') + + if owner != o: + self.output.add_info('E', pkg, 'permissions-incorrect-owner', + f'{f} belongs to {owner} but should be {o}') + + elif not stat.S_ISLNK(mode): + if f + '/' in self.perms: + self.output.add_info('W', pkg, 'permissions-file-as-dir', + f + ' is a file but listed as directory') + + if mode & (stat.S_ISUID | stat.S_ISGID): + + need_verifyscript = True + msg = '%(file)s is packaged with ' \ + 'setuid/setgid bits (0%(mode)o)' % \ + {'file': f, 'mode': stat.S_IMODE(mode)} + if not stat.S_ISDIR(mode): + self.output.add_info('E', pkg, 'permissions-file-setuid-bit', msg) + else: + self.output.add_info('W', pkg, 'permissions-directory-setuid-bit', msg) + + if stat.S_ISREG(mode): + if 'shared object' not in pkgfile.magic and 'pie executable' not in pkgfile.magic: + self.output.add_info('E', pkg, 'non-position-independent-executable', f) + + if mode & stat.S_IWOTH: + need_verifyscript = True + self.output.add_info('E', pkg, 'permissions-world-writable', + f'{f} is packaged with world writable permissions ({oct(mode)})') + + script = not isinstance(pkg, FakePkg) and (pkg[rpm.RPMTAG_POSTIN] or pkg.scriptprog(rpm.RPMTAG_POSTINPROG)) + found = False + if script: + for line in script.split('\n'): + if 'chkstat -n' in line and f in line: + found = True + break + + if 'SuSEconfig --module permissions' in line \ + or 'run_permissions is obsolete' in line: + found = True + found_suseconfig = True + break + + if need_verifyscript and \ + (f not in self.perms or 'static' not in self.perms[f]): + + if not script or not found: + self.output.add_info('E', pkg, 'permissions-missing-postin', + f'missing %set_permissions {f} in %post') + + need_set_permissions = True + script = pkg[rpm.RPMTAG_VERIFYSCRIPT] or pkg[rpm.RPMTAG_VERIFYSCRIPTPROG] + + found = False + if script: + for line in script.split('\n'): + if '/chkstat' in line and f in line: + found = True + break + + if not script or not found: + self.output.add_info('W', pkg, 'permissions-missing-verifyscript', + f'missing %verify_permissions -e {f}') + + if need_set_permissions: + if 'permissions' not in (x[0] for x in pkg.prereq): + self.output.add_info('E', pkg, 'permissions-missing-requires', + "missing 'permissions' in PreReq") + + if found_suseconfig: + # TODO: nothing in tumbleweed uses this anymore. can probable be dropped. + self.output.add_info('I', pkg, 'permissions-suseconfig-obsolete', + '%run_permissions is obsolete') diff --git a/rpmlint/descriptions/SUIDPermissionsCheck.toml b/rpmlint/descriptions/SUIDPermissionsCheck.toml new file mode 100644 index 000000000..01f4d33a0 --- /dev/null +++ b/rpmlint/descriptions/SUIDPermissionsCheck.toml @@ -0,0 +1,104 @@ +permissions-unauthorized-file=""" +If the package is intended for inclusion in any SUSE product please open a bug +report to request review of the package by the security team. Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for +more information. +""" + + + +permissions-symlink=""" +permissions handling for symlinks is useless. Please contact security@suse.de +to remove the entry. Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for +more information. +""" + + +permissions-dir-without-slash=""" +the entry in the permissions file refers to a directory. Please contact +security@suse.de to append a slash to the entry in order to avoid security +problems. Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for +more information. +""" + + +permissions-file-as-dir=""" +the entry in the permissions file refers to a directory but the package +actually contains a file. Please contact security@suse.de to remove the slash. +Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for +more information. +""" + + +permissions-incorrect=""" +please use the %attr macro to set the correct permissions. +""" + + +permissions-incorrect-owner=""" +please use the %attr macro to set the correct ownership. +""" + + +permissions-file-setuid-bit=""" +If the package is intended for inclusion in any SUSE product please open a bug +report to request review of the program by the security team. Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for +more information. +""" + + +permissions-directory-setuid-bit=""" +If the package is intended for inclusion in any SUSE product please open a bug +report to request review of the package by the security team. Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for +more information. +""" + + +permissions-world-writable=""" +If the package is intended for inclusion in any SUSE product please open a bug +report to request review of the package by the security team. Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for +more information. +""" + + +permissions-fscaps=""" +Packaging file capabilities is currently not supported. Please use normal +permissions instead. You may contact the security team to request an entry that +sets capabilities in /usr/share/permissions/permissions instead. +""" + + +permissions-missing-postin=""" +Please add an appropriate %post section +""" + + +permissions-missing-requires=""" +Please add 'PreReq: permissions' +""" + + +permissions-missing-verifyscript=""" +Please add a %verifyscript section +""" + + +permissions-suseconfig-obsolete=""" +The %run_permissions macro calls SuSEconfig which sets permissions for all +files in the system. Please use %set_permissions instead to only set +permissions for files contained in this package +""" + + +permissions-ghostfile=""" +This package installs a permissions file as a %ghost file. This is not allowed +as it is impossible to review. Please refer to +https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for +more information. +""" diff --git a/test/binary/suid-no-scripts-0-0.x86_64.rpm b/test/binary/suid-no-scripts-0-0.x86_64.rpm new file mode 100644 index 0000000000000000000000000000000000000000..02bde8545d5e129837339d2c1b948b6c3cb653bb GIT binary patch literal 6284 zcmeHLU2GLa6rOFbP%6aAkAe_$B}OS`Z}#a>C-UnkKR$D>(jNnnZ3IA3pf*uq4JN64G=8`U}CZ ze-N+<7M=n)0Sp_&O1Ai zR(;zJY{k+w-?vR)(KR(NG~075TXSrtIJ(a%u{@vYRMD9lSX}j$z~CxX9D^B(sxm{R zzG3U!Hci)cIp?mcYP!o^>M`4=Jyk;NJ?4bta7ePVid*G5&<$s44POf}>8j2h<7lD$D?c`4Yz9 zer^YZzJ&9T&lG-V3=6Y_f4Sgy2tFkEJV3}VeFcd5myL}}SNA98NmuvR4t`wkBp|NW zDdJBHz77!mxL@>d5dKSoZxsF!!8eWZIy@)Fdw{T?x;cQjA7Ai6!MWgj1rG#&MewrV zuL@oR1ik7G0b>3>K-gd1kug3E^PvA2Agnj-OF&%zYr#(gVm!V-^q&EQ_J1TpDFuQ&(#e^4Hv42s%JZ<<~gQ$Ju@M36s6-I<4%f|*-{d-Qe5pN z@Xg5DxR~+k%!4z?meGttHr{BEqa1qkTBF`-%#(!sZJ^P^w3jE%MEG!IN!SgdULWhA z1LcTj!O3#k$2(#kg#Fj12zPtZxwV<-C8cD@Dltk{Ny!kOW=YHXs#$^$95&e2HG}(- zHF*D|0V~lrRHL&>byp@7o;vGw`C&|2ys*+jT8O+tE6UwPxz{p^asWq3;3CLnUg=JI z7AQq|Nmtv7Rjb?E)^$CyV$G`7r3)ZDNK&{kay5nv!O|#(ky>R-Sn=hQdT?ui(`2B+ zdg7=O!NvKPldx(Sk^Zg36n8~VY6+~E-5zlgI2-S+rUL|anyoN^j)qzq$PQ6?U{VHm z!?88rX1YlY)8-o0Jk?TyfP$ZD4!0@gfu$HKQ@Fx>%d|Yo{lLLrV)k1Z`yVknI&uMc z(-nJbCwzPxfBtLoTXp0v2|x5{%bB-NN{^GZ8_CQmKNi><@$9~X-G@(H`XXHX`GS>? z9lH40o6er&-#mX{>a6`o`;L+w4?O+C<@IwXz5LqFq^$0laNqY62OIWITHX2ao%1K$ zJJc8t2kr8yXLq=}7w%K_wv({Nllt_P)rTH!oqKrpw63R~d1v42yWbmKwExrN>iY|K dwH(_(q@B_Tyx{{M%^&>m>`!yDm)*E-{Xf@Jo^SvF literal 0 HcmV?d00001 diff --git a/test/binary/suid-with-scripts-0-0.x86_64.rpm b/test/binary/suid-with-scripts-0-0.x86_64.rpm new file mode 100644 index 0000000000000000000000000000000000000000..5b5e04d1b15643b8bb2d77f139b05792cf5e57dd GIT binary patch literal 6596 zcmeHMTWl3Y7@pHppd2A0pn?#x@PHKAbM}5oq+nVKO;j#|DF`&PduH3E=a$_)y`V;; zh9F>p;ERAHVB-tL5b^Q=MvTT7pL`$&qedi>6eB2x5E6m<|9b|g^hKZDY5x7@n}6n? z*>7g{B!4d*z5IQeKuLvFKQH$BQI{CHfnSM2Q4%He{QHt3P|aBMim+a?e)%j_~{{{%-Nd*kXr+)`T|I1)7KK%zE`d
    pV3S+vhsgj~go@sijod>sK z7^Y-1#icfNRMYZYXmXXjt{AdyDzYIln2?Z$Mc3qG-yU3~oR@AL$R0Utu7xGN5WA8` zqClcRqClcRqClcRqClcRqClcRqClcRqCld+|6PGNCo(cJvIm5KoCYDI@+LZ@H)ccA z5-`YX)K3D$K6$VPFf5q&z&r`&P3kzm$oX{6f93ok&M}WN5o`wMmpGrp`DH-Nt1RUl z^DG#@h4U+b=!YCfJpVO7^tW?<9T4Mh=KLlg`nPa?i}Om2W2o5Q%Q-fn4T8Zt)#F%9 zRSj{Dv5_C=JOhaRF~@}c?*l}APH~R9A;)>qk9jBPAE&2~-(T~mun%(780x91oTHAg zh7|4v461(+5XV2x{SO0T{}#?sSM=li;l9n}{#~3uQsZ^FPV_GTg!pv}0WrSGxy*SJ z=O1$3%=ssrF9(GA>b?TR^{?Rk8_wGRVSROHImdm%{y%WOp8J2{d?WYcIl*|Fxc@5W zxUUc|jk==W0fhCZQ72q~p7U*-V~Py@J303_AK<*e`5w-zfUv&wARxx?0mS)+YJA#0 z?mq|jU07ATaXd|g+GA7?iB>3~nP)NkWNT^V^n|3k*bD~1X zBFK*lo?l=gj3=A_oH@VP9nfkhmdX)x%H?isCd*en*`ttkrLe+W-}70X6#P=PkMvna zhoKR-t2>NnB#0#;=lG=@E}|nSmm}eBwIlBbxi}bqygs~a7^^rZ8dDv~)ilM1Jf!T( znoT{&(_G884bQO^O))9cWTqLK?bxd3Qq9(FTXsxKR~$=+@5$YQbCY`>W0b*6a5fCw zY)~n}Q20*8WlX=Znl6C%77eWtMvViT3zosn$c(m{5?$mrR#;HRBbO%X(HO89@+o)i8(`Icgh4v6luVK@`LOFk(ePWcTi@+%#Vi zaCqjw#JF=Z;^QD-kUIk177CXhpKX*hHn=Bo!86`~`TJ2h{5ZoOAGUa(;u;38g(W#6 zu2clEQxJ=WCWszL9K!Jz3#`-`buE?zv8AJR<(jo^t?N6US-Eaa^RmUz-Gk%oc8iq& zE`b}B18Aw1di_#fjHm;51vm}+OKw+CE|uZ3{KHAyieDyuTZzu^hZt5vm@&RF{7vA! zSg{iI6If|HLq9qis*xvJL}rk_cj1g%rjj>ZRi~P6GKDG*q~tx1f}biDGbv@BA!)KJ zG0Dvvx&i+H$a@xkf#PpO?YDJgH}Q*N4D+c&@&{rRs+hwI4P6#igFxeM={OFd7X z-9%^yPy>gWE_&la!VGIZti+t%JQU++9Jb>^XyJtxWbCti8u`i6NE z-+XIFSdjOQf9$&n0}cBou5CXxXW{rqk7j~luT4C^ZM(g@X}_$to`X4FRHv`5JoQ}j zyc4sgb-eWIyZaC9es5&Sp-<1q?>FtreX)_GcBF2=4j=h=;lM{1f0`X{c4qyCzX4bJ B=92&b literal 0 HcmV?d00001 diff --git a/test/suid_permissions_check.py b/test/suid_permissions_check.py new file mode 100644 index 000000000..486b42296 --- /dev/null +++ b/test/suid_permissions_check.py @@ -0,0 +1,150 @@ +import os +import stat + +import pytest +import rpm +from rpmlint.checks.SUIDPermissionsCheck import SUIDCheck +from rpmlint.filter import Filter +from rpmlint.pkg import FakePkg, PkgFile + +from Testing import CONFIG, get_tested_package + + +@pytest.fixture(scope='function', autouse=True) +def suid_check(tmpdir): + CONFIG.info = True + output = Filter(CONFIG) + test = SUIDCheck(CONFIG, output) + + # set the allow list of expected/allowed permissions, matching suid-test-0-0.x86_64.rpm + test.perms = { + '/bin/suid_root': {'owner': 'root:root', 'mode': 0o4750}, + '/suid_root_dir/': {'owner': 'root:root', 'mode': 0o2750}, + } + + return output, test + + +@pytest.mark.parametrize('package', ['binary/suid-with-scripts']) +def test_suid_ok(tmpdir, package, suid_check): + output, test = suid_check + test.check(get_tested_package(package, tmpdir)) + out = output.print_results(output.results) + assert out == '' + + +@pytest.mark.parametrize('package', ['binary/suid-with-scripts']) +def test_suid_dir_without_slash(tmpdir, package, suid_check): + output, test = suid_check + test.perms['/suid_root_dir'] = test.perms['/suid_root_dir/'] + del test.perms['/suid_root_dir/'] + + test.check(get_tested_package(package, tmpdir)) + out = output.print_results(output.results) + assert 'noproblem' not in out + assert 'permissions-dir-without-slash /suid_root_dir' in out + + +@pytest.mark.parametrize('package', ['binary/suid-with-scripts']) +def test_suid_file_as_dir(tmpdir, package, suid_check): + output, test = suid_check + test.perms['/bin/suid_root/'] = test.perms['/bin/suid_root'] + del test.perms['/bin/suid_root'] + + test.check(get_tested_package(package, tmpdir)) + out = output.print_results(output.results) + assert 'noproblem' not in out + assert 'permissions-file-as-dir /bin/suid_root' in out + assert 'non-position-independent-executable /bin/suid_root' in out + assert 'permissions-file-setuid-bit /bin/suid_root' in out + + +@pytest.mark.parametrize('package', ['binary/suid-no-scripts']) +def test_suid_missing_scripts(tmpdir, package, suid_check): + output, test = suid_check + test.check(get_tested_package(package, tmpdir)) + out = output.print_results(output.results) + assert 'noproblem' not in out + assert 'permissions-missing-requires' in out + assert 'permissions-missing-verifyscript' in out + assert 'permissions-missing-postin' in out + + +@pytest.mark.parametrize('package', ['binary/suid-with-scripts']) +def test_suid_non_allowed(tmpdir, package, suid_check): + output, test = suid_check + test.perms = {} + + test.check(get_tested_package(package, tmpdir)) + out = output.print_results(output.results) + assert 'noproblem' not in out + assert 'permissions-directory-setuid-bit' in out + assert 'permissions-file-setuid-bit' in out + + +def test_suid_unauthorized_permissions(suid_check): + output, test = suid_check + + unauth_file = PkgFile('/usr/share/permissions/permissions.d/unauthorized') + unauth_file.user = 'root' + unauth_file.group = 'root' + unauth_file.filecaps = '+caps' + ghost_file = PkgFile('/etc/permissions.d/ghost') + ghost_file.user = 'root' + ghost_file.group = 'root' + ghost_file.flags = rpm.RPMFILE_GHOST + + with FakePkg('suid', [unauth_file, ghost_file]) as package: + package.is_source = False + + os.makedirs(os.path.dirname(package.dirName() + '/' + unauth_file.name)) + open(package.dirName() + '/' + unauth_file.name, 'w').close() + + test.check(package) + + out = output.print_results(output.results) + assert 'noproblem' not in out + assert 'permissions-unauthorized-file /usr/share/permissions/permissions.d/unauthorized' in out + assert 'permissions-fscaps /usr/share/permissions/permissions.d/unauthorized' in out + assert 'permissions-ghostfile /etc/permissions.d/ghost' in out + + +@pytest.mark.parametrize('package', ['binary/suid-with-scripts']) +def test_suid_world_writable(tmpdir, package, suid_check): + output, test = suid_check + + pkg = get_tested_package(package, tmpdir) + pkg.files['/bin/suid_root'].mode |= stat.S_IWOTH + del test.perms['/bin/suid_root'] + test.check(pkg) + + out = output.print_results(output.results) + assert 'noproblem' not in out + assert 'permissions-world-writable /bin/suid_root' in out + + +@pytest.mark.parametrize('package', ['binary/suid-with-scripts']) +def test_suid_incorrect(tmpdir, package, suid_check): + output, test = suid_check + + test.perms['/bin/suid_root']['owner'] = 'foo:bar' + test.perms['/bin/suid_root']['mode'] = 0 + test.check(get_tested_package(package, tmpdir)) + + out = output.print_results(output.results) + assert 'noproblem' not in out + assert 'permissions-incorrect-owner /bin/suid_root' in out + assert 'permissions-incorrect /bin/suid_root' in out + + +@pytest.mark.parametrize('package', ['binary/suid-with-scripts']) +def test_suid_symlink(tmpdir, package, suid_check): + output, test = suid_check + + pkg = get_tested_package(package, tmpdir) + pkg.files['/bin/suid_root'].mode = (pkg.files['/bin/suid_root'].mode ^ stat.S_IFREG) | stat.S_IFLNK + test.check(pkg) + + out = output.print_results(output.results) + assert 'noproblem' not in out + assert 'permissions-symlink /bin/suid_root' in out From a8cfc3093258bb72e0822dd8d3a89d79fc6224fb Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Thu, 25 Jun 2020 10:56:27 +0200 Subject: [PATCH 08/20] fix lints for allow lists --- rpmlint/checks/Allowlisting.py | 4 ++-- rpmlint/checks/SUIDPermissionsCheck.py | 8 +++++--- test/test_polkit_default_privs_allowlist.py | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/rpmlint/checks/Allowlisting.py b/rpmlint/checks/Allowlisting.py index f2c0a1ee8..198260ed4 100644 --- a/rpmlint/checks/Allowlisting.py +++ b/rpmlint/checks/Allowlisting.py @@ -166,7 +166,7 @@ def compareDigests(self, pkg): dig_res = DigestVerificationResult(path, alg, digest, encountered) results.append(dig_res) - return (all([res.matches() for res in results]), results) + return all(res.matches() for res in results), results def _verifyBugNr(self): """Perform some sanity checks on the bug nr associated with this audit @@ -343,7 +343,7 @@ def collect_restricted_files(self, pkg): def is_restricted(f): return any(f.startswith(restricted) for restricted in self.restricted_paths) - return set(f for f in pkg.files.keys() if is_restricted(f)) + return {f for f in pkg.files.keys() if is_restricted(f)} def check_binary(self, pkg): restricted_files = self.collect_restricted_files(pkg) diff --git a/rpmlint/checks/SUIDPermissionsCheck.py b/rpmlint/checks/SUIDPermissionsCheck.py index 6c0c0fe54..55605c9e2 100644 --- a/rpmlint/checks/SUIDPermissionsCheck.py +++ b/rpmlint/checks/SUIDPermissionsCheck.py @@ -11,8 +11,9 @@ import sys import rpm -from ..pkg import FakePkg + from .AbstractCheck import AbstractCheck +from ..pkg import FakePkg _permissions_d_allowed = ( 'lprng', @@ -77,7 +78,10 @@ def _parsefile(self, fname): # don't need special handling if fname in self._paths_to('permissions'): self.perms[fn]['static'] = True + + lastfn = fn else: + lastfn = None print(f'{fname}: Malformatted line {lnr}: {" ".join(line)}...', file=sys.stderr) def check_binary(self, pkg): @@ -124,8 +128,6 @@ def check_binary(self, pkg): need_verifyscript = True - m = 0 - o = 'invalid' if stat.S_ISDIR(mode): if f in self.perms: self.output.add_info('W', pkg, 'permissions-dir-without-slash', f) diff --git a/test/test_polkit_default_privs_allowlist.py b/test/test_polkit_default_privs_allowlist.py index 67f04ef7f..64594538f 100644 --- a/test/test_polkit_default_privs_allowlist.py +++ b/test/test_polkit_default_privs_allowlist.py @@ -31,7 +31,7 @@ def polkit_actions_check(tmpdir): return output, test -@pytest.mark.parametrize('package', ['binary/polkit']) +@pytest.mark.parametrize('package', ['binary/polkit-0']) def test_polkit_actions(tmpdir, package, polkit_actions_check): output, test = polkit_actions_check test.check(get_tested_package(package, tmpdir)) From ed09d88d4aa869940b23fed359921b6afc4c0261 Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Thu, 25 Jun 2020 13:04:52 +0200 Subject: [PATCH 09/20] Allowlisting.py: turn code into (more) idiomatic python --- rpmlint/checks/Allowlisting.py | 150 +++++++++---------------- rpmlint/checks/CronJobsCheck.py | 1 - rpmlint/checks/SUIDPermissionsCheck.py | 1 - 3 files changed, 54 insertions(+), 98 deletions(-) diff --git a/rpmlint/checks/Allowlisting.py b/rpmlint/checks/Allowlisting.py index 198260ed4..b1e7a06db 100644 --- a/rpmlint/checks/Allowlisting.py +++ b/rpmlint/checks/Allowlisting.py @@ -1,10 +1,10 @@ -# vim: sw=4 ts=4 sts=4 et : ############################################################################# # Author : Matthias Gerstner # Purpose : reusable code for dealing with security allow lists ############################################################################# import abc +from dataclasses import make_dataclass import hashlib import json import os @@ -15,36 +15,17 @@ from rpmlint.checks.AbstractCheck import AbstractCheck -class DigestVerificationResult(object): +class DigestVerificationResult(make_dataclass('', ['path', 'algorithm', 'expected', 'encountered'])): """This type represents the result of a digest verification as returned - from AuditEntry.compareDigests().""" - - def __init__(self, path, alg, expected, encountered): - - self.m_path = path - self.m_alg = alg - self.m_expected = expected - self.m_encountered = encountered - - def path(self): - return self.m_path - - def algorithm(self): - return self.m_alg + from AuditEntry.compare_digests().""" def matches(self): """Returns a boolean whether the encountered digest matches the expected digest.""" - return self.m_expected == self.m_encountered - - def expected(self): - return self.m_expected - - def encountered(self): - return self.m_encountered + return self.expected == self.encountered -class AuditEntry(object): +class AuditEntry: """This object represents a single audit entry as found in an allow entry like: "bsc#1234": { @@ -58,48 +39,30 @@ class AuditEntry(object): """ def __init__(self, bug): + self.bug = bug + self._verify_bug_nr() + self.comment = '' + self.digests = {} - self.m_bug = bug - self._verifyBugNr() - self.m_comment = '' - self.m_digests = {} - - def bug(self): - return self.m_bug - - def setComment(self, comment): - self.m_comment = comment - - def comment(self): - return self.m_comment - - def setDigests(self, digests): + def _set_digests(self, digests): for path, digest in digests.items(): - self._verifyPath(path) - self._verifyDigestSyntax(digest) - - self.m_digests = digests + self._verify_path(path) + self._verify_digest_syntax(digest) - def digests(self): - """Returns a dictionary specifying file paths and their allow listed - digests. The digests are suitable for the - Python hashlib module. They're of the form ':'. As a - special case the digest entry can be 'skip:' which indicates - that no digest verification should be performed and the file is - acceptable regardless of its contents.""" - return self.m_digests + self.digests = digests - def isSkipDigest(self, digest): + @staticmethod + def is_skip_digest(digest): """Returns whether the given digest entry denotes the special 'skip digest' case which means not to check the file digest at all.""" return digest == 'skip:' - def coversAllFiles(self, file_paths): + def covers_all_files(self, file_paths): """Returns a boolean indicating whether all files from the set 'file_paths' are covered by this audit.""" - return file_paths.issubset(self.digests().keys()) + return file_paths.issubset(self.digests.keys()) - def compareDigests(self, pkg): + def compare_digests(self, pkg): """Compares the digests recorded in this AuditEntry against the actual files coming from the given rpmlint @pkg. Returns a tuple of (boolean, [DigestVerificationResult, ...]). The boolean indicates the @@ -111,15 +74,15 @@ def compareDigests(self, pkg): results = [] # NOTE: syntax and algorithm validity of stored digests was already - # checked in setDigests() so we can skip the respective error handling + # checked in _set_digests() so we can skip the respective error handling # here. fileinfos = pkg.files - for path, digest in self.digests().items(): + for path, digest in self.digests.items(): alg, digest = digest.split(':', 1) - if self.isSkipDigest(f'{alg}:{digest}'): + if self.is_skip_digest(f'{alg}:{digest}'): results.append(DigestVerificationResult(path, alg, digest, digest)) continue @@ -129,7 +92,7 @@ def compareDigests(self, pkg): src_info = fileinfos.get(path, None) if not src_info: - raise Exception('expected file {} is not part of the RPM'.format(path)) + raise Exception(f'expected file {path} is not part of the RPM') # resolve potential symbolic links # @@ -143,7 +106,7 @@ def compareDigests(self, pkg): dst_info = pkg.readlink(src_info) if not dst_info: - raise Exception('symlink {} -> {} is broken or pointing outside this RPM'.format(src_info.path, src_info.linkto)) + raise Exception(f'symlink {src_info.path} -> {src_info.linkto} is broken or pointing outside this RPM') # NOTE: this path is dynamic, rpmlint unpacks the RPM # contents into a temporary directory even when outside the @@ -168,19 +131,19 @@ def compareDigests(self, pkg): return all(res.matches() for res in results), results - def _verifyBugNr(self): + def _verify_bug_nr(self): """Perform some sanity checks on the bug nr associated with this audit entry.""" - parts = self.m_bug.split('#') + parts = self.bug.split('#') if len(parts) != 2 or \ parts[0] not in ('bsc', 'boo', 'bnc') or \ not parts[1].isdigit(): - raise Exception('Bad bug nr# "{}"'.format(self.m_bug)) + raise Exception(f'Bad bug nr# "{self.bug}"') - def _verifyDigestSyntax(self, digest): - if self.isSkipDigest(digest): + def _verify_digest_syntax(self, digest): + if self.is_skip_digest(digest): return parts = digest.split(':') @@ -194,13 +157,13 @@ def _verifyDigestSyntax(self, digest): except ValueError: raise Exception('Bad digest algorithm in ' + digest) - def _verifyPath(self, path): + def _verify_path(self, path): if not path.startswith(os.path.sep): raise Exception('Bad allow listing path ' + path) def allowlist_for_package(allowlist_path, pkg_name): - class AllowlistParser(object): + class AllowlistParser: """This type knows how to parse the JSON allow listing format. The format is documented in [1]. @@ -211,13 +174,13 @@ def __init__(self, wl_path): """Creates a new instance of AllowlistParser that operates on @wl_path.""" - self.m_path = wl_path + self.path = wl_path def parse(self, package): """Parses the allow list file for the current package and returns a list of AuditEntry objects.""" try: - with open(self.m_path, 'r') as fd: + with open(self.path, 'r') as fd: data = json.load(fd) try: @@ -225,15 +188,13 @@ def parse(self, package): except KeyError: return [] - return self._parseAllowlistEntry(package, config) + return self._parse_allowlist_entry(package, config) except Exception as e: _, _, tb = sys.exc_info() fn, ln, _, _ = traceback.extract_tb(tb)[-1] - raise Exception(self._getErrorPrefix() + 'Failed to parse JSON file: {}:{}: {}'.format( - fn, ln, str(e) - )) + raise Exception(self._get_error_prefix() + f'Failed to parse JSON file: {fn}:{ln}: {e}') - def _parseAllowlistEntry(self, package, config): + def _parse_allowlist_entry(self, package, config): """Parses a single JSON allow entry and returns a AuditEntry() object for it. On non-critical error conditions None is returned, otherwise an exception is raised.""" @@ -243,13 +204,13 @@ def _parseAllowlistEntry(self, package, config): audits = config.get('audits') if not audits: - raise Exception(self._getErrorPrefix() + f"no 'audits' entries for package {package}") + raise Exception(self._get_error_prefix() + f"no 'audits' entries for package {package}") for bug, data in audits.items(): try: - audit = self._parseAuditEntry(bug, data) + audit = self._parse_audit_entry(bug, data) except Exception as e: - raise Exception(self._getErrorPrefix() + 'Failed to parse audit entries: ' + str(e)) + raise Exception(self._get_error_prefix() + 'Failed to parse audit entries: ' + str(e)) # missing audit is soft error, continue parsing if audit: @@ -257,7 +218,7 @@ def _parseAllowlistEntry(self, package, config): return ret - def _parseAuditEntry(self, bug, data): + def _parse_audit_entry(self, bug, data): """Parses a single JSON audit sub-entry returns an AuditEntry() object for it. On non-critical error conditions None is returned, otherwise an exception is raised""" @@ -266,22 +227,19 @@ def _parseAuditEntry(self, bug, data): comment = data.get('comment') if comment: - ret.setComment(comment) + ret.comment = comment digests = data.get('digests') if not digests: - raise Exception(self._getErrorPrefix() + f"no 'digests' entry for '{bug}'") + raise Exception(self._get_error_prefix() + f"no 'digests' entry for '{bug}'") - ret.setDigests(digests) + ret._set_digests(digests) return ret - def _getErrorPrefix(self): - return self.m_path + ': ERROR: ' - - def _getWarnPrefix(self): - return self.m_path + ': WARN: ' + def _get_error_prefix(self): + return self.path + ': ERROR: ' parser = AllowlistParser(allowlist_path) return parser.parse(pkg_name) @@ -290,6 +248,11 @@ def _getWarnPrefix(self): class AbstractAllowlistCheck(AbstractCheck, metaclass=abc.ABCMeta): """An abstract base class for comparing files found in an RPM against an allow list with hashed file contents.""" + def __init__(self, config, output): + for req_key in ('unauthorized', 'changed', 'ghost'): + assert req_key in self.error_map, f'Missing error mapping in class {type(self)}' + super().__init__(config, output) + @property @abc.abstractmethod def allowlist_filenames(self): @@ -312,11 +275,6 @@ def error_map(self): desired rpmlint error identifiers for this check. """ pass - def __init__(self, config, output): - for req_key in ('unauthorized', 'changed', 'ghost'): - assert req_key in self.error_map, f'Missing error mapping in class {type(self)}' - super().__init__(config, output) - def read_allowlist(self, pkg): """ Retrieves the allow list for a package. @@ -364,20 +322,20 @@ def check_binary(self, pkg): return for audit in allow_list: - digest_matches, results = audit.compareDigests(pkg) + digest_matches, results = audit.compare_digests(pkg) - if digest_matches and audit.coversAllFiles(restricted_files): + if digest_matches and audit.covers_all_files(restricted_files): break else: # print the encountered and expected digests and paths for diagnostic purposes for result in results: - restricted_files -= {result.path()} + restricted_files -= {result.path} if result.matches(): continue - print(f'{result.path()}: expected {result.algorithm()} digest {result.expected()} but encountered {result.encountered()}', file=sys.stderr) - self.output.add_info('E', pkg, self.error_map['changed'], result.path()) + print(f'{result.path}: expected {result.algorithm} digest {result.expected} but encountered {result.encountered}', file=sys.stderr) + self.output.add_info('E', pkg, self.error_map['changed'], result.path) for f in restricted_files: self.output.add_info('E', pkg, self.error_map['unauthorized'], f) diff --git a/rpmlint/checks/CronJobsCheck.py b/rpmlint/checks/CronJobsCheck.py index 1f67f2162..adcb98c97 100644 --- a/rpmlint/checks/CronJobsCheck.py +++ b/rpmlint/checks/CronJobsCheck.py @@ -1,4 +1,3 @@ -# vim: sw=4 ts=4 sts=4 et : ############################################################################# # Author : Matthias Gerstner # Purpose : Enforce cron jobs in /etc/cron.* directories to be on a allow list diff --git a/rpmlint/checks/SUIDPermissionsCheck.py b/rpmlint/checks/SUIDPermissionsCheck.py index 55605c9e2..7d7e95f2d 100644 --- a/rpmlint/checks/SUIDPermissionsCheck.py +++ b/rpmlint/checks/SUIDPermissionsCheck.py @@ -1,4 +1,3 @@ -# vim: sw=4 et sts=4 ts=4 : ############################################################################# # File : CheckSUIDPermissions.py # Package : rpmlint From 2b8dcbb3e5271ecd3c466f54792c0f023c2e9e47 Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Tue, 7 Jul 2020 18:29:42 +0200 Subject: [PATCH 10/20] allow lists: change comment headers to doc strings --- rpmlint/checks/Allowlisting.py | 5 +---- rpmlint/checks/CronJobsCheck.py | 5 +---- rpmlint/checks/SUIDPermissionsCheck.py | 7 +------ 3 files changed, 3 insertions(+), 14 deletions(-) diff --git a/rpmlint/checks/Allowlisting.py b/rpmlint/checks/Allowlisting.py index b1e7a06db..cd8832013 100644 --- a/rpmlint/checks/Allowlisting.py +++ b/rpmlint/checks/Allowlisting.py @@ -1,7 +1,4 @@ -############################################################################# -# Author : Matthias Gerstner -# Purpose : reusable code for dealing with security allow lists -############################################################################# +"""reusable code for dealing with security allow lists""" import abc from dataclasses import make_dataclass diff --git a/rpmlint/checks/CronJobsCheck.py b/rpmlint/checks/CronJobsCheck.py index adcb98c97..b0bb45513 100644 --- a/rpmlint/checks/CronJobsCheck.py +++ b/rpmlint/checks/CronJobsCheck.py @@ -1,7 +1,4 @@ -############################################################################# -# Author : Matthias Gerstner -# Purpose : Enforce cron jobs in /etc/cron.* directories to be on a allow list -############################################################################# +""" Enforce cron jobs in /etc/cron.* directories to be on a allow list """ from .Allowlisting import AbstractAllowlistCheck diff --git a/rpmlint/checks/SUIDPermissionsCheck.py b/rpmlint/checks/SUIDPermissionsCheck.py index 7d7e95f2d..46fb2e0f0 100644 --- a/rpmlint/checks/SUIDPermissionsCheck.py +++ b/rpmlint/checks/SUIDPermissionsCheck.py @@ -1,9 +1,4 @@ -############################################################################# -# File : CheckSUIDPermissions.py -# Package : rpmlint -# Author : Ludwig Nussel -# Purpose : Check for /usr/share/permissions violations -############################################################################# +""" Check for /usr/share/permissions violations """ import os import stat From 4d78c11cce220de495042d5cac754257f0bfb9d7 Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Tue, 7 Jul 2020 18:30:30 +0200 Subject: [PATCH 11/20] allow lists: use absolute python module imports --- rpmlint/checks/CheckDBUSServices.py | 2 +- rpmlint/checks/CronJobsCheck.py | 2 +- rpmlint/checks/PolkitDefaultPrivsAllowlist.py | 4 ++-- rpmlint/checks/PolkitRulesAllowedCheck.py | 2 +- rpmlint/checks/SUIDPermissionsCheck.py | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/rpmlint/checks/CheckDBUSServices.py b/rpmlint/checks/CheckDBUSServices.py index f5e647136..87dbd85d6 100644 --- a/rpmlint/checks/CheckDBUSServices.py +++ b/rpmlint/checks/CheckDBUSServices.py @@ -1,4 +1,4 @@ -from .Allowlisting import AbstractSimpleAllowlistCheck +from rpmlint.checks.Allowlisting import AbstractSimpleAllowlistCheck class DBUSServiceCheck(AbstractSimpleAllowlistCheck): diff --git a/rpmlint/checks/CronJobsCheck.py b/rpmlint/checks/CronJobsCheck.py index b0bb45513..ba04c82d0 100644 --- a/rpmlint/checks/CronJobsCheck.py +++ b/rpmlint/checks/CronJobsCheck.py @@ -1,6 +1,6 @@ """ Enforce cron jobs in /etc/cron.* directories to be on a allow list """ -from .Allowlisting import AbstractAllowlistCheck +from rpmlint.checks.Allowlisting import AbstractAllowlistCheck class CronCheck(AbstractAllowlistCheck): diff --git a/rpmlint/checks/PolkitDefaultPrivsAllowlist.py b/rpmlint/checks/PolkitDefaultPrivsAllowlist.py index 7db560bb9..a9a025122 100644 --- a/rpmlint/checks/PolkitDefaultPrivsAllowlist.py +++ b/rpmlint/checks/PolkitDefaultPrivsAllowlist.py @@ -3,8 +3,8 @@ from xml.dom.minidom import parse -from .AbstractCheck import AbstractFilesCheck -from .Allowlisting import AbstractSimpleAllowlistCheck +from rpmlint.checks.AbstractCheck import AbstractFilesCheck +from rpmlint.checks.Allowlisting import AbstractSimpleAllowlistCheck POLKIT_DEFAULT_PRIVS_D = '/etc/polkit-default-privs.d/' diff --git a/rpmlint/checks/PolkitRulesAllowedCheck.py b/rpmlint/checks/PolkitRulesAllowedCheck.py index 564980d04..6a52433be 100644 --- a/rpmlint/checks/PolkitRulesAllowedCheck.py +++ b/rpmlint/checks/PolkitRulesAllowedCheck.py @@ -1,4 +1,4 @@ -from .Allowlisting import AbstractAllowlistCheck +from rpmlint.checks.Allowlisting import AbstractAllowlistCheck class PolkitRulesAllowedCheck(AbstractAllowlistCheck): diff --git a/rpmlint/checks/SUIDPermissionsCheck.py b/rpmlint/checks/SUIDPermissionsCheck.py index 46fb2e0f0..576023547 100644 --- a/rpmlint/checks/SUIDPermissionsCheck.py +++ b/rpmlint/checks/SUIDPermissionsCheck.py @@ -6,8 +6,8 @@ import rpm -from .AbstractCheck import AbstractCheck -from ..pkg import FakePkg +from rpmlint.checks.AbstractCheck import AbstractCheck +from rpmlint.pkg import FakePkg _permissions_d_allowed = ( 'lprng', From c246228be6e1cb078ba97ca5fd8d9ad806eb7507 Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Tue, 7 Jul 2020 18:31:34 +0200 Subject: [PATCH 12/20] drop obsolete permissions-suseconfig-obsolete check --- rpmlint/checks/SUIDPermissionsCheck.py | 16 ++-------------- rpmlint/descriptions/SUIDPermissionsCheck.toml | 7 ------- 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/rpmlint/checks/SUIDPermissionsCheck.py b/rpmlint/checks/SUIDPermissionsCheck.py index 576023547..97360e650 100644 --- a/rpmlint/checks/SUIDPermissionsCheck.py +++ b/rpmlint/checks/SUIDPermissionsCheck.py @@ -104,7 +104,6 @@ def check_binary(self, pkg): break need_set_permissions = False - found_suseconfig = False # second pass, find permissions violations for f, pkgfile in pkg.files.items(): @@ -112,7 +111,7 @@ def check_binary(self, pkg): self.output.add_info('E', pkg, 'permissions-fscaps', f'{f} has fscaps "{pkgfile.filecaps}"') mode = pkgfile.mode - owner = pkgfile.user + ':' + pkgfile.group + owner = f'{pkgfile.user}:{pkgfile.group}' need_verifyscript = False if f in self.perms or (stat.S_ISDIR(mode) and f + '/' in self.perms): @@ -151,7 +150,7 @@ def check_binary(self, pkg): elif not stat.S_ISLNK(mode): if f + '/' in self.perms: self.output.add_info('W', pkg, 'permissions-file-as-dir', - f + ' is a file but listed as directory') + f'{f} is a file but listed as directory') if mode & (stat.S_ISUID | stat.S_ISGID): @@ -181,12 +180,6 @@ def check_binary(self, pkg): found = True break - if 'SuSEconfig --module permissions' in line \ - or 'run_permissions is obsolete' in line: - found = True - found_suseconfig = True - break - if need_verifyscript and \ (f not in self.perms or 'static' not in self.perms[f]): @@ -212,8 +205,3 @@ def check_binary(self, pkg): if 'permissions' not in (x[0] for x in pkg.prereq): self.output.add_info('E', pkg, 'permissions-missing-requires', "missing 'permissions' in PreReq") - - if found_suseconfig: - # TODO: nothing in tumbleweed uses this anymore. can probable be dropped. - self.output.add_info('I', pkg, 'permissions-suseconfig-obsolete', - '%run_permissions is obsolete') diff --git a/rpmlint/descriptions/SUIDPermissionsCheck.toml b/rpmlint/descriptions/SUIDPermissionsCheck.toml index 01f4d33a0..a9c7aab76 100644 --- a/rpmlint/descriptions/SUIDPermissionsCheck.toml +++ b/rpmlint/descriptions/SUIDPermissionsCheck.toml @@ -89,13 +89,6 @@ Please add a %verifyscript section """ -permissions-suseconfig-obsolete=""" -The %run_permissions macro calls SuSEconfig which sets permissions for all -files in the system. Please use %set_permissions instead to only set -permissions for files contained in this package -""" - - permissions-ghostfile=""" This package installs a permissions file as a %ghost file. This is not allowed as it is impossible to review. Please refer to From 9059d9cb23750fd6705189b9fb522d41002b63cf Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Tue, 7 Jul 2020 18:32:08 +0200 Subject: [PATCH 13/20] permissions-missing-requires: do not suggest obsolete PreReq but still allow it, to allow packagers to move to normal 'Requires' on their own pace --- rpmlint/checks/SUIDPermissionsCheck.py | 13 ++++++++++--- rpmlint/descriptions/SUIDPermissionsCheck.toml | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/rpmlint/checks/SUIDPermissionsCheck.py b/rpmlint/checks/SUIDPermissionsCheck.py index 97360e650..67ff413b7 100644 --- a/rpmlint/checks/SUIDPermissionsCheck.py +++ b/rpmlint/checks/SUIDPermissionsCheck.py @@ -5,7 +5,6 @@ import sys import rpm - from rpmlint.checks.AbstractCheck import AbstractCheck from rpmlint.pkg import FakePkg @@ -202,6 +201,14 @@ def check_binary(self, pkg): f'missing %verify_permissions -e {f}') if need_set_permissions: - if 'permissions' not in (x[0] for x in pkg.prereq): + # we used to require the legacy statement 'Prereq: permissions' + # allow this for now, to give packagers some time to update to 'Requires: /usr/bin/chkstat' and 'Requires(post): /usr/bin/chkstat' + legacy = 'permissions' in (x[0] for x in pkg.prereq) + req = '/usr/bin/chkstat' in (x[0] for x in pkg.requires) + req_post = '/usr/bin/chkstat' in (x[0] for x in pkg.prereq) + if not legacy and not req: + self.output.add_info('E', pkg, 'permissions-missing-requires', + "missing '/usr/bin/chkstat' in Requires") + if not legacy and not req_post: self.output.add_info('E', pkg, 'permissions-missing-requires', - "missing 'permissions' in PreReq") + "missing '/usr/bin/chkstat' in Requires(post)") diff --git a/rpmlint/descriptions/SUIDPermissionsCheck.toml b/rpmlint/descriptions/SUIDPermissionsCheck.toml index a9c7aab76..5d393c81d 100644 --- a/rpmlint/descriptions/SUIDPermissionsCheck.toml +++ b/rpmlint/descriptions/SUIDPermissionsCheck.toml @@ -80,7 +80,7 @@ Please add an appropriate %post section permissions-missing-requires=""" -Please add 'PreReq: permissions' +Please add 'Requires: /usr/bin/chkstat' and 'Requires(post): /usr/bin/chkstat' """ From 6b4a77370f5163a856dac62ea0fd038b113de40b Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Wed, 8 Jul 2020 15:28:47 +0200 Subject: [PATCH 14/20] allow list descriptions: drop openSUSE references --- rpmlint/descriptions/CronJobs.toml | 20 ++----- rpmlint/descriptions/DBUSServices.toml | 13 ++--- rpmlint/descriptions/PAMModulesCheck.toml | 7 +-- .../PolkitDefaultPrivsAllowlist.toml | 22 ++----- .../descriptions/PolkitRulesAllowedCheck.toml | 21 ++----- .../descriptions/SUIDPermissionsCheck.toml | 58 +++++++------------ 6 files changed, 45 insertions(+), 96 deletions(-) diff --git a/rpmlint/descriptions/CronJobs.toml b/rpmlint/descriptions/CronJobs.toml index 9f0205988..47fad00b8 100644 --- a/rpmlint/descriptions/CronJobs.toml +++ b/rpmlint/descriptions/CronJobs.toml @@ -1,22 +1,14 @@ cronjob-unauthorized-file=""" -A cron job file is installed by this package. If the package is -intended for inclusion in any SUSE product please open a bug report to request -review of the package by the security team. Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for more -information. +An unknown cron job file is installed by this package. Please contact the +security team to get it allowed. """ cronjob-changed-file=""" -A cron job or cron job related file installed by this package changed -in content. Please open a bug report to request follow-up review of the -introduced changes by the security team. Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for more -information. +A cron job or cron job related file installed by this package changed in +content. Please contact the security team to get this allowed. """ cronjob-ghost-file=""" -A cron job path has been marked as %ghost file by this package. -This is not allowed as it is impossible to review. Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for more -information. +A cron job path has been marked as %ghost file by this package. This is not +allowed as it is impossible to review. """ diff --git a/rpmlint/descriptions/DBUSServices.toml b/rpmlint/descriptions/DBUSServices.toml index 7c8e4876d..c502073fd 100644 --- a/rpmlint/descriptions/DBUSServices.toml +++ b/rpmlint/descriptions/DBUSServices.toml @@ -1,14 +1,9 @@ suse-dbus-unauthorized-service=""" -The package installs a DBUS system service file. If the package -is intended for inclusion in any SUSE product please open a bug -report to request review of the service by the security team. Please -refer to https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs -for more information. +The package installs an unknown DBUS system service file. Please contact the +security team to get it allowed. """ suse-dbus-ghost-service=""" -This package installs a DBUS system service marked as %ghost. -This is not allowed, since it is impossible to review. Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for more -information. +This package installs a DBUS system service marked as %ghost. This is not +allowed, since it is impossible to review. """ diff --git a/rpmlint/descriptions/PAMModulesCheck.toml b/rpmlint/descriptions/PAMModulesCheck.toml index fe882272c..cbde618fe 100644 --- a/rpmlint/descriptions/PAMModulesCheck.toml +++ b/rpmlint/descriptions/PAMModulesCheck.toml @@ -1,7 +1,4 @@ pam-unauthorized-module=""" -The package installs a PAM module. If the package -is intended for inclusion in any SUSE product please open a bug -report to request review of the service by the security team. -Please refer to https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs -for more information. +The package installs an unknown PAM module. Please contact the security team to +get it allowed. """ diff --git a/rpmlint/descriptions/PolkitDefaultPrivsAllowlist.toml b/rpmlint/descriptions/PolkitDefaultPrivsAllowlist.toml index 40e914ed1..1f2074ceb 100644 --- a/rpmlint/descriptions/PolkitDefaultPrivsAllowlist.toml +++ b/rpmlint/descriptions/PolkitDefaultPrivsAllowlist.toml @@ -1,31 +1,21 @@ polkit-unauthorized-file=""" -A custom polkit rule file is installed by this package. If the package is -intended for inclusion in any SUSE product please open a bug report to request -review of the package by the security team. Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for -more information. +A custom polkit rule file is installed by this package. Please contact the +security team. """ polkit-unauthorized-privilege=""" The package allows unprivileged users to carry out privileged operations without authentication. This could cause security problems if not done -carefully. If the package is intended for inclusion in any SUSE product please -open a bug report to request review of the package by the security team. Please -refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for -more information. +carefully. Please contact the security team to have this reviewed. """ polkit-untracked-privilege=""" The privilege is not listed in /etc/polkit-default-privs.* which makes it -harder for admins to find. Furthermore polkit authorization checks can easily -introduce security issues. If the package is intended for inclusion in any -SUSE product please open a bug report to request review of the package by the -security team. Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for -more information. +harder for admins to find. Furthermore polkit authorization checks can easily +introduce security issues. Please contact the security team to have this +reviewed. """ diff --git a/rpmlint/descriptions/PolkitRulesAllowedCheck.toml b/rpmlint/descriptions/PolkitRulesAllowedCheck.toml index 11778a738..ebbebd5b8 100644 --- a/rpmlint/descriptions/PolkitRulesAllowedCheck.toml +++ b/rpmlint/descriptions/PolkitRulesAllowedCheck.toml @@ -1,25 +1,16 @@ polkit-unauthorized-rules=""" -A polkit rules file installed by this package is not whitelisted in the -polkit-whitelisting package. If the package is intended for inclusion in any -SUSE product please open a bug report to request review of the package by the -security team. Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for -more information. +A polkit rules file installed by this package is not allowed in the +polkit-whitelisting package. Please contact the security team. """ polkit-changed-rules=""" -A polkit rules file installed by this package changed in content. Please open a -bug report to request follow-up review of the introduced changes by the -security team. Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for -more information. +A polkit rules file installed by this package changed in content. Please +contact the security team. """ polkit-ghost-file=""" -This package installs a polkit rule or policy as %ghost file. This is not -allowed as it is impossible to review. For more information please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for -more information. +This package installs a polkit rule or policy as %ghost file. This is not +allowed as it is impossible to review. """ diff --git a/rpmlint/descriptions/SUIDPermissionsCheck.toml b/rpmlint/descriptions/SUIDPermissionsCheck.toml index 5d393c81d..b0711af5c 100644 --- a/rpmlint/descriptions/SUIDPermissionsCheck.toml +++ b/rpmlint/descriptions/SUIDPermissionsCheck.toml @@ -1,69 +1,54 @@ permissions-unauthorized-file=""" -If the package is intended for inclusion in any SUSE product please open a bug -report to request review of the package by the security team. Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for -more information. +An unknown file is packaged with sensitive permissions. Please contact the +security team to request review of the package by the security team. """ permissions-symlink=""" -permissions handling for symlinks is useless. Please contact security@suse.de -to remove the entry. Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for -more information. +A symlink is packaged at a path where a file with sensitive permissions is +expected. Please contact the security team to remove the entry. """ permissions-dir-without-slash=""" -the entry in the permissions file refers to a directory. Please contact -security@suse.de to append a slash to the entry in order to avoid security -problems. Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for -more information. +The entry in the permissions file refers to a directory. Please contact the +security team to append a slash to the entry in order to avoid security +problems. """ permissions-file-as-dir=""" -the entry in the permissions file refers to a directory but the package -actually contains a file. Please contact security@suse.de to remove the slash. -Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for -more information. +The entry in the permissions file refers to a directory but the package +actually contains a file. Please contact the security team to remove the slash. """ permissions-incorrect=""" -please use the %attr macro to set the correct permissions. +Please use the %attr macro to set the correct permissions. """ permissions-incorrect-owner=""" -please use the %attr macro to set the correct ownership. +Please use the %attr macro to set the correct ownership. """ permissions-file-setuid-bit=""" -If the package is intended for inclusion in any SUSE product please open a bug -report to request review of the program by the security team. Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for -more information. +The file has a setuid or setgid bit set. Please contact the security team to +have it allowed. """ permissions-directory-setuid-bit=""" -If the package is intended for inclusion in any SUSE product please open a bug -report to request review of the package by the security team. Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for -more information. +The directory has a setuid or setgid bit set. Please contact the security team +to have it allowed. """ permissions-world-writable=""" -If the package is intended for inclusion in any SUSE product please open a bug -report to request review of the package by the security team. Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for -more information. +The file or directory is world-writable. Please contact the security team to +have that allowed. """ @@ -75,7 +60,7 @@ sets capabilities in /usr/share/permissions/permissions instead. permissions-missing-postin=""" -Please add an appropriate %post section +Please call the %set_permissions macro for this file in the %post section. """ @@ -85,13 +70,12 @@ Please add 'Requires: /usr/bin/chkstat' and 'Requires(post): /usr/bin/chkstat' permissions-missing-verifyscript=""" -Please add a %verifyscript section +Please add call the %verify_permissions macro for this file in the +%verifyscript section. """ permissions-ghostfile=""" This package installs a permissions file as a %ghost file. This is not allowed -as it is impossible to review. Please refer to -https://en.opensuse.org/openSUSE:Package_security_guidelines#audit_bugs for -more information. +as it is impossible to review. """ From 9c349accb3c64740001ed5195689d1661f69aa0a Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Thu, 23 Jul 2020 14:07:26 +0200 Subject: [PATCH 15/20] DBus services: drop 'suse-' prefix from lint names --- rpmlint/checks/CheckDBUSServices.py | 4 ++-- rpmlint/descriptions/DBUSServices.toml | 4 ++-- test/test_dbus_services.py | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/rpmlint/checks/CheckDBUSServices.py b/rpmlint/checks/CheckDBUSServices.py index 87dbd85d6..7e1c1150f 100644 --- a/rpmlint/checks/CheckDBUSServices.py +++ b/rpmlint/checks/CheckDBUSServices.py @@ -9,7 +9,7 @@ class DBUSServiceCheck(AbstractSimpleAllowlistCheck): '/etc/dbus-1/system.d/' ) error_map = { - 'ghost': 'suse-dbus-ghost-service', - 'unauthorized': 'suse-dbus-unauthorized-service', + 'ghost': 'dbus-ghost-service', + 'unauthorized': 'dbus-unauthorized-service', } allowlist_config_key = 'DBUSServices.WhiteList' diff --git a/rpmlint/descriptions/DBUSServices.toml b/rpmlint/descriptions/DBUSServices.toml index c502073fd..7f86bd555 100644 --- a/rpmlint/descriptions/DBUSServices.toml +++ b/rpmlint/descriptions/DBUSServices.toml @@ -1,9 +1,9 @@ -suse-dbus-unauthorized-service=""" +dbus-unauthorized-service=""" The package installs an unknown DBUS system service file. Please contact the security team to get it allowed. """ -suse-dbus-ghost-service=""" +dbus-ghost-service=""" This package installs a DBUS system service marked as %ghost. This is not allowed, since it is impossible to review. """ diff --git a/test/test_dbus_services.py b/test/test_dbus_services.py index 9d7c8b456..e9c779444 100644 --- a/test/test_dbus_services.py +++ b/test/test_dbus_services.py @@ -23,10 +23,10 @@ def test_dbus_services(tmpdir, package, dbusservicescheck): out = output.print_results(output.results) assert 'noproblem' not in out # these are allowed and should NOT produce errors - assert 'suse-dbus-unauthorized-service /usr/share/dbus-1/system-services/1' not in out - assert 'suse-dbus-unauthorized-service /usr/share/dbus-1/system.d/2' not in out - assert 'suse-dbus-unauthorized-service /etc/dbus-1/system.d/3' not in out + assert 'dbus-unauthorized-service /usr/share/dbus-1/system-services/1' not in out + assert 'dbus-unauthorized-service /usr/share/dbus-1/system.d/2' not in out + assert 'dbus-unauthorized-service /etc/dbus-1/system.d/3' not in out # these are not allowed and should produce errors - assert 'suse-dbus-unauthorized-service /usr/share/dbus-1/system-services/a' in out - assert 'suse-dbus-unauthorized-service /usr/share/dbus-1/system.d/b' in out - assert 'suse-dbus-unauthorized-service /etc/dbus-1/system.d/c' in out + assert 'dbus-unauthorized-service /usr/share/dbus-1/system-services/a' in out + assert 'dbus-unauthorized-service /usr/share/dbus-1/system.d/b' in out + assert 'dbus-unauthorized-service /etc/dbus-1/system.d/c' in out From c8f9b68c0527fcfff698b9bf3ce82fd09419b4a0 Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Thu, 1 Oct 2020 10:39:02 +0200 Subject: [PATCH 16/20] Do not use Python 3.7 feature (dataclasses). --- rpmlint/checks/Allowlisting.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/rpmlint/checks/Allowlisting.py b/rpmlint/checks/Allowlisting.py index cd8832013..78739a89e 100644 --- a/rpmlint/checks/Allowlisting.py +++ b/rpmlint/checks/Allowlisting.py @@ -1,7 +1,6 @@ """reusable code for dealing with security allow lists""" import abc -from dataclasses import make_dataclass import hashlib import json import os @@ -12,10 +11,16 @@ from rpmlint.checks.AbstractCheck import AbstractCheck -class DigestVerificationResult(make_dataclass('', ['path', 'algorithm', 'expected', 'encountered'])): +class DigestVerificationResult: """This type represents the result of a digest verification as returned from AuditEntry.compare_digests().""" + def __init__(self, path, algorithm, expected, encountered): + self.path = path + self.algorithm = algorithm + self.expected = expected + self.encountered = encountered + def matches(self): """Returns a boolean whether the encountered digest matches the expected digest.""" From 781a14f79f6903f507861297990106698b9990c5 Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Thu, 1 Oct 2020 09:41:29 +0200 Subject: [PATCH 17/20] Add openSUSE config and port fill SUIDAllowedPermissions. --- rpmlint/checks/SUIDPermissionsCheck.py | 18 ++---------------- rpmlint/configdefaults.toml | 3 +++ rpmlint/configs/openSUSE/config.toml | 16 ++++++++++++++++ 3 files changed, 21 insertions(+), 16 deletions(-) create mode 100644 rpmlint/configs/openSUSE/config.toml diff --git a/rpmlint/checks/SUIDPermissionsCheck.py b/rpmlint/checks/SUIDPermissionsCheck.py index 67ff413b7..18f60273e 100644 --- a/rpmlint/checks/SUIDPermissionsCheck.py +++ b/rpmlint/checks/SUIDPermissionsCheck.py @@ -8,26 +8,12 @@ from rpmlint.checks.AbstractCheck import AbstractCheck from rpmlint.pkg import FakePkg -_permissions_d_allowed = ( - 'lprng', - 'lprng.paranoid', - 'mail-server', - 'mail-server.paranoid', - 'postfix', - 'postfix.paranoid', - 'sendmail', - 'sendmail.paranoid', - 'squid', - 'texlive', - 'texlive.texlive', - 'otrs', # bsc#1118049 -) - class SUIDCheck(AbstractCheck): def __init__(self, config, output): super().__init__(config, output) + self.permissions_d_allowed = config.configuration['SUIDAllowedPermissions'] self.perms = {} for fname in self._paths_to('permissions', 'permissions.secure'): @@ -88,7 +74,7 @@ def check_binary(self, pkg): continue bn = f[len(prefix):] - if bn not in _permissions_d_allowed: + if bn not in self.permissions_d_allowed: self.output.add_info('E', pkg, 'permissions-unauthorized-file', f) bn = 'permissions.d/' + bn.split('.')[0] diff --git a/rpmlint/configdefaults.toml b/rpmlint/configdefaults.toml index 85fde73e4..2e4af37d0 100644 --- a/rpmlint/configdefaults.toml +++ b/rpmlint/configdefaults.toml @@ -291,6 +291,9 @@ ValidLicenseExceptions = [] # Default white list for PAM modules PAMModulesWhiteList = [] +# Default list of allowed permissions +SUIDAllowedPermissions = [] + # Additional warnings on specific function calls [WarnOnFunction] #[WarnOnFunction.testname] diff --git a/rpmlint/configs/openSUSE/config.toml b/rpmlint/configs/openSUSE/config.toml new file mode 100644 index 000000000..164ca8829 --- /dev/null +++ b/rpmlint/configs/openSUSE/config.toml @@ -0,0 +1,16 @@ +# openSUSE specific configuration + +SUIDAllowedPermissions = [ + "lprng", + "lprng.paranoid", + "mail-server", + "mail-server.paranoid", + "postfix", + "postfix.paranoid", + "sendmail", + "sendmail.paranoid", + "squid", + "texlive", + "texlive.texlive", + "otrs", # bsc#1118049 +] From 6c270e6a50a3b9c571b130fffb524fae72a0f5ac Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Thu, 1 Oct 2020 10:22:39 +0200 Subject: [PATCH 18/20] Allow the checks in openSUSE config file. --- rpmlint/configs/openSUSE/config.toml | 35 ++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/rpmlint/configs/openSUSE/config.toml b/rpmlint/configs/openSUSE/config.toml index 164ca8829..9f23185a0 100644 --- a/rpmlint/configs/openSUSE/config.toml +++ b/rpmlint/configs/openSUSE/config.toml @@ -1,5 +1,40 @@ # openSUSE specific configuration +Checks = [ + "AlternativesCheck", + "AppDataCheck", + "BinariesCheck", + "BuildDateCheck", + 'BuildRootCheck', + "ConfigFilesCheck", + "CronCheck", + "DBusPolicyCheck", + "DBUSServiceCheck", + 'DuplicatesCheck', + "DocCheck", + "ErlangCheck", + "FHSCheck", + "FilesCheck", + "IconSizesCheck", + "I18NCheck", + "LogrotateCheck", + "MenuCheck", + "MenuXDGCheck", + "MixedOwnershipCheck", + "PkgConfigCheck", + "PolkitActionsCheck", + "PolkitDefaultPrivsOverrideCheck", + "PolkitRulesAllowedCheck", + "PostCheck", + "SignatureCheck", + "SourceCheck", + "SpecCheck", + "SUIDCheck", + "TagsCheck", + "ZipCheck", + "ZyppSyntaxCheck", +] + SUIDAllowedPermissions = [ "lprng", "lprng.paranoid", From 815b023bf42739090ec02faefcc47b8f2de5d9b4 Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Wed, 7 Oct 2020 18:08:32 +0200 Subject: [PATCH 19/20] allowlisting: use TOML instead of JSON --- rpmlint/checks/Allowlisting.py | 34 +++++++++------------------------ rpmlint/checks/CronJobsCheck.py | 2 +- test/test_cron_jobs.py | 4 ++-- test/test_polkit_allowlist.py | 10 +++++----- 4 files changed, 17 insertions(+), 33 deletions(-) diff --git a/rpmlint/checks/Allowlisting.py b/rpmlint/checks/Allowlisting.py index 78739a89e..244fa2eee 100644 --- a/rpmlint/checks/Allowlisting.py +++ b/rpmlint/checks/Allowlisting.py @@ -2,13 +2,13 @@ import abc import hashlib -import json import os import os.path import sys import traceback from rpmlint.checks.AbstractCheck import AbstractCheck +import toml class DigestVerificationResult: @@ -30,20 +30,13 @@ def matches(self): class AuditEntry: """This object represents a single audit entry as found in an allow entry like: - "bsc#1234": { - "comment": "some comment", - "digests": { - "/some/file": ":", - ... - } - } - + [."bsc#1234".digests] + "/some/file" = ":" """ def __init__(self, bug): self.bug = bug self._verify_bug_nr() - self.comment = '' self.digests = {} def _set_digests(self, digests): @@ -166,7 +159,7 @@ def _verify_path(self, path): def allowlist_for_package(allowlist_path, pkg_name): class AllowlistParser: - """This type knows how to parse the JSON allow listing format. The format + """This type knows how to parse the TOML allow listing format. The format is documented in [1]. [1]: https://github.com/openSUSE/rpmlint-security-whitelistings/blob/master/README.md @@ -183,7 +176,7 @@ def parse(self, package): try: with open(self.path, 'r') as fd: - data = json.load(fd) + data = toml.load(fd) try: config = data[package] @@ -194,21 +187,16 @@ def parse(self, package): except Exception as e: _, _, tb = sys.exc_info() fn, ln, _, _ = traceback.extract_tb(tb)[-1] - raise Exception(self._get_error_prefix() + f'Failed to parse JSON file: {fn}:{ln}: {e}') + raise Exception(self._get_error_prefix() + f'Failed to parse TOML file: {fn}:{ln}: {e}') def _parse_allowlist_entry(self, package, config): - """Parses a single JSON allow entry and returns a AuditEntry() + """Parses a single TOML allow entry and returns a AuditEntry() object for it. On non-critical error conditions None is returned, otherwise an exception is raised.""" ret = [] - audits = config.get('audits') - - if not audits: - raise Exception(self._get_error_prefix() + f"no 'audits' entries for package {package}") - - for bug, data in audits.items(): + for bug, data in config.items(): try: audit = self._parse_audit_entry(bug, data) except Exception as e: @@ -221,16 +209,12 @@ def _parse_allowlist_entry(self, package, config): return ret def _parse_audit_entry(self, bug, data): - """Parses a single JSON audit sub-entry returns an AuditEntry() object + """Parses a single TOML audit sub-entry returns an AuditEntry() object for it. On non-critical error conditions None is returned, otherwise an exception is raised""" ret = AuditEntry(bug) - comment = data.get('comment') - if comment: - ret.comment = comment - digests = data.get('digests') if not digests: diff --git a/rpmlint/checks/CronJobsCheck.py b/rpmlint/checks/CronJobsCheck.py index ba04c82d0..d83f51651 100644 --- a/rpmlint/checks/CronJobsCheck.py +++ b/rpmlint/checks/CronJobsCheck.py @@ -4,7 +4,7 @@ class CronCheck(AbstractAllowlistCheck): - allowlist_filenames = ('cron-whitelist.json',) + allowlist_filenames = ('cron-whitelist.toml',) restricted_paths = ( '/etc/cron.d/', '/etc/cron.hourly/', '/etc/cron.daily/', '/etc/cron.weekly/', '/etc/cron.monthly/' diff --git a/test/test_cron_jobs.py b/test/test_cron_jobs.py index d7090931a..54d25b16b 100644 --- a/test/test_cron_jobs.py +++ b/test/test_cron_jobs.py @@ -1,10 +1,10 @@ -import json import os import os.path import pytest from rpmlint.checks.CronJobsCheck import CronCheck from rpmlint.filter import Filter +import toml from Testing import CONFIG, get_tested_package @@ -21,7 +21,7 @@ def cronjobscheck(tmpdir): # make an empty allow list with open(os.path.join(tmpdir, test.allowlist_filenames[0]), 'w') as fd: - json.dump({}, fd) + toml.dump({}, fd) return output, test diff --git a/test/test_polkit_allowlist.py b/test/test_polkit_allowlist.py index 7e7a18706..ed6262b2b 100644 --- a/test/test_polkit_allowlist.py +++ b/test/test_polkit_allowlist.py @@ -1,10 +1,10 @@ -import json import os import os.path import pytest from rpmlint.checks.PolkitRulesAllowedCheck import PolkitRulesAllowedCheck from rpmlint.filter import Filter +import toml from Testing import CONFIG, get_tested_package @@ -15,16 +15,16 @@ def polkit_rules_allowed_check(tmpdir): # make the check search for its allow list in 'tmpdir' CONFIG.configuration['WhitelistDataDir'] = [tmpdir] - CONFIG.configuration['PolkitRulesWhitelist'] = ('allowed-polkit-rules.json',) + CONFIG.configuration['PolkitRulesWhitelist'] = ('allowed-polkit-rules.toml',) output = Filter(CONFIG) test = PolkitRulesAllowedCheck(CONFIG, output) # make an empty allow list - with open(os.path.join(tmpdir, 'allowed-polkit-rules.json'), 'w') as fd: - json.dump({'polkit-rules-test': {'audits': {'bsc#0': {'comment': '', 'digests': { + with open(os.path.join(tmpdir, 'allowed-polkit-rules.toml'), 'w') as fd: + toml.dump({'polkit-rules-test': {'bsc#0': {'digests': { '/usr/share/polkit-1/rules.d/regular': 'sha256:invalid' - }}}}}, fd) + }}}}, fd) return output, test From 153f6c54be936d5b49ee8991c8ec06c6d7ca9926 Mon Sep 17 00:00:00 2001 From: Malte Kraus Date: Thu, 8 Oct 2020 13:40:25 +0200 Subject: [PATCH 20/20] test allow parsing in PolkitDefaultPrivsAllowlist --- test/test_polkit_default_privs_allowlist.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/test_polkit_default_privs_allowlist.py b/test/test_polkit_default_privs_allowlist.py index 64594538f..50449c3e5 100644 --- a/test/test_polkit_default_privs_allowlist.py +++ b/test/test_polkit_default_privs_allowlist.py @@ -31,6 +31,15 @@ def polkit_actions_check(tmpdir): return output, test +@pytest.mark.parametrize('package', ['binary/polkit-def-privs-test']) +def test_polkit_actions_parse(tmpdir, package, polkit_actions_check): + # the RPM contains a file with allowed actions that should parse successfully + output, test = polkit_actions_check + test.check(get_tested_package(package, tmpdir)) + out = output.print_results(output.results) + assert out == '' + + @pytest.mark.parametrize('package', ['binary/polkit-0']) def test_polkit_actions(tmpdir, package, polkit_actions_check): output, test = polkit_actions_check