Merge lp:~mvo/aptdaemon/support-for-whitelisted-repositories into lp:aptdaemon
- support-for-whitelisted-repositories
- Merge into main
Status: | Merged |
---|---|
Merged at revision: | 858 |
Proposed branch: | lp:~mvo/aptdaemon/support-for-whitelisted-repositories |
Merge into: | lp:aptdaemon |
Diff against target: |
821 lines (+506/-36) 14 files modified
aptdaemon/core.py (+31/-1) aptdaemon/policykit1.py (+4/-1) aptdaemon/test.py (+15/-5) aptdaemon/utils.py (+1/-0) aptdaemon/worker.py (+112/-16) data/org.debian.apt.policy.in (+24/-0) tests/data/high-trust-repository-whitelist-broken.cfg (+10/-0) tests/data/high-trust-repository-whitelist.cfg (+10/-0) tests/repo/whitelisted/Packages (+34/-0) tests/repo/whitelisted/Packages.gpg (+34/-0) tests/repo/whitelisted/Release (+17/-0) tests/test_client.py (+1/-0) tests/test_high_trust_repository_whitelist.py (+201/-0) tests/test_worker.py (+12/-13) |
To merge this branch: | bzr merge lp:~mvo/aptdaemon/support-for-whitelisted-repositories |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Michael Vogt | Needs Resubmitting | ||
Martin Pitt (community) | Approve | ||
Review via email: mp+121200@code.launchpad.net |
Commit message
Description of the change
This branch implements the whitelist feature discussed in #1035207
It adds a new type of policykit priv "org.debian.
that can be overriden by to allow passwordless installs. If its not
overriden it has the same requirements as install-
The whitelist is defined via /etc/aptdaemon/
[webapps test]
origin = Ubuntu
component = universe
pkgnames = 2vcard
[more whitelist]
origin = Ubuntu
component = main
pkgnames = ^unity.*
- 861. By Michael Vogt
-
aptdaemon/
worker. py: more paranoia
- 862. By Michael Vogt
-
make config parser more robust against errors in the config file and add tests
- 863. By Michael Vogt
-
small cleanup/comment updates
- 864. By Michael Vogt
-
move the whitelsit releated tests into a single file
- 865. By Michael Vogt
-
test that the right polkit action is used for whitelisted packages
- 866. By Michael Vogt
-
test that non-whitelisted package fails to install with aptdaemon.
policykit1. PK_ACTION_ INSTALL_ PACKAGES_ FROM_WHITELISTE D_REPO - 867. By Michael Vogt
-
aptdaemon/test.py: make the debug in start_session_aptd option to have less noise
- 868. By Michael Vogt
-
documation updates and ensure that the is install-
from-whitelist only applies to install
Michael Vogt (mvo) wrote : | # |
On Mon, Aug 27, 2012 at 01:55:22PM -0000, Martin Pitt wrote:
> Review: Needs Fixing
Thanks for the review.
> As a general nitpick, I'd prefer to rename "whitelisted" in any public API/properties etc. to "passwordless" or something similarly meaningful, to point out what this list actually does. This especially applies to the conffile, as renaming it later is painful.
Right. I tend to agree but its not neseccarily passwordless, i.e. the
default implementation of this has auth_admin. Its just easier to
override this via e.g. the policykit-
webapps packages that does the same. Maybe a better name would be
"potentially_
bit long :/ Any hints appreciated.
> The test case does not actually verify that the expected PK privilege is being used, as it only goes until the simulation stage (or did I get that wrong?). You could tell the mock polkitd to permit org.debian.
Indeed good points, I added tests for that now too and also
consolidated the whitelist test stuff into a single file.
> I also think read_repository
Thanks, good point. This is more robust now I added tests and logging
(and test for the logging).
This should be ready for re-review, except that we need to find a
better name for whitelisted.
Thanks,
Michael
Martin Pitt (pitti) wrote : | # |
The code/tests look good to me now, many thanks for the changes!
Some other ideas about the privilege name:
- org.debian.
- org.debian.
Perhaps you can also extend the comment to say what kind of packages this privilege is meant to be used for, i. e. webapps wrappers.
- 869. By Michael Vogt
-
rename read_repository
_whitelist to read_high_ trust_repositor y_whitelist - 870. By Michael Vogt
-
rename transaction_
only_installs_ packages_ from_whiteliste d_repos to transaction_ only_installs_ packages_ from_high_ trust_repos - 871. By Michael Vogt
-
rename self._whitelist
ed_packages to self._high_ trust_whitelist ed_packages - 872. By Michael Vogt
-
rename PK_ACTION_
INSTALL_ PACKAGES_ FROM_WHITELISTE D_REPO to PK_ACTION_ INSTALL_ PACKAGES_ FROM_HIGH_ TRUST_WHITELIST ED_REPO - 873. By Michael Vogt
-
rename /etc/aptdaemon/
repository- whitelist. cfg to /etc/aptdaemon/ high-trust- repository- whitelist. cfg - 874. By Michael Vogt
-
read the high-trust repository whitelist from /etc/aptdaemon/
high-trust- repository- whitelist. d by default and make it a .d directory to ensure packages like e.g. webapps can drop there whitelist in
Michael Vogt (mvo) wrote : | # |
Thanks Martin! I like the "high-trust" one best, the current code calls it:
"org.debian.
I also tried to make the naming clearer and improve the examples. A quick
double check would be very cool
Martin Pitt (pitti) wrote : | # |
There are still a few places where this hasn't been renamed consistently:
+ def _get_whiteliste
+class WhiteListedRepo
+ """ Test the whitelist feature inside the chroot """
and a few more.
Do we really need both "high trust" and "whitelisted" in the name? But anyway, that's bikeshedding at some point, at least it more self-explaining now.
Thanks!
- 875. By Michael Vogt
-
rename AptWorker.
_whitelisted_ repositories to _high_trust_ repositories
Michael Vogt (mvo) wrote : | # |
On Fri, Aug 31, 2012 at 08:38:20AM -0000, Martin Pitt wrote:
> There are still a few places where this hasn't been renamed consistently:
>
> + def _get_whiteliste
>
> +class WhiteListedRepo
> + """ Test the whitelist feature inside the chroot """
>
> and a few more.
Thanks, I look over the diff once more and eliminiate the remaining
ones.
> Do we really need both "high trust" and "whitelisted" in the name? But anyway, that's bikeshedding at some point, at least it more self-explaining now.
I guess you are right about this, its a bit too much, so I will get
rid of more of the "whitelisted" strings.
- 876. By Michael Vogt
-
-PK_ACTION_
INSTALL_ PACKAGES_ FROM_HIGH_ TRUST_WHITELIST ED_REPO -> +PK_ACTION_ INSTALL_ PACKAGES_ FROM_HIGH_ TRUST_REPO - 877. By Michael Vogt
-
trans.whitelist
ed_packages, -> trans.high_ trust_packages - 878. By Michael Vogt
-
more renaming
- 879. By Michael Vogt
-
data/org.
debian. apt.policy. in: fix name to match org.debian. apt.install- packages. high-trust- repo - 880. By Michael Vogt
-
some more renames
Preview Diff
1 | === modified file 'aptdaemon/core.py' |
2 | --- aptdaemon/core.py 2012-06-06 13:53:40 +0000 |
3 | +++ aptdaemon/core.py 2012-09-03 09:53:19 +0000 |
4 | @@ -63,7 +63,11 @@ |
5 | from defer import inline_callbacks, return_value, Deferred |
6 | from defer.utils import dbus_deferred_method |
7 | from . import policykit1 |
8 | -from .worker import AptWorker, DummyWorker |
9 | +from .worker import ( |
10 | + AptWorker, |
11 | + DummyWorker, |
12 | + transaction_only_installs_packages_from_high_trust_repos, |
13 | + ) |
14 | from .loop import mainloop |
15 | |
16 | # Setup i18n |
17 | @@ -407,6 +411,8 @@ |
18 | for pkgs in packages], |
19 | signature="asasasasasas") |
20 | self._unauthenticated = dbus.Array([], signature=dbus.Signature('s')) |
21 | + self._high_trust_packages = dbus.Array( |
22 | + [], signature=dbus.Signature('s')) |
23 | # Add a timeout which removes the transaction from the bus if it |
24 | # hasn't been setup and run for the TRANSACTION_IDLE_TIMEOUT period |
25 | self._idle_watch = GObject.timeout_add_seconds( |
26 | @@ -607,6 +613,24 @@ |
27 | doc="Unauthenticated packages in this " |
28 | "transaction") |
29 | |
30 | + # package that can have a different auth schema, useful for e.g. |
31 | + # lightweight packages like unity-webapps or packages comming from |
32 | + # a high trust repository (e.g. a internal company repo) |
33 | + def _get_high_trust_packages(self): |
34 | + return self._high_trust_packages |
35 | + |
36 | + def _set_high_trust_packages(self, whitelisted_packages): |
37 | + self._high_trust_packages = dbus.Array( |
38 | + whitelisted_packages, signature="s") |
39 | + self.PropertyChanged( |
40 | + "HighTrustWhitelistedPackages", |
41 | + self._high_trust_packages) |
42 | + |
43 | + high_trust_packages = property( |
44 | + _get_high_trust_packages, |
45 | + _set_high_trust_packages, |
46 | + doc="High trust packages in this transaction") |
47 | + |
48 | def _get_depends(self): |
49 | return self._depends |
50 | |
51 | @@ -911,6 +935,12 @@ |
52 | action = self.ROLE_ACTION_MAP[self.role] |
53 | if action is None: |
54 | raise StopIteration |
55 | + # Special case if InstallPackages only touches stuff from the |
56 | + # high trust whitelist |
57 | + if (self.role in (enums.ROLE_INSTALL_PACKAGES, |
58 | + enums.ROLE_COMMIT_PACKAGES) and |
59 | + transaction_only_installs_packages_from_high_trust_repos(self)): |
60 | + action = policykit1.PK_ACTION_INSTALL_PACKAGES_FROM_HIGH_TRUST_REPO |
61 | # Special case if CommitPackages only upgrades |
62 | if self.role == enums.ROLE_COMMIT_PACKAGES and \ |
63 | not self.packages[enums.PKGS_INSTALL] and \ |
64 | |
65 | === modified file 'aptdaemon/policykit1.py' |
66 | --- aptdaemon/policykit1.py 2012-05-08 18:50:57 +0000 |
67 | +++ aptdaemon/policykit1.py 2012-09-03 09:53:19 +0000 |
68 | @@ -28,7 +28,8 @@ |
69 | "PK_ACTION_GET_TRUSTED_VENDOR_KEYS", |
70 | "PK_ACTION_INSTALL_FILE", |
71 | "PK_ACTION_INSTALL_OR_REMOVE_PACKAGES", |
72 | - "PK_ACTION_INSTALL_PACKAGES_FROM_NRE_REPO", |
73 | + "PK_ACTION_INSTALL_PACKAGES_FROM_NEW_REPO", |
74 | + "PK_ACTION_INSTALL_PACKAGES_FROM_HIGH_TRUST_REPO", |
75 | "PK_ACTION_INSTALL_PURCHASED_PACKAGES", |
76 | "PK_ACTION_UPDATE_CACHE", "PK_ACTION_UPGRADE_PACKAGES", |
77 | "PK_ACTION_SET_PROXY", "PK_ACTION_CLEAN") |
78 | @@ -44,6 +45,8 @@ |
79 | "org.debian.apt.install-purchased-packages" |
80 | PK_ACTION_INSTALL_PACKAGES_FROM_NEW_REPO = \ |
81 | "org.debian.apt.install-packages-from-new-repo" |
82 | +PK_ACTION_INSTALL_PACKAGES_FROM_HIGH_TRUST_REPO = \ |
83 | + "org.debian.apt.install-packages.high-trust-repo" |
84 | PK_ACTION_INSTALL_FILE = "org.debian.apt.install-file" |
85 | PK_ACTION_UPGRADE_PACKAGES = "org.debian.apt.upgrade-packages" |
86 | PK_ACTION_UPDATE_CACHE = "org.debian.apt.update-cache" |
87 | |
88 | === modified file 'aptdaemon/test.py' |
89 | --- aptdaemon/test.py 2012-06-13 07:57:12 +0000 |
90 | +++ aptdaemon/test.py 2012-09-03 09:53:19 +0000 |
91 | @@ -45,6 +45,14 @@ |
92 | PY3K = sys.version_info.major > 2 |
93 | |
94 | |
95 | +class MockQueue(object): |
96 | + |
97 | + """A fake TransactionQueue which only provides a limbo attribute.""" |
98 | + |
99 | + def __init__(self): |
100 | + self.limbo = {} |
101 | + |
102 | + |
103 | class Chroot(object): |
104 | |
105 | """Provides a chroot which can be used by APT.""" |
106 | @@ -195,7 +203,7 @@ |
107 | self.addCleanup(self._kill_process, proc) |
108 | return proc |
109 | |
110 | - def start_session_aptd(self, chroot="/"): |
111 | + def start_session_aptd(self, chroot="/", debug=False): |
112 | """Start an aptdaemon which listens on the session D-Bus. |
113 | |
114 | :param chroot: path to the chroot |
115 | @@ -210,10 +218,12 @@ |
116 | path = "/usr/sbin/aptd" |
117 | else: |
118 | path = os.path.join(dir, "../aptd") |
119 | - proc = subprocess.Popen(["python3", path, "--debug", |
120 | - "--disable-timeout", "--disable-plugins", |
121 | - "--chroot", chroot], |
122 | - env=env) |
123 | + cmd = ["python3", path, |
124 | + "--disable-timeout", "--disable-plugins", |
125 | + "--chroot", chroot] |
126 | + if debug: |
127 | + cmd.append("--debug") |
128 | + proc = subprocess.Popen(cmd, env=env) |
129 | self.addCleanup(self._kill_process, proc) |
130 | return proc |
131 | |
132 | |
133 | === modified file 'aptdaemon/utils.py' |
134 | --- aptdaemon/utils.py 2012-05-09 00:49:57 +0000 |
135 | +++ aptdaemon/utils.py 2012-09-03 09:53:19 +0000 |
136 | @@ -29,6 +29,7 @@ |
137 | import warnings |
138 | from xml.etree import ElementTree |
139 | |
140 | + |
141 | def deprecated(func): |
142 | """This is a decorator which can be used to mark functions |
143 | as deprecated. It will result in a warning being emitted |
144 | |
145 | === modified file 'aptdaemon/worker.py' |
146 | --- aptdaemon/worker.py 2012-06-13 19:31:39 +0000 |
147 | +++ aptdaemon/worker.py 2012-09-03 09:53:19 +0000 |
148 | @@ -22,6 +22,7 @@ |
149 | __all__ = ("AptWorker", "DummyWorker") |
150 | |
151 | import contextlib |
152 | +import glob |
153 | import logging |
154 | import os |
155 | import platform |
156 | @@ -32,9 +33,14 @@ |
157 | import time |
158 | import traceback |
159 | try: |
160 | - from urllib.parse import urlsplit, urlunsplit |
161 | -except ImportError: |
162 | - from urlparse import urlsplit, urlunsplit |
163 | + from urllib.parse import urlsplit, urlunsplit |
164 | +except ImportError: |
165 | + from urlparse import urlsplit, urlunsplit |
166 | + |
167 | +try: |
168 | + from configparser import ConfigParser |
169 | +except ImportError: |
170 | + from ConfigParser import ConfigParser |
171 | |
172 | import apt |
173 | import apt.auth |
174 | @@ -52,15 +58,17 @@ |
175 | from .errors import * |
176 | from . import lock |
177 | from .loop import mainloop |
178 | -from .progress import DaemonOpenProgress, \ |
179 | - DaemonInstallProgress, \ |
180 | - DaemonAcquireProgress, \ |
181 | - DaemonAcquireRepoProgress, \ |
182 | - DaemonDpkgInstallProgress, \ |
183 | - DaemonDpkgReconfigureProgress, \ |
184 | - DaemonDpkgRecoverProgress, \ |
185 | - DaemonLintianProgress, \ |
186 | - DaemonForkProgress |
187 | +from .progress import ( |
188 | + DaemonOpenProgress, |
189 | + DaemonInstallProgress, |
190 | + DaemonAcquireProgress, |
191 | + DaemonAcquireRepoProgress, |
192 | + DaemonDpkgInstallProgress, |
193 | + DaemonDpkgReconfigureProgress, |
194 | + DaemonDpkgRecoverProgress, |
195 | + DaemonLintianProgress, |
196 | + DaemonForkProgress, |
197 | + ) |
198 | |
199 | log = logging.getLogger("AptDaemon.Worker") |
200 | |
201 | @@ -69,6 +77,62 @@ |
202 | _ = lambda s: s |
203 | |
204 | |
205 | +def transaction_only_installs_packages_from_high_trust_repos(trans, |
206 | + whitelist=set()): |
207 | + """ Return True if this transaction only touches packages in the |
208 | + aptdaemon repoisotry high trust repository whitelist |
209 | + """ |
210 | + # the transaction *must* be simulated before |
211 | + if not trans.simulated: |
212 | + return False |
213 | + # we never allow unauthenticated ones |
214 | + if trans.unauthenticated: |
215 | + return False |
216 | + # paranoia: wrong role |
217 | + if not trans.role in (ROLE_INSTALL_PACKAGES, ROLE_COMMIT_PACKAGES): |
218 | + return False |
219 | + # if there is anything touched that is not a install bail out |
220 | + for enum in (PKGS_REINSTALL, PKGS_REMOVE, PKGS_PURGE, PKGS_DOWNGRADE, |
221 | + PKGS_UPGRADE): |
222 | + if trans.packages[enum]: |
223 | + return False |
224 | + # paranoia(2): we must want to install something |
225 | + if not trans.packages[PKGS_INSTALL]: |
226 | + return False |
227 | + # if the install packages matches the whitelisted set we are good |
228 | + return set(trans.packages[PKGS_INSTALL]) == set(trans.high_trust_packages) |
229 | + |
230 | + |
231 | +def read_high_trust_repository_dir(whitelist_cfg_d): |
232 | + """Return a set of (origin, component, pkgname-regexp) from a |
233 | + high-trust-repository-whitelist.d directory |
234 | + """ |
235 | + whitelist = set() |
236 | + for path in glob.glob(os.path.join(whitelist_cfg_d, "*.cfg")): |
237 | + whitelist |= _read_high_trust_repository_whitelist_file(path) |
238 | + return whitelist |
239 | + |
240 | + |
241 | +def _read_high_trust_repository_whitelist_file(path): |
242 | + """Read a individual high-trust-repository whitelist file and return |
243 | + a set of tuples (origin, component, pkgname-regexp) |
244 | + """ |
245 | + parser = ConfigParser() |
246 | + whitelist = set() |
247 | + try: |
248 | + parser.read(path) |
249 | + except Exception as e: |
250 | + log.error("Failed to read repository whitelist '%s' (%s)" % ( |
251 | + path, e)) |
252 | + return whitelist |
253 | + for section in parser.sections(): |
254 | + origin = parser.get(section, "origin") |
255 | + component = parser.get(section, "component") |
256 | + pkgnames = parser.get(section, "pkgnames") |
257 | + whitelist.add( (origin, component, pkgnames) ) |
258 | + return whitelist |
259 | + |
260 | + |
261 | class AptWorker(GObject.GObject): |
262 | |
263 | """Worker which processes transactions from the queue.""" |
264 | @@ -92,7 +156,7 @@ |
265 | # Store the the tid of the transaction whose changes are currently |
266 | # marked in the cache |
267 | self.marked_tid = None |
268 | - |
269 | + |
270 | # Change to a given chroot |
271 | if chroot: |
272 | apt_conf_file = os.path.join(chroot, "etc/apt/apt.conf") |
273 | @@ -118,6 +182,16 @@ |
274 | lock.lists_lock.path = os.path.join(lists_dir, "lock") |
275 | apt_pkg.init_system() |
276 | |
277 | + # a set of tuples of the type (origin, component, pkgname-regexp) |
278 | + # that on install will trigger a different kind of polkit |
279 | + # authentication request (see LP: #1035207), useful for e.g. |
280 | + # webapps/company repos |
281 | + self._high_trust_repositories = read_high_trust_repository_dir( |
282 | + os.path.join(apt_pkg.config.find_dir("Dir"), |
283 | + "etc/aptdaemon/high-trust-repository-whitelist.d")) |
284 | + log.debug( |
285 | + "using high-trust whitelist: '%s'" % self._high_trust_repositories) |
286 | + |
287 | self._status_orig = apt_pkg.config.find_file("Dir::State::status") |
288 | self._status_frozen = None |
289 | self.plugins = {} |
290 | @@ -347,6 +421,25 @@ |
291 | version = release = None |
292 | return name, version, release |
293 | |
294 | + def _get_high_trust_packages(self): |
295 | + """ Return a list of packages that come from a high-trust repo """ |
296 | + def _in_high_trust_repository(pkgname, pkgorigin): |
297 | + for origin, component, regexp in self._high_trust_repositories: |
298 | + if (origin == pkgorigin.origin and |
299 | + component == pkgorigin.component and |
300 | + re.match(regexp, pkgname)): |
301 | + return True |
302 | + return False |
303 | + # loop |
304 | + from_high_trust_repo = [] |
305 | + for pkg in self._iterate_packages(): |
306 | + if pkg.marked_install: |
307 | + for origin in pkg.candidate.origins: |
308 | + if _in_high_trust_repository(pkg.name, origin): |
309 | + from_high_trust_repo.append(pkg.name) |
310 | + break |
311 | + return from_high_trust_repo |
312 | + |
313 | def _get_unauthenticated(self): |
314 | """Return a list of unauthenticated package names """ |
315 | unauthenticated = [] |
316 | @@ -1041,7 +1134,7 @@ |
317 | def _simulate(self, trans): |
318 | try: |
319 | trans.depends, trans.download, trans.space, \ |
320 | - trans.unauthenticated = self.__simulate(trans) |
321 | + trans.unauthenticated, trans.high_trust_packages = self.__simulate(trans) |
322 | except TransactionFailed as excep: |
323 | trans.error = excep |
324 | trans.exit = EXIT_FAILED |
325 | @@ -1068,6 +1161,7 @@ |
326 | def __simulate(self, trans): |
327 | depends = [[], [], [], [], [], [], []] |
328 | unauthenticated = [] |
329 | + high_trust_packages = [] |
330 | skip_pkgs = [] |
331 | size = 0 |
332 | installs = reinstalls = removals = purges = upgrades = upgradables = \ |
333 | @@ -1079,7 +1173,7 @@ |
334 | ROLE_UPGRADE_SYSTEM, ROLE_REMOVE_PACKAGES, |
335 | ROLE_COMMIT_PACKAGES, ROLE_INSTALL_FILE, |
336 | ROLE_FIX_BROKEN_DEPENDS]: |
337 | - return depends, 0, 0, [] |
338 | + return depends, 0, 0, [], [] |
339 | |
340 | # If a transaction is currently running use the former status file |
341 | if self._status_frozen: |
342 | @@ -1157,6 +1251,7 @@ |
343 | changes_names.append(pkg.name) |
344 | # get the unauthenticated packages |
345 | unauthenticated = self._get_unauthenticated() |
346 | + high_trust_packages = self._get_high_trust_packages() |
347 | # Check for skipped upgrades |
348 | for pkg in upgradables: |
349 | if pkg.marked_keep: |
350 | @@ -1174,7 +1269,8 @@ |
351 | |
352 | required_space = size + self._cache.required_space |
353 | |
354 | - return depends, required_download, required_space, unauthenticated |
355 | + return depends, required_download, required_space, unauthenticated, \ |
356 | + high_trust_packages |
357 | |
358 | def _check_deb_file(self, trans, path, force): |
359 | """Perform some basic checks for the Debian package. |
360 | |
361 | === modified file 'data/org.debian.apt.policy.in' |
362 | --- data/org.debian.apt.policy.in 2011-03-17 11:07:09 +0000 |
363 | +++ data/org.debian.apt.policy.in 2012-09-03 09:53:19 +0000 |
364 | @@ -106,6 +106,30 @@ |
365 | </defaults> |
366 | </action> |
367 | |
368 | + <action id="org.debian.apt.install-packages.high-trust-repo"> |
369 | + <!-- This priviledge will be requested when installing a package |
370 | + from a high trusted repository that can be explicitely whitelisted. |
371 | + |
372 | + The defaults for this action are the same as |
373 | + "org.debian.apt.install-or-remove-packages". |
374 | + |
375 | + The admin can override them to e.g. allow passwordless installs for |
376 | + leightweight applications like unity-webapps or for packages |
377 | + comming from high trust repositories (like internal repositories) |
378 | + --> |
379 | + <_description> |
380 | + Install software from a high-trust whitelisted repository. |
381 | + </_description> |
382 | + <_message> |
383 | + To install software, you need to authenticate. |
384 | + </_message> |
385 | + <defaults> |
386 | + <allow_any>auth_admin</allow_any> |
387 | + <allow_inactive>auth_admin</allow_inactive> |
388 | + <allow_active>auth_admin_keep</allow_active> |
389 | + </defaults> |
390 | + </action> |
391 | + |
392 | <action id="org.debian.apt.install-packages-from-new-repo"> |
393 | <!-- This privilege allows to call AddRepository, UpdateCache(Partially) |
394 | and InstallPackages in a row and only authenticating once. |
395 | |
396 | === added directory 'tests/data' |
397 | === added file 'tests/data/high-trust-repository-whitelist-broken.cfg' |
398 | --- tests/data/high-trust-repository-whitelist-broken.cfg 1970-01-01 00:00:00 +0000 |
399 | +++ tests/data/high-trust-repository-whitelist-broken.cfg 2012-09-03 09:53:19 +0000 |
400 | @@ -0,0 +1,10 @@ |
401 | + |
402 | +[repo name1] |
403 | +origin = Ubuntu |
404 | +component = main |
405 | +pkgnames = foo.* |
406 | + |
407 | +[repo name1] |
408 | +origin = Debian-Security |
409 | +component = non-free |
410 | +pkgnames = ^bar$ |
411 | \ No newline at end of file |
412 | |
413 | === added file 'tests/data/high-trust-repository-whitelist.cfg' |
414 | --- tests/data/high-trust-repository-whitelist.cfg 1970-01-01 00:00:00 +0000 |
415 | +++ tests/data/high-trust-repository-whitelist.cfg 2012-09-03 09:53:19 +0000 |
416 | @@ -0,0 +1,10 @@ |
417 | + |
418 | +[repo name1] |
419 | +origin = Ubuntu |
420 | +component = main |
421 | +pkgnames = foo.* |
422 | + |
423 | +[repo whitelist2] |
424 | +origin = Debian-Security |
425 | +component = non-free |
426 | +pkgnames = ^bar$ |
427 | \ No newline at end of file |
428 | |
429 | === added directory 'tests/data/high-trust-repository-whitelist.d' |
430 | === added symlink 'tests/data/high-trust-repository-whitelist.d/high-trust-repository-whitelist-broken.cfg' |
431 | === target is u'../high-trust-repository-whitelist-broken.cfg' |
432 | === added symlink 'tests/data/high-trust-repository-whitelist.d/high-trust-repository-whitelist.cfg' |
433 | === target is u'../high-trust-repository-whitelist.cfg' |
434 | === added directory 'tests/repo/whitelisted' |
435 | === added file 'tests/repo/whitelisted/Packages' |
436 | --- tests/repo/whitelisted/Packages 1970-01-01 00:00:00 +0000 |
437 | +++ tests/repo/whitelisted/Packages 2012-09-03 09:53:19 +0000 |
438 | @@ -0,0 +1,34 @@ |
439 | +Package: silly-base |
440 | +Priority: extra |
441 | +Section: admin |
442 | +Installed-Size: 44 |
443 | +Maintainer: Sebastian Heinlein <devel@glatzor.de> |
444 | +Architecture: all |
445 | +Source: silly-packages (0.1-0) |
446 | +Version: 0.1-0update1 |
447 | +Filename: ./silly-base_0.1-0update1_all.deb |
448 | +Size: 1934 |
449 | +MD5sum: 8e20af56a63a1e0cf40db3b0d07e7989 |
450 | +SHA1: 7ce87423d9c7a734478c464021994944d07fbf1b |
451 | +SHA256: d3693c0e3e7a9519b2833fdf1301c7e03e0620edf15b95b4c7329d9eb0bee553 |
452 | +SHA512: e9eded74e2449a98b02828539c55a83de85a762d2361cd8c929292eb9c5a6e5a9b8eb9b64c26c45d7b73280e12a280cd799a9b831126e484bcf55b56456d559f |
453 | +Description: working package |
454 | + Silly packages is a set of packages which will break your package |
455 | + management tool. They are created only for debugging purposes. |
456 | + . |
457 | + This package doesn't contain any files and should always be installable. |
458 | + |
459 | +Package: other-pkg |
460 | +Priority: extra |
461 | +Section: admin |
462 | +Installed-Size: 44 |
463 | +Maintainer: Sebastian Heinlein <devel@glatzor.de> |
464 | +Architecture: all |
465 | +Version: 2.0 |
466 | +Filename: ./other-pkg_2.0_all.deb |
467 | +Size: 1934 |
468 | +MD5sum: 8e20af56a63a1e0cf40db3b0d07e7989 |
469 | +SHA1: 7ce87423d9c7a734478c464021994944d07fbf1b |
470 | +SHA256: d3693c0e3e7a9519b2833fdf1301c7e03e0620edf15b95b4c7329d9eb0bee553 |
471 | +SHA512: e9eded74e2449a98b02828539c55a83de85a762d2361cd8c929292eb9c5a6e5a9b8eb9b64c26c45d7b73280e12a280cd799a9b831126e484bcf55b56456d559f |
472 | +Description: another working package |
473 | |
474 | === added file 'tests/repo/whitelisted/Packages.gpg' |
475 | --- tests/repo/whitelisted/Packages.gpg 1970-01-01 00:00:00 +0000 |
476 | +++ tests/repo/whitelisted/Packages.gpg 2012-09-03 09:53:19 +0000 |
477 | @@ -0,0 +1,34 @@ |
478 | +-----BEGIN PGP SIGNED MESSAGE----- |
479 | +Hash: SHA256 |
480 | + |
481 | +Package: silly-base |
482 | +Priority: extra |
483 | +Section: admin |
484 | +Installed-Size: 44 |
485 | +Maintainer: Sebastian Heinlein <devel@glatzor.de> |
486 | +Architecture: all |
487 | +Source: silly-packages (0.1-0) |
488 | +Version: 0.1-0update1 |
489 | +Filename: ./silly-base_0.1-0update1_all.deb |
490 | +Size: 1934 |
491 | +MD5sum: 8e20af56a63a1e0cf40db3b0d07e7989 |
492 | +SHA1: 7ce87423d9c7a734478c464021994944d07fbf1b |
493 | +SHA256: d3693c0e3e7a9519b2833fdf1301c7e03e0620edf15b95b4c7329d9eb0bee553 |
494 | +SHA512: e9eded74e2449a98b02828539c55a83de85a762d2361cd8c929292eb9c5a6e5a9b8eb9b64c26c45d7b73280e12a280cd799a9b831126e484bcf55b56456d559f |
495 | +Description: working package |
496 | + Silly packages is a set of packages which will break your package |
497 | + management tool. They are created only for debugging purposes. |
498 | + . |
499 | + This package doesn't contain any files and should always be installable. |
500 | + |
501 | +-----BEGIN PGP SIGNATURE----- |
502 | +Version: GnuPG v1.4.11 (GNU/Linux) |
503 | + |
504 | +iQEcBAEBCAAGBQJO3ZylAAoJEGg8U8fPmC0YooQH/inTQPInDCiN3pTDvzWfV16F |
505 | +Ea+UpwBN0vzuyS6f3xmhLRxLQpuz/yk8buc1H+f/XKn6eygydJRFwIEgtdWAN/Tk |
506 | +eG9I4c5zYiHzZnNWe4XNBhRdPVkIHPkbmbRs/RvDiM5Cq0LvXIe0X0RV+empJyrC |
507 | +EgKbt3PJxh8qpMfrf/OIF+GkSqAug4tq0i0n6QxLOi0raeb9PjfDwErmBpbLDSFg |
508 | +XyDnNvPET5BtWxjgupOwoFqs2QRkrLv10JBdGRz+7qG6WhH1BCAOfzYxxCtn++Ip |
509 | +kmwo8c/pmtOr1BzZyyNMWP8nvVtB728eb/M84WGYynIEyCObUQ+Q2HVsXcsPQLc= |
510 | +=j3Nx |
511 | +-----END PGP SIGNATURE----- |
512 | |
513 | === added file 'tests/repo/whitelisted/Release' |
514 | --- tests/repo/whitelisted/Release 1970-01-01 00:00:00 +0000 |
515 | +++ tests/repo/whitelisted/Release 2012-09-03 09:53:19 +0000 |
516 | @@ -0,0 +1,17 @@ |
517 | +Origin: Ubuntu |
518 | +Label: Ubuntu-Whitelisted |
519 | +Components: main |
520 | +Suite: sid |
521 | +Date: Tue, 06 Dec 2011 04:40:53 UTC |
522 | +MD5Sum: |
523 | + 0bd018b62f0031d1ee4993896f50ddd5 782 Packages |
524 | + d41d8cd98f00b204e9800998ecf8427e 0 Release |
525 | +SHA1: |
526 | + 42201c0ab580cb2d557ca22a7ed04717a68b2e6e 782 Packages |
527 | + da39a3ee5e6b4b0d3255bfef95601890afd80709 0 Release |
528 | +SHA256: |
529 | + 950233659ae7e86ac11ae886298963e57f4414691881af1357456c6a4038ef91 782 Packages |
530 | + e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 0 Release |
531 | +SHA512: |
532 | + 984c9203b8e534b9b9cc486b21da94cf807428dc1d2daa56427a870f4d53111c58ce749c35509a5fcf7112433d7a403830378a0f482354fb382c0aa7a25bdbc2 782 Packages |
533 | + cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e 0 Release |
534 | |
535 | === added file 'tests/repo/whitelisted/Release.gpg' |
536 | === added symlink 'tests/repo/whitelisted/silly-base_0.1-0update1_all.deb' |
537 | === target is u'../silly-base_0.1-0update1_all.deb' |
538 | === modified file 'tests/test_client.py' |
539 | --- tests/test_client.py 2012-05-09 22:47:34 +0000 |
540 | +++ tests/test_client.py 2012-09-03 09:53:19 +0000 |
541 | @@ -111,6 +111,7 @@ |
542 | aptdaemon.enums.ERROR_DEP_RESOLUTION_FAILED) |
543 | |
544 | |
545 | + |
546 | if __name__ == "__main__": |
547 | if DEBUG: |
548 | logging.basicConfig(level=logging.DEBUG) |
549 | |
550 | === added file 'tests/test_high_trust_repository_whitelist.py' |
551 | --- tests/test_high_trust_repository_whitelist.py 1970-01-01 00:00:00 +0000 |
552 | +++ tests/test_high_trust_repository_whitelist.py 2012-09-03 09:53:19 +0000 |
553 | @@ -0,0 +1,201 @@ |
554 | +#!/usr/bin/env python |
555 | +# -*- coding: utf-8 -*- |
556 | +"""Provides unit tests for the APTDAEMON high-trust-repo feature.""" |
557 | +# Copyright (C) 2011 Sebastian Heinlein <devel@glatzor.de> |
558 | +# |
559 | +# Licensed under the GNU General Public License Version 2 |
560 | +# |
561 | +# This program is free software; you can redistribute it and/or modify |
562 | +# it under the terms of the GNU General Public License as published by |
563 | +# the Free Software Foundation; either version 2 of the License, or |
564 | +# at your option) any later version. |
565 | +# |
566 | +# This program is distributed in the hope that it will be useful, |
567 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of |
568 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
569 | +# GNU General Public License for more details. |
570 | +# |
571 | +# You should have received a copy of the GNU General Public License |
572 | +# along with this program; if not, write to the Free Software |
573 | +# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. |
574 | +# Licensed under the GNU General Public License Version 2 |
575 | + |
576 | +__author__ = "Michael Vogt <michael.vogt@ubuntu.com>" |
577 | + |
578 | +import os |
579 | +import sys |
580 | +import time |
581 | +import unittest |
582 | + |
583 | +import dbus |
584 | +from gi.repository import GObject |
585 | +from mock import ( |
586 | + patch, |
587 | + ) |
588 | + |
589 | +import aptdaemon.client |
590 | +import aptdaemon.policykit1 |
591 | +import aptdaemon.test |
592 | + |
593 | +from aptdaemon.worker import ( |
594 | + _read_high_trust_repository_whitelist_file, |
595 | + read_high_trust_repository_dir, |
596 | + transaction_only_installs_packages_from_high_trust_repos, |
597 | + AptWorker, |
598 | + ) |
599 | +from aptdaemon.core import Transaction |
600 | +from aptdaemon import enums |
601 | + |
602 | + |
603 | +REPO_PATH = os.path.join(aptdaemon.test.get_tests_dir(), "repo") |
604 | + |
605 | +PY3K = sys.version_info.major > 2 |
606 | + |
607 | + |
608 | +class BaseHighTrustTestCase(aptdaemon.test.AptDaemonTestCase): |
609 | + |
610 | + def setUp(self): |
611 | + self.chroot = aptdaemon.test.Chroot() |
612 | + self.chroot.setup() |
613 | + self.addCleanup(self.chroot.remove) |
614 | + self.loop = GObject.MainLoop() |
615 | + |
616 | + |
617 | +class HighTrustRepositoryTestCase(BaseHighTrustTestCase): |
618 | + """ Test the worker low-level bits of the high-trust repo implementation""" |
619 | + |
620 | + |
621 | + def setUp(self): |
622 | + super(HighTrustRepositoryTestCase, self).setUp() |
623 | + self.queue = aptdaemon.test.MockQueue() |
624 | + self.worker = AptWorker(chroot=self.chroot.path, load_plugins=False) |
625 | + self.worker.connect("transaction-done", lambda w,t: self.loop.quit()) |
626 | + self.worker.connect("transaction-simulated", |
627 | + lambda w,t: self.loop.quit()) |
628 | + |
629 | + def test_read_high_trust_repository_whitelist_dir(self): |
630 | + whitelist = read_high_trust_repository_dir( |
631 | + os.path.join(aptdaemon.test.get_tests_dir(), |
632 | + "data/high-trust-repository-whitelist.d")) |
633 | + self.assertEqual( |
634 | + whitelist, set( [ |
635 | + ("Ubuntu", "main", "foo.*"), |
636 | + ("Debian-Security", "non-free", "^bar$"), |
637 | + ])) |
638 | + |
639 | + def test_read_high_trust_repository_whitelist(self): |
640 | + whitelist = _read_high_trust_repository_whitelist_file( |
641 | + os.path.join(aptdaemon.test.get_tests_dir(), |
642 | + "data/high-trust-repository-whitelist.cfg")) |
643 | + self.assertEqual( |
644 | + whitelist, set( [ |
645 | + ("Ubuntu", "main", "foo.*"), |
646 | + ("Debian-Security", "non-free", "^bar$"), |
647 | + ])) |
648 | + |
649 | + @patch("aptdaemon.worker.log") |
650 | + def test_read_high_trust_repository_whitelist_broken(self, mock_log): |
651 | + whitelist = _read_high_trust_repository_whitelist_file( |
652 | + os.path.join(aptdaemon.test.get_tests_dir(), |
653 | + "data/high-trust-repository-whitelist-broken.cfg")) |
654 | + self.assertEqual(whitelist, set()) |
655 | + # ensure we log a error if the config file is broken |
656 | + mock_log.error.assert_called() |
657 | + |
658 | + @patch("aptdaemon.worker.log") |
659 | + def test_read_high_trust_repository_whitelist_not_there(self, mock_log): |
660 | + whitelist = _read_high_trust_repository_whitelist_file( |
661 | + "lalalala-not-there-really.cfg") |
662 | + self.assertEqual(whitelist, set()) |
663 | + # ensure we log no error if there is no config file |
664 | + self.assertFalse(mock_log.called) |
665 | + |
666 | + def test_high_trust_repository(self): |
667 | + """Test if using a high_trust repo is working """ |
668 | + self.chroot.add_repository(os.path.join(aptdaemon.test.get_tests_dir(), |
669 | + "repo/whitelisted")) |
670 | + # setup a whitelist |
671 | + self.worker._high_trust_repositories.add( |
672 | + ("Ubuntu", "", "silly.*")) |
673 | + # a high-trust whitelisted pkg and a non-whitelisted one |
674 | + trans = Transaction(None, enums.ROLE_INSTALL_PACKAGES, self.queue, |
675 | + os.getpid(), os.getuid(), sys.argv[0], |
676 | + "org.debian.apt.test", connect=False, |
677 | + packages=[["silly-base", "other-pkg"],[],[],[],[], []]) |
678 | + self.worker.simulate(trans) |
679 | + self.loop.run() |
680 | + self.assertEqual(trans.high_trust_packages, ["silly-base"]) |
681 | + self.assertFalse( |
682 | + transaction_only_installs_packages_from_high_trust_repos( |
683 | + trans, self.worker._high_trust_repositories)) |
684 | + # whitelisted only |
685 | + trans = Transaction(None, enums.ROLE_INSTALL_PACKAGES, self.queue, |
686 | + os.getpid(), os.getuid(), sys.argv[0], |
687 | + "org.debian.apt.test", connect=False, |
688 | + packages=[["silly-base"],[],[],[],[], []]) |
689 | + self.worker.simulate(trans) |
690 | + self.loop.run() |
691 | + self.assertTrue( |
692 | + transaction_only_installs_packages_from_high_trust_repos( |
693 | + trans, self.worker._high_trust_repositories)) |
694 | + |
695 | + |
696 | +class HighTrustRepositoryIntegrationTestCase(BaseHighTrustTestCase): |
697 | + """ Test the whitelist feature inside the chroot """ |
698 | + |
699 | + def setUp(self): |
700 | + super(HighTrustRepositoryIntegrationTestCase, self).setUp() |
701 | + # Start aptdaemon with the chroot on the session bus |
702 | + self.start_dbus_daemon() |
703 | + self.bus = dbus.bus.BusConnection(self.dbus_address) |
704 | + # setup the environment first including the high-trust whitelist |
705 | + self.chroot.add_repository(os.path.join(aptdaemon.test.get_tests_dir(), |
706 | + "repo/whitelisted")) |
707 | + whitelist_file = os.path.join( |
708 | + self.chroot.path, "etc", "aptdaemon", |
709 | + "high-trust-repository-whitelist.d", "test.cfg") |
710 | + os.makedirs(os.path.dirname(whitelist_file)) |
711 | + |
712 | + with open(whitelist_file, "w") as f: |
713 | + f.write(""" |
714 | +[test repo"] |
715 | +origin = Ubuntu |
716 | +component = |
717 | +pkgnames = silly.* |
718 | +""") |
719 | + # *after* that start the aptdaemon |
720 | + self.start_session_aptd(self.chroot.path) |
721 | + # start policykit and *only* allow from-whitelisted repo pk action |
722 | + self.start_fake_polkitd( |
723 | + aptdaemon.policykit1.PK_ACTION_INSTALL_PACKAGES_FROM_HIGH_TRUST_REPO) |
724 | + time.sleep(1) |
725 | + self.client = aptdaemon.client.AptClient(self.bus) |
726 | + |
727 | + def test_high_trust_polkit_ok(self): |
728 | + # test that the high trust whitelist works |
729 | + trans = self.client.install_packages(["silly-base"]) |
730 | + trans.simulate() |
731 | + trans.connect("finished", lambda a,b: self.loop.quit()) |
732 | + trans.run() |
733 | + self.loop.run() |
734 | + self.assertEqual(trans.exit, aptdaemon.enums.EXIT_SUCCESS) |
735 | + # plus ensure removal will not work |
736 | + trans = self.client.remove_packages(["silly-base"]) |
737 | + with self.assertRaises(aptdaemon.errors.NotAuthorizedError): |
738 | + trans.run() |
739 | + |
740 | + def test_high_trust_polkit_not_ok(self): |
741 | + # ensure that non-whitelisted packages can not be installed |
742 | + trans = self.client.install_packages(["other-pkg"]) |
743 | + trans.simulate() |
744 | + trans.connect("finished", lambda a,b: self.loop.quit()) |
745 | + with self.assertRaises(aptdaemon.errors.NotAuthorizedError): |
746 | + trans.run() |
747 | + |
748 | + |
749 | +if __name__ == "__main__": |
750 | + #import logging |
751 | + #logging.basicConfig(level=logging.DEBUG) |
752 | + unittest.main() |
753 | + |
754 | +# vim: ts=4 et sts=4 |
755 | |
756 | === modified file 'tests/test_worker.py' |
757 | --- tests/test_worker.py 2012-08-10 13:59:10 +0000 |
758 | +++ tests/test_worker.py 2012-09-03 09:53:19 +0000 |
759 | @@ -31,26 +31,25 @@ |
760 | |
761 | import apt_pkg |
762 | from gi.repository import GObject |
763 | -import dbus |
764 | +from mock import ( |
765 | + Mock, |
766 | + patch, |
767 | + ) |
768 | |
769 | import aptdaemon.test |
770 | -from aptdaemon.worker import AptWorker |
771 | +from aptdaemon.worker import ( |
772 | + transaction_only_installs_packages_from_high_trust_repos, |
773 | + AptWorker, |
774 | + ) |
775 | from aptdaemon.core import Transaction |
776 | from aptdaemon import enums, errors |
777 | |
778 | + |
779 | REPO_PATH = os.path.join(aptdaemon.test.get_tests_dir(), "repo") |
780 | |
781 | PY3K = sys.version_info.major > 2 |
782 | |
783 | |
784 | -class MockQueue(object): |
785 | - |
786 | - """A fake TransactionQueue which only provides a limbo attribute.""" |
787 | - |
788 | - def __init__(self): |
789 | - self.limbo = {} |
790 | - |
791 | - |
792 | class WorkerTestCase(aptdaemon.test.AptDaemonTestCase): |
793 | |
794 | """Test suite for the worker which performs the actual package |
795 | @@ -61,7 +60,7 @@ |
796 | self.chroot.setup() |
797 | self.addCleanup(self.chroot.remove) |
798 | self.loop = GObject.MainLoop() |
799 | - self.queue = MockQueue() |
800 | + self.queue = aptdaemon.test.MockQueue() |
801 | self.worker = AptWorker(chroot=self.chroot.path, load_plugins=False) |
802 | self.worker.connect("transaction-done", lambda w,t: self.loop.quit()) |
803 | self.worker.connect("transaction-simulated", |
804 | @@ -438,8 +437,6 @@ |
805 | """Test if credentials of repositories are store securely in a |
806 | separate file. |
807 | """ |
808 | - from mock import Mock |
809 | - |
810 | source_file_name = "private_source.list" |
811 | self.worker.add_repository(Mock(), "deb", |
812 | "https://user:pass@host.example.com/path", |
813 | @@ -469,6 +466,8 @@ |
814 | |
815 | |
816 | if __name__ == "__main__": |
817 | + #import logging |
818 | + #logging.basicConfig(level=logging.DEBUG) |
819 | unittest.main() |
820 | |
821 | # vim: ts=4 et sts=4 |
As a general nitpick, I'd prefer to rename "whitelisted" in any public API/properties etc. to "passwordless" or something similarly meaningful, to point out what this list actually does. This especially applies to the conffile, as renaming it later is painful.
The test case does not actually verify that the expected PK privilege is being used, as it only goes until the simulation stage (or did I get that wrong?). You could tell the mock polkitd to permit org.debian. apt.install- packages- from-whiteliste d-repo and nothing else, and verify that installation of silly-* works, but other-pkg fails due to a permission error. It would also be nice to then try to remove the package again and verify that for this case (i. e. any but "install" it requires the "full" privilege.
I also think read_repository _whitelist( ) should become more robust against errors in the conffile. ConfigParser defaults to "strict" and thus excepts out if there a duplicate keys, etc. We don't want aptdaemon to crash on a ConfigParser.* exception; I think it should just be logged and it should return an empty set then. (Also easy to testcase).
Otherwise this looks good to me. Thanks!