Merge lp:~mardy/signon-apparmor-extension/apparmor-1489489 into lp:signon-apparmor-extension

Proposed by Alberto Mardegan on 2016-09-21
Status: Merged
Approved by: Alberto Mardegan on 2016-10-05
Approved revision: 31
Merged at revision: 25
Proposed branch: lp:~mardy/signon-apparmor-extension/apparmor-1489489
Merge into: lp:signon-apparmor-extension
Diff against target: 446 lines (+183/-41)
11 files modified
.qmake.conf (+2/-0)
common-project-config.pri (+6/-2)
debian/control (+4/-3)
src/access-control-manager.cpp (+18/-9)
src/access-control-manager.h (+3/-0)
tests/dbus_apparmor.py (+50/-0)
tests/fake_dbus.h (+52/-0)
tests/mock/apparmor.cpp (+0/-11)
tests/mock/mock.pro (+2/-3)
tests/tst_extension.cpp (+38/-11)
tests/tst_extension.pro (+8/-2)
To merge this branch: bzr merge lp:~mardy/signon-apparmor-extension/apparmor-1489489
Reviewer Review Type Date Requested Status
Alexandre Abreu (community) 2016-09-21 Approve on 2016-10-04
Review via email: mp+306316@code.launchpad.net

Commit message

Use GetConnectionCredentials() method instead of the deprecated apparmor-specific method.

Description of the change

Use GetConnectionCredentials() method instead of the deprecated apparmor-specific method.

To post a comment you must log in.
28. By Alberto Mardegan on 2016-09-29

Rename mock file

29. By Alberto Mardegan on 2016-09-29

Update apparmor dep version

30. By Alberto Mardegan on 2016-09-29

Enable C++11

Alexandre Abreu (abreu-alexandre) wrote :

a few comments and questions

review: Needs Information
Alberto Mardegan (mardy) :
31. By Alberto Mardegan on 2016-10-04

Address review comments

