Merge lp:~thomas-voss/trust-store/do-not-prompt-or-cache-unconfined into lp:trust-store

[Thomas Voß]

Add an agent implementation that allows for selectively whitelisting certain apps.

Add an agent implementation that allows for selectively whitelisting certain apps.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:58
http://jenkins.qa.ubuntu.com/job/trust-store-ci/77/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/trust-store-utopic-amd64-ci/77/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/trust-store-utopic-armhf-ci/77/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/trust-store-utopic-i386-ci/77/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/trust-store-ci/77/rebuild

review: Needs Fixing (continuous-integration)

[Pete Woods (pete-woods)]

Looks good at a reasonably superficial level, has tests, etc

review: Approve

[lp:~thomas-voss/trust-store/do-not-prompt-or-cache-unconfined]

59. By Thomas Voß on 2014-10-15

Add new symbols for core::trust::WhiteListingAgent.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:59
http://jenkins.qa.ubuntu.com/job/trust-store-ci/78/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/trust-store-utopic-amd64-ci/78
    SUCCESS: http://jenkins.qa.ubuntu.com/job/trust-store-utopic-armhf-ci/78
        deb: http://jenkins.qa.ubuntu.com/job/trust-store-utopic-armhf-ci/78/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/trust-store-utopic-i386-ci/78

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/trust-store-ci/78/rebuild

review: Approve (continuous-integration)

[Alberto Aguirre (albaguirre)]

LGTM

review: Approve

[Jamie Strandboge (jdstrand)]

Reviewing the code, I approve of the overall concept-- unconfined apps should not get a trust store prompt and this should achieve that for all trust-store consumers.

It is less clear from this MP how different trusted helpers will be able to precache other apps. Eg, perhaps location service wants to say that the Canonical camera app should not prompt. We wouldn't necessarily want to hardcode camera-app in the same way as unconfined, because it wouldn't show up in System Settings and the camera-app would be allowed to talk to any trust-store consumer without prompting. For precaching non-unconfined apps, would trusted helpers reimplement their own WhiteListingAgent? (Note, this is not a NAK-- Thomas mentioned that non-unconfined would be handled in another MP).

review: Approve

=== modified file 'debian/libtrust-store1.symbols'
--- debian/libtrust-store1.symbols	2014-10-08 11:27:14 +0000
+++ debian/libtrust-store1.symbols	2014-10-15 17:39:09 +0000
@@ -1,4 +1,11 @@
 libtrust-store.so.1 libtrust-store1 #MINVER#
+ (c++)"core::trust::WhiteListingAgent::always_grant_for_unconfined()@Base" 0replaceme
+ (c++)"core::trust::WhiteListingAgent::authenticate_request_with_parameters(core::trust::Agent::RequestParameters const&)@Base" 0replaceme
+ (c++)"core::trust::WhiteListingAgent::WhiteListingAgent(std::function<bool (core::trust::Agent::RequestParameters const&)>, std::shared_ptr<core::trust::Agent> const&)@Base" 0replaceme
+ (c++)"core::trust::WhiteListingAgent::~WhiteListingAgent()@Base" 0replaceme
+ (c++)"typeinfo for core::trust::WhiteListingAgent@Base" 0replaceme
+ (c++)"typeinfo name for core::trust::WhiteListingAgent@Base" 0replaceme
+ (c++)"vtable for core::trust::WhiteListingAgent@Base" 0replaceme
  (c++)"core::trust::i18n::default_text_domain()@Base" 1.1.0+14.10.20141008.1
  (c++)"core::trust::i18n::service_text_domain()@Base" 1.1.0+14.10.20141008.1
  (c++)"core::trust::i18n::set_service_text_domain(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)@Base" 1.1.0+14.10.20141008.1

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2014-09-19 21:18:18 +0000
+++ src/CMakeLists.txt	2014-10-15 17:39:09 +0000
@@ -43,6 +43,8 @@
   core/trust/app_id_formatting_trust_agent.h
   core/trust/app_id_formatting_trust_agent.cpp
 
+  # An agent-implementation that allows for selectively whitelisting app ids
+  core/trust/white_listing_agent.cpp
   # An agent-implementation using a store instance to cache user replies.
   core/trust/cached_agent.cpp
   core/trust/cached_agent_glog_reporter.cpp

=== modified file 'src/core/trust/daemon.cpp'
--- src/core/trust/daemon.cpp	2014-10-07 19:27:18 +0000
+++ src/core/trust/daemon.cpp	2014-10-15 17:39:09 +0000
@@ -23,6 +23,7 @@
 #include <core/trust/expose.h>
 #include <core/trust/i18n.h>
 #include <core/trust/store.h>
