Merge lp:~thomas-voss/trust-store/fix-1504022 into lp:trust-store/15.04

Proposed by Thomas Voß
Status: Merged
Approved by: Alberto Mardegan
Approved revision: 141
Merged at revision: 140
Proposed branch: lp:~thomas-voss/trust-store/fix-1504022
Merge into: lp:trust-store/15.04
Prerequisite: lp:~thomas-voss/trust-store/integrate-xdg
Diff against target: 1270 lines (+687/-167)
17 files modified
CMakeLists.txt (+1/-1)
debian/control (+1/-0)
po/trust-store.pot (+10/-11)
src/CMakeLists.txt (+7/-0)
src/core/trust/daemon.cpp (+1/-1)
src/core/trust/mir/agent.cpp (+22/-42)
src/core/trust/mir/agent.h (+44/-16)
src/core/trust/mir/click_desktop_entry_app_info_resolver.cpp (+139/-0)
src/core/trust/mir/click_desktop_entry_app_info_resolver.h (+49/-0)
src/core/trust/mir/prompt_main.cpp (+44/-41)
src/core/trust/mir/prompt_main.h (+16/-4)
src/core/trust/mir/prompt_main.qml (+66/-30)
tests/CMakeLists.txt (+20/-0)
tests/click_desktop_entry_app_name_resolver_test.cpp (+75/-0)
tests/mir_agent_test.cpp (+131/-21)
tests/share/applications/valid.pkg_app_0.0.0.desktop (+56/-0)
tests/test_data.h.in (+5/-0)
To merge this branch: bzr merge lp:~thomas-voss/trust-store/fix-1504022
Reviewer Review Type Date Requested Status
Matthew Paul Thomas (community) design Approve
Tyler Hicks Approve
Alberto Mardegan (community) Approve
PS Jenkins bot continuous-integration Pending
Review via email: mp+278418@code.launchpad.net

This proposal supersedes a proposal from 2015-11-20.

Commit message

Add interface mir::AppNameResolver.

mir::AppNameResolver is used by mir::Agent to map application IDs
to localized application names.

Description of the change

Add interface mir::AppNameResolver.

mir::AppNameResolver is used by mir::Agent to map application IDs
to localized application names.

To post a comment you must log in.
Revision history for this message
Alberto Mardegan (mardy) wrote : Posted in a previous version of this proposal

A couple of inline comments, I'm not convinced that this way of finding the desktop files is reliable.

review: Needs Fixing
Revision history for this message
Alberto Mardegan (mardy) wrote : Posted in a previous version of this proposal

Looks good!

review: Approve
Revision history for this message
Alberto Mardegan (mardy) wrote : Posted in a previous version of this proposal

LGTM

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Tyler Hicks (tyhicks) wrote : Posted in a previous version of this proposal

Hello - Thomas had asked for a security review and I just took a look at the code.

The code itself looks nearly perfect. I have a couple questions below but they're minor.

I'm mainly concerned about the concept of using the localized Name of the desktop file in a trust prompt. For something like a click app that comes from an untrusted developer, we have no idea if the localized name in their provided desktop file is an accurate translation or a malicious translation intended to trick the user into granting their app access. I don't know of any logic that could be used in the click reviewers tools to verify that the translations are non-malicious. How do we handle such a situation?

As for the code, I have a couple questions/comments:

 1) What UID does resolve_desktop_entry_or_throw() run under? I ask because it goes mucking around in the user's home dir and is vulnerable to minor TOCTOU issues such as checking to see if /home/USER/applications/APP_ID.desktop is a regular file and then later passing that path to g_key_file_load_from_file(). The user could modify the path to be a symlink to an area of the filesystem that he/she doesn't own. If resolve_desktop_entry_or_throw() runs as the user, there's probably nothing to worry about.

 2) The external XDG stuff that you wrote is a nice idea. I'd recommend packaging it up as a library so that it can be reused in a controllable fashion instead of copying and pasting the code into all of the projects that want to use it.

review: Needs Information
Revision history for this message
Tyler Hicks (tyhicks) wrote : Posted in a previous version of this proposal

After speaking with jdstrand, he explained why the click reviewers tools don't currently verify the Name field of a desktop file. The desktop file name field can't be forced to match the click package name because the desktop file name should be "pretty" while the click package name is alphanumerics and '-'.

Because of this, I don't see how the desktop file name field can be trusted enough to be used in trust store prompts. :/

Revision history for this message
Thomas Voß (thomas-voss) wrote : Posted in a previous version of this proposal

Hey Tyler,

thanks for the detailed review and your comments. Let me first try to clarify on the use of the localized name in trust prompts. I actually raised the same issue as you, and it turns out that we are "vulnerable" to applications faking their identity in a couple of places throughout the system. First and foremost: Unity itself relies on the .desktop file entry for displaying the name. We might want to fix the default approach for handling localized application names meant for human consumption in snappy, though.

One possible way to resolve the issue is by altering the prompt to show the localized name alongside the actual application id. Ideally, we would have an expendable field "Details" as part of the prompt containing in-depth information about the request, at the very least the actual app ID together with the UID (and the human-readable user name).

(@1:) The function always runs under the actual user. I will however add a check to make sure that the current user id matches the user id of the request. If not, we will return early, and deny the request.

(@2:) Sure, happy to. I mainly added the external code here to guarantee a swift landing. I will do the packaging and carry out the requesting asap, but would like to make sure that a fix for this bug is not blocked by package/MIR review processes.

Revision history for this message
Thomas Voß (thomas-voss) wrote : Posted in a previous version of this proposal

One other idea to further clarify the identity of the app might be to include its official icon. At the very least, this would make it easier for the user to associate the source of the trust request with what is visible in the dash and in the launcher.

Revision history for this message
Thomas Voß (thomas-voss) wrote : Posted in a previous version of this proposal

I reported the potential privilege escalation issue in https://bugs.launchpad.net/trust-store/+bug/1518883. The bug has got a fix attached.

Revision history for this message
Alberto Mardegan (mardy) wrote : Posted in a previous version of this proposal

LGTM!

review: Approve
Revision history for this message
Sebastien Bacher (seb128) wrote : Posted in a previous version of this proposal

@Tyler, note that the current situation is not any better, the prompt are using the name of the hook included in the click, which is also not restricted...

Having the appid mentioned looks like a good idea to me though

Revision history for this message
Thomas Voß (thomas-voss) wrote : Posted in a previous version of this proposal

> @Tyler, note that the current situation is not any better, the prompt are
> using the name of the hook included in the click, which is also not
> restricted...
>

I think that's an issue with our review tools tbh. The trust store relies on apparmor to query the profile name. With that, if an app can just request any name in there, we have got a problem from my pov.

> Having the appid mentioned looks like a good idea to me though

Revision history for this message
Tyler Hicks (tyhicks) wrote : Posted in a previous version of this proposal

Thomas, I'm not sure that you need to add a user ID check. I was just curious what UID the code was expected to be running in. I'm happy enough with your answer that it runs as the user.

If you think the XDG code will be reused in other projects, it'd be good to package up separately.

Revision history for this message
Tyler Hicks (tyhicks) wrote : Posted in a previous version of this proposal

> @Tyler, note that the current situation is not any better, the prompt are
> using the name of the hook included in the click, which is also not
> restricted...
>
> Having the appid mentioned looks like a good idea to me though

Yes, I think the appid approach has a chance of greatly improving the situation.

Revision history for this message
Matthew Paul Thomas (mpt) wrote : Posted in a previous version of this proposal
Download full text (3.5 KiB)

The standard permission prompt message is of the form
    <appname> wants to access <property>.
for example
    Frobnicator Turbo wants to access your location.
so the attack you have in mind is for Frobnicator Turbo to pretend to be another app:
    Browser wants to access your location.

Especially while people are used to hardly anything happening in the background, this might often fail. People would think, "Huh? I'm not using Browser at the moment. Why is it asking me now?"

Showing the app icon would help now, but it would lose its effectiveness once bug 1453795 is fixed.

Including the application id might protect geeks, but I expect most people would ignore it, or think it was a bug, on the grounds that it looks like code. If it was embedded in the prompt text,
    <appname> (<appid>) wants to access <property>.
