mir/agent: fix indefinite wait if fail to start prompt session

For some reason, Mir still provide an FD for a prompt session even if it
fails to start. When this FD is passed to the prompt provider, it will
just hang, causing the whole trust store user chain to just wait. To fix
this, make sure the agent checks for the prompt session's state before
continue.

Interestingly, the only way to know the session's state is through the
state callback. This cause a bit of a problem in the test as now we have
to call the state callback in the mock. Thus, a custom ACTION is written
and tests are changed to call it.

Related: ubports/ubuntu-touch#1668

src/core/trust/mir/agent.cpp

@@ -56,6 +56,11 @@ mir::PromptSessionVirtualTable::PromptSessionVirtualTable()
 {
 }
 
+std::string mir::PromptSessionVirtualTable::error_message()
+{
+    return std::string(mir_prompt_session_error_message(prompt_session));
+}
+
 int mir::PromptSessionVirtualTable::new_fd_for_prompt_provider()
 {
     static const unsigned int fd_count = 1;
@@ -166,14 +171,16 @@ void mir::Agent::on_trust_session_changed_state(
         // The context of type context.
         void* context)
 {
-    if (mir_prompt_session_state_stopped != state)
-        return;
-
     auto ctxt = static_cast<mir::Agent::OnTrustSessionStateChangedCallbackContext*>(context);
 
     if (not ctxt)
         return;
 
+    ctxt->state = state;
+
+    if (mir_prompt_session_state_stopped != state)
+        return;
+
     std::error_code ec;
     // If the trust session ended (for whatever reason), we send a SIG_KILL to the
     // prompt provider process. We hereby ensure that we never return Answer::granted
@@ -216,6 +223,7 @@ core::trust::Request::Answer mir::Agent::authenticate_request_with_parameters(co
     // instance here.
     mir::Agent::OnTrustSessionStateChangedCallbackContext cb_context
     {
+        mir_prompt_session_state_stopped,
         core::posix::ChildProcess::invalid()
     };
 
@@ -241,10 +249,19 @@ core::trust::Request::Answer mir::Agent::authenticate_request_with_parameters(co
                     parameters.application.pid,
                     Agent::on_trust_session_changed_state,
                     &cb_context),
-        // Acquire a new fd for the prompt provider.
-        scope.prompt_session->new_fd_for_prompt_provider()
+        /* fd */ -1
     };
 
+    if (cb_context.state == mir_prompt_session_state_stopped) {
+        throw std::runtime_error{
+            "Unable to create a prompt session: " +
+            scope.prompt_session->error_message()
+        };
+    }
+
+    // Acquire a new fd for the prompt provider.
+    scope.fd = scope.prompt_session->new_fd_for_prompt_provider();
+
     // And prepare the actual execution in a child process.
     mir::PromptProviderHelper::InvocationArguments args
     {

src/core/trust/mir/agent.h

@@ -64,6 +64,9 @@ class CORE_TRUST_DLL_PUBLIC PromptSessionVirtualTable
     PromptSessionVirtualTable(MirPromptSession* prompt_session);
     virtual ~PromptSessionVirtualTable() = default;
 
+    // Retrieve a text description of the last error.
+    virtual std::string error_message();
+
     // Requests a new, pre-authenticated fd for associating prompt providers.
     // Returns the fd or throws std::runtime_error.
     virtual int new_fd_for_prompt_provider();
@@ -184,6 +187,9 @@ struct CORE_TRUST_DLL_PUBLIC Agent : public core::trust::Agent
     // Used in prompt_user_for_request to wait for the trust session to be stopped.
     struct OnTrustSessionStateChangedCallbackContext
     {
+        // The current state of the prompt session. Used to detect an error
+        // when starting prompt session.
+        MirPromptSessionState state;
         // The process that provides the prompting UI.
         core::posix::ChildProcess prompt_provider_process;
     };

tests/mir_agent_test.cpp

@@ -42,6 +42,9 @@ struct MockPromptSessionVirtualTable : public core::trust::mir::PromptSessionVir
     {
     }
 
+    // Retrieve a text description of the last error.
+    MOCK_METHOD0(error_message, std::string());
+
     // Requests a new, pre-authenticated fd for associating prompt providers.
     // Returns the fd or throws std::runtime_error.
     MOCK_METHOD0(new_fd_for_prompt_provider, int());
@@ -127,6 +130,17 @@ std::shared_ptr<MockAppInfoResolver> a_mocked_app_info_resolver()
 {
     return std::make_shared<MockAppInfoResolver>();
 }
+
+ACTION_P2(ReturnPromptSession, prompt_session_vtable, success) {
+    arg1(
+        /* Mir's prompt session, not really used */ nullptr,
+        success ? mir_prompt_session_state_started : mir_prompt_session_state_stopped,
+        /* context */ arg2
+        );
+
+    return prompt_session_vtable;
+}
+
 }
 
 TEST(DefaultProcessStateTranslator, throws_for_signalled_process)
@@ -232,7 +246,7 @@ TEST(MirAgent, creates_prompt_session_and_execs_helper_with_preauthenticated_fd)
     auto app_info_resolver = a_mocked_app_info_resolver();
 
     ON_CALL(*connection_vtable, create_prompt_session_sync(_, _, _))
-            .WillByDefault(Return(prompt_session_vtable));
+            .WillByDefault(ReturnPromptSession(prompt_session_vtable, /* success */ true));
 
     ON_CALL(*prompt_session_vtable, new_fd_for_prompt_provider())
             .WillByDefault(Return(pre_authenticated_fd));
@@ -303,7 +317,7 @@ TEST(MirAgent, calls_into_app_info_resolver_with_app_id)
     auto app_info_resolver = a_mocked_app_info_resolver();
 
     ON_CALL(*connection_vtable, create_prompt_session_sync(_, _, _))
-            .WillByDefault(Return(prompt_session_vtable));
+            .WillByDefault(ReturnPromptSession(prompt_session_vtable, /* success */ true));
 
     ON_CALL(*prompt_session_vtable, new_fd_for_prompt_provider())
             .WillByDefault(Return(pre_authenticated_fd));
@@ -400,7 +414,7 @@ TEST(MirAgent, sig_kills_prompt_provider_process_on_status_change)
             .WillByDefault(
                 DoAll(
                     SaveArg<2>(&prompt_session_state_callback_context),
-                    Return(prompt_session_vtable)));
+                    ReturnPromptSession(prompt_session_vtable, /* success */ true)));
 
     core::trust::mir::Agent agent
     {
@@ -445,6 +459,65 @@ TEST(MirAgent, sig_kills_prompt_provider_process_on_status_change)
         asynchronously_stop_the_prompting_session.join();
 }
 
+TEST(MirAgent, dont_exec_provider_when_unable_to_create_a_prompt_session)
+{
+    using namespace ::testing;
+
+    const core::trust::Pid app_pid {21};
+    const std::string app_icon{"/tmp"};
+    const std::string app_name {"Does not exist"};
+    const std::string app_id {"does.not.exist.application"};
+    const core::trust::Feature feature{42};
+    const std::string app_description {"This is just an extended description %1%"};
+    const int pre_authenticated_fd {42};
+
+    auto connection_vtable = a_mocked_connection_vtable();
+    auto prompt_session_vtable = a_mocked_prompt_session_vtable();
+
+    auto prompt_provider_exec_helper = a_mocked_prompt_provider_calling_bin_false();
+    auto app_info_resolver = a_mocked_app_info_resolver();
+
+    ON_CALL(*connection_vtable, create_prompt_session_sync(_, _, _))
+            .WillByDefault(ReturnPromptSession(prompt_session_vtable, /* success */ false));
+
+    ON_CALL(*prompt_session_vtable, error_message())
+            .WillByDefault(Return(std::string("Fabricated error.")));
+    ON_CALL(*prompt_session_vtable, new_fd_for_prompt_provider())
+            .WillByDefault(Return(pre_authenticated_fd));
+
+    ON_CALL(*app_info_resolver, resolve(_))
+            .WillByDefault(Return(core::trust::mir::AppInfo{app_icon, app_name, app_id}));
+
+    EXPECT_CALL(*connection_vtable, create_prompt_session_sync(app_pid, _, _)).Times(1);
+    EXPECT_CALL(*prompt_session_vtable, error_message()).Times(1);
+    // We have an error. New FD shouldn't be requested, and the prompt provider
+    // shouldn't be exec'ed, otherwise the provider can hang.
+    EXPECT_CALL(*prompt_session_vtable, new_fd_for_prompt_provider()).Times(0);
+    EXPECT_CALL(*prompt_provider_exec_helper,
+                exec_prompt_provider_with_arguments(_)).Times(0);
+
+    core::trust::mir::Agent agent
+    {
+        core::trust::mir::Agent::Configuration
+        {
+            connection_vtable,
+            prompt_provider_exec_helper,
+            core::trust::mir::Agent::translator_only_accepting_exit_status_success(),
+            app_info_resolver
+        }
+    };
+
+    EXPECT_THROW(agent.authenticate_request_with_parameters(
+                  core::trust::Agent::RequestParameters
+                  {
+                     core::trust::Uid{::getuid()},
+                     app_pid,
+                     app_id,
+                     feature,
+                     app_description
+                  }), std::runtime_error);
+}
+
 TEST(TrustPrompt, aborts_for_missing_title)
 {
     // And we pass in an empty argument vector