Merge lp:~mardy/signon-apparmor-extension/new-signond into lp:signon-apparmor-extension

[Alberto Mardegan]

Update to new signon API, add support for p2p connections

Dependency changes: latest signond, added build deps on dbus-1 and apparmor.

Update to new signon API, add support for p2p connections

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:7
http://jenkins.qa.ubuntu.com/job/signon-apparmor-extension-ci/4/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/signon-apparmor-extension-saucy-amd64-ci/4/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/signon-apparmor-extension-ci/4/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:7
http://jenkins.qa.ubuntu.com/job/signon-apparmor-extension-ci/5/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/signon-apparmor-extension-saucy-amd64-ci/5/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/signon-apparmor-extension-ci/5/rebuild

review: Needs Fixing (continuous-integration)

[lp:~mardy/signon-apparmor-extension/new-signond]

8. By Alberto Mardegan on 2013-09-04

Tests: mock libapparmor and QDBusConnection::call()

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:8
http://jenkins.qa.ubuntu.com/job/signon-apparmor-extension-ci/6/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/signon-apparmor-extension-saucy-amd64-ci/6

Click here to trigger a rebuild:
http://s-jenkins:8080/job/signon-apparmor-extension-ci/6/rebuild

review: Approve (continuous-integration)

[Ken VanDine (ken-vandine)]

Approve

review: Approve

=== modified file '.bzrignore'
--- .bzrignore	2013-08-23 14:35:22 +0000
+++ .bzrignore	2013-09-04 06:56:12 +0000
@@ -4,6 +4,7 @@
 *.substvars
 /debian/files
 /debian/signon-apparmor-extension/
-Makefile
+Makefile*
 moc_*
+/tests/mock/libapparmor.so*
 /tests/tst_extension

=== modified file 'debian/changelog'
--- debian/changelog	2013-08-06 12:17:11 +0000
+++ debian/changelog	2013-09-04 06:56:12 +0000
@@ -1,5 +1,6 @@
-signon-apparmor-extension (0.0-0ubuntu1) UNRELEASED; urgency=low
+signon-apparmor-extension (0.1-0ubuntu0) UNRELEASED; urgency=low
 
   * Initial implementation.
+  * Support p2p connections.
 
- -- Alberto Mardegan <[e-mail]>  Tue, 06 Aug 2013 14:52:28 +0300
+ -- Alberto Mardegan <[e-mail]>  Tue, 27 Aug 2013 15:31:46 +0300

=== modified file 'debian/control'
--- debian/control	2013-08-13 14:39:53 +0000
+++ debian/control	2013-09-04 06:56:12 +0000
@@ -5,7 +5,10 @@
                pkg-config,
                qt5-default,
                qt5-qmake,
-               signond-dev,
+               libapparmor-dev,
+               libdbus-1-dev,
+               signond-dev (>= 8.53),
+               dbus-test-runner,
 Standards-Version: 3.9.4
 Section: libs
 Homepage: https://launchpad.net/signon-apparmor-extension

=== modified file 'src/access-control-manager.cpp'
--- src/access-control-manager.cpp	2013-08-07 11:41:08 +0000
+++ src/access-control-manager.cpp	2013-09-04 06:56:12 +0000
@@ -24,9 +24,19 @@
 #include <QDBusConnection>
 #include <QDBusMessage>
 #include <QDebug>
+#include <dbus/dbus.h>
+#include <sys/apparmor.h>
 
 static const char keychainAppId[] = "SignondKeychain";
 
+AccessReply::AccessReply(const SignOn::AccessRequest &request,
+                         QObject *parent):
+    SignOn::AccessReply(request, parent)
+{
+    /* Always decline */
+    QMetaObject::invokeMethod(this, "decline", Qt::QueuedConnection);
+}
+
 AccessControlManager::AccessControlManager(QObject *parent):
     SignOn::AbstractAccessControlManager(parent)
 {
@@ -41,10 +51,12 @@
     return QLatin1String(keychainAppId);
 }
 
-bool AccessControlManager::isPeerAllowedToAccess(const QDBusMessage &peerMessage,
-                                                 const QString &securityContext)
+bool AccessControlManager::isPeerAllowedToAccess(
+                                       const QDBusConnection &peerConnection,
+                                       const QDBusMessage &peerMessage,
+                                       const QString &securityContext)
 {
-    QString appId = appIdOfPeer(peerMessage);
+    QString appId = appIdOfPeer(peerConnection, peerMessage);
 
     bool allowed = (appId == securityContext ||
                     securityContext == QLatin1String("*"));
@@ -53,27 +65,57 @@
     return allowed;
 }
 
-QString AccessControlManager::appIdOfPeer(const QDBusMessage &peerMessage)
+QString AccessControlManager::appIdOfPeer(const QDBusConnection &peerConnection,
+                                          const QDBusMessage &peerMessage)
 {
     QString uniqueConnectionId = peerMessage.service();
     QString appId;
 
-    QDBusMessage msg =
-        QDBusMessage::createMethodCall("org.freedesktop.DBus",
-                                       "/org/freedesktop/DBus",
-                                       "org.freedesktop.DBus",
-                                       "GetConnectionAppArmorSecurityContext");
-    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();
-        qDebug() << "App ID:" << appId;
+    if (uniqueConnectionId.isEmpty()) {
+        /* it's a p2p connection; get the fd of the socket, and ask apparmor to
+         * identify the peer. */
+        DBusConnection *connection =
+            (DBusConnection *)peerConnection.internalPointer();
+        int fd = 0;
+        dbus_bool_t ok = dbus_connection_get_unix_fd(connection, &fd);
+        if (Q_LIKELY(ok)) {
+            char *con = NULL, *mode = NULL;
+            int ret = aa_getpeercon(fd, &con, &mode);
+            if (Q_LIKELY(ret >= 0)) {
+                appId = QString::fromUtf8(con);
+                qDebug() << "App ID:" << appId;
+                free(con);
+                if (mode) free(mode);
+            } else {
+                qWarning() << "Couldn't get apparmor profile of peer";
+            }
+        } else {
+            qWarning() << "Couldn't get fd of caller!";
+        }
     } else {
-        qWarning() << "Error getting app ID:" << reply.errorName() <<
-            reply.errorMessage();
+        QDBusMessage msg =
+            QDBusMessage::createMethodCall("org.freedesktop.DBus",
+                                           "/org/freedesktop/DBus",
+                                           "org.freedesktop.DBus",
+                                           "GetConnectionAppArmorSecurityContext");
+        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();
+            qDebug() << "App ID:" << appId;
+        } else {
+            qWarning() << "Error getting app ID:" << reply.errorName() <<
+                reply.errorMessage();
+        }
     }
     return appId;
 }
+
+SignOn::AccessReply *
+AccessControlManager::handleRequest(const SignOn::AccessRequest &request)
+{
+    return new AccessReply(request, this);
+}

=== modified file 'src/access-control-manager.h'
--- src/access-control-manager.h	2013-08-06 11:24:08 +0000
+++ src/access-control-manager.h	2013-09-04 06:56:12 +0000
@@ -24,6 +24,14 @@
 
 #include <SignOn/AbstractAccessControlManager>
 