+#include <core/trust/white_listing_agent.h>
 
 #include <core/trust/mir_agent.h>
 
@@ -365,13 +366,16 @@
                     core::trust::CachedAgentGlogReporter::Configuration{})
         });
     auto formatting_agent = std::make_shared<core::trust::AppIdFormattingTrustAgent>(cached_agent);
+    auto whitelisting_agent = std::make_shared<core::trust::WhiteListingAgent>(
+                core::trust::WhiteListingAgent::always_grant_for_unconfined(),
+                formatting_agent);
     auto remote_agent = remote_agent_factory(service_name, formatting_agent, dict);
 
     return core::trust::Daemon::Skeleton::Configuration
     {
         service_name,
         bus_from_name(vm[Parameters::StoreBus::name].as<std::string>()),
-        {local_store, formatting_agent},
+        {local_store, whitelisting_agent},
         {remote_agent}
     };
 }

=== added file 'src/core/trust/white_listing_agent.cpp'
--- src/core/trust/white_listing_agent.cpp	1970-01-01 00:00:00 +0000
+++ src/core/trust/white_listing_agent.cpp	2014-10-15 17:39:09 +0000
@@ -0,0 +1,47 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * 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 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 program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Thomas Voß <[e-mail]>
+ */
+
+#include <core/trust/white_listing_agent.h>
+
+core::trust::WhiteListingAgent::WhiteListingPredicate core::trust::WhiteListingAgent::always_grant_for_unconfined()
+{
+    return [](const core::trust::Agent::RequestParameters& params) -> bool
+    {
+        return params.application.id == "unconfined";
+    };
+}
+
+core::trust::WhiteListingAgent::WhiteListingAgent(
+        core::trust::WhiteListingAgent::WhiteListingPredicate white_listing_predicate,
+        const std::shared_ptr<core::trust::Agent>& impl)
+    : white_listing_predicate{white_listing_predicate},
+      impl{impl}
+{
+    if (not impl) throw std::runtime_error
+    {
+        "Missing agent implementation."
+    };
+}
+
+core::trust::Request::Answer core::trust::WhiteListingAgent::authenticate_request_with_parameters(const core::trust::Agent::RequestParameters& parameters)
+{
+    if (white_listing_predicate(parameters))
+        return core::trust::Request::Answer::granted;
+
+    return impl->authenticate_request_with_parameters(parameters);
+}

=== added file 'src/core/trust/white_listing_agent.h'
--- src/core/trust/white_listing_agent.h	1970-01-01 00:00:00 +0000
+++ src/core/trust/white_listing_agent.h	2014-10-15 17:39:09 +0000
@@ -0,0 +1,52 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * 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 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 program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Thomas Voß <[e-mail]>
+ */
+
+#ifndef CORE_TRUST_WHITE_LISTING_AGENT_H_
+#define CORE_TRUST_WHITE_LISTING_AGENT_H_
+
+#include <core/trust/agent.h>
+
+namespace core
+{
+namespace trust
+{
+// An agent implementation that allows for selectively whitelisting app ids.
+class CORE_TRUST_DLL_PUBLIC WhiteListingAgent : public core::trust::Agent
+{
+public:
+    // A functor that is evaluated for every incoming requests.
+    // If it returns true, the request is immediately granted, otherwise passed on
+    // to the next agent.
+    typedef std::function<bool(const RequestParameters&)> WhiteListingPredicate;
+
+    // Returns a predicate that returns true iff the app id is 'unconfined'.
+    static WhiteListingPredicate always_grant_for_unconfined();
+
+    WhiteListingAgent(WhiteListingPredicate white_listing_predicate, const std::shared_ptr<Agent>& impl);
+
+    // From core::trust::Agent
+    Request::Answer authenticate_request_with_parameters(const RequestParameters& parameters) override;
+
+private:
+    WhiteListingPredicate white_listing_predicate;
+    std::shared_ptr<Agent> impl;
+};
+}
+}
+
+#endif // CORE_TRUST_WHITE_LISTING_AGENT_H_

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2014-08-19 14:11:30 +0000
+++ tests/CMakeLists.txt	2014-10-15 17:39:09 +0000
@@ -49,6 +49,11 @@
 )
 
 add_executable(
+  white_listing_agent_test
+  white_listing_agent_test.cpp
+)
+
+add_executable(
   cached_agent_test
   cached_agent_test.cpp
 )
