Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FdoSecrets: warning when client executable path can't be determined #6459

Closed
michaelk83 opened this issue Apr 28, 2021 · 1 comment · Fixed by #6915
Closed

FdoSecrets: warning when client executable path can't be determined #6459

michaelk83 opened this issue Apr 28, 2021 · 1 comment · Fixed by #6915

Comments

@michaelk83
Copy link

michaelk83 commented Apr 28, 2021

When an FdoSecrets client requests access to the DB, KPXC attempts to determine its executable path for authorization by the user. Sometimes, the executable path can't be determined (e.g. when /proc/$pid/exe is inaccessible). In that case:

Current Behavior

The executable path is not displayed in the authorization prompt (based on the PR discussions; I haven't tested in practice).

If anything goes wrong no exe name is presented to the user to prevent a false sense of security.

Expected Behavior

Show a bright warning instead of the missing executable path: "Client executable path could not be determined."

Context

A missing executable path is easy to miss in a hurry. A bright warning would draw user attention.

A counter-argument is that providing this warning may induce a false sense of security when the executable path is determined successfully. I'm not sure which is the lesser evil. But I would argue that the whole point of these authorization prompts is to detect when a suspicious process attempts to access the DB. If the user isn't aware that the executable path can be spoofed, they already provide a false sense of security as it is.

Displaying a warning in both cases (path determined vs undetermined) wouldn't be any better then not displaying them at all: the two cases would appear too similar to an inattentive user.

This may make more sense together with client fingerprinting (#6458). If we can't determine the executable path, we can't obtain its fingerprint.

Steps to Reproduce

  1. Create a test client whose /proc/$pid/exe is inaccessible, e.g. by calling prctl(PR_SET_DUMPABLE, 0).
  2. In KPXC, enable Secret Service integration and access prompts.
  3. Create a secret for this client in KPXC.
  4. Attempt to access the secret with the client. (KPXC should display an access prompt but fail to read the client's /proc/$pid/exe).

Alternative:
Same as above, but instead of creating a special test client, add temporary debug code that pretends the /proc/$pid/exe is inaccessible.

Extra Info

Operating system: Linux
KPXC version: 2.7.0
(Reporting based on the PR discussions only.)

@michaelk83
Copy link
Author

Hmm, come to think of it, since these prompts were added in #5747 , which is marked for 2.7.0, that would make this a pre-release bug, wouldn't it?

@droidmonkey droidmonkey modified the milestone: v2.7.0 May 2, 2021
@Aetf Aetf self-assigned this Sep 14, 2021
droidmonkey added a commit that referenced this issue Oct 1, 2021
* Fixes #6459 

Improves the overall handling of FdoSecrets showing client executable paths to the user. It does the following:

* Check executable file existence as described in [RFC] fdosecrets: add optional confirmation to secret access (#4733)
* Show application PID and dbus address in the client list
* When the executable file is inaccessible, depending on where the client name is shown:
    * when shown inline, e.g. in notification text, where space is limited, clearly say that the path is invalid
    * when shown in auth dialog, show warning and print detailed info about the client
    * when shown in the client list, draw a warning icon

Co-authored-by: Jonathan White <support@dmapps.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants