Package: lomiri-download-manager
Severity: normal
Tags: patch
User: ubuntu-de...@lists.ubuntu.com
Usertags: origin-ubuntu noble ubuntu-patch

Dear Maintainer,

During runtime, lomiri-download-manager still tries to consume the
GetConnectionAppArmorSecurityContext DBus method, which was added in
Ubuntu around 10 years ago and never made it upstream. Instead, upstream
DBus added another way to access the AppArmor security context through
GetConnectionCredentials [1].

The GetConnectionAppArmorSecurityContext method does not exist anymore
in Debian ; so there is no point trying to use it.

I also submitted the patch upstream [2].

In Ubuntu, the attached patch was applied to achieve the following:

  * Replace deprecated calls to GetConnectionAppArmorSecurityContext by calls
    to GetConnectionCredentials (LP: #1489489).


Thanks for considering the patch.

[1] 
https://dbus.freedesktop.org/doc/dbus-specification.html#bus-messages-get-connection-credentials
[2] 
https://gitlab.com/ubports/development/core/lomiri-download-manager/-/merge_requests/24

-- System Information:
Debian Release: trixie/sid
  APT prefers mantic-updates
  APT policy: (500, 'mantic-updates'), (500, 'mantic-security'), (500, 'mantic')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 6.1.0-16-generic (SMP w/8 CPU threads; PREEMPT)
Kernel taint flags: TAINT_PROPRIETARY_MODULE, TAINT_OOT_MODULE
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled
diff -Nru 
lomiri-download-manager-0.1.2/debian/patches/1001-drop-deprecated-GetConnectionAppArmorSecurityContext.patch
 
lomiri-download-manager-0.1.2/debian/patches/1001-drop-deprecated-GetConnectionAppArmorSecurityContext.patch
--- 
lomiri-download-manager-0.1.2/debian/patches/1001-drop-deprecated-GetConnectionAppArmorSecurityContext.patch
        1970-01-01 01:00:00.000000000 +0100
+++ 
lomiri-download-manager-0.1.2/debian/patches/1001-drop-deprecated-GetConnectionAppArmorSecurityContext.patch
        2023-12-05 10:20:56.000000000 +0100
@@ -0,0 +1,386 @@
+Description: Drop deprecated calls to GetConnectionAppArmorSecurityContext
+ Around 10 years ago, we introduced GetConnectionAppArmorSecurityContext() in
+ DBus in Ubuntu. Upstream pushed back and instead exposed the AppArmor's
+ context in org.freedesktop.DBus.GetConnectionCredentials().
+ So far, we have maintained GetconnectionAppArmorSecurityContext() as a delta
+ in Ubuntu so that existing software do not break.
+ However, the recommended way (and the only portable way) is to use
+ GetConnectionCredentials() instead. Furthermore, it is about time we drop
+ support for the former in Ubuntu.
+ .
+ The patch is inspired from 4c4d7961261b41ac41f24c8ce75563ab12f6b74c from the
+ https://gitlab.com/ubports/development/core/lomiri-thumbnailer/ repository.
+Author: Olivier Gayot <olivier.ga...@canonical.com>
+Bug-Ubuntu: https://launchpad.net/bugs/1489489
+Forwarded: 
https://gitlab.com/ubports/development/core/lomiri-download-manager/-/merge_requests/24
+Last-Update: 2023-12-05
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+--- a/src/common/priv/lomiri/transfers/system/apparmor.cpp
++++ b/src/common/priv/lomiri/transfers/system/apparmor.cpp
+@@ -19,6 +19,7 @@
+ #include <errno.h>
+ #include <lomiri/util/Dbus.h>
+ #include <lomiri/util/SnapPath.h>
++#include <assert.h>
+ #include <sys/types.h>
+ #include <unistd.h>
+ #include <QDBusConnection>
+@@ -37,6 +38,8 @@
+ namespace System {
+ 
+ QString AppArmor::UNCONFINED_ID = "unconfined";
++QString AppArmor::LINUX_SECURITY_LABEL = "LinuxSecurityLabel";
++
+ 
+ AppArmor::AppArmor(DBusConnection* connection,
+                 QObject* parent)
+@@ -68,15 +71,38 @@
+ }
+ 
+ QString
++AppArmor::labelFromCredentials(const QVariantMap &map)
++{
++    // The contents of this map are described in the specification here:
++    // 
http://dbus.freedesktop.org/doc/dbus-specification.html#bus-messages-get-connection-credentials
++    QByteArray label = map.value(LINUX_SECURITY_LABEL).value<QByteArray>();
++
++    if (label.size() == 0) {
++        return "";
++    }
++
++    // The label is null terminated.
++    assert(label[label.size()-1] == '\0');
++    label.truncate(label.size() - 1);
++    // Trim the mode off the end of the label.
++    int pos = label.lastIndexOf(' ');
++    if (pos > 0 && label.endsWith(')') && label[pos+1] == '(')
++    {
++        label.truncate(pos);  // LCOV_EXCL_LINE
++    }
++    return QString::fromUtf8(label.constData(), label.size());
++}
++
++QString
+ AppArmor::appId(QString caller) {
+-    QScopedPointer<PendingReply<QString> > reply(
+-        _dbus->GetConnectionAppArmorSecurityContext(caller));
++    QScopedPointer<PendingReply<QVariantMap> > reply(
++        _dbus->GetConnectionCredentials(caller));
+     // blocking but should be ok for now
+     reply->waitForFinished();
+     if (reply->isError()) {
+         return "";
+     }
+-    return reply->value();
++    return labelFromCredentials(reply->value());
+ }
+ 
+ bool
+@@ -97,8 +123,8 @@
+         return;
+     }
+ 
+-    QScopedPointer<PendingReply<QString> > reply (
+-        _dbus->GetConnectionAppArmorSecurityContext(connName));
++    QScopedPointer<PendingReply<QVariantMap> > reply(
++        _dbus->GetConnectionCredentials(connName));
+     // blocking but should be ok for now
+     reply->waitForFinished();
+     if (reply->isError()) {
+@@ -109,7 +135,7 @@
+         return;
+     } else {
+         // use the returned value
+-        details->appId = reply->value();
++        details->appId = labelFromCredentials(reply->value());
+ 
+         if (details->appId.isEmpty() || details->appId == UNCONFINED_ID) {
+             LOG(INFO) << "UNCONFINED APP";
+--- a/src/common/priv/lomiri/transfers/system/apparmor.h
++++ b/src/common/priv/lomiri/transfers/system/apparmor.h
+@@ -60,11 +60,13 @@
+     virtual bool isConfined(QString appId);
+ 
+     static QString UNCONFINED_ID;
++    static QString LINUX_SECURITY_LABEL;
+ 
+  private:
+     void getSecurityDetails(const QString& connName,
+                             SecurityDetails* details);
+     QString getLocalPath(const QString& appId);
++    QString labelFromCredentials(const QVariantMap &map);
+ 
+  private:
+     const char* BASE_ACCOUNT_URL = "/com/lomiri/applications/%1";
+--- a/src/common/priv/lomiri/transfers/system/dbus_proxy.h
++++ b/src/common/priv/lomiri/transfers/system/dbus_proxy.h
+@@ -54,14 +54,14 @@
+         return 
asyncCallWithArgumentList(QLatin1String("GetAdtAuditSessionData"), 
argumentList);
+     }
+ 
+-    virtual PendingReply<QString>* GetConnectionAppArmorSecurityContext(const 
QString &in0)
++    virtual PendingReply<QVariantMap>* GetConnectionCredentials(const QString 
&in0)
+     {
+         QList<QVariant> argumentList;
+         argumentList << QVariant::fromValue(in0);
+-        QDBusPendingReply<QString> reply = asyncCallWithArgumentList(
+-            QLatin1String("GetConnectionAppArmorSecurityContext"),
+-            argumentList);
+-        auto wrapper = new PendingReply<QString>(reply);
++        QDBusPendingReply<QVariantMap> reply = asyncCallWithArgumentList(
++           QLatin1String("GetConnectionCredentials"),
++           argumentList);
++        auto wrapper = new PendingReply<QVariantMap>(reply);
+         return wrapper;
+     }
+ 
+--- a/src/common/priv/lomiri/transfers/system/org.freedesktop.DBus.xml
++++ b/src/common/priv/lomiri/transfers/system/org.freedesktop.DBus.xml
+@@ -63,9 +63,9 @@
+       <arg direction="in" type="s"/>
+       <arg direction="out" type="ay"/>
+     </method>
+-    <method name="GetConnectionAppArmorSecurityContext">
++    <method name="GetConnectionCredentials">
+       <arg direction="in" type="s"/>
+-      <arg direction="out" type="s"/>
++      <arg direction="out" type="a{sv}"/>
+     </method>
+     <method name="ReloadConfig">
+     </method>
+--- a/tests/downloads/daemon/dbus_proxy.h
++++ b/tests/downloads/daemon/dbus_proxy.h
+@@ -37,8 +37,8 @@
+     MOCK_METHOD1(AddMatch, QDBusPendingReply<>(const QString&));
+     MOCK_METHOD1(GetAdtAuditSessionData,
+         QDBusPendingReply<QByteArray>(const QString&));
+-    MOCK_METHOD1(GetConnectionAppArmorSecurityContext,
+-        PendingReply<QString>*(const QString&));
++    MOCK_METHOD1(GetConnectionCredentials,
++        PendingReply<QVariantMap>*(const QString&));
+     MOCK_METHOD1(GetConnectionSELinuxSecurityContext,
+         QDBusPendingReply<QByteArray>(const QString&));
+     MOCK_METHOD1(GetConnectionUnixProcessID,
+--- a/tests/downloads/daemon/test_apparmor.cpp
++++ b/tests/downloads/daemon/test_apparmor.cpp
+@@ -48,13 +48,13 @@
+     QString caller = "my app";
+     auto dbusProxy = new MockDBusProxy();
+     auto conn = new MockDBusConnection();
+-    auto reply = new MockPendingReply<QString>();
++    auto reply = new MockPendingReply<QVariantMap>();
+ 
+     EXPECT_CALL(*_dbusProxyFactory, createDBusProxy(conn, _))
+         .Times(1)
+         .WillOnce(Return(dbusProxy));
+ 
+-    EXPECT_CALL(*dbusProxy, GetConnectionAppArmorSecurityContext(caller))
++    EXPECT_CALL(*dbusProxy, GetConnectionCredentials(caller))
+         .Times(1)
+         .WillOnce(Return(reply));
+ 
+@@ -78,15 +78,18 @@
+ TestAppArmor::testAppId() {
+     QString caller = "my app";
+     QString expectedAppId = "APPID";
++    QVariantMap credentials = {
++        {AppArmor::LINUX_SECURITY_LABEL, QByteArray("APPID (enforce)") + 
'\0'},
++    };
+     auto dbusProxy = new MockDBusProxy();
+     auto conn = new MockDBusConnection();
+-    auto reply = new MockPendingReply<QString>();
++    auto reply = new MockPendingReply<QVariantMap>();
+ 
+     EXPECT_CALL(*_dbusProxyFactory, createDBusProxy(conn, _))
+         .Times(1)
+         .WillOnce(Return(dbusProxy));
+ 
+-    EXPECT_CALL(*dbusProxy, GetConnectionAppArmorSecurityContext(caller))
++    EXPECT_CALL(*dbusProxy, GetConnectionCredentials(caller))
+         .Times(1)
+         .WillOnce(Return(reply));
+ 
+@@ -99,7 +102,7 @@
+ 
+     EXPECT_CALL(*reply, value())
+         .Times(1)
+-        .WillOnce(Return(expectedAppId));
++        .WillOnce(Return(credentials));
+ 
+     QScopedPointer<AppArmor> appArmor(new AppArmor(conn));
+ 
+--- a/tests/downloads/daemon/test_download_manager.cpp
++++ b/tests/downloads/daemon/test_download_manager.cpp
+@@ -145,13 +145,16 @@
+     SignalBarrier spy(_man, SIGNAL(downloadCreated(QDBusObjectPath)));
+     DownloadStruct downStruct(url, metadata, headers);
+ 
++    QVariantMap credentials = {
++        {AppArmor::LINUX_SECURITY_LABEL, QByteArray("unconfined") + '\0'},
++    };
+     auto dbusProxy = new MockDBusProxy();
+-    auto reply = new MockPendingReply<QString>();
++    auto reply = new MockPendingReply<QVariantMap>();
+     EXPECT_CALL(*_dbusProxyFactory, createDBusProxy(_conn, _))
+             .Times(1)
+             .WillOnce(Return(dbusProxy));
+ 
+-    EXPECT_CALL(*dbusProxy, GetConnectionAppArmorSecurityContext(_))
++    EXPECT_CALL(*dbusProxy, GetConnectionCredentials(_))
+             .Times(1)
+             .WillOnce(Return(reply));
+ 
+@@ -164,7 +167,7 @@
+ 
+     EXPECT_CALL(*reply, value())
+             .Times(1)
+-            .WillOnce(Return("TEST_APP_ID"));
++            .WillOnce(Return(credentials));
+ 
+     // set the expectations of the factory since is the one that
+     // creates the downloads. The matchers will ensure that the
+@@ -277,13 +280,16 @@
+             .Times(1)
+             .WillRepeatedly(Return(down.data()));
+ 
++    QVariantMap credentials = {
++        {AppArmor::LINUX_SECURITY_LABEL, QByteArray("TEST_APP_ID (enforce)") 
+ '\0'},
++    };
+     auto dbusProxy = new MockDBusProxy();
+-    auto reply = new MockPendingReply<QString>();
++    auto reply = new MockPendingReply<QVariantMap>();
+     EXPECT_CALL(*_dbusProxyFactory, createDBusProxy(_conn, _))
+             .Times(1)
+             .WillOnce(Return(dbusProxy));
+ 
+-    EXPECT_CALL(*dbusProxy, GetConnectionAppArmorSecurityContext(_))
++    EXPECT_CALL(*dbusProxy, GetConnectionCredentials(_))
+             .Times(1)
+             .WillOnce(Return(reply));
+ 
+@@ -296,7 +302,7 @@
+ 
+     EXPECT_CALL(*reply, value())
+             .Times(1)
+-            .WillOnce(Return("TEST_APP_ID"));
++            .WillOnce(Return(credentials));
+ 
+     // expected actions to be performed on the download
+     EXPECT_CALL(*down.data(), setThrottle(_man->defaultThrottle()))
+@@ -508,14 +514,17 @@
+ void
+ TestDownloadManager::testGetAllDownloadsUnconfined() {
+     QString expectedAppId = "unconfined";
++    QVariantMap credentials = {
++        {AppArmor::LINUX_SECURITY_LABEL, QByteArray("unconfined") + '\0'},
++    };
+     auto dbusProxy = new MockDBusProxy();
+-    auto reply = new MockPendingReply<QString>();
++    auto reply = new MockPendingReply<QVariantMap>();
+ 
+     EXPECT_CALL(*_dbusProxyFactory, createDBusProxy(_conn, _))
+         .Times(1)
+         .WillOnce(Return(dbusProxy));
+ 
+-    EXPECT_CALL(*dbusProxy, GetConnectionAppArmorSecurityContext(_))
++    EXPECT_CALL(*dbusProxy, GetConnectionCredentials(_))
+         .Times(1)
+         .WillOnce(Return(reply));
+ 
+@@ -528,7 +537,7 @@
+ 
+     EXPECT_CALL(*reply, value())
+         .Times(1)
+-        .WillOnce(Return(expectedAppId));
++        .WillOnce(Return(credentials));
+ 
+     QStringList expectedPaths;
+     expectedPaths.append("/first/path/object");
+@@ -552,14 +561,17 @@
+ void
+ TestDownloadManager::testGetAllDownloadsConfined() {
+     QString expectedAppId = "APPID";
++    QVariantMap credentials = {
++        {AppArmor::LINUX_SECURITY_LABEL, QByteArray("APPID (enforce)") + 
'\0'},
++    };
+     auto dbusProxy = new MockDBusProxy();
+-    auto reply = new MockPendingReply<QString>();
++    auto reply = new MockPendingReply<QVariantMap>();
+ 
+     EXPECT_CALL(*_dbusProxyFactory, createDBusProxy(_conn, _))
+         .Times(1)
+         .WillOnce(Return(dbusProxy));
+ 
+-    EXPECT_CALL(*dbusProxy, GetConnectionAppArmorSecurityContext(_))
++    EXPECT_CALL(*dbusProxy, GetConnectionCredentials(_))
+         .Times(1)
+         .WillOnce(Return(reply));
+ 
+@@ -572,7 +584,7 @@
+ 
+     EXPECT_CALL(*reply, value())
+         .Times(1)
+-        .WillOnce(Return(expectedAppId));
++        .WillOnce(Return(credentials));
+ 
+     // create several downloads with diff app ids to check that we
+     // do getonly those with the correct app id
+@@ -621,14 +633,17 @@
+ void
+ TestDownloadManager::testAllDownloadsWithMetadataUnconfined() {
+     QString expectedAppId = "unconfined";
++    QVariantMap credentials = {
++        {AppArmor::LINUX_SECURITY_LABEL, QByteArray("unconfined") + '\0'},
++    };
+     auto dbusProxy = new MockDBusProxy();
+-    auto reply = new MockPendingReply<QString>();
++    auto reply = new MockPendingReply<QVariantMap>();
+ 
+     EXPECT_CALL(*_dbusProxyFactory, createDBusProxy(_conn, _))
+         .Times(1)
+         .WillOnce(Return(dbusProxy));
+ 
+-    EXPECT_CALL(*dbusProxy, GetConnectionAppArmorSecurityContext(_))
++    EXPECT_CALL(*dbusProxy, GetConnectionCredentials(_))
+         .Times(1)
+         .WillOnce(Return(reply));
+ 
+@@ -641,7 +656,7 @@
+ 
+     EXPECT_CALL(*reply, value())
+         .Times(1)
+-        .WillOnce(Return(expectedAppId));
++        .WillOnce(Return(credentials));
+ 
+     // create downloads to be used to filter
+     auto key = QString("filter");
+@@ -700,14 +715,17 @@
+ void
+ TestDownloadManager::testAllDownloadsWithMetadataConfined() {
+     QString expectedAppId = "APPID";
++    QVariantMap credentials = {
++        {AppArmor::LINUX_SECURITY_LABEL, QByteArray("APPID (enforce)") + 
'\0'},
++    };
+     auto dbusProxy = new MockDBusProxy();
+-    auto reply = new MockPendingReply<QString>();
++    auto reply = new MockPendingReply<QVariantMap>();
+ 
+     EXPECT_CALL(*_dbusProxyFactory, createDBusProxy(_conn, _))
+         .Times(1)
+         .WillOnce(Return(dbusProxy));
+ 
+-    EXPECT_CALL(*dbusProxy, GetConnectionAppArmorSecurityContext(_))
++    EXPECT_CALL(*dbusProxy, GetConnectionCredentials(_))
+         .Times(1)
+         .WillOnce(Return(reply));
+ 
+@@ -720,7 +738,7 @@
+ 
+     EXPECT_CALL(*reply, value())
+         .Times(1)
+-        .WillOnce(Return(expectedAppId));
++        .WillOnce(Return(credentials));
+ 
+     auto key = QString("filter");
+     auto value = QString("coconut");
diff -Nru lomiri-download-manager-0.1.2/debian/patches/series 
lomiri-download-manager-0.1.2/debian/patches/series
--- lomiri-download-manager-0.1.2/debian/patches/series 2023-07-27 
01:44:02.000000000 +0200
+++ lomiri-download-manager-0.1.2/debian/patches/series 2023-12-05 
10:14:03.000000000 +0100
@@ -1,2 +1,3 @@
 2001_build-without-Werror.patch
 0001-upgrade-cpp-standard-to-cpp17.patch
+1001-drop-deprecated-GetConnectionAppArmorSecurityContext.patch

Reply via email to