+class AccessReply: public SignOn::AccessReply
+{
+    Q_OBJECT
+
+public:
+    AccessReply(const SignOn::AccessRequest &request, QObject *parent = 0);
+};
+
 class AccessControlManager: public SignOn::AbstractAccessControlManager
 {
     Q_OBJECT
@@ -32,29 +40,17 @@
     AccessControlManager(QObject *parent = 0);
     ~AccessControlManager();
 
-    /*!
-     * Checks if a client process is allowed to access objects with a certain
-     * security context.
-     * The access type to be checked is read or execute.
-     * @param peerMessage, the request message sent over DBUS by the process.
-     * @param securityContext, the securityContext to be checked against.
-     * @returns true, if the peer is allowed, false otherwise.
-     */
-    bool isPeerAllowedToAccess(const QDBusMessage &peerMessage,
+    bool isPeerAllowedToAccess(const QDBusConnection &peerConnection,
+                               const QDBusMessage &peerMessage,
                                const QString &securityContext) Q_DECL_OVERRIDE;
 
-    /*!
-     * Looks up for the application identifier of a specific client process.
-     * @param peerMessage, the request message sent over DBUS by the process.
-     * @returns the application identifier of the process, or an empty string
-     * if none found.
-     */
-    QString appIdOfPeer(const QDBusMessage &peerMessage) Q_DECL_OVERRIDE;
+    QString appIdOfPeer(const QDBusConnection &peerConnection,
+                        const QDBusMessage &peerMessage) Q_DECL_OVERRIDE;
 
-    /*!
-     * @returns the application identifier of the keychain widget
-     */
     QString keychainWidgetAppId() Q_DECL_OVERRIDE;
+
+    SignOn::AccessReply *handleRequest(const SignOn::AccessRequest &request)
+        Q_DECL_OVERRIDE;
 };
 
 #endif // SIGNON_APPARMOR_ACCESS_CONTROL_MANAGER_H

=== modified file 'src/src.pro'
--- src/src.pro	2013-08-06 12:17:11 +0000
+++ src/src.pro	2013-09-04 06:56:12 +0000
@@ -19,7 +19,9 @@
     -fvisibility=hidden
 
 PKGCONFIG += \
-    SignOnExtension
+    SignOnExtension \
+    dbus-1 \
+    libapparmor
 
 # The following use of pkg-config + sed is a hack to workaround a qmake
 # limitation: the CFLAGS variable is not used with the moc compiler, so the

=== added directory 'tests/mock'
=== added file 'tests/mock/apparmor.cpp'
--- tests/mock/apparmor.cpp	1970-01-01 00:00:00 +0000
+++ tests/mock/apparmor.cpp	2013-09-04 06:56:12 +0000
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2013 Canonical Ltd.
+ *
+ * Contact: Alberto Mardegan <[e-mail]>
+ *
+ * This file is part of the signon-apparmor-extension
+ *
+ * This library 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.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY 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 library.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <QDBusConnection>
+#include <QDBusMessage>
+#include <QDebug>
+#include <sys/apparmor.h>
+
+#define UNCONFINED_PROFILE "unconfined"
+
+int aa_getpeercon(int fd, char **con, char **mode)
+{
+    Q_UNUSED(fd);
+    Q_UNUSED(mode);
+    *con = strdup(UNCONFINED_PROFILE);
+    return 0;
+}
+
+QDBusMessage QDBusConnection::call(const QDBusMessage &message,
+                                   QDBus::CallMode mode,
+                                   int timeout) const
+{
+    Q_UNUSED(mode);
+    Q_UNUSED(timeout);
+    return message.createReply(QVariantList() << QStringLiteral(UNCONFINED_PROFILE));
+}

=== added file 'tests/mock/mock.pro'
--- tests/mock/mock.pro	1970-01-01 00:00:00 +0000
+++ tests/mock/mock.pro	2013-09-04 06:56:12 +0000
@@ -0,0 +1,22 @@
+include(../../common-project-config.pri)
+
+TARGET = apparmor
+TEMPLATE = lib
+
+CONFIG += \
+    debug \
+    link_pkgconfig \
+    qt
+
+QT += \
+    core \
+    dbus
+
+PKGCONFIG += \
+    libapparmor
+
+# Error on undefined symbols
+QMAKE_LFLAGS += $$QMAKE_LFLAGS_NOUNDEF
+
+SOURCES += \
+    apparmor.cpp

=== added file 'tests/tests.pro'
--- tests/tests.pro	1970-01-01 00:00:00 +0000
+++ tests/tests.pro	2013-09-04 06:56:12 +0000
@@ -0,0 +1,5 @@
+TEMPLATE = subdirs
+CONFIG += ordered
+SUBDIRS = \
+    mock \
+    tst_extension.pro

=== modified file 'tests/tst_extension.cpp'
--- tests/tst_extension.cpp	2013-08-07 11:41:08 +0000
+++ tests/tst_extension.cpp	2013-09-04 06:56:12 +0000
@@ -19,28 +19,44 @@
  * along with this library.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <QDBusConnection>
 #include <QDBusMessage>
+#include <QDBusServer>
 #include <QDebug>
 #include <QPluginLoader>
 #include <QTest>
 
 #include <SignOn/AbstractAccessControlManager>
 
+#define P2P_SOCKET "unix:path=/tmp/tst_extension_%1"
+
 class ExtensionTest: public QObject
 {
     Q_OBJECT
 
+public:
+    ExtensionTest();
+
 private Q_SLOTS:
     void initTestCase();
     void test_appId();
+    void test_appId_p2p();
     void test_access();
     void test_accessWildcard();
     void cleanupTestCase();
 
 private:
     SignOn::AbstractAccessControlManager *m_acm;
+    QDBusConnection m_busConnection;
+    QDBusConnection m_p2pConnection;
 };
 
+ExtensionTest::ExtensionTest():
+    m_busConnection(QDBusConnection::sessionBus()),
+    m_p2pConnection(QStringLiteral("uninitialized"))
+{
+}
+
 void ExtensionTest::initTestCase()
 {
     QPluginLoader pluginLoader(PLUGIN_PATH);
@@ -53,25 +69,55 @@
 
     m_acm = interface->accessControlManager(this);
     QVERIFY(m_acm != 0);
+
+    QDBusServer *server;
+    for (int i = 0; i < 10; i++) {
+        server = new QDBusServer(QString::fromLatin1(P2P_SOCKET).arg(i), this);
+        if (!server->isConnected()) {
+            delete server;
+        } else {
+            break;
+        }
+    }
+
+    QVERIFY(server->isConnected());
+
+    m_p2pConnection = QDBusConnection::connectToPeer(server->address(),
+                                                     QStringLiteral("tst"));
+    QVERIFY(m_p2pConnection.isConnected());
 }
 
 void ExtensionTest::test_appId()
 {
     /* forge a QDBusMessage */
     QDBusMessage msg =
-        QDBusMessage::createMethodCall(":0.3", "/", "my.interface", "hi");
-    QString appId = m_acm->appIdOfPeer(msg);
-    /* At the moment, AppArmor doesn't implement the
-     * GetConnectionAppArmorSecurityContext method, so expect an error. */
-    QCOMPARE(appId, QString());
+        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"));
+}
+
+void ExtensionTest::test_appId_p2p()
+{
+    /* forge a QDBusMessage */
+    QDBusMessage msg =
+        QDBusMessage::createMethodCall("", "/", "my.interface", "hi");
+    QString appId = m_acm->appIdOfPeer(m_p2pConnection, msg);
+    /* At the moment, AppArmor doesn't implement the
+     * GetConnectionAppArmorSecurityContext method, so expect an error. */
+    QCOMPARE(appId, QStringLiteral("unconfined"));
 }
 
 void ExtensionTest::test_access()
 {
     /* forge a QDBusMessage */
     QDBusMessage msg =
-        QDBusMessage::createMethodCall(":0.3", "/", "my.interface", "hi");
-    bool allowed = m_acm->isPeerAllowedToAccess(msg, "anyContext");
+        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);
@@ -81,8 +127,9 @@
 {
     /* forge a QDBusMessage */
     QDBusMessage msg =
-        QDBusMessage::createMethodCall(":0.3", "/", "my.interface", "hi");
-    bool allowed = m_acm->isPeerAllowedToAccess(msg, "*");
+        QDBusMessage::createMethodCall(m_busConnection.baseService(),
+                                       "/", "my.interface", "hi");
+    bool allowed = m_acm->isPeerAllowedToAccess(m_busConnection, msg, "*");
     /* Everything is allowed to access "*" */
     QVERIFY(allowed);
 }

=== renamed file 'tests/tests.pro' => 'tests/tst_extension.pro'
--- tests/tests.pro	2013-08-07 11:41:08 +0000
+++ tests/tst_extension.pro	2013-09-04 06:56:12 +0000
@@ -22,7 +22,7 @@
 SOURCES = \
     tst_extension.cpp
 
-check.commands = "./$${TARGET}"
+check.commands = "LD_PRELOAD=mock/libapparmor.so dbus-test-runner -t ./$${TARGET}"
 check.depends = $${TARGET}
 QMAKE_EXTRA_TARGETS += check