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

[Alberto Mardegan]

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

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

[lp:~mardy/signon-apparmor-extension/apparmor-1489489]

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)]

a few comments and questions

review: Needs Information

[lp:~mardy/signon-apparmor-extension/apparmor-1489489]

31. By Alberto Mardegan on 2016-10-04

Address review comments

[Alexandre Abreu (abreu-alexandre)]

LGTM

review: Approve

=== 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 @@
+TOP_SRC_DIR = $$PWD
+TOP_BUILD_DIR = $$shadowed($$PWD)
 
=== 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 @@
 # Common configuration for all projects.
 #-----------------------------------------------------------------------------
 
+CONFIG += c++11
+
 # we don't like warnings...
 QMAKE_CXXFLAGS += -Werror
 # Disable RTTI
 QMAKE_CXXFLAGS += -fno-exceptions -fno-rtti
 
-TOP_SRC_DIR = $$PWD
-TOP_BUILD_DIR = $${TOP_SRC_DIR}/$(BUILD_DIR)
+!defined(TOP_SRC_DIR, var) {
+    TOP_SRC_DIR = $$PWD
+    TOP_BUILD_DIR = $${TOP_SRC_DIR}/$(BUILD_DIR)
+}
 
 include(coverage.pri)
 
=== 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 @@
 Source: signon-apparmor-extension
 Priority: optional
 Maintainer: Ubuntu Desktop Team <[e-mail]>
-Build-Depends: dbus-test-runner,
-               debhelper (>= 9),
-               libapparmor-dev,
+Build-Depends: debhelper (>= 9),
+               libapparmor-dev (>= 2.9.1-0ubuntu9.1),
                libdbus-1-dev,
+               libqtdbusmock1-dev,
+               libqtdbustest1-dev,
                pkg-config,
                qt5-default,
                qt5-qmake,
 
=== 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 @@
 #include "access-control-manager.h"
 
 #include <QDBusConnection>
+#include <QDBusError>
 #include <QDBusMessage>
+#include <QDBusReply>
 #include <QDebug>
 #include <QStringList>
 #include <sys/apparmor.h>
@@ -38,7 +40,9 @@
 }
 
 AccessControlManager::AccessControlManager(QObject *parent):
-    SignOn::AbstractAccessControlManager(parent)
+    SignOn::AbstractAccessControlManager(parent),
+    m_dbusService(qEnvironmentVariableIsSet("SIGNON_APPARMOR_TEST") ?
+                  "fake.freedesktop.DBus" : "org.freedesktop.DBus")
 {
 }
 
@@ -85,21 +89,26 @@
         appId = "unconfined";
     } else {
         QDBusMessage msg =
-            QDBusMessage::createMethodCall("org.freedesktop.DBus",
+            QDBusMessage::createMethodCall(m_dbusService,
                                            "/org/freedesktop/DBus",
                                            "org.freedesktop.DBus",
-                                           "GetConnectionAppArmorSecurityContext");
+                                           "GetConnectionCredentials");
         QVariantList args;
         args << uniqueConnectionId;
         msg.setArguments(args);
-        QDBusMessage reply =
-            QDBusConnection::sessionBus().call(msg, QDBus::Block);
-        if (reply.type() == QDBusMessage::ReplyMessage) {
-            appId = reply.arguments().value(0, QString()).toString();
+        QDBusReply<QVariantMap> reply = QDBusConnection::sessionBus().call(msg, QDBus::Block);
+        if (reply.isValid()) {
+            QVariantMap map = reply.value();
+            QByteArray context = map.value("LinuxSecurityLabel").toByteArray();
+            if (!context.isEmpty()) {
+                aa_splitcon(context.data(), NULL);
+                appId = QString::fromUtf8(context);
+            }
             qDebug() << "App ID:" << appId;
         } else {
-            qWarning() << "Error getting app ID:" << reply.errorName() <<
-                reply.errorMessage();
+            QDBusError error = reply.error();
+            qWarning() << "Error getting app ID:" << error.name() <<
+                error.message();
         }
     }
     return appId;
 
=== 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 @@
 
 private:
     QString stripVersion(const QString &appId) const;
+
+private:
+    QString m_dbusService;
 };
 
 #endif // SIGNON_APPARMOR_ACCESS_CONTROL_MANAGER_H
 