then an app could even explain it away with a devious name:
    Browser (downloadable from
producing the prompt
    Browser (downloadable from (frobnicator-turbo.comorg) wants to access your location.
The profusion of TLDs leaves that plausible even to geeks who aren't familiar with the exact format of the prompt. Few would notice the unmatched bracket. So if it's included at all, I think it should be secondary text, like in the "Details" section for PolicyKit prompts. <https://wiki.ubuntu.com/AccountPrivileges#Details>

Meanwhile, an app could do much more devious things with its name. Maybe we can address these at the same time, or maybe they need to be tackled separately. For example, an app could give itself a name something like
    Browser wants to access only your bookmarks. It does not
which would produce prompt text of the form
    Browser wants to access only your bookmarks. It does not wants to access your location.
This would be slightly ungrammatical ("does not wants"), but would still fool many people.

How to mitigate this? We could put the app name in quote marks:
    “Browser wants to access only your bookmarks. It does not” wants to access your location.
But then the app could jimmy its own opposing quote marks into its name:
    Browser” wants to access only your bookmarks. It does “*not*
producing the only-slightly-less-believable prompt:
    “Browser” wants to access only your bookmarks. It does “*not*” wants to access your location.
(This idea adapted from <https://www.squarefree.com/dialogs2010/exec-warning-injected.png>.)

(We couldn't prevent that by banning use of quote marks, because there are many other Unicode characters that look like quote marks, including apostrophes: Baldur’s Gate, Sid Meier's Civilization, Tony Hawk's Pro Skater 2, etc.)

We could limit the length of app names to something like 30 characters:
    “Browser wants to access only
That would put the kibosh on those obfuscation attacks ... in most languages. Not in kanji ones, unfortunately:
    浏览器要访问您的书签。它不希望访问您的位置。
But maybe mitigating the attack in most but not all languages is better than mitigating it in none of them.

We could put the app name in color, while the rest of the text is the normal UI color. But apparently that hasn't stopped malware in the past. <https://www.squarefree.com/dialogs2010/activex.png>

Perhaps a har...

Read more...

Revision history for this message
Thomas Voß (thomas-voss) wrote : Posted in a previous version of this proposal
Download full text (3.7 KiB)

> The standard permission prompt message is of the form
> <appname> wants to access <property>.
> for example
> Frobnicator Turbo wants to access your location.
> so the attack you have in mind is for Frobnicator Turbo to pretend to be
> another app:
> Browser wants to access your location.
>
> Especially while people are used to hardly anything happening in the
> background, this might often fail. People would think, "Huh? I'm not using
> Browser at the moment. Why is it asking me now?"
>
> Showing the app icon would help now, but it would lose its effectiveness once
> bug 1453795 is fixed.
>
> Including the application id might protect geeks, but I expect most people
> would ignore it, or think it was a bug, on the grounds that it looks like
> code. If it was embedded in the prompt text,
> <appname> (<appid>) wants to access <property>.
> then an app could even explain it away with a devious name:
> Browser (downloadable from
> producing the prompt
> Browser (downloadable from (frobnicator-turbo.comorg) wants to access your
> location.
> The profusion of TLDs leaves that plausible even to geeks who aren't familiar
> with the exact format of the prompt. Few would notice the unmatched bracket.
> So if it's included at all, I think it should be secondary text, like in the
> "Details" section for PolicyKit prompts.
> <https://wiki.ubuntu.com/AccountPrivileges#Details>
>
> Meanwhile, an app could do much more devious things with its name. Maybe we
> can address these at the same time, or maybe they need to be tackled
> separately. For example, an app could give itself a name something like
> Browser wants to access only your bookmarks. It does not
> which would produce prompt text of the form
> Browser wants to access only your bookmarks. It does not wants to access
> your location.
> This would be slightly ungrammatical ("does not wants"), but would still fool
> many people.
>
> How to mitigate this? We could put the app name in quote marks:
> “Browser wants to access only your bookmarks. It does not” wants to access
> your location.
> But then the app could jimmy its own opposing quote marks into its name:
> Browser” wants to access only your bookmarks. It does “*not*
> producing the only-slightly-less-believable prompt:
> “Browser” wants to access only your bookmarks. It does “*not*” wants to
> access your location.
> (This idea adapted from <https://www.squarefree.com/dialogs2010/exec-warning-
> injected.png>.)
>
> (We couldn't prevent that by banning use of quote marks, because there are
> many other Unicode characters that look like quote marks, including
> apostrophes: Baldur’s Gate, Sid Meier's Civilization, Tony Hawk's Pro Skater
> 2, etc.)
>
> We could limit the length of app names to something like 30 characters:
> “Browser wants to access only
> That would put the kibosh on those obfuscation attacks ... in most languages.
> Not in kanji ones, unfortunately:
> 浏览器要访问您的书签。它不希望访问您的位置。
> But maybe mitigating the attack in most but not all languages is better than
> mitigating it in none of them.
>
> We could put the app name in color, while the rest of the text is the normal
> U...

Read more...

Revision history for this message
Thomas Voß (thomas-voss) wrote : Posted in a previous version of this proposal

> Thomas, I'm not sure that you need to add a user ID check. I was just curious
> what UID the code was expected to be running in. I'm happy enough with your
> answer that it runs as the user.
>

You were raising a perfectly valid issue. Requests *should* always be handled by user-specific store instances. The branch I proposed ensures that this actually happens or it fails loudly :)

> If you think the XDG code will be reused in other projects, it'd be good to
> package up separately.

Yup, I will tackle it next after this landing. That will also include changes to account for snappy/click-specific filesystem layouts.

Revision history for this message
Alberto Mardegan (mardy) wrote :

LGTM!

review: Approve
Revision history for this message
Thomas Voß (thomas-voss) wrote :

Please find a screenshot of the current version here: http://pasteboard.co/2qhB6op7.png

142. By Thomas Voß

Address comments in review and implement new prompt design.

143. By Thomas Voß

Ensure that rendering is consistent with app icon rendering on launcher/dash.

144. By Thomas Voß

Update pot to account for string changes.

Revision history for this message
Matthew Paul Thomas (mpt) wrote :

A couple of nitpicks, but the screenshot looks good otherwise.

review: Needs Fixing (design)
145. By Thomas Voß

Change color of negative action to neutral, see:
  https://developer.ubuntu.com/api/apps/qml/sdk-14.10/Ubuntu.Components.UbuntuColors/#lightGrey-prop
Fix apostrophe to be a real typographic apostrophe.

146. By Thomas Voß

Align with app scope display of icons, see:
  https://code.launchpad.net/~cimi/unity8/new-shadows-1.3/+merge/271611

Revision history for this message
Tyler Hicks (tyhicks) wrote :

Looks good to me. Nice work on finding a better solution!

review: Approve
Revision history for this message
Matthew Paul Thomas (mpt) :
review: Approve (design)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'CMakeLists.txt'
--- CMakeLists.txt 2015-11-30 10:46:27 +0000
+++ CMakeLists.txt 2015-11-30 10:46:27 +0000
@@ -28,7 +28,7 @@
28ENDIF(CMAKE_BUILD_TYPE MATCHES [cC][oO][vV][eE][rR][aA][gG][eE])28ENDIF(CMAKE_BUILD_TYPE MATCHES [cC][oO][vV][eE][rR][aA][gG][eE])
2929
30find_package(PkgConfig)30find_package(PkgConfig)
31find_package(Boost COMPONENTS program_options system REQUIRED)31find_package(Boost COMPONENTS filesystem program_options system REQUIRED)
3232
33add_subdirectory(3rd_party/xdg)33add_subdirectory(3rd_party/xdg)
3434
3535
=== modified file 'debian/control'
--- debian/control 2015-11-30 10:46:27 +0000
+++ debian/control 2015-11-30 10:46:27 +0000
@@ -14,6 +14,7 @@
14 libdbus-cpp-dev (>= 4.0.0),14 libdbus-cpp-dev (>= 4.0.0),
15 libdbus-1-dev,15 libdbus-1-dev,
16 libgflags-dev,16 libgflags-dev,
17 libglib2.0-dev,
17 libgoogle-glog-dev,18 libgoogle-glog-dev,
18 libgtest-dev,19 libgtest-dev,
19 libjson-c-dev,20 libjson-c-dev,
2021
=== modified file 'po/trust-store.pot'
--- po/trust-store.pot 2015-08-20 17:46:04 +0000
+++ po/trust-store.pot 2015-11-30 10:46:27 +0000
@@ -8,7 +8,7 @@
8msgstr ""8msgstr ""
9"Project-Id-Version: trust-store\n"9"Project-Id-Version: trust-store\n"
10"Report-Msgid-Bugs-To: \n"10"Report-Msgid-Bugs-To: \n"
11"POT-Creation-Date: 2015-08-20 13:41-0400\n"11"POT-Creation-Date: 2015-11-27 13:56+0100\n"
12"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"12"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
13"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"13"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
14"Language-Team: LANGUAGE <LL@li.org>\n"14"Language-Team: LANGUAGE <LL@li.org>\n"
@@ -17,15 +17,14 @@
17"Content-Type: text/plain; charset=CHARSET\n"17"Content-Type: text/plain; charset=CHARSET\n"
18"Content-Transfer-Encoding: 8bit\n"18"Content-Transfer-Encoding: 8bit\n"
1919
20#: /tmp/trust-store-i18n/src/core/trust/daemon.cpp:26520#: /home/tvoss/ubuntu/scratch/fix-1504022/src/core/trust/daemon.cpp:265
21#, boost-format21msgid "is trying to access"
22msgid "Application %1% is trying to access"22msgstr ""
23msgstr ""23
2424#: /home/tvoss/ubuntu/scratch/fix-1504022/src/core/trust/mir/prompt_main.qml:78
25#: /tmp/trust-store-i18n/src/core/trust/mir/prompt_main.qml:42
26msgid "Deny"
27msgstr ""
28
29#: /tmp/trust-store-i18n/src/core/trust/mir/prompt_main.qml:49
30msgid "Allow"25msgid "Allow"
31msgstr ""26msgstr ""
27
28#: /home/tvoss/ubuntu/scratch/fix-1504022/src/core/trust/mir/prompt_main.qml:83
29msgid "Don't Allow"
30msgstr ""
3231
=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt 2015-11-30 10:46:27 +0000
+++ src/CMakeLists.txt 2015-11-30 10:46:27 +0000
@@ -20,6 +20,8 @@
2020
21pkg_check_modules(DBUS_CPP dbus-cpp REQUIRED)21pkg_check_modules(DBUS_CPP dbus-cpp REQUIRED)
22pkg_check_modules(DBUS dbus-1 REQUIRED)22pkg_check_modules(DBUS dbus-1 REQUIRED)
23pkg_check_modules(GLIB glib-2.0 REQUIRED)
24pkg_check_modules(GOBJECT gobject-2.0 REQUIRED)
23pkg_check_modules(LIBAPPARMOR libapparmor REQUIRED)25pkg_check_modules(LIBAPPARMOR libapparmor REQUIRED)
24pkg_check_modules(SQLITE3 sqlite3 REQUIRED)26pkg_check_modules(SQLITE3 sqlite3 REQUIRED)
2527
@@ -28,6 +30,8 @@
28include_directories(30include_directories(
29 ${DBUS_CPP_INCLUDE_DIRS}31 ${DBUS_CPP_INCLUDE_DIRS}
30 ${DBUS_INCLUDE_DIRS}32 ${DBUS_INCLUDE_DIRS}
33 ${GLIB_INCLUDE_DIRS}
34 ${GOBJECT_INCLUDE_DIRS}
31 ${LIBAPPARMOR_INCLUDE_DIRS}35 ${LIBAPPARMOR_INCLUDE_DIRS}
32 ${SQLITE3_INCLUDE_DIRS}36 ${SQLITE3_INCLUDE_DIRS}
33)37)
@@ -70,6 +74,7 @@
70 # to prompt the user for trusting an application to access a trusted74 # to prompt the user for trusting an application to access a trusted
71 # system service.75 # system service.
72 core/trust/mir/agent.cpp76 core/trust/mir/agent.cpp
77 core/trust/mir/click_desktop_entry_app_info_resolver.cpp
73 )78 )
7479
75 # Make sure Qt does not inject evil macros like 'signals' and 'slots'.80 # Make sure Qt does not inject evil macros like 'signals' and 'slots'.
@@ -186,6 +191,8 @@
186 ${DBUS_LIBRARIES}191 ${DBUS_LIBRARIES}
187 ${GFLAGS_LDFLAGS}192 ${GFLAGS_LDFLAGS}
188 ${GLOG_LDFLAGS}193 ${GLOG_LDFLAGS}
194 ${GLIB_LDFLAGS}
195 ${GOBJECT_LDFLAGS}
189 ${LIBAPPARMOR_LDFLAGS}196 ${LIBAPPARMOR_LDFLAGS}
190 ${MIR_CLIENT_LDFLAGS}197 ${MIR_CLIENT_LDFLAGS}
191 ${PROCESS_CPP_LDFLAGS}198 ${PROCESS_CPP_LDFLAGS}
192199
=== modified file 'src/core/trust/daemon.cpp'
--- src/core/trust/daemon.cpp 2015-08-31 14:00:41 +0000
+++ src/core/trust/daemon.cpp 2015-11-30 10:46:27 +0000
@@ -262,7 +262,7 @@
262 core::trust::remote::helpers::aa_get_task_con_app_id_resolver(),262 core::trust::remote::helpers::aa_get_task_con_app_id_resolver(),
263 dict.count("description-pattern") > 0 ?263 dict.count("description-pattern") > 0 ?
264 dict.at("description-pattern") :264 dict.at("description-pattern") :
265 core::trust::i18n::tr("Application %1% is trying to access") + " " + service_name + ".",265 core::trust::i18n::tr("is trying to access") + " " + service_name + ".",
266 dict.count("verify-process-timestamp") > 0266 dict.count("verify-process-timestamp") > 0
267 };267 };
268268
269269
=== modified file 'src/core/trust/mir/agent.cpp'
--- src/core/trust/mir/agent.cpp 2015-08-31 13:16:20 +0000
+++ src/core/trust/mir/agent.cpp 2015-11-30 10:46:27 +0000
@@ -33,6 +33,11 @@
3333
34namespace mir = core::trust::mir;34namespace mir = core::trust::mir;
3535
36bool mir::operator==(const mir::AppInfo& lhs, const mir::AppInfo& rhs)
37{
38 return lhs.icon == rhs.icon && lhs.id == rhs.id && lhs.name == rhs.name;
39}
40
36// Invoked whenever a request for creation of pre-authenticated fds succeeds.41// Invoked whenever a request for creation of pre-authenticated fds succeeds.
37void mir::PromptSessionVirtualTable::mir_client_fd_callback(MirPromptSession */*prompt_session*/, size_t count, int const* fds, void* context)42void mir::PromptSessionVirtualTable::mir_client_fd_callback(MirPromptSession */*prompt_session*/, size_t count, int const* fds, void* context)
38{43{
@@ -134,27 +139,15 @@
134{139{
135 static auto child_setup = []() {};140 static auto child_setup = []() {};
136141
137 // We translate to human readable strings here, and do it a non-translateable way first142 auto app_name = args.app_info.name;
138 // We post-process the application id and try to extract the unversioned package name.143 auto description = i18n::tr(args.description, i18n::service_text_domain());
139 // Please see https://wiki.ubuntu.com/AppStore/Interfaces/ApplicationId.
140 static const std::regex regex_full_app_id{"(.*)_(.*)_(.*)"};
141 static const std::regex regex_short_app_id{"(.*)_(.*)"};
142 static constexpr std::size_t index_app{2};
143
144 auto app_name = args.application_id;
145
146 std::smatch match;
147 if (std::regex_match(app_name, match, regex_full_app_id))
148 app_name = std::string{match[index_app]};
149 else if (std::regex_match(app_name, match, regex_short_app_id))
150 app_name = std::string{match[index_app]};
151
152 auto description = (boost::format(i18n::tr(args.description, i18n::service_text_domain())) % app_name).str();
153144
154 std::vector<std::string> argv145 std::vector<std::string> argv
155 {146 {
156 "--" + std::string{core::trust::mir::cli::option_server_socket}, "fd://" + std::to_string(args.fd),147 "--" + std::string{core::trust::mir::cli::option_server_socket}, "fd://" + std::to_string(args.fd),
157 "--" + std::string{core::trust::mir::cli::option_title}, app_name,148 "--" + std::string{core::trust::mir::cli::option_icon}, args.app_info.icon,
149 "--" + std::string{core::trust::mir::cli::option_name}, args.app_info.name,
150 "--" + std::string{core::trust::mir::cli::option_id}, args.app_info.id,
158 "--" + std::string{core::trust::mir::cli::option_description}, description151 "--" + std::string{core::trust::mir::cli::option_description}, description
159 };152 };
160153
@@ -219,17 +212,8 @@
219 };212 };
220}213}
221214
222mir::Agent::Agent(215mir::Agent::Agent(const mir::Agent::Configuration& config)
223 // VTable object providing access to Mir's trusted prompting functionality.216 : config(config)
224 const mir::ConnectionVirtualTable::Ptr& connection_vtable,
225 // Exec helper for starting up prompt provider child processes with the correct setup
226 // of command line arguments and environment variables.
227 const mir::PromptProviderHelper::Ptr& exec_helper,
228 // A translator function for mapping child process exit states to trust::Request answers.
229 const std::function<core::trust::Request::Answer(const core::posix::wait::Result&)>& translator)
230 : connection_vtable(connection_vtable),
231 exec_helper(exec_helper),
232 translator(translator)
233{217{
234}218}
235219
@@ -260,7 +244,7 @@
260 } scope244 } scope
261 {245 {
262 // We setup the prompt session and wire up to our own internal callback helper.246 // We setup the prompt session and wire up to our own internal callback helper.
263 connection_vtable->create_prompt_session_sync(247 config.connection_vtable->create_prompt_session_sync(
264 parameters.application.pid,248 parameters.application.pid,
265 Agent::on_trust_session_changed_state,249 Agent::on_trust_session_changed_state,
266 &cb_context)250 &cb_context)
@@ -273,24 +257,25 @@
273 mir::PromptProviderHelper::InvocationArguments args257 mir::PromptProviderHelper::InvocationArguments args
274 {258 {
275 fd,259 fd,
276 parameters.application.id,260 config.app_info_resolver->resolve(parameters.application.id),
277 parameters.description261 parameters.description
278 };262 };
279263
280 // Ask the helper to fire up the prompt provider.264 // Ask the helper to fire up the prompt provider.
281 cb_context.prompt_provider_process = exec_helper->exec_prompt_provider_with_arguments(args);265 cb_context.prompt_provider_process = config.exec_helper->exec_prompt_provider_with_arguments(args);
282 // And subsequently wait for it to finish.266 // And subsequently wait for it to finish.
283 auto result = cb_context.prompt_provider_process.wait_for(core::posix::wait::Flags::untraced);267 auto result = cb_context.prompt_provider_process.wait_for(core::posix::wait::Flags::untraced);
284268
285 return translator(result);269 return config.translator(result);
286}270}
287271
288bool mir::operator==(const mir::PromptProviderHelper::InvocationArguments& lhs, const mir::PromptProviderHelper::InvocationArguments& rhs)272bool mir::operator==(const mir::PromptProviderHelper::InvocationArguments& lhs, const mir::PromptProviderHelper::InvocationArguments& rhs)
289{273{
290 return std::tie(lhs.application_id, lhs.description, lhs.fd) == std::tie(rhs.application_id, rhs.description, rhs.fd);274 return std::tie(lhs.app_info, lhs.description, lhs.fd) == std::tie(rhs.app_info, rhs.description, rhs.fd);
291}275}
292276
293#include "config.h"277#include "config.h"
278#include "click_desktop_entry_app_info_resolver.h"
294279
295MirConnection* mir::connect(const std::string& endpoint, const std::string& name)280MirConnection* mir::connect(const std::string& endpoint, const std::string& name)
296{281{
@@ -318,13 +303,8 @@
318 }303 }
319 };304 };
320305
321 return mir::Agent::Ptr306 mir::AppInfoResolver::Ptr anr{new mir::ClickDesktopEntryAppInfoResolver{}};
322 {307
323 new mir::Agent308 mir::Agent::Configuration config{cvt, pph, mir::Agent::translator_only_accepting_exit_status_success(), anr};
324 {309 return mir::Agent::Ptr{new mir::Agent{config}};
325 cvt,
326 pph,
327 mir::Agent::translator_only_accepting_exit_status_success()
328 }
329 };
330}310}
331311
=== modified file 'src/core/trust/mir/agent.h'
--- src/core/trust/mir/agent.h 2014-10-07 19:27:18 +0000
+++ src/core/trust/mir/agent.h 2015-11-30 10:46:27 +0000
@@ -27,6 +27,8 @@
27#include <mirclient/mir_toolkit/mir_client_library.h>27#include <mirclient/mir_toolkit/mir_client_library.h>
28#include <mirclient/mir_toolkit/mir_prompt_session.h>28#include <mirclient/mir_toolkit/mir_prompt_session.h>
2929
30#include <boost/filesystem.hpp>
31
30#include <condition_variable>32#include <condition_variable>
31#include <functional>33#include <functional>
32#include <mutex>34#include <mutex>
@@ -37,6 +39,17 @@
37{39{
38namespace mir40namespace mir
39{41{
42// Info bundles information about an application.
43struct AppInfo
44{
45 std::string icon; // The icon of the application.
46 std::string name; // The human-readable, localized name of the application.
47 std::string id; // The unique id of the application.
48};
49
50// operator== returns true iff lhs is exactly equal to rhs.
51bool operator==(const AppInfo& lhs, const AppInfo& rhs);
52
40// We wrap the Mir prompt session API into a struct to53// We wrap the Mir prompt session API into a struct to
41// ease with testing and mocking.54// ease with testing and mocking.
42class CORE_TRUST_DLL_PUBLIC PromptSessionVirtualTable55class CORE_TRUST_DLL_PUBLIC PromptSessionVirtualTable
@@ -129,8 +142,8 @@
129 // The pre-authenticated fd that the helper142 // The pre-authenticated fd that the helper
130 // should use for connecting to Mir.143 // should use for connecting to Mir.
131 int fd; 144 int fd;
132 // The application id of the requesting app.145 // Application-specific information goes here.
133 std::string application_id;146 AppInfo app_info;
134 // The extended description that should be presented to the user.147 // The extended description that should be presented to the user.
135 std::string description;148 std::string description;
136 };149 };
@@ -146,6 +159,17 @@
146 CreationArguments creation_arguments;159 CreationArguments creation_arguments;
147};160};
148161
162// An AppNameResolver resolves an application id to a localized application name.
163struct AppInfoResolver
164{
165 // Save us some typing.
166 typedef std::shared_ptr<AppInfoResolver> Ptr;
167
168 virtual ~AppInfoResolver() = default;
169 // resolve maps app_id to a localized application name.
170 virtual AppInfo resolve(const std::string& app_id) = 0;
171};
172
149// Implements the trust::Agent interface and dispatches calls to a helper173// Implements the trust::Agent interface and dispatches calls to a helper
150// prompt provider, tying it together with the requesting service and app174// prompt provider, tying it together with the requesting service and app
151// by leveraging Mir's trusted session/prompting support.175// by leveraging Mir's trusted session/prompting support.
@@ -154,6 +178,20 @@
154 // Convenience typedef178 // Convenience typedef
155 typedef std::shared_ptr<Agent> Ptr;179 typedef std::shared_ptr<Agent> Ptr;
156180
181 // A Configuration bundles creation-time configuration options.
182 struct Configuration
183 {
184 // VTable object providing access to Mir's trusted prompting functionality.
185 ConnectionVirtualTable::Ptr connection_vtable;
186 // Exec helper for starting up prompt provider child processes with the correct setup
187 // of command line arguments and environment variables.
188 PromptProviderHelper::Ptr exec_helper;
189 // A translator function for mapping child process exit states to trust::Request answers.
190 std::function<core::trust::Request::Answer(const core::posix::wait::Result&)> translator;
191 // AppNameResolver used by the agent to map incoming request app ids to application names.
192 AppInfoResolver::Ptr app_info_resolver;
193 };
194
157 // Helper struct for injecting state into on_trust_changed_state_state callbacks.195 // Helper struct for injecting state into on_trust_changed_state_state callbacks.
158 // Used in prompt_user_for_request to wait for the trust session to be stopped.196 // Used in prompt_user_for_request to wait for the trust session to be stopped.
159 struct OnTrustSessionStateChangedCallbackContext197 struct OnTrustSessionStateChangedCallbackContext
@@ -177,26 +215,16 @@
177 // Throws std::logic_error if the process did not exit but was signaled.215 // Throws std::logic_error if the process did not exit but was signaled.
178 static std::function<core::trust::Request::Answer(const core::posix::wait::Result&)> translator_only_accepting_exit_status_success();216 static std::function<core::trust::Request::Answer(const core::posix::wait::Result&)> translator_only_accepting_exit_status_success();
179217
180 // Creates a new MirAgent instance with the given MirConnectionVirtualTable instance.218 // Creates a new MirAgent instance with the given Configuration.
181 Agent(// VTable object providing access to Mir's trusted prompting functionality.219 Agent(const Configuration& config);
182 const ConnectionVirtualTable::Ptr& connection_vtable,
183 // Exec helper for starting up prompt provider child processes with the correct setup
184 // of command line arguments and environment variables.
185 const PromptProviderHelper::Ptr& exec_helper,
186 // A translator function for mapping child process exit states to trust::Request answers.
187 const std::function<core::trust::Request::Answer(const core::posix::wait::Result&)>& translator);
188220
189 // From core::trust::Agent:221 // From core::trust::Agent:
190 // Throws a std::logic_error if anything unforeseen happens during execution, thus222 // Throws a std::logic_error if anything unforeseen happens during execution, thus
191 // indicating that no conclusive answer could be obtained from the user.223 // indicating that no conclusive answer could be obtained from the user.
192 core::trust::Request::Answer authenticate_request_with_parameters(const RequestParameters& parameters) override;224 core::trust::Request::Answer authenticate_request_with_parameters(const RequestParameters& parameters) override;
193225
194 // The connection VTable used for creating trusted prompting sessions.226 // The configured options.
195 ConnectionVirtualTable::Ptr connection_vtable;227 Configuration config;
196 // Execution helper for firing up prompt provider executables.
197 PromptProviderHelper::Ptr exec_helper;
198 // Translator instance.
199 std::function<core::trust::Request::Answer(const core::posix::wait::Result&)> translator;
200};228};
201229
202CORE_TRUST_DLL_PUBLIC bool operator==(const PromptProviderHelper::InvocationArguments&, const PromptProviderHelper::InvocationArguments&);230CORE_TRUST_DLL_PUBLIC bool operator==(const PromptProviderHelper::InvocationArguments&, const PromptProviderHelper::InvocationArguments&);
203231
=== added file 'src/core/trust/mir/click_desktop_entry_app_info_resolver.cpp'
--- src/core/trust/mir/click_desktop_entry_app_info_resolver.cpp 1970-01-01 00:00:00 +0000
+++ src/core/trust/mir/click_desktop_entry_app_info_resolver.cpp 2015-11-30 10:46:27 +0000
@@ -0,0 +1,139 @@
1/*
2 * Copyright © 2015 Canonical Ltd.
3 *
4 * This program is free software: you can redistribute it and/or modify it
5 * under the terms of the GNU Lesser General Public License version 3,
6 * as published by the Free Software Foundation.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU Lesser General Public License for more details.
12 *
13 * You should have received a copy of the GNU Lesser General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 *
16 * Authored by: Thomas Voß <thomas.voss@canonical.com>
17 */
18
19#include <core/trust/mir/click_desktop_entry_app_info_resolver.h>
20
21#include <glib.h>
22#include <xdg.h>
23
24#include <core/posix/this_process.h>
25
26#include <boost/algorithm/string.hpp>
27#include <boost/filesystem.hpp>
28#include <boost/format.hpp>
29
30#include <memory>
31#include <regex>
32#include <string>
33#include <stdexcept>
34#include <vector>
35
36namespace env = core::posix::this_process::env;
37namespace fs = boost::filesystem;
38namespace mir = core::trust::mir;
39
40namespace
41{
42fs::path resolve_desktop_entry_or_throw(const std::string& app_id)
43{
44 auto p = xdg::data().home() / "applications" / (app_id + ".desktop");
45 if (fs::is_regular_file(p))
46 return p;
47
48 fs::path applications{xdg::data().home() / "applications"};
49 fs::directory_iterator it(applications), itE;
50 while (it != itE)
51 {
52 if (it->path().filename().string().find(app_id) == 0)
53 return it->path();
54 ++it;
55 }
56
57 for (auto dir : xdg::data().dirs())
58 {
59 auto p = dir / "applications" / (app_id + ".desktop");
60 if (fs::is_regular_file(p))
61 return p;
62 }
63
64 throw std::runtime_error{"Could not resolve desktop entry for " + app_id};
65}
66
67// Wrap up a GError with an RAII approach, easing
68// cleanup if we throw an exception.
69struct Error
70{
71 ~Error() { clear(); }
72 void clear() { g_clear_error(&error); }
73
74 GError* error = nullptr;
75};
76
77std::string name_from_desktop_entry_or_throw(const fs::path& fn)
78{
79 Error g;
80 std::shared_ptr<GKeyFile> key_file{g_key_file_new(), [](GKeyFile* file) { if (file) g_key_file_free(file); }};
81
82 if (not g_key_file_load_from_file(key_file.get(), fn.string().c_str(), G_KEY_FILE_NONE, &g.error)) throw std::runtime_error
83 {
84 "Failed to load desktop entry [" + std::string(g.error->message) + "]"
85 };
86
87 auto app_name = g_key_file_get_locale_string(key_file.get(), G_KEY_FILE_DESKTOP_GROUP, G_KEY_FILE_DESKTOP_KEY_NAME, nullptr, &g.error);
88
89 if (g.error) throw std::runtime_error
90 {
91 "Failed to query localized name [" + std::string(g.error->message) + "]"
92 };
93
94 return app_name;
95}
96
97std::string icon_from_desktop_entry_or_throw(const fs::path& fn)
98{
99 Error g;
100 std::shared_ptr<GKeyFile> key_file{g_key_file_new(), [](GKeyFile* file) { if (file) g_key_file_free(file); }};
101
102 if (not g_key_file_load_from_file(key_file.get(), fn.string().c_str(), G_KEY_FILE_NONE, &g.error)) throw std::runtime_error
103 {
104 "Failed to load desktop entry [" + std::string(g.error->message) + "]"
105 };
106
107 auto app_icon = g_key_file_get_locale_string(key_file.get(), G_KEY_FILE_DESKTOP_GROUP, G_KEY_FILE_DESKTOP_KEY_ICON, nullptr, &g.error);
108
109 if (g.error) throw std::runtime_error
110 {
111 "Failed to query icon [" + std::string(g.error->message) + "]"
112 };
113
114 // We expect an absolute path to a regular file representing the icon. Ideally, we should
115 // also run further checks like the file not being part of another app for example.
116 fs::path p{app_icon};
117 if (not p.is_absolute() || not fs::is_regular_file(fs::status(p))) throw std::runtime_error
118 {
119 "Icon path is either not absolute or not pointing to a regular file [" + std::string{app_icon} + "]"
120 };
121
122 return app_icon;
123}
124}
125
126mir::ClickDesktopEntryAppInfoResolver::ClickDesktopEntryAppInfoResolver()
127{
128}
129
130mir::AppInfo mir::ClickDesktopEntryAppInfoResolver::resolve(const std::string& app_id)
131{
132 auto de = resolve_desktop_entry_or_throw(app_id);
133 return mir::AppInfo
134 {
135 icon_from_desktop_entry_or_throw(de),
136 name_from_desktop_entry_or_throw(de),
137 app_id
138 };
139}
0140
=== added file 'src/core/trust/mir/click_desktop_entry_app_info_resolver.h'
--- src/core/trust/mir/click_desktop_entry_app_info_resolver.h 1970-01-01 00:00:00 +0000
+++ src/core/trust/mir/click_desktop_entry_app_info_resolver.h 2015-11-30 10:46:27 +0000
@@ -0,0 +1,49 @@
1/*
2 * Copyright © 2015 Canonical Ltd.
3 *
4 * This program is free software: you can redistribute it and/or modify it
5 * under the terms of the GNU Lesser General Public License version 3,
6 * as published by the Free Software Foundation.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU Lesser General Public License for more details.
12 *
13 * You should have received a copy of the GNU Lesser General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 *
16 * Authored by: Thomas Voß <thomas.voss@canonical.com>
17 */
18
19#ifndef CORE_TRUST_MIR_CLICK_DESKTOP_ENTRY_APP_NAME_RESOLVER_H_
20#define CORE_TRUST_MIR_CLICK_DESKTOP_ENTRY_APP_NAME_RESOLVER_H_
21
22#include <core/trust/mir/agent.h>
23
24namespace core
25{
26namespace trust
27{
28namespace mir
29{
30// A ClickDesktopEntryAppNameResolver queries the click database of installed
31// packages to resolve an app's installation folder in the local filesystem.
32// The directory is searched for a .desktop file, that is then loaded and queried
33// for the app's localized name.
34class CORE_TRUST_DLL_PUBLIC ClickDesktopEntryAppInfoResolver : public AppInfoResolver
35{
36public:
37 // ClickDesktopEntryAppNameResolver sets up an instance with default dbs.
38 ClickDesktopEntryAppInfoResolver();
39
40 // resolve queries the click index and an apps desktop file entry for
41 // obtaining a localized application name. Throws std::runtime_error in
42 // case of issues.
43 AppInfo resolve(const std::string& app_id) override;
44};
45}
46}
47}
48
49#endif // CORE_TRUST_MIR_CLICK_DESKTOP_ENTRY_APP_NAME_RESOLVER_H_
050
=== modified file 'src/core/trust/mir/prompt_main.cpp'
--- src/core/trust/mir/prompt_main.cpp 2015-08-31 13:16:20 +0000
+++ src/core/trust/mir/prompt_main.cpp 2015-11-30 10:46:27 +0000
@@ -74,9 +74,19 @@
74{74{
75 // We are throwing exceptions here, which immediately calls abort and still gives a75 // We are throwing exceptions here, which immediately calls abort and still gives a
76 // helpful error message on the terminal.76 // helpful error message on the terminal.
77 if (vm.count(cli::option_title) == 0) throw std::logic_error77 if (vm.count(cli::option_icon) == 0) throw std::logic_error
78 {78 {
79 "Missing option title."79 "Missing option icon."
80 };
81
82 if (vm.count(cli::option_name) == 0) throw std::logic_error
83 {
84 "Missing option name."
85 };
86
87 if (vm.count(cli::option_id) == 0) throw std::logic_error
88 {
89 "Missing option id."
80 };90 };
8191
82 if (vm.count(cli::option_description) == 0) throw std::logic_error92 if (vm.count(cli::option_description) == 0) throw std::logic_error
@@ -84,17 +94,13 @@
84 "Missing option description"94 "Missing option description"
85 };95 };
8696
87 if (vm.count(cli::option_server_socket) == 0) throw std::logic_error97 if (vm.count(cli::option_server_socket) > 0)
88 {98 {
89 "Missing option mir_server_socket"99 if (vm[cli::option_server_socket].as<std::string>().find("fd://") != 0) throw std::logic_error
90 };100 {
91101 "mir_server_socket does not being with fd://"
92 std::string mir_server_socket = vm[cli::option_server_socket].as<std::string>();102 };
93103 }
94 if (mir_server_socket.find("fd://") != 0) throw std::logic_error
95 {
96 "mir_server_socket does not being with fd://"
97 };
98}104}
99}105}
100}106}
@@ -104,8 +110,10 @@
104 boost::program_options::options_description options;110 boost::program_options::options_description options;
105 options.add_options()111 options.add_options()
106 (cli::option_server_socket, boost::program_options::value<std::string>(), "Mir server socket to connect to.")112 (cli::option_server_socket, boost::program_options::value<std::string>(), "Mir server socket to connect to.")
113 (cli::option_icon, boost::program_options::value<std::string>(), "Icon of the requesting application.")
114 (cli::option_name, boost::program_options::value<std::string>(), "Name of the requesting application.")
115 (cli::option_id, boost::program_options::value<std::string>(), "Id of the requesting application.")
107 (cli::option_description, boost::program_options::value<std::string>(), "Extended description of the prompt.")116 (cli::option_description, boost::program_options::value<std::string>(), "Extended description of the prompt.")
108 (cli::option_title, boost::program_options::value<std::string>(), "Title of the prompt.")
109 (cli::option_testing, "Only checks command-line parameters and does not execute any actions.")117 (cli::option_testing, "Only checks command-line parameters and does not execute any actions.")
110 (cli::option_testability, "Loads the Qt Testability plugin if provided.");118 (cli::option_testability, "Loads the Qt Testability plugin if provided.");
111119
@@ -124,30 +132,29 @@
124 boost::program_options::store(parsed_options, vm);132 boost::program_options::store(parsed_options, vm);
125 boost::program_options::notify(vm);133 boost::program_options::notify(vm);
126134
135 // We immediately bail out if verification of command line arguments fails.
136 testing::validate_command_line_arguments(vm);
137
127 // We just verify command line arguments in testing and immediately return.138 // We just verify command line arguments in testing and immediately return.
128 if (vm.count(cli::option_testing) > 0)139 if (vm.count(cli::option_testing) > 0)
129 {140 return 0;
130 testing::validate_command_line_arguments(vm); return 0;141
131 }142 auto icon = vm[cli::option_icon].as<std::string>();
132143 auto name = vm[cli::option_name].as<std::string>();
133 // Let's validate the arguments.144 auto id = vm[cli::option_id].as<std::string>();
134 if (vm.count(cli::option_title) == 0)145 // As per design, we replace "_" in app ids with a "/", thereby
135 abort(); // We have to call abort here to make sure that we get signalled.146 // rendering the app id a little nicer. See:
136147 //
137 std::string title = vm[cli::option_title].as<std::string>();148 // http://pasteboard.co/2qhB6op7.png
138149 //
139 std::string description;150 // for an example.
140 if (vm.count(cli::option_description) > 0)151 std::replace(id.begin(), id.end(), '_', '/');
141 description = vm[cli::option_description].as<std::string>();152 auto description = vm[cli::option_description].as<std::string>();
142153
143 if (vm.count(cli::option_server_socket) > 0)154 if (vm.count(cli::option_server_socket) > 0)
144 {155 {
145 // We make sure that the env variable is not set prior to adusting it.
146 this_env::unset_or_throw(env::option_mir_socket);156 this_env::unset_or_throw(env::option_mir_socket);
147157 this_env::set_or_throw(env::option_mir_socket, vm[cli::option_server_socket].as<std::string>());
148 this_env::set_or_throw(
149 env::option_mir_socket,
150 vm[cli::option_server_socket].as<std::string>());
151 }158 }
152159
153 // We install our default gettext domain prior to anything qt.160 // We install our default gettext domain prior to anything qt.
@@ -202,14 +209,10 @@
202 view->setTitle(QGuiApplication::applicationName());209 view->setTitle(QGuiApplication::applicationName());
203210
204 // Make some properties known to the root context.211 // Make some properties known to the root context.
205 // The title of the dialog.212 view->rootContext()->setContextProperty(cli::option_icon, icon.c_str());
206 view->rootContext()->setContextProperty(213 view->rootContext()->setContextProperty(cli::option_name, name.c_str());
207 cli::option_title,214 view->rootContext()->setContextProperty(cli::option_id, id.c_str());
208 title.c_str());215 view->rootContext()->setContextProperty(cli::option_description, description.c_str());
209 // The description of the dialog.
210 view->rootContext()->setContextProperty(
211 cli::option_description,
212 description.c_str());
213216
214 // Point the engine to the right directory. Please note that217 // Point the engine to the right directory. Please note that
215 // the respective value changes with the installation state.218 // the respective value changes with the installation state.
216219
=== modified file 'src/core/trust/mir/prompt_main.h'
--- src/core/trust/mir/prompt_main.h 2015-02-11 13:04:24 +0000
+++ src/core/trust/mir/prompt_main.h 2015-11-30 10:46:27 +0000
@@ -46,10 +46,22 @@
46 "mir_server_socket"46 "mir_server_socket"
47};47};
4848
49/** @brief Title of the prompt. */49/** @brief Icon of the requesting app. */
50static constexpr const char* option_title50static constexpr const char* option_icon
51{51{
52 "title"52 "icon"
53};
54
55/** @brief Name of the requesting app. */
56static constexpr const char* option_name
57{
58 "name"
59};
60
61/** @brief Id of the requesting app. */
62static constexpr const char* option_id
63{
64 "id"
53};65};
5466
55/** @brief Extended description of the prompt. */67/** @brief Extended description of the prompt. */
5668
=== modified file 'src/core/trust/mir/prompt_main.qml'
--- src/core/trust/mir/prompt_main.qml 2014-11-06 10:38:33 +0000
+++ src/core/trust/mir/prompt_main.qml 2015-11-30 10:46:27 +0000
@@ -15,40 +15,76 @@
15 */15 */
1616
17import QtQuick 2.017import QtQuick 2.0
18import Ubuntu.Components 1.118import Ubuntu.Components 1.3
19import Ubuntu.Components.Popups 1.019import Ubuntu.Components.Popups 1.3
2020
21Rectangle {21Item {
22 width: units.gu(40)22
23 height: units.gu(71)23 property var appIcon: icon
24 color: "transparent"24 property var appName: name
2525 property var appId: id
26 property var dialogTitle: title26 property var serviceDescription: description
27 property var dialogDescription: description
2827
29 signal quit(int code)28 signal quit(int code)
3029
31 Component.onCompleted: dialog.show();30 Component {
32
33 Dialog {
34 id: dialog31 id: dialog
3532 Dialog {
36 title: dialogTitle33 id: dialogue
37 text: dialogDescription34 Column {
38 fadingAnimation: PropertyAnimation { duration: 0 }35 spacing: units.gu(0.5)
3936 UbuntuShape {
40 Button {37 anchors.horizontalCenter: parent.horizontalCenter
41 objectName: "deny"38 id: iconShape
42 text: i18n.tr("Deny")39 radius: "medium"
43 color: UbuntuColors.red40 aspect: UbuntuShape.DropShadow
44 onClicked: quit(1)41 anchors.margins: units.gu(1)
45 }42 sourceFillMode: UbuntuShape.PreserveAspectCrop
4643 source: Image {
47 Button {44 id: icon
48 objectName: "allow"45 sourceSize.width: iconShape.width
49 text: i18n.tr("Allow")46 sourceSize.height: iconShape.height
50 color: UbuntuColors.green47 source: appIcon
51 onClicked: quit(0)48 }
49 }
50 Label {
51 anchors.horizontalCenter: parent.horizontalCenter
52 text: appName
53 horizontalAlignment: Text.AlignHCenter
54 width: parent.width
55 elide: Text.ElideRight
56 wrapMode: Text.Wrap
57 maximumLineCount: 2
58 }
59 Label {
60 anchors.horizontalCenter: parent.horizontalCenter
61 text: appId
62 color: UbuntuColors.lightGrey
63 fontSize: "small"
64 width: parent.width
65 horizontalAlignment: Text.AlignHCenter
66 elide: Text.ElideMiddle
67 maximumLineCount: 1
68 }
69 }
70 Label {
71 anchors.horizontalCenter: parent.horizontalCenter
72 text: serviceDescription
73 horizontalAlignment: Text.AlignHCenter
74 width: parent.width
75 wrapMode: Text.Wrap
76 }
77 Button {
78 text: i18n.tr("Allow")
79 color: UbuntuColors.green
80 onClicked: quit(0)
81 }
82 Button {
83 text: i18n.tr("Don’t Allow")
84 color: UbuntuColors.lightGrey
85 onClicked: quit(1)
86 }
52 }87 }
53 }88 }
89 Component.onCompleted: PopupUtils.open(dialog)
54}90}
5591
=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt 2015-11-30 10:46:27 +0000
+++ tests/CMakeLists.txt 2015-11-30 10:46:27 +0000
@@ -242,6 +242,11 @@
242 mir_agent_test.cpp242 mir_agent_test.cpp
243 )243 )
244244
245 add_executable(
246 click_desktop_entry_app_name_resolver_test
247 click_desktop_entry_app_name_resolver_test.cpp
248 )
249
245 target_link_libraries(250 target_link_libraries(
246 mir_agent_test251 mir_agent_test
247252
@@ -256,7 +261,22 @@
256 ${PROCESS_CPP_LIBRARIES}261 ${PROCESS_CPP_LIBRARIES}
257 )262 )
258263
264 target_link_libraries(
265 click_desktop_entry_app_name_resolver_test
266
267 trust-store
268
269 gmock
270
271 gtest
272 gtest_main
273
274 ${PROCESS_CPP_LIBRARIES}
275 )
276
277
259 add_test(mir_agent_test ${CMAKE_CURRENT_BINARY_DIR}/mir_agent_test --gtest_filter=*-*requires_mir)278 add_test(mir_agent_test ${CMAKE_CURRENT_BINARY_DIR}/mir_agent_test --gtest_filter=*-*requires_mir)
279 add_test(click_desktop_entry_app_name_resolver_test ${CMAKE_CURRENT_BINARY_DIR}/click_desktop_entry_app_name_resolver_test)
260280
261 install(281 install(
262 TARGETS mir_agent_test282 TARGETS mir_agent_test
263283
=== added file 'tests/click_desktop_entry_app_name_resolver_test.cpp'
--- tests/click_desktop_entry_app_name_resolver_test.cpp 1970-01-01 00:00:00 +0000
+++ tests/click_desktop_entry_app_name_resolver_test.cpp 2015-11-30 10:46:27 +0000
@@ -0,0 +1,75 @@
1/*
2 * Copyright © 2013 Canonical Ltd.
3 *
4 * This program is free software: you can redistribute it and/or modify it
5 * under the terms of the GNU Lesser General Public License version 3,
6 * as published by the Free Software Foundation.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU Lesser General Public License for more details.
12 *
13 * You should have received a copy of the GNU Lesser General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 *
16 * Authored by: Thomas Voß <thomas.voss@canonical.com>
17 */
18
19#include <core/trust/mir/click_desktop_entry_app_info_resolver.h>
20#include <core/posix/this_process.h>
21#include "test_data.h"
22
23#include <gtest/gtest.h>
24
25#include <fstream>
26
27namespace env = core::posix::this_process::env;
28namespace mir = core::trust::mir;
29
30namespace
31{
32bool ensure_icon_file()
33{
34 std::ofstream out{"/tmp/test.icon"};
35 out << "test.icon";
36 return out.good();
37}
38}
39
40TEST(ClickDesktopEntryAppInfoResolver, throws_for_invalid_app_id)
41{
42 ASSERT_TRUE(ensure_icon_file());
43 mir::ClickDesktopEntryAppInfoResolver resolver;
44 EXPECT_THROW(resolver.resolve("something:not:right"), std::runtime_error);
45}
46
47TEST(ClickDesktopEntryAppInfoResolver, resolves_existing_desktop_entry_from_xdg_data_home)
48{
49 ASSERT_TRUE(ensure_icon_file());
50 env::unset_or_throw("XDG_DATA_HOME");
51 env::set_or_throw("XDG_DATA_HOME", core::trust::testing::current_source_dir + std::string("/share"));
52 mir::ClickDesktopEntryAppInfoResolver resolver;
53 EXPECT_NO_THROW(resolver.resolve("valid.pkg_app_0.0.0"));
54}
55
56TEST(ClickDesktopEntryAppInfoResolver, robustly_resolves_existing_desktop_entry_from_xdg_data_home)
57{
58 ASSERT_TRUE(ensure_icon_file());
59 env::unset_or_throw("XDG_DATA_HOME");
60 env::set_or_throw("XDG_DATA_HOME", core::trust::testing::current_source_dir + std::string("/share"));
61
62 mir::ClickDesktopEntryAppInfoResolver resolver;
63 EXPECT_NO_THROW(resolver.resolve("valid.pkg_app"));
64}
65
66TEST(ClickDesktopEntryAppInfoResolver, resolves_existing_desktop_entry_from_xdg_data_dirs)
67{
68 ASSERT_TRUE(ensure_icon_file());
69 env::unset_or_throw("XDG_DATA_HOME"); env::unset_or_throw("XDG_DATA_DIRS");
70 env::set_or_throw("XDG_DATA_HOME", core::trust::testing::current_source_dir + std::string("/empty"));
71 env::set_or_throw("XDG_DATA_DIRS", core::trust::testing::current_source_dir + std::string("/share"));
72
73 mir::ClickDesktopEntryAppInfoResolver resolver;
74 resolver.resolve("valid.pkg_app_0.0.0");
75}
076
=== added directory 'tests/empty'
=== added directory 'tests/empty/applications'
=== modified file 'tests/mir_agent_test.cpp'
--- tests/mir_agent_test.cpp 2015-11-30 10:46:27 +0000
+++ tests/mir_agent_test.cpp 2015-11-30 10:46:27 +0000
@@ -99,6 +99,11 @@
99 MOCK_METHOD1(translate, core::trust::Request::Answer(const core::posix::wait::Result&));99 MOCK_METHOD1(translate, core::trust::Request::Answer(const core::posix::wait::Result&));
100};100};
101101
102struct MockAppInfoResolver : public core::trust::mir::AppInfoResolver
103{
104 MOCK_METHOD1(resolve, core::trust::mir::AppInfo(const std::string&));
105};
106
102std::shared_ptr<MockConnectionVirtualTable> a_mocked_connection_vtable()107std::shared_ptr<MockConnectionVirtualTable> a_mocked_connection_vtable()
103{108{
104 return std::make_shared<testing::NiceMock<MockConnectionVirtualTable>>();109 return std::make_shared<testing::NiceMock<MockConnectionVirtualTable>>();
@@ -117,6 +122,11 @@
117 "/bin/false"122 "/bin/false"
118 });123 });
119}124}
125
126std::shared_ptr<MockAppInfoResolver> a_mocked_app_info_resolver()
127{
128 return std::make_shared<MockAppInfoResolver>();
129}
120}130}
121131
122TEST(DefaultProcessStateTranslator, throws_for_signalled_process)132TEST(DefaultProcessStateTranslator, throws_for_signalled_process)
@@ -172,7 +182,11 @@
172 core::trust::mir::PromptProviderHelper::InvocationArguments iargs182 core::trust::mir::PromptProviderHelper::InvocationArguments iargs
173 {183 {
174 42,184 42,
175 "does.not.exist.application",185 {
186 "/tmp",
187 "Does not exist",
188 "does.not.exist.application"
189 },
176 "Just an extended description for %1%"190 "Just an extended description for %1%"
177 };191 };
178192
@@ -193,6 +207,8 @@
193 using namespace ::testing;207 using namespace ::testing;
194208
195 const core::trust::Pid app_pid {21};209 const core::trust::Pid app_pid {21};
210 const std::string app_icon{"/tmp"};
211 const std::string app_name {"Does not exist"};
196 const std::string app_id {"does.not.exist.application"};212 const std::string app_id {"does.not.exist.application"};
197 const core::trust::Feature feature{42};213 const core::trust::Feature feature{42};
198 const std::string app_description {"This is just an extended description %1%"};214 const std::string app_description {"This is just an extended description %1%"};
@@ -201,7 +217,11 @@
201 const core::trust::mir::PromptProviderHelper::InvocationArguments reference_invocation_args217 const core::trust::mir::PromptProviderHelper::InvocationArguments reference_invocation_args
202 {218 {
203 pre_authenticated_fd,219 pre_authenticated_fd,
204 app_id,220 {
221 app_icon,
222 app_name,
223 app_id
224 },
205 app_description225 app_description
206 };226 };
207227
@@ -209,6 +229,7 @@
209 auto prompt_session_vtable = a_mocked_prompt_session_vtable();229 auto prompt_session_vtable = a_mocked_prompt_session_vtable();
210230
211 auto prompt_provider_exec_helper = a_mocked_prompt_provider_calling_bin_false();231 auto prompt_provider_exec_helper = a_mocked_prompt_provider_calling_bin_false();
232 auto app_info_resolver = a_mocked_app_info_resolver();
212233
213 ON_CALL(*connection_vtable, create_prompt_session_sync(_, _, _))234 ON_CALL(*connection_vtable, create_prompt_session_sync(_, _, _))
214 .WillByDefault(Return(prompt_session_vtable));235 .WillByDefault(Return(prompt_session_vtable));
@@ -219,6 +240,9 @@
219 ON_CALL(*prompt_session_vtable, add_prompt_provider_sync(_))240 ON_CALL(*prompt_session_vtable, add_prompt_provider_sync(_))
220 .WillByDefault(Return(true));241 .WillByDefault(Return(true));
221242
243 ON_CALL(*app_info_resolver, resolve(_))
244 .WillByDefault(Return(core::trust::mir::AppInfo{app_icon, app_name, app_id}));
245
222 EXPECT_CALL(*connection_vtable, create_prompt_session_sync(app_pid, _, _)).Times(1);246 EXPECT_CALL(*connection_vtable, create_prompt_session_sync(app_pid, _, _)).Times(1);
223 EXPECT_CALL(*prompt_session_vtable, new_fd_for_prompt_provider()).Times(1);247 EXPECT_CALL(*prompt_session_vtable, new_fd_for_prompt_provider()).Times(1);
224248
@@ -228,21 +252,94 @@
228252
229 core::trust::mir::Agent agent253 core::trust::mir::Agent agent
230 {254 {
231 connection_vtable,255 core::trust::mir::Agent::Configuration
232 prompt_provider_exec_helper,256 {
233 core::trust::mir::Agent::translator_only_accepting_exit_status_success()257 connection_vtable,
234 };258 prompt_provider_exec_helper,
235259 core::trust::mir::Agent::translator_only_accepting_exit_status_success(),
236 EXPECT_EQ(core::trust::Request::Answer::denied, // /bin/false exits with failure.260 app_info_resolver
237 agent.authenticate_request_with_parameters(261 }
238 core::trust::Agent::RequestParameters262 };
239 {263
240 core::trust::Uid{::getuid()},264 EXPECT_EQ(core::trust::Request::Answer::denied, // /bin/false exits with failure.
241 app_pid,265 agent.authenticate_request_with_parameters(
242 app_id,266 core::trust::Agent::RequestParameters
243 feature,267 {
244 app_description268 core::trust::Uid{::getuid()},
245 }));269 app_pid,
270 app_id,
271 feature,
272 app_description
273 }));
274}
275
276TEST(MirAgent, calls_into_app_info_resolver_with_app_id)
277{
278 using namespace ::testing;
279
280 const core::trust::Pid app_pid {21};
281 const std::string app_id {"does.not.exist.application"};
282 const std::string app_icon{"/tmp"};
283 const std::string app_name{"MeMyselfAndI"};
284 const core::trust::Feature feature{42};
285 const std::string app_description {"This is just an extended description for %1%"};
286 const int pre_authenticated_fd {42};
287
288 const core::trust::mir::PromptProviderHelper::InvocationArguments reference_invocation_args
289 {
290 pre_authenticated_fd,
291 {
292 app_icon,
293 app_name,
294 app_id
295 },
296 app_description
297 };
298
299 auto connection_vtable = a_mocked_connection_vtable();
300 auto prompt_session_vtable = a_mocked_prompt_session_vtable();
301
302 auto prompt_provider_exec_helper = a_mocked_prompt_provider_calling_bin_false();
303 auto app_info_resolver = a_mocked_app_info_resolver();
304
305 ON_CALL(*connection_vtable, create_prompt_session_sync(_, _, _))
306 .WillByDefault(Return(prompt_session_vtable));
307
308 ON_CALL(*prompt_session_vtable, new_fd_for_prompt_provider())
309 .WillByDefault(Return(pre_authenticated_fd));
310
311 ON_CALL(*prompt_session_vtable, add_prompt_provider_sync(_))
312 .WillByDefault(Return(true));
313
314 EXPECT_CALL(*app_info_resolver, resolve(app_id)).Times(1)
315 .WillRepeatedly(
316 Return(core::trust::mir::AppInfo{app_icon, app_name, app_id}));
317 EXPECT_CALL(*prompt_provider_exec_helper,
318 exec_prompt_provider_with_arguments(
319 reference_invocation_args)).Times(1);
320
321 core::trust::mir::Agent agent
322 {
323 core::trust::mir::Agent::Configuration
324 {
325 connection_vtable,
326 prompt_provider_exec_helper,
327 core::trust::mir::Agent::translator_only_accepting_exit_status_success(),
328 app_info_resolver
329 }
330 };
331
332 EXPECT_EQ(core::trust::Request::Answer::denied, // /bin/false exits with failure.
333 agent.authenticate_request_with_parameters(
334 core::trust::Agent::RequestParameters
335 {
336 core::trust::Uid{::getuid()},
337 app_pid,
338 app_id,
339 feature,
340 app_description
341 }));
342
246}343}
247344
248TEST(MirAgent, sig_kills_prompt_provider_process_on_status_change)345TEST(MirAgent, sig_kills_prompt_provider_process_on_status_change)
@@ -250,6 +347,8 @@
250 using namespace ::testing;347 using namespace ::testing;
251348
252 const core::trust::Pid app_pid {21};349 const core::trust::Pid app_pid {21};
350 const std::string app_icon{"/tmp"};
351 const std::string app_name{"MeMyselfAndI"};
253 const std::string app_id {"does.not.exist.application"};352 const std::string app_id {"does.not.exist.application"};
254 const core::trust::Feature feature{42};353 const core::trust::Feature feature{42};
255 const std::string app_description {"This is just an extended description for %1%"};354 const std::string app_description {"This is just an extended description for %1%"};
@@ -270,6 +369,8 @@
270 auto prompt_provider_helper = std::make_shared<MockPromptProviderHelper>(369 auto prompt_provider_helper = std::make_shared<MockPromptProviderHelper>(
271 core::trust::mir::PromptProviderHelper::CreationArguments{"/bin/false"});370 core::trust::mir::PromptProviderHelper::CreationArguments{"/bin/false"});
272371
372 auto app_info_resolver = a_mocked_app_info_resolver();
373
273 void* prompt_session_state_callback_context{nullptr};374 void* prompt_session_state_callback_context{nullptr};
274375
275 ON_CALL(*prompt_provider_helper, exec_prompt_provider_with_arguments(_))376 ON_CALL(*prompt_provider_helper, exec_prompt_provider_with_arguments(_))
@@ -289,6 +390,9 @@
289 Return(390 Return(
290 true));391 true));
291392
393 ON_CALL(*app_info_resolver, resolve(_))
394 .WillByDefault(Return(core::trust::mir::AppInfo{app_icon, app_name, app_id}));
395
292 // An invocation results in a session being created. In addition,396 // An invocation results in a session being created. In addition,
293 // we store pointers to callback and context provided by the implementation397 // we store pointers to callback and context provided by the implementation
294 // for being able to later trigger the callback.398 // for being able to later trigger the callback.
@@ -300,9 +404,13 @@
300404
301 core::trust::mir::Agent agent405 core::trust::mir::Agent agent
302 {406 {
303 connection_vtable,407 core::trust::mir::Agent::Configuration
304 prompt_provider_helper,408 {
305 core::trust::mir::Agent::translator_only_accepting_exit_status_success()409 connection_vtable,
410 prompt_provider_helper,
411 core::trust::mir::Agent::translator_only_accepting_exit_status_success(),
412 app_info_resolver
413 }
306 };414 };
307415
308 std::thread asynchronously_stop_the_prompting_session416 std::thread asynchronously_stop_the_prompting_session
@@ -416,7 +524,9 @@
416 std::vector<std::string> argv524 std::vector<std::string> argv
417 {525 {
418 "--" + std::string{core::trust::mir::cli::option_server_socket} + "=" + mir_socket(),526 "--" + std::string{core::trust::mir::cli::option_server_socket} + "=" + mir_socket(),
419 "--" + std::string{core::trust::mir::cli::option_title} + "=" + pretty_function,527 "--" + std::string{core::trust::mir::cli::option_icon}, pretty_function,
528 "--" + std::string{core::trust::mir::cli::option_name}, pretty_function,
529 "--" + std::string{core::trust::mir::cli::option_id}, pretty_function,
420 "--" + std::string{core::trust::mir::cli::option_description} + "=" + pretty_function,530 "--" + std::string{core::trust::mir::cli::option_description} + "=" + pretty_function,
421 // We have to circumvent unity8's authentication mechanism and just provide531 // We have to circumvent unity8's authentication mechanism and just provide
422 // the desktop_file_hint as part of the command line.532 // the desktop_file_hint as part of the command line.
423533
=== added directory 'tests/share'
=== added directory 'tests/share/applications'
=== added file 'tests/share/applications/valid.pkg_app_0.0.0.desktop'
--- tests/share/applications/valid.pkg_app_0.0.0.desktop 1970-01-01 00:00:00 +0000
+++ tests/share/applications/valid.pkg_app_0.0.0.desktop 2015-11-30 10:46:27 +0000
@@ -0,0 +1,56 @@
1[Desktop Entry]
2Type=Application
3Icon=/tmp/test.icon
4Name=Camera
5Name[am]=ካሜራ
6Name[ar]=الكاميرا
7Name[ast]=Cámara
8Name[az]=Kamera
9Name[bg]=Камера
10Name[br]=Kamera
11Name[bs]=Kamera
12Name[ca]=Càmera
13Name[ca@valencia]=Càmera
14Name[cs]=Fotoaparát
15Name[cy]=Camera
16Name[da]=Kamera
17Name[de]=Kamera
18Name[el]=Κάμερα
19Name[en_AU]=Camera
20Name[en_GB]=Camera
21Name[es]=Cámara
22Name[eu]=Kamera
23Name[fa]=دوربین
24Name[fi]=Kamera
25Name[fr]=Appareil photo
26Name[gd]=Camara
27Name[gl]=Cámara
28Name[he]=מצלמה
29Name[hi]=कैमरा
30Name[hr]=Fotoaparat
31Name[hu]=Kamera
32Name[hy]=Կամերա
33Name[is]=Myndavél
34Name[it]=Fotocamera
35Name[ja]=カメラ
36Name[km]=ម៉ាស៊ីន<U+200B>ថ<U+200B>ត
37Name[ko]=카메라
38Name[lo]=ກ້ອງ
39Name[lv]=Fotokamera
40Name[ml]=ക്യാമറ
41Name[ms]=Kamera
42Name[my]=ကင<U+103A>မရာ
43Name[nb]=Kamera
44Name[nl]=Camera
45Name[pa]=ਕੈਮਰਾ
46Name[pl]=Aparat
47Name[pt]=Câmara
48Name[pt_BR]=Câmera
49Name[ro]=Aparat foto
50Name[ru]=Камера
51Name[sl]=Fotoaparat
52Name[sr]=Камера
53Name[sv]=Kamera
54Name[ta]=புகைப்பட கருவி
55Name[te]=కెమేరా
56Name[tr]=Fotoğraf Makinesi
057
=== modified file 'tests/test_data.h.in'
--- tests/test_data.h.in 2014-08-12 14:25:41 +0000
+++ tests/test_data.h.in 2015-11-30 10:46:27 +0000
@@ -25,6 +25,11 @@
25{25{
26namespace testing26namespace testing
27{27{
28static constexpr const char* current_source_dir
29{
30 "@CMAKE_CURRENT_SOURCE_DIR@"
31};
32
28static constexpr const char* trust_prompt_executable_in_build_dir33static constexpr const char* trust_prompt_executable_in_build_dir
29{34{
30 "@CMAKE_BINARY_DIR@/src/trust-prompt"35 "@CMAKE_BINARY_DIR@/src/trust-prompt"

Subscribers

People subscribed via source and target branches