@@ -132,6 +137,19 @@
 )
 
 target_link_libraries(
+  white_listing_agent_test
+
+  trust-store
+
+  gmock
+
+  gtest
+  gtest_main
+
+  ${PROCESS_CPP_LIBRARIES}
+)
+
+target_link_libraries(
   cached_agent_test
 
   trust-store
@@ -190,6 +208,7 @@
 add_test(remote_agent_test ${CMAKE_CURRENT_BINARY_DIR}/remote_agent_test)
 add_test(app_id_formatting_trust_agent_test ${CMAKE_CURRENT_BINARY_DIR}/app_id_formatting_trust_agent_test)
 add_test(cached_agent_test ${CMAKE_CURRENT_BINARY_DIR}/cached_agent_test)
+add_test(white_listing_agent_test ${CMAKE_CURRENT_BINARY_DIR}/white_listing_agent_test)
 # TODO(tvoss) Re-enable daemon tests once CI issues are resolved.
 # add_test(daemon_test ${CMAKE_CURRENT_BINARY_DIR}/daemon_test)
 add_test(dbus_test ${CMAKE_CURRENT_BINARY_DIR}/dbus_test)

=== added file 'tests/white_listing_agent_test.cpp'
--- tests/white_listing_agent_test.cpp	1970-01-01 00:00:00 +0000
+++ tests/white_listing_agent_test.cpp	2014-10-15 17:39:09 +0000
@@ -0,0 +1,115 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * 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 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 program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Thomas Voß <[e-mail]>
+ */
+
+#include <core/trust/white_listing_agent.h>
+
+#include "mock_agent.h"
+#include "the.h"
+
+#include <gmock/gmock.h>
+
+namespace
+{
+std::shared_ptr<testing::NiceMock<MockAgent>> a_mocked_agent()
+{
+    return std::make_shared<testing::NiceMock<MockAgent>>();
+}
+
+struct MockWhiteListingPredicate
+{
+    core::trust::WhiteListingAgent::WhiteListingPredicate to_functional()
+    {
+        return [this](const core::trust::Agent::RequestParameters& params)
+        {
+            return is_whitelisted(params);
+        };
+    }
+
+    MOCK_METHOD1(is_whitelisted, bool(const core::trust::Agent::RequestParameters&));
+};
+}
+
+TEST(WhiteListingAgent, ctor_throws_for_null_agent)
+{
+    EXPECT_ANY_THROW(core::trust::WhiteListingAgent
+    (
+        core::trust::WhiteListingAgent::always_grant_for_unconfined(),
+        std::shared_ptr<core::trust::Agent>()
+    ));
+}
+
+TEST(WhiteListingAgent, invokes_predicate_for_incoming_request_and_dispatches_to_impl_for_non_whitelisted)
+{
+    using namespace ::testing;
+
+    auto mock_agent = a_mocked_agent();
+
+    auto params = the::default_request_parameters_for_testing();
+    params.application.id = params.application.id + std::string{"_app"} + std::string{"_1.2.3"};
+
+    MockWhiteListingPredicate wlp;
+    EXPECT_CALL(wlp, is_whitelisted(params))
+            .Times(1)
+            .WillRepeatedly(Return(false));
+
+    EXPECT_CALL(*mock_agent, authenticate_request_with_parameters(params))
+            .Times(1)
+            .WillRepeatedly(Return(core::trust::Request::Answer::denied));
+
+    core::trust::WhiteListingAgent agent{wlp.to_functional(), mock_agent};
+
+    EXPECT_EQ(core::trust::Request::Answer::denied,
+              agent.authenticate_request_with_parameters(params));
+}
+
+TEST(WhiteListingAgent, invokes_predicate_for_incoming_request_and_returns_immediately_for_non_whitelisted)
+{
+    using namespace ::testing;
+
+    auto mock_agent = a_mocked_agent();
+
+    auto params = the::default_request_parameters_for_testing();
+    params.application.id = params.application.id + std::string{"_app"} + std::string{"_1.2.3"};
+
+    MockWhiteListingPredicate wlp;
+    EXPECT_CALL(wlp, is_whitelisted(params))
+            .Times(1)
+            .WillRepeatedly(Return(true));
+
+    EXPECT_CALL(*mock_agent, authenticate_request_with_parameters(params))
+            .Times(0);
+
+    core::trust::WhiteListingAgent agent{wlp.to_functional(), mock_agent};
+
+    EXPECT_EQ(core::trust::Request::Answer::granted,
+              agent.authenticate_request_with_parameters(params));
+}
+
+TEST(WhiteListingAgent, unconfined_predicate_only_returns_true_for_unconfined)
+{
+    using namespace ::testing;
+
+    auto predicate = core::trust::WhiteListingAgent::always_grant_for_unconfined();
+
+    auto params = the::default_request_parameters_for_testing();
+    params.application.id = params.application.id + std::string{"_app"} + std::string{"_1.2.3"};
+
+    EXPECT_FALSE(predicate(params));
+    params.application.id = "unconfined";
+    EXPECT_TRUE(predicate(params));
+}