Merge lp://staging/~jamesh/thumbnailer/dbus-aa-credentials into lp://staging/thumbnailer/devel

Proposed by James Henstridge
Status: Merged
Approved by: Michi Henning
Approved revision: 223
Merged at revision: 214
Proposed branch: lp://staging/~jamesh/thumbnailer/dbus-aa-credentials
Merge into: lp://staging/thumbnailer/devel
Diff against target: 526 lines (+324/-7)
11 files modified
CMakeLists.txt (+1/-0)
debian/control (+1/-0)
src/service/CMakeLists.txt (+8/-2)
src/service/bus.xml (+9/-0)
src/service/credentialscache.cpp (+179/-0)
src/service/credentialscache.h (+81/-0)
src/service/dbusinterface.cpp (+16/-3)
src/service/dbusinterface.h (+4/-0)
src/service/handler.cpp (+21/-1)
src/service/handler.h (+3/-0)
tests/qml/CMakeLists.txt (+1/-1)
To merge this branch: bzr merge lp://staging/~jamesh/thumbnailer/dbus-aa-credentials
Reviewer Review Type Date Requested Status
Michi Henning (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+261824@code.staging.launchpad.net

Commit message

Track the credentials (user ID, AppArmor label) of clients connecting to the D-Bus service. This is not yet used to make security decisions.

Description of the change

This is the first step of the changing our security policy to rely on aa_query_label().

I've introduced a new step for the request handler to determine the AppArmor security context of the client. At the moment we're only printing it out in a log message, but eventually this information will be pushed down to the ThumbnailRequest where it can be used to make the security decision.

You can test this on the desktop by running thumbnailer-service in one terminal and in another run something like:

    aa-exec -p $profile thumbnailer-admin get $filename outdir/

You can get a list of available profiles on the system with "sudo aa-status". When run on the phone, you should see the labels for confined clients.

To avoid excessive GetConnectionCredentials() calls to the bus daemon, we cache the results in the CredentialsCache class.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
221. By James Henstridge

A few style changes.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Michi Henning (michihenning) wrote :

Looks really nice, thank you!

Just to show that I read it, I have to make at least one anal-retentive comment ;-)

qWarning() << "CredentialsCache::received_credentials: error retrieving credentials for" << peer << ":" << reply.error().message();

Should be

... CredentialsCache::received_credentials(): error ...

Splitting the line into two shorter ones would make it a little bit more readable.

review: Needs Fixing
222. By James Henstridge

Make qml_test depend on thumbnailer-qml.

223. By James Henstridge

Fix up warning message, from Michi's review.

Revision history for this message
Michi Henning (michihenning) wrote :

Sweet, thank you!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: