diff --git a/src/core/trust/mir/agent.cpp b/src/core/trust/mir/agent.cpp index 9f08bd6..6dfd840 100644 --- a/src/core/trust/mir/agent.cpp +++ b/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(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 { diff --git a/src/core/trust/mir/agent.h b/src/core/trust/mir/agent.h index b793fcc..fabcd99 100644 --- a/src/core/trust/mir/agent.h +++ b/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; }; diff --git a/tests/mir_agent_test.cpp b/tests/mir_agent_test.cpp index 632c37c..c7306f7 100644 --- a/tests/mir_agent_test.cpp +++ b/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 a_mocked_app_info_resolver() { return std::make_shared(); } + +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