=== 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 @@
+'''dbus mock template
+
+This creates the expected methods and properties of the
+org.freedesktop.DBus service.
+'''
+
+# This program is free software; you can redistribute it and/or modify it under
+# the terms of the GNU Lesser General Public License as published by the Free
+# Software Foundation; either version 3 of the License, or (at your option) any
+# later version.  See http://www.gnu.org/copyleft/lgpl.html for the full text
+# of the license.
+
+__author__ = 'Alberto Mardegan'
+__email__ = '[e-mail]'
+__copyright__ = '(c) 2016 Canonical Ltd.'
+__license__ = 'LGPL 3+'
+
+import dbus
+import time
+
+from dbusmock import MOCK_IFACE
+import dbusmock
+
+BUS_NAME = 'fake.freedesktop.DBus'
+MAIN_OBJ = '/org/freedesktop/DBus'
+MAIN_IFACE = 'org.freedesktop.DBus'
+SYSTEM_BUS = False
+
+ERROR_PREFIX = 'org.freedesktop.DBus.Error.'
+ERROR_NAME_HAS_NO_OWNER = ERROR_PREFIX + 'NameHasNoOwner'
+
+def get_credentials(self, service):
+    if service not in self.credentials:
+        raise dbus.exceptions.DBusException('Service not found',
+                                            name=ERROR_NAME_HAS_NO_OWNER)
+    return self.credentials[service]
+
+
+def load(mock, parameters):
+    mock.get_credentials = get_credentials
+    mock.AddMethods(MAIN_IFACE, [
+        ('GetConnectionCredentials', 's', 'a{sv}', 'ret = self.get_credentials(self, args[0])'),
+    ])
+
+    mock.credentials = {}
+
+
[e-mail](MOCK_IFACE, in_signature='sa{sv}', out_signature='')
+def SetCredentials(self, service, credentials):
+    self.credentials[service] = credentials
 
=== 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 @@
+/*
+ * This file is part of signon-apparmor-extension
+ *
+ * Copyright (C) 2016 Canonical Ltd.
+ *
+ * Contact: Alberto Mardegan <[e-mail]>
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef AP_FAKE_DBUS_H
+#define AP_FAKE_DBUS_H
+
+#include <QVariantMap>
+#include <libqtdbusmock/DBusMock.h>
+
+class FakeDBus
+{
+public:
+    FakeDBus(QtDBusMock::DBusMock *mock): m_mock(mock) {
+        m_mock->registerTemplate("fake.freedesktop.DBus",
+                                 DBUS_MOCK_TEMPLATE,
+                                 QDBusConnection::SessionBus);
+    }
+
+    void setCredentials(const QString &service, const QVariantMap &credentials) {
+        mockedDBusService().call("SetCredentials", service, credentials);
+    }
+
+private:
+    OrgFreedesktopDBusMockInterface &mockedDBusService() {
+        return m_mock->mockInterface("fake.freedesktop.DBus",
+                                     "/org/freedesktop/DBus",
+                                     "org.freedesktop.DBus",
+                                     QDBusConnection::SessionBus);
+    }
+
+private:
+    QtDBusMock::DBusMock *m_mock;
+};
+
+#endif // AP_FAKE_DBUS_H
 
=== 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 @@
  */
 
 #include <QByteArray>
-#include <QDBusConnection>
-#include <QDBusMessage>
 #include <QDebug>
 #include <sys/apparmor.h>
 
@@ -40,12 +38,3 @@
     *con = strdup(mockedProfile().constData());
     return 0;
 }
-
-QDBusMessage QDBusConnection::call(const QDBusMessage &message,
-                                   QDBus::CallMode mode,
-                                   int timeout) const
-{
-    Q_UNUSED(mode);
-    Q_UNUSED(timeout);
-    return message.createReply(QVariantList() << mockedProfile().constData());
-}
 
=== 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 @@
 include(../../common-project-config.pri)
 
-TARGET = apparmor
+TARGET = fakeapparmor
 TEMPLATE = lib
 
 CONFIG += \
@@ -9,8 +9,7 @@
     qt
 
 QT += \
-    core \
-    dbus
+    core
 
 PKGCONFIG += \
     libapparmor
 
=== 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 @@
 /*
- * Copyright (C) 2013 Canonical Ltd.
+ * Copyright (C) 2016 Canonical Ltd.
  *
  * Contact: Alberto Mardegan <[e-mail]>
  *
@@ -19,6 +19,8 @@
  * along with this library.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "fake_dbus.h"
+
 #include <QDBusConnection>
 #include <QDBusMessage>
 #include <QDBusServer>
@@ -28,8 +30,13 @@
 
 #include <SignOn/AbstractAccessControlManager>
 
+#include <libqtdbusmock/DBusMock.h>
+
+
 #define P2P_SOCKET "unix:path=/tmp/tst_extension_%1"
 
+using namespace QtDBusMock;
+
 static void setMockedProfile(const char *profile)
 {
     if (profile) {
@@ -47,6 +54,13 @@
 {
     Q_OBJECT
 
+    struct SetupEnvironment {
+        /* The test is run with a LD_PRELOAD in order to mock aa_getpeercon();
+         * but we don't want this to propagate to our children: dbus-daemon in
+         * particular won't like it. */
+        SetupEnvironment() { qunsetenv("LD_PRELOAD"); }
+    };
+
 public:
     ExtensionTest();
 
