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
=== added file '.qmake.conf'
--- .qmake.conf 1970-01-01 00:00:00 +0000
+++ .qmake.conf 2016-10-04 07:11:40 +0000
@@ -0,0 +1,2 @@
1TOP_SRC_DIR = $$PWD
2TOP_BUILD_DIR = $$shadowed($$PWD)
03
=== modified file 'common-project-config.pri'
--- common-project-config.pri 2013-08-23 14:35:22 +0000
+++ common-project-config.pri 2016-10-04 07:11:40 +0000
@@ -2,12 +2,16 @@
2# Common configuration for all projects.2# Common configuration for all projects.
3#-----------------------------------------------------------------------------3#-----------------------------------------------------------------------------
44
5CONFIG += c++11
6
5# we don't like warnings...7# we don't like warnings...
6QMAKE_CXXFLAGS += -Werror8QMAKE_CXXFLAGS += -Werror
7# Disable RTTI9# Disable RTTI
8QMAKE_CXXFLAGS += -fno-exceptions -fno-rtti10QMAKE_CXXFLAGS += -fno-exceptions -fno-rtti
911
10TOP_SRC_DIR = $$PWD12!defined(TOP_SRC_DIR, var) {
11TOP_BUILD_DIR = $${TOP_SRC_DIR}/$(BUILD_DIR)13 TOP_SRC_DIR = $$PWD
14 TOP_BUILD_DIR = $${TOP_SRC_DIR}/$(BUILD_DIR)
15}
1216
13include(coverage.pri)17include(coverage.pri)
1418
=== modified file 'debian/control'
--- debian/control 2014-05-15 17:28:41 +0000
+++ debian/control 2016-10-04 07:11:40 +0000
@@ -1,10 +1,11 @@
1Source: signon-apparmor-extension1Source: signon-apparmor-extension
2Priority: optional2Priority: optional
3Maintainer: Ubuntu Desktop Team <ubuntu-desktop@lists.ubuntu.com>3Maintainer: Ubuntu Desktop Team <ubuntu-desktop@lists.ubuntu.com>
4Build-Depends: dbus-test-runner,4Build-Depends: debhelper (>= 9),
5 debhelper (>= 9),5 libapparmor-dev (>= 2.9.1-0ubuntu9.1),
6 libapparmor-dev,
7 libdbus-1-dev,6 libdbus-1-dev,
7 libqtdbusmock1-dev,
8 libqtdbustest1-dev,
8 pkg-config,9 pkg-config,
9 qt5-default,10 qt5-default,
10 qt5-qmake,11 qt5-qmake,
1112
=== modified file 'src/access-control-manager.cpp'
--- src/access-control-manager.cpp 2016-06-07 08:05:54 +0000
+++ src/access-control-manager.cpp 2016-10-04 07:11:40 +0000
@@ -22,7 +22,9 @@
22#include "access-control-manager.h"22#include "access-control-manager.h"
2323
24#include <QDBusConnection>24#include <QDBusConnection>
25#include <QDBusError>
25#include <QDBusMessage>26#include <QDBusMessage>
27#include <QDBusReply>
26#include <QDebug>28#include <QDebug>
27#include <QStringList>29#include <QStringList>
28#include <sys/apparmor.h>30#include <sys/apparmor.h>
@@ -38,7 +40,9 @@
38}40}
3941
40AccessControlManager::AccessControlManager(QObject *parent):42AccessControlManager::AccessControlManager(QObject *parent):
41 SignOn::AbstractAccessControlManager(parent)43 SignOn::AbstractAccessControlManager(parent),
44 m_dbusService(qEnvironmentVariableIsSet("SIGNON_APPARMOR_TEST") ?
45 "fake.freedesktop.DBus" : "org.freedesktop.DBus")
42{46{
43}47}
4448
@@ -85,21 +89,26 @@
85 appId = "unconfined";89 appId = "unconfined";
86 } else {90 } else {
87 QDBusMessage msg =91 QDBusMessage msg =
88 QDBusMessage::createMethodCall("org.freedesktop.DBus",92 QDBusMessage::createMethodCall(m_dbusService,
89 "/org/freedesktop/DBus",93 "/org/freedesktop/DBus",
90 "org.freedesktop.DBus",94 "org.freedesktop.DBus",
91 "GetConnectionAppArmorSecurityContext");95 "GetConnectionCredentials");
92 QVariantList args;96 QVariantList args;
93 args << uniqueConnectionId;97 args << uniqueConnectionId;
94 msg.setArguments(args);98 msg.setArguments(args);
95 QDBusMessage reply =99 QDBusReply<QVariantMap> reply = QDBusConnection::sessionBus().call(msg, QDBus::Block);
96 QDBusConnection::sessionBus().call(msg, QDBus::Block);100 if (reply.isValid()) {
97 if (reply.type() == QDBusMessage::ReplyMessage) {101 QVariantMap map = reply.value();
98 appId = reply.arguments().value(0, QString()).toString();102 QByteArray context = map.value("LinuxSecurityLabel").toByteArray();
103 if (!context.isEmpty()) {
104 aa_splitcon(context.data(), NULL);
105 appId = QString::fromUtf8(context);
106 }
99 qDebug() << "App ID:" << appId;107 qDebug() << "App ID:" << appId;
100 } else {108 } else {
101 qWarning() << "Error getting app ID:" << reply.errorName() <<109 QDBusError error = reply.error();
102 reply.errorMessage();110 qWarning() << "Error getting app ID:" << error.name() <<
111 error.message();
103 }112 }
104 }113 }
105 return appId;114 return appId;
106115
=== modified file 'src/access-control-manager.h'
--- src/access-control-manager.h 2014-01-31 15:39:01 +0000
+++ src/access-control-manager.h 2016-10-04 07:11:40 +0000
@@ -54,6 +54,9 @@
5454
55private:55private:
56 QString stripVersion(const QString &appId) const;56 QString stripVersion(const QString &appId) const;
57
58private:
59 QString m_dbusService;
57};60};
5861
59#endif // SIGNON_APPARMOR_ACCESS_CONTROL_MANAGER_H62#endif // SIGNON_APPARMOR_ACCESS_CONTROL_MANAGER_H
6063
=== added file 'tests/dbus_apparmor.py'
--- tests/dbus_apparmor.py 1970-01-01 00:00:00 +0000
+++ tests/dbus_apparmor.py 2016-10-04 07:11:40 +0000
@@ -0,0 +1,50 @@
1'''dbus mock template
2
3This creates the expected methods and properties of the
4org.freedesktop.DBus service.
5'''
6
7# This program is free software; you can redistribute it and/or modify it under
8# the terms of the GNU Lesser General Public License as published by the Free
9# Software Foundation; either version 3 of the License, or (at your option) any
10# later version. See http://www.gnu.org/copyleft/lgpl.html for the full text
11# of the license.
12
13__author__ = 'Alberto Mardegan'
14__email__ = 'alberto.mardegan@canonical.com'
15__copyright__ = '(c) 2016 Canonical Ltd.'
16__license__ = 'LGPL 3+'
17
18import dbus
19import time
20
21from dbusmock import MOCK_IFACE
22import dbusmock
23
24BUS_NAME = 'fake.freedesktop.DBus'
25MAIN_OBJ = '/org/freedesktop/DBus'
26MAIN_IFACE = 'org.freedesktop.DBus'
27SYSTEM_BUS = False
28
29ERROR_PREFIX = 'org.freedesktop.DBus.Error.'
30ERROR_NAME_HAS_NO_OWNER = ERROR_PREFIX + 'NameHasNoOwner'
31
32def get_credentials(self, service):
33 if service not in self.credentials:
34 raise dbus.exceptions.DBusException('Service not found',
35 name=ERROR_NAME_HAS_NO_OWNER)
36 return self.credentials[service]
37
38
39def load(mock, parameters):
40 mock.get_credentials = get_credentials
41 mock.AddMethods(MAIN_IFACE, [
42 ('GetConnectionCredentials', 's', 'a{sv}', 'ret = self.get_credentials(self, args[0])'),
43 ])
44
45 mock.credentials = {}
46
47
48@dbus.service.method(MOCK_IFACE, in_signature='sa{sv}', out_signature='')
49def SetCredentials(self, service, credentials):
50 self.credentials[service] = credentials
051
=== added file 'tests/fake_dbus.h'
--- tests/fake_dbus.h 1970-01-01 00:00:00 +0000
+++ tests/fake_dbus.h 2016-10-04 07:11:40 +0000
@@ -0,0 +1,52 @@
1/*
2 * This file is part of signon-apparmor-extension
3 *
4 * Copyright (C) 2016 Canonical Ltd.
5 *
6 * Contact: Alberto Mardegan <alberto.mardegan@canonical.com>
7 *
8 * This program is free software: you can redistribute it and/or modify it
9 * under the terms of the GNU Lesser General Public License version 3, as
10 * published by the Free Software Foundation.
11 *
12 * This program is distributed in the hope that it will be useful, but
13 * WITHOUT ANY WARRANTY; without even the implied warranties of
14 * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
15 * PURPOSE. See the GNU Lesser General Public License for more details.
16 *
17 * You should have received a copy of the GNU Lesser General Public License
18 * along with this program. If not, see <http://www.gnu.org/licenses/>.
19 */
20
21#ifndef AP_FAKE_DBUS_H
22#define AP_FAKE_DBUS_H
23
24#include <QVariantMap>
25#include <libqtdbusmock/DBusMock.h>
26
27class FakeDBus
28{
29public:
30 FakeDBus(QtDBusMock::DBusMock *mock): m_mock(mock) {
31 m_mock->registerTemplate("fake.freedesktop.DBus",
32 DBUS_MOCK_TEMPLATE,
33 QDBusConnection::SessionBus);
34 }
35
36 void setCredentials(const QString &service, const QVariantMap &credentials) {
37 mockedDBusService().call("SetCredentials", service, credentials);
38 }
39
40private:
41 OrgFreedesktopDBusMockInterface &mockedDBusService() {
42 return m_mock->mockInterface("fake.freedesktop.DBus",
43 "/org/freedesktop/DBus",
44 "org.freedesktop.DBus",
45 QDBusConnection::SessionBus);
46 }
47
48private:
49 QtDBusMock::DBusMock *m_mock;
50};
51
52#endif // AP_FAKE_DBUS_H
053
=== modified file 'tests/mock/apparmor.cpp'
--- tests/mock/apparmor.cpp 2014-02-03 08:35:47 +0000
+++ tests/mock/apparmor.cpp 2016-10-04 07:11:40 +0000
@@ -20,8 +20,6 @@
20 */20 */
2121
22#include <QByteArray>22#include <QByteArray>
23#include <QDBusConnection>
24#include <QDBusMessage>
25#include <QDebug>23#include <QDebug>
26#include <sys/apparmor.h>24#include <sys/apparmor.h>
2725
@@ -40,12 +38,3 @@
40 *con = strdup(mockedProfile().constData());38 *con = strdup(mockedProfile().constData());
41 return 0;39 return 0;
42}40}
43
44QDBusMessage QDBusConnection::call(const QDBusMessage &message,
45 QDBus::CallMode mode,
46 int timeout) const
47{
48 Q_UNUSED(mode);
49 Q_UNUSED(timeout);
50 return message.createReply(QVariantList() << mockedProfile().constData());
51}
5241
=== modified file 'tests/mock/mock.pro'
--- tests/mock/mock.pro 2013-09-04 06:55:40 +0000
+++ tests/mock/mock.pro 2016-10-04 07:11:40 +0000
@@ -1,6 +1,6 @@
1include(../../common-project-config.pri)1include(../../common-project-config.pri)
22
3TARGET = apparmor3TARGET = fakeapparmor
4TEMPLATE = lib4TEMPLATE = lib
55
6CONFIG += \6CONFIG += \
@@ -9,8 +9,7 @@
9 qt9 qt
1010
11QT += \11QT += \
12 core \12 core
13 dbus
1413
15PKGCONFIG += \14PKGCONFIG += \
16 libapparmor15 libapparmor
1716
=== modified file 'tests/tst_extension.cpp'
--- tests/tst_extension.cpp 2015-01-28 15:30:34 +0000
+++ tests/tst_extension.cpp 2016-10-04 07:11:40 +0000
@@ -1,5 +1,5 @@
1/*1/*
2 * Copyright (C) 2013 Canonical Ltd.2 * Copyright (C) 2016 Canonical Ltd.
3 *3 *
4 * Contact: Alberto Mardegan <alberto.mardegan@canonical.com>4 * Contact: Alberto Mardegan <alberto.mardegan@canonical.com>
5 *5 *
@@ -19,6 +19,8 @@
19 * along with this library. If not, see <http://www.gnu.org/licenses/>.19 * along with this library. If not, see <http://www.gnu.org/licenses/>.
20 */20 */
2121
22#include "fake_dbus.h"
23
22#include <QDBusConnection>24#include <QDBusConnection>
23#include <QDBusMessage>25#include <QDBusMessage>
24#include <QDBusServer>26#include <QDBusServer>
@@ -28,8 +30,13 @@
2830
29#include <SignOn/AbstractAccessControlManager>31#include <SignOn/AbstractAccessControlManager>
3032
33#include <libqtdbusmock/DBusMock.h>
34
35
31#define P2P_SOCKET "unix:path=/tmp/tst_extension_%1"36#define P2P_SOCKET "unix:path=/tmp/tst_extension_%1"
3237
38using namespace QtDBusMock;
39
33static void setMockedProfile(const char *profile)40static void setMockedProfile(const char *profile)
34{41{
35 if (profile) {42 if (profile) {
@@ -47,6 +54,13 @@
47{54{
48 Q_OBJECT55 Q_OBJECT
4956
57 struct SetupEnvironment {
58 /* The test is run with a LD_PRELOAD in order to mock aa_getpeercon();
59 * but we don't want this to propagate to our children: dbus-daemon in
60 * particular won't like it. */
61 SetupEnvironment() { qunsetenv("LD_PRELOAD"); }
62 };
63
50public:64public:
51 ExtensionTest();65 ExtensionTest();
5266
@@ -62,18 +76,29 @@
6276
63private:77private:
64 SignOn::AbstractAccessControlManager *m_acm;78 SignOn::AbstractAccessControlManager *m_acm;
79 SetupEnvironment m_env;
80 QtDBusTest::DBusTestRunner m_dbus;
81 QtDBusMock::DBusMock m_mock;
82 FakeDBus m_fakeDBus;
65 QDBusConnection m_busConnection;83 QDBusConnection m_busConnection;
66 QDBusConnection m_p2pConnection;84 QDBusConnection m_p2pConnection;
67};85};
6886
69ExtensionTest::ExtensionTest():87ExtensionTest::ExtensionTest():
70 m_busConnection(QDBusConnection::sessionBus()),88 m_mock(m_dbus),
89 m_fakeDBus(&m_mock),
90 m_busConnection(m_dbus.sessionConnection()),
71 m_p2pConnection(QStringLiteral("uninitialized"))91 m_p2pConnection(QStringLiteral("uninitialized"))
72{92{
93 DBusMock::registerMetaTypes();
73}94}
7495
75void ExtensionTest::initTestCase()96void ExtensionTest::initTestCase()
76{97{
98 m_dbus.startServices();
99
100 qputenv("SIGNON_APPARMOR_TEST", "1");
101
77 QPluginLoader pluginLoader(PLUGIN_PATH);102 QPluginLoader pluginLoader(PLUGIN_PATH);
78 QObject *plugin = pluginLoader.instance();103 QObject *plugin = pluginLoader.instance();
79 QVERIFY(plugin != 0);104 QVERIFY(plugin != 0);
@@ -104,15 +129,15 @@
104129
105void ExtensionTest::test_appId()130void ExtensionTest::test_appId()
106{131{
107 QSKIP("Disable because of QTBUG-36475");132 m_fakeDBus.setCredentials(m_busConnection.baseService(), {
133 { "LinuxSecurityLabel", QByteArray("unconfined") },
134 });
108135
109 /* forge a QDBusMessage */136 /* forge a QDBusMessage */
110 QDBusMessage msg =137 QDBusMessage msg =
111 QDBusMessage::createMethodCall(m_busConnection.baseService(),138 QDBusMessage::createMethodCall(m_busConnection.baseService(),
112 "/", "my.interface", "hi");139 "/", "my.interface", "hi");
113 QString appId = m_acm->appIdOfPeer(m_busConnection, msg);140 QString appId = m_acm->appIdOfPeer(m_busConnection, msg);
114 /* At the moment, AppArmor doesn't implement the
115 * GetConnectionAppArmorSecurityContext method, so expect an error. */
116 QCOMPARE(appId, QStringLiteral("unconfined"));141 QCOMPARE(appId, QStringLiteral("unconfined"));
117}142}
118143
@@ -127,6 +152,9 @@
127152
128void ExtensionTest::test_click_version()153void ExtensionTest::test_click_version()
129{154{
155 m_fakeDBus.setCredentials(":0.1", {
156 { "LinuxSecurityLabel", QByteArray("com.ubuntu.myapp_myapp_0.2 (enforce)") },
157 });
130 /* forge a QDBusMessage */158 /* forge a QDBusMessage */
131 setMockedProfile("com.ubuntu.myapp_myapp_0.2");159 setMockedProfile("com.ubuntu.myapp_myapp_0.2");
132 QDBusMessage msg =160 QDBusMessage msg =
@@ -148,23 +176,22 @@
148176
149void ExtensionTest::test_access()177void ExtensionTest::test_access()
150{178{
151 QSKIP("Disable because of QTBUG-36475");179 m_fakeDBus.setCredentials(m_busConnection.baseService(), {
152180 });
153 /* forge a QDBusMessage */181 /* forge a QDBusMessage */
154 QDBusMessage msg =182 QDBusMessage msg =
155 QDBusMessage::createMethodCall(m_busConnection.baseService(),183 QDBusMessage::createMethodCall(m_busConnection.baseService(),
156 "/", "my.interface", "hi");184 "/", "my.interface", "hi");
157 bool allowed = m_acm->isPeerAllowedToAccess(m_busConnection, msg,185 bool allowed = m_acm->isPeerAllowedToAccess(m_busConnection, msg,
158 "anyContext");186 "anyContext");
159 /* At the moment, AppArmor doesn't implement the
160 * GetConnectionAppArmorSecurityContext method, so expect an error. */
161 QVERIFY(!allowed);187 QVERIFY(!allowed);
162}188}
163189
164void ExtensionTest::test_accessWildcard()190void ExtensionTest::test_accessWildcard()
165{191{
166 QSKIP("Disable because of QTBUG-36475");192 m_fakeDBus.setCredentials(":0.1", {
167193 { "LinuxSecurityLabel", QByteArray("com.ubuntu.myapp_myapp_0.2 (enforce)") },
194 });
168 /* forge a QDBusMessage */195 /* forge a QDBusMessage */
169 QDBusMessage msg =196 QDBusMessage msg =
170 QDBusMessage::createMethodCall(m_busConnection.baseService(),197 QDBusMessage::createMethodCall(m_busConnection.baseService(),
171198
=== modified file 'tests/tst_extension.pro'
--- tests/tst_extension.pro 2013-09-04 06:55:40 +0000
+++ tests/tst_extension.pro 2016-10-04 07:11:40 +0000
@@ -14,15 +14,21 @@
14QT -= gui14QT -= gui
1515
16PKGCONFIG += \16PKGCONFIG += \
17 SignOnExtension17 SignOnExtension \
18 libqtdbusmock-1 \
19 libqtdbustest-1
1820
19DEFINES += \21DEFINES += \
22 DBUS_MOCK_TEMPLATE=\\\"$${PWD}/dbus_apparmor.py\\\" \
20 PLUGIN_PATH=\\\"../src/libsignon-apparmor-ac.so\\\"23 PLUGIN_PATH=\\\"../src/libsignon-apparmor-ac.so\\\"
2124
22SOURCES = \25SOURCES = \
23 tst_extension.cpp26 tst_extension.cpp
2427
25check.commands = "LD_PRELOAD=mock/libapparmor.so dbus-test-runner -t ./$${TARGET}"28HEADERS += \
29 fake_dbus.h
30
31check.commands = "LD_PRELOAD=mock/libfakeapparmor.so ./$${TARGET}"
26check.depends = $${TARGET}32check.depends = $${TARGET}
27QMAKE_EXTRA_TARGETS += check33QMAKE_EXTRA_TARGETS += check
2834

Subscribers

People subscribed via source and target branches