Skip to content
This repository has been archived by the owner on Jan 13, 2022. It is now read-only.

Commit

Permalink
mir/agent: fix indefinite wait if fail to start prompt session
Browse files Browse the repository at this point in the history
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 error before continue.

Related: ubports/ubuntu-touch#1668
  • Loading branch information
peat-psuwit committed Feb 14, 2021
1 parent ad94b8b commit 25d3d82
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 2 deletions.
18 changes: 16 additions & 2 deletions src/core/trust/mir/agent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -241,10 +246,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
};

auto error = scope.prompt_session->error_message();
if (!error.empty()) {
throw std::runtime_error{
"Unable to create a prompt session: " + error
};
}

// 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
{
Expand Down
3 changes: 3 additions & 0 deletions src/core/trust/mir/agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
71 changes: 71 additions & 0 deletions tests/mir_agent_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -234,6 +237,9 @@ TEST(MirAgent, creates_prompt_session_and_execs_helper_with_preauthenticated_fd)
ON_CALL(*connection_vtable, create_prompt_session_sync(_, _, _))
.WillByDefault(Return(prompt_session_vtable));

ON_CALL(*prompt_session_vtable, error_message())
.WillByDefault(Return(std::string()));

ON_CALL(*prompt_session_vtable, new_fd_for_prompt_provider())
.WillByDefault(Return(pre_authenticated_fd));

Expand Down Expand Up @@ -305,6 +311,9 @@ TEST(MirAgent, calls_into_app_info_resolver_with_app_id)
ON_CALL(*connection_vtable, create_prompt_session_sync(_, _, _))
.WillByDefault(Return(prompt_session_vtable));

ON_CALL(*prompt_session_vtable, error_message())
.WillByDefault(Return(std::string()));

ON_CALL(*prompt_session_vtable, new_fd_for_prompt_provider())
.WillByDefault(Return(pre_authenticated_fd));

Expand Down Expand Up @@ -380,6 +389,9 @@ TEST(MirAgent, sig_kills_prompt_provider_process_on_status_change)
a_spinning_process,
core::posix::StandardStream::empty)));

ON_CALL(*prompt_session_vtable, error_message())
.WillByDefault(Return(std::string()));

ON_CALL(*prompt_session_vtable, new_fd_for_prompt_provider())
.WillByDefault(
Return(
Expand Down Expand Up @@ -445,6 +457,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(Return(prompt_session_vtable));

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
Expand Down

0 comments on commit 25d3d82

Please sign in to comment.