@@ -62,18 +76,29 @@
 
 private:
     SignOn::AbstractAccessControlManager *m_acm;
+    SetupEnvironment m_env;
+    QtDBusTest::DBusTestRunner m_dbus;
+    QtDBusMock::DBusMock m_mock;
+    FakeDBus m_fakeDBus;
     QDBusConnection m_busConnection;
     QDBusConnection m_p2pConnection;
 };
 
 ExtensionTest::ExtensionTest():
-    m_busConnection(QDBusConnection::sessionBus()),
+    m_mock(m_dbus),
+    m_fakeDBus(&m_mock),
+    m_busConnection(m_dbus.sessionConnection()),
     m_p2pConnection(QStringLiteral("uninitialized"))
 {
+    DBusMock::registerMetaTypes();
 }
 
 void ExtensionTest::initTestCase()
 {
+    m_dbus.startServices();
+
+    qputenv("SIGNON_APPARMOR_TEST", "1");
+
     QPluginLoader pluginLoader(PLUGIN_PATH);
     QObject *plugin = pluginLoader.instance();
     QVERIFY(plugin != 0);
@@ -104,15 +129,15 @@
 
 void ExtensionTest::test_appId()
 {
-    QSKIP("Disable because of QTBUG-36475");
+    m_fakeDBus.setCredentials(m_busConnection.baseService(), {
+        { "LinuxSecurityLabel", QByteArray("unconfined") },
+    });
 
     /* forge a QDBusMessage */
     QDBusMessage msg =
         QDBusMessage::createMethodCall(m_busConnection.baseService(),
                                        "/", "my.interface", "hi");
     QString appId = m_acm->appIdOfPeer(m_busConnection, msg);
-    /* At the moment, AppArmor doesn't implement the
-     * GetConnectionAppArmorSecurityContext method, so expect an error. */
     QCOMPARE(appId, QStringLiteral("unconfined"));
 }
 
@@ -127,6 +152,9 @@
 
 void ExtensionTest::test_click_version()
 {
+    m_fakeDBus.setCredentials(":0.1", {
+        { "LinuxSecurityLabel", QByteArray("com.ubuntu.myapp_myapp_0.2 (enforce)") },
+    });
     /* forge a QDBusMessage */
     setMockedProfile("com.ubuntu.myapp_myapp_0.2");
     QDBusMessage msg =
@@ -148,23 +176,22 @@
 
 void ExtensionTest::test_access()
 {
-    QSKIP("Disable because of QTBUG-36475");
-
+    m_fakeDBus.setCredentials(m_busConnection.baseService(), {
+    });
     /* forge a QDBusMessage */
     QDBusMessage msg =
         QDBusMessage::createMethodCall(m_busConnection.baseService(),
                                        "/", "my.interface", "hi");
     bool allowed = m_acm->isPeerAllowedToAccess(m_busConnection, msg,
                                                 "anyContext");
-    /* At the moment, AppArmor doesn't implement the
-     * GetConnectionAppArmorSecurityContext method, so expect an error. */
     QVERIFY(!allowed);
 }
 
 void ExtensionTest::test_accessWildcard()
 {
-    QSKIP("Disable because of QTBUG-36475");
-
+    m_fakeDBus.setCredentials(":0.1", {
+        { "LinuxSecurityLabel", QByteArray("com.ubuntu.myapp_myapp_0.2 (enforce)") },
+    });
     /* forge a QDBusMessage */
     QDBusMessage msg =
         QDBusMessage::createMethodCall(m_busConnection.baseService(),
 
=== 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 @@
 QT -= gui
 
 PKGCONFIG += \
-    SignOnExtension
+    SignOnExtension \
+    libqtdbusmock-1 \
+    libqtdbustest-1
 
 DEFINES += \
+    DBUS_MOCK_TEMPLATE=\\\"$${PWD}/dbus_apparmor.py\\\" \
     PLUGIN_PATH=\\\"../src/libsignon-apparmor-ac.so\\\"
 
 SOURCES = \
     tst_extension.cpp
 
-check.commands = "LD_PRELOAD=mock/libapparmor.so dbus-test-runner -t ./$${TARGET}"
+HEADERS += \
+    fake_dbus.h
+
+check.commands = "LD_PRELOAD=mock/libfakeapparmor.so ./$${TARGET}"
 check.depends = $${TARGET}
 QMAKE_EXTRA_TARGETS += check