Merge lp:~phablet-team/media-hub/fix-1533779 into lp:media-hub

[Jim Hodapp]

Make sure that an apparmor profile_name of the format com.ubuntu.my-app is supported as well as improve the error handling and reporting for when a client does not have the proper apparmor permissions to play a given media URI

Make sure that an apparmor profile_name of the format com.ubuntu.my-app is supported as well as improve the error handling and reporting for when a client does not have the proper apparmor permissions to play a given media URI

[lp:~phablet-team/media-hub/fix-1533779]

166. By Jim Hodapp on 2016-01-14

Also raise an error if insufficient apparmor permissions for open_uri extended

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:165
http://jenkins.qa.ubuntu.com/job/media-hub-ci/386/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/226
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/226
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/226/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/226

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/386/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:166
http://jenkins.qa.ubuntu.com/job/media-hub-ci/387/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/227
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/227
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/227/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/227

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/387/rebuild

review: Approve (continuous-integration)

[Bill Filler (bfiller)]

Tested this and functionality it's all working with silo 52. Able to record/play audio with messaging-app and playback video.

review: Approve

[Thomas Voß (thomas-voss)]

As per our discussion on IRC: http://pastebin.ubuntu.com/14499825/

review: Needs Fixing

[lp:~phablet-team/media-hub/fix-1533779]

167. By Jim Hodapp on 2016-01-15

Tighten up the messaging-app apparmor permission check

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:167
http://jenkins.qa.ubuntu.com/job/media-hub-ci/388/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/228
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/228
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/228/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/228

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/388/rebuild

review: Approve (continuous-integration)

[lp:~phablet-team/media-hub/fix-1533779]

168. By Jim Hodapp on 2016-01-15

Remove app_id_ since it isn't used

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:168
http://jenkins.qa.ubuntu.com/job/media-hub-ci/389/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-amd64-ci/229
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/229
        deb: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-armhf-ci/229/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/media-hub-vivid-i386-ci/229

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/media-hub-ci/389/rebuild

review: Approve (continuous-integration)

[Thomas Voß (thomas-voss)]

LGTM.

review: Approve

=== modified file 'include/core/media/player.h'
--- include/core/media/player.h	2015-04-17 16:37:02 +0000
+++ include/core/media/player.h	2016-01-15 15:44:13 +0000
@@ -56,6 +56,11 @@
         {
             OutOfProcessBufferStreamingNotSupported();
         };
+
+        struct InsufficientAppArmorPermissions : public std::runtime_error
+        {
+            InsufficientAppArmorPermissions(const std::string& err);
+        };
     };
 
     struct Configuration;

=== modified file 'src/core/media/apparmor/ubuntu.cpp'
--- src/core/media/apparmor/ubuntu.cpp	2015-09-03 07:23:16 +0000
+++ src/core/media/apparmor/ubuntu.cpp	2016-01-15 15:44:13 +0000
@@ -75,17 +75,26 @@
 
 // Returns true if the context name is a valid Ubuntu app id.
 // If it is, out is populated with the package and app name.
-bool process_context_name(const std::string& s, std::smatch& out)
+bool process_context_name(const std::string& s, std::smatch& out,
+        std::string& pkg_name)
 {
     // See https://wiki.ubuntu.com/AppStore/Interfaces/ApplicationId.
     static const std::regex short_re{"(.*)_(.*)"};
     static const std::regex full_re{"(.*)_(.*)_(.*)"};
-
-    if (std::regex_match(s, out, full_re))
-        return true;
-
-    if (std::regex_match(s, out, short_re))
-        return true;
+    static const std::regex trust_store_re{"(.*)-(.*)"};
+
+    if (s == "messaging-app"
+            and std::regex_match(s, out, trust_store_re))
+    {
+        pkg_name = s;
+        return true;
+    }
+
+    if (std::regex_match(s, out, full_re) or std::regex_match(s, out, short_re))
+    {
+        pkg_name = out[index_package];
+        return true;
+    }
 
     return false;
 }
@@ -94,8 +103,11 @@
 apparmor::ubuntu::Context::Context(const std::string& name)
     : apparmor::Context{name},
       unconfined_{str() == ubuntu::unconfined},
-      has_package_name_{process_context_name(str(), match_)}
+      has_package_name_{process_context_name(str(), match_, pkg_name_)}
 {
+    std::cout << "apparmor profile name: " << name;
+    std::cout << ", is_unconfined(): " << is_unconfined();
+    std::cout << ", has_package_name(): " << has_package_name() << std::endl;
     if (not is_unconfined() && not has_package_name()) throw std::logic_error
     {
         "apparmor::ubuntu::Context: Invalid profile name " + str()
@@ -112,10 +124,14 @@
     return has_package_name_;
 }
 
-
 std::string apparmor::ubuntu::Context::package_name() const
 {
-    return std::string{match_[index_package]};
+    return pkg_name_;
+}
+
+std::string apparmor::ubuntu::Context::profile_name() const
+{
+    return std::string{match_[index_package]} + "-" + std::string{match_[index_app]};
 }
 
 apparmor::ubuntu::DBusDaemonRequestContextResolver::DBusDaemonRequestContextResolver(const core::dbus::Bus::Ptr& bus) : dbus_daemon{bus}
@@ -149,6 +165,18 @@
             "Client can access content in ~/.local/share/" + context.package_name() + " or ~/.cache/" + context.package_name()
         };
     }
+    // Check for trust-store compatible path name using full messaging-app profile_name
+    else if (context.profile_name() == "messaging-app" &&
+             /* Since the full APP_ID is not available yet (see aa_query_file_path()), add an exception: */
+             (parsed_uri.path.find(std::string(".local/share/com.ubuntu." + context.profile_name() + "/")) != std::string::npos ||
+             parsed_uri.path.find(std::string(".cache/com.ubuntu." + context.profile_name() + "/")) != std::string::npos))
+    {
+        return Result
+        {
+            true,
+            "Client can access content in ~/.local/share/" + context.profile_name() + " or ~/.cache/" + context.profile_name()
+        };
+    }
     else if (parsed_uri.path.find(std::string("opt/click.ubuntu.com/")) != std::string::npos &&
              parsed_uri.path.find(context.package_name()) != std::string::npos)
     {

=== modified file 'src/core/media/apparmor/ubuntu.h'
--- src/core/media/apparmor/ubuntu.h	2014-12-15 14:05:29 +0000
+++ src/core/media/apparmor/ubuntu.h	2016-01-15 15:44:13 +0000
@@ -73,8 +73,11 @@
     // Returns the package name or throws if no package name can be found.
     virtual std::string package_name() const;
 
+    virtual std::string profile_name() const;
+
 private:
     std::smatch match_;
+    std::string pkg_name_;
     const bool unconfined_;
     const bool has_package_name_;
 };

=== modified file 'src/core/media/mpris/player.h'
--- src/core/media/mpris/player.h	2015-09-29 11:07:54 +0000
+++ src/core/media/mpris/player.h	2016-01-15 15:44:13 +0000
@@ -116,6 +116,14 @@
                 "mpris.Player.Error.OutOfProcessBufferStreamingNotSupported"
             };
         };
+
+        struct InsufficientAppArmorPermissions
+        {
+            static constexpr const char* name
+            {
+                "mpris.Player.Error.InsufficientAppArmorPermissions"
+            };
+        };
     };
 
     typedef std::map<std::string, core::dbus::types::Variant> Dictionary;

=== modified file 'src/core/media/player.cpp'
--- src/core/media/player.cpp	2015-04-14 02:39:32 +0000
+++ src/core/media/player.cpp	2016-01-15 15:44:13 +0000
@@ -27,6 +27,12 @@
 {
 }
 