Alexandre Abreu (abreu-alexandre) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file '.qmake.conf'
2--- .qmake.conf 1970-01-01 00:00:00 +0000
3+++ .qmake.conf 2016-10-04 07:11:40 +0000
4@@ -0,0 +1,2 @@
5+TOP_SRC_DIR = $$PWD
6+TOP_BUILD_DIR = $$shadowed($$PWD)
7
8=== modified file 'common-project-config.pri'
9--- common-project-config.pri 2013-08-23 14:35:22 +0000
10+++ common-project-config.pri 2016-10-04 07:11:40 +0000
11@@ -2,12 +2,16 @@
12 # Common configuration for all projects.
13 #-----------------------------------------------------------------------------
14
15+CONFIG += c++11
16+
17 # we don't like warnings...
18 QMAKE_CXXFLAGS += -Werror
19 # Disable RTTI
20 QMAKE_CXXFLAGS += -fno-exceptions -fno-rtti
21
22-TOP_SRC_DIR = $$PWD
23-TOP_BUILD_DIR = $${TOP_SRC_DIR}/$(BUILD_DIR)
24+!defined(TOP_SRC_DIR, var) {
25+ TOP_SRC_DIR = $$PWD
26+ TOP_BUILD_DIR = $${TOP_SRC_DIR}/$(BUILD_DIR)
27+}
28
29 include(coverage.pri)
30
31=== modified file 'debian/control'
32--- debian/control 2014-05-15 17:28:41 +0000
33+++ debian/control 2016-10-04 07:11:40 +0000
34@@ -1,10 +1,11 @@
35 Source: signon-apparmor-extension
36 Priority: optional
37 Maintainer: Ubuntu Desktop Team <ubuntu-desktop@lists.ubuntu.com>
38-Build-Depends: dbus-test-runner,
39- debhelper (>= 9),
40- libapparmor-dev,
41+Build-Depends: debhelper (>= 9),
42+ libapparmor-dev (>= 2.9.1-0ubuntu9.1),
43 libdbus-1-dev,
44+ libqtdbusmock1-dev,
45+ libqtdbustest1-dev,
46 pkg-config,
47 qt5-default,
48 qt5-qmake,
49
50=== modified file 'src/access-control-manager.cpp'
51--- src/access-control-manager.cpp 2016-06-07 08:05:54 +0000
52+++ src/access-control-manager.cpp 2016-10-04 07:11:40 +0000
53@@ -22,7 +22,9 @@
54 #include "access-control-manager.h"
55
56 #include <QDBusConnection>
57+#include <QDBusError>
58 #include <QDBusMessage>
59+#include <QDBusReply>
60 #include <QDebug>
61 #include <QStringList>
62 #include <sys/apparmor.h>
63@@ -38,7 +40,9 @@
64 }
65
66 AccessControlManager::AccessControlManager(QObject *parent):
67- SignOn::AbstractAccessControlManager(parent)
68+ SignOn::AbstractAccessControlManager(parent),
69+ m_dbusService(qEnvironmentVariableIsSet("SIGNON_APPARMOR_TEST") ?
70+ "fake.freedesktop.DBus" : "org.freedesktop.DBus")
71 {
72 }
73
74@@ -85,21 +89,26 @@
75 appId = "unconfined";
76 } else {
77 QDBusMessage msg =
78- QDBusMessage::createMethodCall("org.freedesktop.DBus",
79+ QDBusMessage::createMethodCall(m_dbusService,
80 "/org/freedesktop/DBus",
81 "org.freedesktop.DBus",
82- "GetConnectionAppArmorSecurityContext");
83+ "GetConnectionCredentials");
84 QVariantList args;
85 args << uniqueConnectionId;
86 msg.setArguments(args);
87- QDBusMessage reply =
88- QDBusConnection::sessionBus().call(msg, QDBus::Block);
89- if (reply.type() == QDBusMessage::ReplyMessage) {
90- appId = reply.arguments().value(0, QString()).toString();
91+ QDBusReply<QVariantMap> reply = QDBusConnection::sessionBus().call(msg, QDBus::Block);
92+ if (reply.isValid()) {
93+ QVariantMap map = reply.value();
94+ QByteArray context = map.value("LinuxSecurityLabel").toByteArray();
95+ if (!context.isEmpty()) {
96+ aa_splitcon(context.data(), NULL);
97+ appId = QString::fromUtf8(context);
98+ }
99 qDebug() << "App ID:" << appId;
100 } else {
101- qWarning() << "Error getting app ID:" << reply.errorName() <<
102- reply.errorMessage();
103+ QDBusError error = reply.error();
104+ qWarning() << "Error getting app ID:" << error.name() <<
105+ error.message();
106 }
107 }
108 return appId;
109
110=== modified file 'src/access-control-manager.h'
111--- src/access-control-manager.h 2014-01-31 15:39:01 +0000
112+++ src/access-control-manager.h 2016-10-04 07:11:40 +0000
113@@ -54,6 +54,9 @@
114
115 private:
116 QString stripVersion(const QString &appId) const;
117+
118+private:
119+ QString m_dbusService;
120 };
121
122 #endif // SIGNON_APPARMOR_ACCESS_CONTROL_MANAGER_H
123
124=== added file 'tests/dbus_apparmor.py'
125--- tests/dbus_apparmor.py 1970-01-01 00:00:00 +0000
126+++ tests/dbus_apparmor.py 2016-10-04 07:11:40 +0000
127@@ -0,0 +1,50 @@
128+'''dbus mock template
129+
130+This creates the expected methods and properties of the
131+org.freedesktop.DBus service.
132+'''
133+
134+# This program is free software; you can redistribute it and/or modify it under
135+# the terms of the GNU Lesser General Public License as published by the Free
136+# Software Foundation; either version 3 of the License, or (at your option) any
137+# later version. See http://www.gnu.org/copyleft/lgpl.html for the full text
138+# of the license.
139+
140+__author__ = 'Alberto Mardegan'
141+__email__ = 'alberto.mardegan@canonical.com'
142+__copyright__ = '(c) 2016 Canonical Ltd.'
143+__license__ = 'LGPL 3+'
144+
145+import dbus
146+import time
147+
148+from dbusmock import MOCK_IFACE
149+import dbusmock
150+
151+BUS_NAME = 'fake.freedesktop.DBus'
152+MAIN_OBJ = '/org/freedesktop/DBus'
153+MAIN_IFACE = 'org.freedesktop.DBus'
154+SYSTEM_BUS = False
155+
156+ERROR_PREFIX = 'org.freedesktop.DBus.Error.'
157+ERROR_NAME_HAS_NO_OWNER = ERROR_PREFIX + 'NameHasNoOwner'
158+
159+def get_credentials(self, service):
160+ if service not in self.credentials:
161+ raise dbus.exceptions.DBusException('Service not found',
162+ name=ERROR_NAME_HAS_NO_OWNER)
163+ return self.credentials[service]
164+
165+
166+def load(mock, parameters):
167+ mock.get_credentials = get_credentials
168+ mock.AddMethods(MAIN_IFACE, [
169+ ('GetConnectionCredentials', 's', 'a{sv}', 'ret = self.get_credentials(self, args[0])'),
170+ ])
171+
172+ mock.credentials = {}
173+
174+
175+@dbus.service.method(MOCK_IFACE, in_signature='sa{sv}', out_signature='')
176+def SetCredentials(self, service, credentials):
177+ self.credentials[service] = credentials
178
179=== added file 'tests/fake_dbus.h'
180--- tests/fake_dbus.h 1970-01-01 00:00:00 +0000
181+++ tests/fake_dbus.h 2016-10-04 07:11:40 +0000
182@@ -0,0 +1,52 @@
183+/*
184+ * This file is part of signon-apparmor-extension
185+ *
186+ * Copyright (C) 2016 Canonical Ltd.
187+ *
188+ * Contact: Alberto Mardegan <alberto.mardegan@canonical.com>
189+ *
190+ * This program is free software: you can redistribute it and/or modify it
191+ * under the terms of the GNU Lesser General Public License version 3, as
192+ * published by the Free Software Foundation.
193+ *
194+ * This program is distributed in the hope that it will be useful, but
195+ * WITHOUT ANY WARRANTY; without even the implied warranties of
196+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
197+ * PURPOSE. See the GNU Lesser General Public License for more details.
198+ *
199+ * You should have received a copy of the GNU Lesser General Public License
200+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
201+ */
202+
203+#ifndef AP_FAKE_DBUS_H
204+#define AP_FAKE_DBUS_H
205+
206+#include <QVariantMap>
207+#include <libqtdbusmock/DBusMock.h>
208+
209+class FakeDBus
210+{
211+public:
212+ FakeDBus(QtDBusMock::DBusMock *mock): m_mock(mock) {
213+ m_mock->registerTemplate("fake.freedesktop.DBus",
214+ DBUS_MOCK_TEMPLATE,
215+ QDBusConnection::SessionBus);
216+ }
217+
218+ void setCredentials(const QString &service, const QVariantMap &credentials) {
219+ mockedDBusService().call("SetCredentials", service, credentials);
220+ }
221+
222+private:
223+ OrgFreedesktopDBusMockInterface &mockedDBusService() {
224+ return m_mock->mockInterface("fake.freedesktop.DBus",
225+ "/org/freedesktop/DBus",
226+ "org.freedesktop.DBus",
227+ QDBusConnection::SessionBus);
228+ }
229+
230+private:
231+ QtDBusMock::DBusMock *m_mock;
232+};
233+
234+#endif // AP_FAKE_DBUS_H
235
236=== modified file 'tests/mock/apparmor.cpp'
237--- tests/mock/apparmor.cpp 2014-02-03 08:35:47 +0000
238+++ tests/mock/apparmor.cpp 2016-10-04 07:11:40 +0000
239@@ -20,8 +20,6 @@
240 */
241
242 #include <QByteArray>
243-#include <QDBusConnection>
244-#include <QDBusMessage>
245 #include <QDebug>
246 #include <sys/apparmor.h>
247
248@@ -40,12 +38,3 @@
249 *con = strdup(mockedProfile().constData());
250 return 0;
251 }
252-
253-QDBusMessage QDBusConnection::call(const QDBusMessage &message,
254- QDBus::CallMode mode,
255- int timeout) const
256-{
257- Q_UNUSED(mode);
258- Q_UNUSED(timeout);
259- return message.createReply(QVariantList() << mockedProfile().constData());
260-}
261
262=== modified file 'tests/mock/mock.pro'
263--- tests/mock/mock.pro 2013-09-04 06:55:40 +0000
264+++ tests/mock/mock.pro 2016-10-04 07:11:40 +0000
265@@ -1,6 +1,6 @@
266 include(../../common-project-config.pri)
267
268-TARGET = apparmor
269+TARGET = fakeapparmor
270 TEMPLATE = lib
271
272 CONFIG += \
273@@ -9,8 +9,7 @@
274 qt
275
276 QT += \
277- core \
278- dbus
279+ core
280
281 PKGCONFIG += \
282 libapparmor
283
284=== modified file 'tests/tst_extension.cpp'
285--- tests/tst_extension.cpp 2015-01-28 15:30:34 +0000
286+++ tests/tst_extension.cpp 2016-10-04 07:11:40 +0000
287@@ -1,5 +1,5 @@
288 /*
289- * Copyright (C) 2013 Canonical Ltd.
290+ * Copyright (C) 2016 Canonical Ltd.
291 *
292 * Contact: Alberto Mardegan <alberto.mardegan@canonical.com>
293 *
294@@ -19,6 +19,8 @@
295 * along with this library. If not, see <http://www.gnu.org/licenses/>.
296 */
297
298+#include "fake_dbus.h"
299+
300 #include <QDBusConnection>
301 #include <QDBusMessage>
302 #include <QDBusServer>
303@@ -28,8 +30,13 @@
304
305 #include <SignOn/AbstractAccessControlManager>
306
307+#include <libqtdbusmock/DBusMock.h>
308+
309+
310 #define P2P_SOCKET "unix:path=/tmp/tst_extension_%1"
311
312+using namespace QtDBusMock;
313+
314 static void setMockedProfile(const char *profile)
315 {
316 if (profile) {
317@@ -47,6 +54,13 @@
318 {
319 Q_OBJECT
320
321+ struct SetupEnvironment {
322+ /* The test is run with a LD_PRELOAD in order to mock aa_getpeercon();
323+ * but we don't want this to propagate to our children: dbus-daemon in
324+ * particular won't like it. */
325+ SetupEnvironment() { qunsetenv("LD_PRELOAD"); }
326+ };
327+
328 public:
329 ExtensionTest();
330
331@@ -62,18 +76,29 @@
332
333 private:
334 SignOn::AbstractAccessControlManager *m_acm;
335+ SetupEnvironment m_env;
336+ QtDBusTest::DBusTestRunner m_dbus;
337+ QtDBusMock::DBusMock m_mock;
338+ FakeDBus m_fakeDBus;
339 QDBusConnection m_busConnection;
340 QDBusConnection m_p2pConnection;
341 };
342
343 ExtensionTest::ExtensionTest():
344- m_busConnection(QDBusConnection::sessionBus()),
345+ m_mock(m_dbus),
346+ m_fakeDBus(&m_mock),
347+ m_busConnection(m_dbus.sessionConnection()),
348 m_p2pConnection(QStringLiteral("uninitialized"))
349 {
350+ DBusMock::registerMetaTypes();
351 }
352
353 void ExtensionTest::initTestCase()
354 {
355+ m_dbus.startServices();
356+
357+ qputenv("SIGNON_APPARMOR_TEST", "1");
358+
359 QPluginLoader pluginLoader(PLUGIN_PATH);
360 QObject *plugin = pluginLoader.instance();
361 QVERIFY(plugin != 0);
362@@ -104,15 +129,15 @@
363
364 void ExtensionTest::test_appId()
365 {
366- QSKIP("Disable because of QTBUG-36475");
367+ m_fakeDBus.setCredentials(m_busConnection.baseService(), {
368+ { "LinuxSecurityLabel", QByteArray("unconfined") },
369+ });
370
371 /* forge a QDBusMessage */
372 QDBusMessage msg =
373 QDBusMessage::createMethodCall(m_busConnection.baseService(),
374 "/", "my.interface", "hi");
375 QString appId = m_acm->appIdOfPeer(m_busConnection, msg);
376- /* At the moment, AppArmor doesn't implement the
377- * GetConnectionAppArmorSecurityContext method, so expect an error. */
378 QCOMPARE(appId, QStringLiteral("unconfined"));
379 }
380
381@@ -127,6 +152,9 @@
382
383 void ExtensionTest::test_click_version()
384 {
385+ m_fakeDBus.setCredentials(":0.1", {
386+ { "LinuxSecurityLabel", QByteArray("com.ubuntu.myapp_myapp_0.2 (enforce)") },
387+ });
388 /* forge a QDBusMessage */
389 setMockedProfile("com.ubuntu.myapp_myapp_0.2");
390 QDBusMessage msg =
391@@ -148,23 +176,22 @@
392
393 void ExtensionTest::test_access()
394 {
395- QSKIP("Disable because of QTBUG-36475");
396-
397+ m_fakeDBus.setCredentials(m_busConnection.baseService(), {
398+ });
399 /* forge a QDBusMessage */
400 QDBusMessage msg =
401 QDBusMessage::createMethodCall(m_busConnection.baseService(),
402 "/", "my.interface", "hi");
403 bool allowed = m_acm->isPeerAllowedToAccess(m_busConnection, msg,
404 "anyContext");
405- /* At the moment, AppArmor doesn't implement the
406- * GetConnectionAppArmorSecurityContext method, so expect an error. */
407 QVERIFY(!allowed);
408 }
409
410 void ExtensionTest::test_accessWildcard()
411 {
412- QSKIP("Disable because of QTBUG-36475");
413-
414+ m_fakeDBus.setCredentials(":0.1", {
415+ { "LinuxSecurityLabel", QByteArray("com.ubuntu.myapp_myapp_0.2 (enforce)") },
416+ });
417 /* forge a QDBusMessage */
418 QDBusMessage msg =
419 QDBusMessage::createMethodCall(m_busConnection.baseService(),
420
421=== modified file 'tests/tst_extension.pro'
422--- tests/tst_extension.pro 2013-09-04 06:55:40 +0000
423+++ tests/tst_extension.pro 2016-10-04 07:11:40 +0000
424@@ -14,15 +14,21 @@
425 QT -= gui
426
427 PKGCONFIG += \
428- SignOnExtension
429+ SignOnExtension \
430+ libqtdbusmock-1 \
431+ libqtdbustest-1
432
433 DEFINES += \
434+ DBUS_MOCK_TEMPLATE=\\\"$${PWD}/dbus_apparmor.py\\\" \
435 PLUGIN_PATH=\\\"../src/libsignon-apparmor-ac.so\\\"
436
437 SOURCES = \
438 tst_extension.cpp
439
440-check.commands = "LD_PRELOAD=mock/libapparmor.so dbus-test-runner -t ./$${TARGET}"
441+HEADERS += \
442+ fake_dbus.h
443+
444+check.commands = "LD_PRELOAD=mock/libfakeapparmor.so ./$${TARGET}"
445 check.depends = $${TARGET}
446 QMAKE_EXTRA_TARGETS += check
447

Subscribers

People subscribed via source and target branches