+media::Player::Errors::InsufficientAppArmorPermissions::InsufficientAppArmorPermissions
+    (const std::string &e)
+    : std::runtime_error{e}
+{
+}
+
 const media::Player::Configuration& media::Player::Client::default_configuration()
 {
     static const media::Player::Configuration config

=== modified file 'src/core/media/player_skeleton.cpp'
--- src/core/media/player_skeleton.cpp	2015-04-20 17:48:48 +0000
+++ src/core/media/player_skeleton.cpp	2016-01-15 15:44:13 +0000
@@ -179,11 +179,23 @@
             Track::UriType uri;
             in->reader() >> uri;
 
+            auto reply = dbus::Message::make_method_return(in);
             // Make sure the client has adequate apparmor permissions to open the URI
-            auto result = request_authenticator->authenticate_open_uri_request(context, uri);
-
-            auto reply = dbus::Message::make_method_return(in);
-            reply->writer() << (std::get<0>(result) ? impl->open_uri(uri) : false);
+            const auto result = request_authenticator->authenticate_open_uri_request(context, uri);
+            if (std::get<0>(result))
+            {
+                reply->writer() << (std::get<0>(result) ? impl->open_uri(uri) : false);
+            }
+            else
+            {
+                const std::string err_str = {"Warning: Failed to authenticate necessary "
+                    "apparmor permissions to open uri: " + std::get<1>(result)};
+                std::cerr << err_str << std::endl;
+                reply = dbus::Message::make_error(
+                            in,
+                            mpris::Player::Error::InsufficientAppArmorPermissions::name,
+                            err_str);
+            }
 
             bus->send(reply);
         });
@@ -198,9 +210,23 @@
 
             in->reader() >> uri >> headers;
 
-            auto result = request_authenticator->authenticate_open_uri_request(context, uri);
             auto reply = dbus::Message::make_method_return(in);
-            reply->writer() << (std::get<0>(result) ? impl->open_uri(uri, headers) : false);
+            // Make sure the client has adequate apparmor permissions to open the URI
+            const auto result = request_authenticator->authenticate_open_uri_request(context, uri);
+            if (std::get<0>(result))
+            {
+                reply->writer() << (std::get<0>(result) ? impl->open_uri(uri, headers) : false);
+            }
+            else
+            {
+                const std::string err_str = {"Warning: Failed to authenticate necessary "
+                    "apparmor permissions to open uri: " + std::get<1>(result)};
+                std::cerr << err_str << std::endl;
+                reply = dbus::Message::make_error(
+                            in,
+                            mpris::Player::Error::InsufficientAppArmorPermissions::name,
+                            err_str);
+            }
 
             bus->send(reply);
         });

=== modified file 'src/core/media/player_stub.cpp'
--- src/core/media/player_stub.cpp	2015-05-01 19:44:11 +0000
+++ src/core/media/player_stub.cpp	2016-01-15 15:44:13 +0000
@@ -261,7 +261,14 @@
 
 bool media::PlayerStub::open_uri(const media::Track::UriType& uri)
 {
-    auto op = d->object->transact_method<mpris::Player::OpenUri, bool>(uri);
+    const auto op = d->object->transact_method<mpris::Player::OpenUri, bool>(uri);
+    if (op.is_error())
+    {
+        if (op.error().name() == mpris::Player::Error::InsufficientAppArmorPermissions::name)
+            throw media::Player::Errors::InsufficientAppArmorPermissions{op.error().print()};
+        else
+            throw std::runtime_error{op.error().print()};
+    }
 
     return op.value();
 }
@@ -269,7 +276,14 @@
 
 bool media::PlayerStub::open_uri(const Track::UriType& uri, const Player::HeadersType& headers)
 {
-    auto op = d->object->transact_method<mpris::Player::OpenUriExtended, bool>(uri, headers);
+    const auto op = d->object->transact_method<mpris::Player::OpenUriExtended, bool>(uri, headers);
+    if (op.is_error())
+    {
+        if (op.error().name() == mpris::Player::Error::InsufficientAppArmorPermissions::name)
+            throw media::Player::Errors::InsufficientAppArmorPermissions{op.error().print()};
+        else
+            throw std::runtime_error{op.error().print()};
+    }
 
     return op.value();
 }

=== modified file 'src/core/media/service_implementation.cpp'
--- src/core/media/service_implementation.cpp	2015-12-17 18:53:47 +0000
+++ src/core/media/service_implementation.cpp	2016-01-15 15:44:13 +0000
@@ -255,7 +255,7 @@
   return std::shared_ptr<media::Player>();
 }
 
-void media::ServiceImplementation::set_current_player(Player::PlayerKey key)
+void media::ServiceImplementation::set_current_player(Player::PlayerKey)
 {
   // no impl
 }