Skip to content

FdoSecrets: remember client authorizations across restarts - #13610

Draft
Aetf wants to merge 12 commits into
keepassxreboot:developfrom
Aetf:kpxc-fingerprint
Draft

FdoSecrets: remember client authorizations across restarts#13610
Aetf wants to merge 12 commits into
keepassxreboot:developfrom
Aetf:kpxc-fingerprint

Conversation

@Aetf

@Aetf Aetf commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Closes #6458.

Note

Draft: the first four commits are the pending stack of #13598 and #13602. This branch will be rebased once those merge; only the eight FdoSecrets: commits from "add persisted client authorization data model" onward belong to this PR.

What this does

Today an authorization lives only as long as the DBus connection it was granted on. The client disconnects, DBusClient goes away, and the next run starts from zero. For secret-tool, pinentry and anything else that runs as a fresh process each time, that leaves two options: click Allow every single time, or turn confirmAccessItem off and stop being asked at all.

This PR lets a decision follow the client application itself, rather than the process it happens to run in this time.

The access request

Access request

Allow and Deny answer the request for the checked entries, and Deny for this program settles a single entry on its own. Scope decides how far that answer reaches: Once covers this request, Selected stores it for the entries listed, and Selected + future stores it for those entries plus everything else the client asks for later.

Keeping the verdict and its scope apart leaves every combination with a meaning. A client-wide decision is by nature one that is stored, so it lives in the scope selector rather than in a separate pair of buttons whose persistence depended on a check box. The connection-lifetime tier that used to sit between "this request" and "stored" is gone as well; users could not perceive it.

An entry denied row by row keeps that denial under every scope. Under Selected + future that is how "everything except this one" is expressed: the entry's own decision outranks the one covering the rest.

How a client is recognized

Access request details

Details shows the calling process and its ancestors, with a Match column that selects what a stored decision is anchored on: for each selected process, its executable path and a SHA-256 digest of the executable content read from /proc/<pid>/exe. The digest is what makes an updated binary ask again. Only the calling process is selected; which ancestor identifies anything is a judgement KeePassXC has no basis to make, and proposing one would suggest a safety it cannot deliver.

Warnings appear when the request deserves a second look, and each of them starts Scope at Once:

  • the executable path is missing or unreadable, so the client cannot be pinned down;
  • the caller is a general purpose tool — a script interpreter, a shell, or a command line client such as secret-tool — which asks for whatever invoked it, so a stored decision would authorize every use of that tool. The warning says storing one is strongly discouraged and stops there; anchoring on an ancestor is left to the documentation, with what it does and does not achieve;
  • the executable content changed since the decision was stored, which is marked per process in the tree.

Managing stored decisions

The database settings list the clients a database recognizes, with an editor for the matching rules and for the decision covering entries that have none of their own:

Database settings

Client authorization editor

Decisions about a single entry are shown and edited on that entry:

Entry decisions

And the Authorization tab shows which stored client each connected application is currently recognized as, or that its executable changed and it will have to be authorized again:

Connected applications

Known limits

Documented in docs/topics/SecretService.adoc: code injected into a legitimate client (LD_PRELOAD, plugins) inherits its authorizations; two scripts run by the same interpreter look identical, since each process runs the interpreter's executable and the command line is never matched on; a process whose executable /proc will not reveal always has to ask; and whoever can write the client's executable becomes that client. A stored decision is worth as much as handing the program the password directly, and the documentation says so.

Implementation notes

Storage, two layers, both in the database. Client identity records go into Database::metadata()->customData(), one key per record (FDO_SECRETS_CLIENT_<DBusClientId>, with the prefix added to CustomData::isProtected()). One key per record rather than a single document, because metadata custom data merges as a whole and decides per key by _LAST_MODIFIED; a single document would silently lose one side's updates whenever two machines both authorized something. Decisions about individual entries live in the entry's own custom data (FDO_SECRETS_AUTH, keyed by DBusClientId), so they travel with the entry through moves, deletion and merges, and the metadata accumulates no dangling uuids. Neither layer is reachable over DBus: Item::attributes() exposes entry attributes, and custom data is not among them.

Matching is two lookups. First the client resolves to at most one record. Records that can match the same client are a configuration smell rather than an error, so resolution is deterministic — a denying catch-all wins, otherwise the earliest created record — and the settings page warns about the overlap. Then the decision is read: the entry's own decision for that DBusClientId, falling back to the record's catch-all. Neither step scans the records at decision time.

Resolution is deliberately not cached. The _LAST_MODIFIED timestamp of a custom data key has second resolution, which cannot distinguish a record edited within the same second. The failure mode of a stale cache here is honoring an authorization the user just revoked.

Hashing. Digests come from /proc/<pid>/exe, which opens the original inode, so the content is readable even when the path has since been replaced or deleted. Conditions are evaluated cheapest-first, so hashing only happens once everything else about a record already matched. Results are cached for the connection lifetime, failures included: a live process cannot change its /proc/<pid>/exe, and a reused pid must not produce a different digest than the process first seen at that depth. Anything unavailable — process gone, different uid, PR_SET_DUMPABLE cleared, unknown algorithm — fails the condition and falls back to asking.

Fingerprint changes. A rule whose non-digest conditions all hold while a digest condition fails identifies the client with changed executable content. That is treated as a mismatch: the user is asked again, with the affected processes marked in the tree, and re-authorizing updates the digests of the existing record in place instead of creating a second one.

Runtime state and stored decisions stay separate. DBusClient holds only what the current connection decided. Stored decisions are never copied into it, which is what keeps a catch-all granted in one database from leaking into another. Service::authDecision() is the single place the two combine: denials win within a layer, and a decision limited to the current request wins over a stored one, so answering a prompt with the Once scope cannot be overruled by what the database holds.

Reading the commits

Commit Contents
1 Data model: record and condition JSON, custom data storage, protected prefix
2 DBusClient::exeHash: algorithm by name, cached per connection, virtual for test fakes
3 Matching and resolution: two phases, cheap conditions first, fingerprint-change detection
4 Enforcement and the access dialog: Service::authDecision as the only combining point
5 Database settings editor
6 Secret Service page on the entry editor
7 Authorization column in the application settings, replacing the Manage column
8 Documentation and changelog

Testing strategy

testfdosecretsclientauth covers serialization, matching semantics, overlap resolution, fingerprint change detection and protected-key survival across a merge (16 tests). testguifdosecrets covers the 46 existing regressions plus persisted authorization, the dialog flow and the three editors (54 tests). Every new behavior was bite-verified: the implementation was broken on purpose to confirm the corresponding test turns red.

Type of change

  • ✅ New feature (change that adds functionality)
  • ✅ Documentation (non-code change)

The majority of the code in this PR was written with Generative AI (Claude Code), with design decisions, review and interactive testing by me.

🤖 Generated with Claude Code

Aetf and others added 12 commits August 17, 2026 18:59
Closing a database tab may spin a nested event loop while the widget
performs save-on-close (Database::save waits for its worker thread in
AsyncTask::waitForFuture). If DatabaseTabWidget::closeDatabaseTab is
re-entered for the same widget from within that nested loop (e.g. two
concurrent FdoSecrets prompts deleting the same collection),
QWidget::close() short-circuits on its internal is_closing flag and
reports success immediately, without delivering another close event.
This behavior is identical in Qt 5 (QWidgetPrivate::close_helper) and
Qt 6 (QWidgetPrivate::handleClose), so none of the downstream guards
(such as DatabaseWidget::lock's m_attemptingLock) ever run for the
re-entrant close. The re-entrant call then removed the tab and
scheduled deletion, and the DeferredDelete event executed inside the
still-running nested loop, destroying the widget while the outer
invocation was still using it; the outer call afterwards repeated
parts of the teardown (stale-index removeTab, deleteLater on the freed
widget, duplicate databaseClosed emission). The bug is long-standing
and was surfaced by scheduler timing under an ASan build with Qt 6.11.

Detected by ASan as a heap-use-after-free in DatabaseWidget::save()
during TestGuiFdoSecrets::testCollectionDeleteConcurrent.

- DatabaseTabWidget::closeDatabaseTab: track widgets with a close in
  progress; a re-entrant call returns false without side effects, and
  only the outermost invocation may remove the tab, schedule deletion,
  and emit databaseClosed (exactly once now). Re-resolve the tab index
  after close() since nested event loops may shift tabs.
- FdoSecrets: PromptBase::prompt now schedules promptSync only once;
  repeated Prompt() calls wait for the same Completed signal instead
  of running the prompt action multiple times.
- FdoSecrets: DeleteCollectionPrompt clears its collection reference on
  Collection::collectionAboutToDelete. The QPointer alone still sees a
  retired collection (removed from D-Bus, m_backend already reset)
  until its deleteLater is delivered, which would violate the
  invariant behind Q_ASSERT(m_backend) in Collection::doDelete. With
  the explicit clearing the invariant "non-null m_collection implies
  usable backend" holds again and the assert stays.
- TestGuiFdoSecrets::testCollectionDeleteConcurrent: fix the second
  prompt proxy accidentally created from the first prompt's path (the
  test was exercising a double-Prompt on one object rather than two
  concurrent prompts), and assert the fail-fast behavior: the losing
  concurrent prompt completes as dismissed while the winner performs
  the deletion.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A Collection retires (removed from D-Bus, backend reset, deleteLater
scheduled) synchronously when its database closes. Prompts, however,
reference collections via QPointer from timer callbacks outside D-Bus
dispatch, and a QPointer still sees the retired object until its
deferred deletion is delivered. This left two holes, both reachable
when a collection goes away while a prompt targeting it is in flight
(e.g. a concurrent Collection.Delete from another client):

- LockCollectionsPrompt/UnlockPrompt could call doLock()/doUnlock() on
  a retired collection, hitting Q_ASSERT(m_backend) in debug builds.
- UnlockPrompt would hang forever: the retired collection never emits
  doneUnlockCollection (its service connections are severed on
  retirement), and the count-based completion condition could no
  longer be satisfied, so the Completed signal never fired and the
  client waited indefinitely. The same accounting also hung when an
  entry was already destroyed by the time promptSync ran.

Following the same principle as DeleteCollectionPrompt: restore the
invariant that a non-null collection reference is usable, instead of
relaxing the asserts.

- LockCollectionsPrompt clears retired collection references on
  collectionAboutToDelete; a retired collection is skipped exactly
  like a destroyed one.
- UnlockPrompt tracks awaited collections in an explicit pending set,
  pre-registered before issuing any doUnlock so that synchronous
  completions (already-unlocked databases) cannot prematurely trigger
  the item unlock step. Retirement is accounted as a rejection and
  re-evaluates completion, so the prompt always terminates.
- Add regression test testServiceUnlockConcurrentDelete covering the
  unlock/delete race; verified to fail (hang detected via signal spy
  timeout) without this fix.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Service::m_unlockingDb tracks databases with an unlock dialog shown to
prevent duplicate dialogs. If the DatabaseWidget is destroyed while
its dialog is open (e.g. the database is deleted via a concurrent
D-Bus call), dialogFinished never fires for it and the entry stayed
forever, keyed by a dangling pointer. The pointer was only ever
compared, not dereferenced, but the stale entry permanently blocked
doUnlockAnyDatabaseInDialog (used by unlock-before-search), and a
future widget allocated at the same address would have been refused an
unlock dialog as well.

Track a cleanup connection on the widget's destroyed signal alongside
the existing databaseUnlocked oneshot, covering both the per-database
and the unlock-any-database flows. Extend the
testServiceUnlockConcurrentDelete regression test to reopen the
database after the concurrent delete and verify the unlock dialog
still appears; without the fix the test times out in SearchItems
waiting for a dialog that never shows.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The item access confirmation dialog used to hold raw Entry pointers in
its model, its deny buttons, and its result signal. When the database
locks or closes while the dialog is open, DatabaseWidget replaces the
Database and destroys every Entry, while the dialog stays up: painting
the model, or answering the dialog, then dereferenced freed entries
(crash in UnlockPrompt::itemUnlockFinished via Entry::uuid, see the
backtrace in keepassxreboot#9104).

Make the dialog self-contained: snapshot each entry's uuid and display
data (title, username, icon) at construction and report decisions as a
QHash<QUuid, AuthDecision>, so the dialog never touches an Entry again
after it is shown.

On the UnlockPrompt side, restore the invariant that a non-null item
reference implies a usable backend: track items with QPointer and clear
all references as soon as an item retires (Item::itemAboutToDelete),
mirroring how collection retirement is handled. A decision for an item
that has retired counts as rejected, so the prompt completes as
dismissed instead of reporting success for nothing.

Finally, match the browser behavior: the dialog now withdraws itself
(rejects) when a collection it is asking about locks or goes away,
instead of keeping a stale question on screen.

Fixes keepassxreboot#9104

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AqHu6RpvLpaoH2aUGSYQzR
Client identity records (match rules + catch-all decision) are stored in
the database metadata customData, one key per record under the new
FDO_SECRETS_CLIENT_ prefix, keyed by a stable DBusClientId. A rule is a
conjunction of conditions on the client's process hierarchy - executable
path, name or content hash at a given ancestor depth - and a record
matches when any of its rules does. Per-entry decisions live in each
entry's own customData, keyed by DBusClientId, so they follow the entry
through moves, deletion and merges; the record's allEntries decision is
the catch-all for entries without one.

One key per record (rather than one blob for all clients) matters for
merging: metadata customData merges whole-map on a single timestamp, so a
monolithic value would lose updates on every cross-machine sync. The
prefix is registered in CustomData::isProtected() so records survive
merges against databases that never saw them. Record parsing fails
closed: a malformed or unknown condition invalidates the whole record
instead of silently broadening what it matches.

Part of keepassxreboot#6458.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016NdKDY7EiJFUFj1bb8x6XP
DBusClient::exeHash(depth, algo) produces a digest of the executable
content of any process in the client's hierarchy. Reading through
/proc/PID/exe reaches the original binary even when its path has been
replaced or deleted, which makes the digest a reliable complement to the
path. The algorithm is taken by name so stored rules can carry theirs;
only sha256 exists today, and an unknown name is logged and yields an
empty digest.

Failures (process gone, different uid, PR_SET_DUMPABLE cleared, no /proc)
also yield an empty digest so hash-based match conditions fail closed.
Results including failures are cached per connection: a live process
cannot change its /proc/PID/exe, a reused pid must not produce a
different hash, and the unsupported-algorithm log fires once instead of
per lookup.

The method is virtual so tests can substitute hashing for their synthetic
process hierarchies.

Part of keepassxreboot#6458.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016NdKDY7EiJFUFj1bb8x6XP
Matching happens in two phases so decisions never iterate records: first
the live client resolves to at most one identity record in a database
(any rule whose conditions all hold selects its record; among overlapping
records a denying catch-all wins, then the earliest created), then the
<entry, client> decision is a plain lookup of the entry's own decision
with the record's catch-all as fallback. Hash conditions are evaluated
last so hashing only happens once everything cheap already matched, and
anything unavailable or not understood fails the condition rather than
skipping it.

A record whose non-hash conditions identify the client while a hash
condition fails is reported as a fingerprint change instead of a match,
with every failing hierarchy depth collected: the previously authorized
executable content has changed and the user must re-decide.
upsertClientRecord() then re-anchors the stale hashes in place instead of
creating a duplicate record; for an unknown client it creates a record
with the default rule, anchoring each requested hierarchy depth by
executable path and content hash - either alone suffices, since a deleted
binary loses its path but stays hashable through /proc/PID/exe.

The default rule anchors the calling process; when that is a script
interpreter (any script it runs could use a stored authorization) the
anchor extends past consecutive interpreter levels to the first
non-interpreter ancestor.

Records are parsed on each decision instead of being cached: the
customData modification time only has second resolution, so a timestamp
validated cache can serve stale authorizations for changes landing within
the same second. Record counts are small and parsing is microseconds.

Part of keepassxreboot#6458.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016NdKDY7EiJFUFj1bb8x6XP
Decisions persisted in the database are consulted wherever the runtime
ones already were: whether an item reports itself as locked, the checks
guarding reading and writing a secret, and the unlock prompt, which no
longer asks about entries the database already answers for.

Service::authDecision() is the only place a stored decision and the state
of the current connection are combined, so no caller can arrive at a
different answer: denials win within a layer, and a decision limited to
the current request wins over a stored one. DBusClient is reduced to that
connection state; a stored catch-all is never copied into it, which is
what keeps it from leaking into another database.

The access dialog answers a request with Allow or Deny for the checked
entries, with per-entry "Deny for this program" for the exceptions, and a
separate Scope: Once, Selected, or Selected + future. Verdict and reach
are independent, so no combination is meaningless — a client-wide decision
is by nature one that is stored, which is why it is a scope rather than a
pair of buttons whose persistence depended on a check box. The
connection-lifetime tier disappears with it; users could not perceive it.

The process tree gains a Match column selecting what a stored decision
anchors on. Only the calling process is proposed: which ancestor
identifies anything is a judgement about the user's setup, and picking one
would suggest a safety this cannot deliver, since the usual ancestors are
shells, terminals and session managers that everything descends from.

Warnings cover an unreadable executable, a general purpose tool as the
caller (interpreters, shells, and clients like secret-tool, which ask on
behalf of whatever invoked them), and executable content that changed
since the decision was stored. Each of them starts the scope at Once, and
the tool warning says outright that storing a decision is discouraged
rather than offering a remedy that would not hold.

Per-entry denials are recorded as intent and mapped when the dialog
concludes, so the order of clicks never changes the outcome, and a denial
stored next to a future-covering allow keeps that entry denied.
The Secret Service page of the database settings lists the client identity
records persisted in the database, with a summary of what each one matches
and what it decided, and lets the user inspect, edit and remove them. A
record editor dialog covers the matching rules (per hierarchy depth: path,
name or executable digest, with a digest computable from a file on disk)
and the catch-all decision; per-entry decisions are listed there but can
only be removed, since they are granted through the access prompt and
changed on the entry itself.

Edits are staged and written when the settings are saved, like every page
of this dialog except the browser one. Nothing is written before that, so
removing a record needs no extra confirmation.

Overlapping records are a configuration smell rather than an error: the
resolver still picks deterministically, so both the list and the editor
only warn about it. recordsOverlap() answers this statically by looking
for a contradiction-free pair of rules, which is why conditions at
depths one record does not constrain never rule out an overlap.

The page has to fit the settings dialog, which gives its pages no
horizontal scrollbar: the warning wraps (an unwrapped message widget makes
its full text the minimum width of the whole page), the rules column gets
whatever the other two do not need, and the exposed group tree is as tall
as its contents instead of taking half the page.
Decisions live in the entry's own customData, so the entry editor gains a
Secret Service page listing them: which client, allow or deny, with a
decision left behind by a since-removed record marked as unknown rather
than shown as a bare uuid. Every row keeps its allow/deny combo open, one
that has to be discovered by double clicking might as well be read-only,
and a decision can be added for any client the database knows about but
that has no decision on this entry yet, searchable by name or by what its
rules match.

The page edits the editor's staging copy of the customData, so its changes
follow the usual Ok/Apply/Cancel and history semantics of the entry editor,
including a revert restoring the decisions of the restored version, the
same as the browser integration settings kept alongside them. The page is
hidden while Secret Service integration is off.
The Authorization tab lists connected applications, so it is the place to
answer "is this the client I already trust?". Each row now names the
identity record it resolves to in every open, exposed database, or reports
that its executable changed and it will have to be authorized again.

The resolution is cached per client and dropped whenever a database is
opened, closed, locked, unlocked or modified: resolving hashes the client's
executable, which is far too expensive to redo on every repaint.

The Manage column goes away with it. Its Reset button cleared decisions
that no longer outlive a single request, and its Disconnect button only
dropped a connection the client reopens on its next call; neither belongs
next to authorizations that live in the database and are managed there.
Since the test override now stands in for a connected client, it is
registered as one so it appears in this list like any other.

All three pages are now called "Secret Service": the settings dialogs
elided the longer name anyway.
The user documentation gains what the access request now asks and what it
means to remember a decision, how a client is recognized and where stored
decisions are managed, and what recognition does not prove: code running
inside a legitimate client inherits its authorizations, scripts sharing an
interpreter are indistinguishable unless the rule reaches further up the
process tree, and a process that hides its executable can never be
recognized.

The screenshots are regenerated, since the access request lost the
"& Future" buttons and both settings pages grew a section, and the tip
about secret-tool prompting on every run is gone: being recognized across
processes is exactly what this feature added.

The implementation README describes the two storage layers, why records
are one custom data key each, the two lookups a decision takes, why
resolution is not cached, and how connection state and stored decisions
combine.
@michaelk83

Copy link
Copy Markdown

The scope labels are very unclear without the explanation, so I would suggest to move them to their own line above the buttons (to give them more room), then rename to "Once" (or "This time" / "Don't remember"), "Remember for selected", and "Remember for all entries".

The "program" wording in the "Deny for ..." buttons is also confusing, because we also have the program that's requesting access. So it looks like this may deny GitHub from requesting access, rather than deny the requesting process from accessing the GitHub entry. Moreover, not all entries represent programs - you could have network passwords, credit card PINs, and so on. And you could have more than one entry per program. So "Deny for this entry" is more appropriate IMO.

What happens (visually and logically) when you click a "Deny for" button?

a denying catch-all wins, otherwise the earliest created record

Shouldn't this be the latest record?

Comment on lines +43 to +51
The decision has two parts. **Allow** and **Deny** answer the request for the checked entries, and **Deny for this program** next to an entry settles that one entry on its own.

**Scope** decides how far the answer reaches:

* **Once** covers this request, and the next one asks again.
* **Selected** stores the answer for the entries listed above. It applies to the same application from then on, across restarts of both KeePassXC and the client, which is what makes short-lived clients workable with confirmations turned on: they are recognized by what they are rather than by the process they happen to run in.
* **Selected + future** stores it for those entries and for every other entry the application asks for, including entries created later.

An entry denied with **Deny for this program** keeps that denial under every scope. Under **Selected + future** this is how "everything except this one" is expressed: the entry's own decision outranks the one covering the rest.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is too technical and partly vague. The following sections also. Imagine you're a new user who knows nothing about how the program works, and you want to learn what the UI does and how to use it.

  1. Explain what the dialog is showing: the path and process ID of the requesting application, the names of the entries, etc. Numbering each section in the screenshot would help.
  2. Focus on the main UI first: the scope and Allow/Deny buttons. Explain how each button interacts with the scope, and what happens to entries outside the scope. For example: "The scope selection and Allow/Deny buttons work together to decide the request outcome:
Selection Outcome
Allow once ...
Allow for selected ... Requests for other entries will ask for authorization.
Allow for all entries ...
Deny once ...
  1. Explain the "Deny for" buttons and how to use them, how they interact with the rest of the dialog.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All points make perfect sense. Let me work on a rewrite.

@Aetf

Aetf commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @michaelk83 for the feedback! Honestly, presenting the scopes intuitively is the part of this design I'm least confident about. I went through a few iterations myself and was never fully satisfied, so suggestions here are very welcome.

Moving the scope selection to its own line with more explicit labels sounds good to me, and I'll rename the per-row button to "Deny for this entry" — you're right that "program" reads as the requesting program, and entries aren't programs anyway.

What happens (visually and logically) when you click a "Deny for" button?

Visually, the row disappears from the list. Logically, the entry is recorded as an explicit deny; whether that deny is remembered is still governed by the scope selected at the moment the dialog is concluded with Allow/Deny. (If every row is denied this way, the dialog concludes by itself — there is nothing left to answer, but the denials are still stored under the chosen scope.)

This differs from leaving an entry unchecked and clicking Allow: an unchecked row gets no decision recorded at all.

The full matrix, using the current labels (Once ≈ don't remember, Selected + future ≈ remember for all entries):

Row state Scope Button Part of this grant Next request
Deny for … Once Allow No Asks again
Checked Once Allow Yes Asks again
Unchecked Once Allow No Asks again
Deny for … Once Deny No Asks again
Checked Once Deny No Asks again
Unchecked Once Deny No Asks again
Deny for … Selected Allow No Denied without asking
Checked Selected Allow Yes Allowed without asking
Unchecked Selected Allow No Asks again
Deny for … Selected Deny No Denied without asking
Checked Selected Deny No Denied without asking
Unchecked Selected Deny No Denied without asking
Deny for … Selected + future Allow No Denied without asking
Checked Selected + future Allow Yes Allowed without asking, incl. other/new entries
Unchecked Selected + future Allow No Allowed without asking
Deny for … Selected + future Deny No Denied without asking, incl. other/new entries
Checked Selected + future Deny No Denied without asking, incl. other/new entries
Unchecked Selected + future Deny No Denied without asking, incl. other/new entries

Two rules generate most of this and are worth calling out:

  • Deny applies to every listed entry, checked or not. Unchecking withholds an entry from a grant; it does not exempt it from a denial.
  • Under Selected + future with Allow, an unchecked row is left out of this grant, but the stored catch-all covers it from the next request on. Unchecking means "not this one, this time" — "never" is what "Deny for this entry" is for.

The table already looks quite confusing :( Maybe the dialog offers too many degrees of freedom in the first place. If the per-row checkboxes were removed — every listed entry shares one decision — the matrix collapses to verdict × scope, and fine-grained per-entry adjustments can still be made later in the entry and database settings. WDYT?

Shouldn't this be the latest record?

Earliest is deliberate. Per-entry decisions are keyed by the record they belong to, so if a newly created overlapping record captured the client, the decisions attached to the older record would silently stop applying — adding a record would change the behavior of a setup that already worked. The overlap itself is flagged in the database settings as something to fix; until then the record that has been resolving keeps resolving. A denying catch-all still wins over both, since when the configuration is ambiguous, failing closed is the safer reading.

@michaelk83

Copy link
Copy Markdown

What happens (visually and logically) when you click a "Deny for" button?

Visually, the row disappears from the list. Logically, the entry is recorded as an explicit deny; whether that deny is remembered is still governed by the scope selected at the moment the dialog is concluded

I was originally thinking of changing "Deny for" to "Reset for" once clicked, so that the denial could be undone without leaving the dialog. But on 2nd thought, and seeing the oversized decision matrix, I think it may be better to simplify:

  • Remove the "Deny for" buttons. (Keep the checkboxes.)
  • Always treat unchecked items as "Deny once".
  • If "Remember for all" is selected, check all items and gray out the checkboxes, to visually indicate that individual selection is irrelevant for that scope. (Ideally, remember the original selection state, so you could restore it if the scope is reduced.)

You could also remember any "Once" items as "Ask", and show them as such in the Access Decisions table in the settings. This would complete the audit trail of requested items, and make it easy to tune the policy.

Then the decision matrix boils down to:

Button Scope Checked Unchecked Other
Allow Once Allow this time, ask next time Deny this time, ask next time Ask
Allow Remember for selected Allow forever (don't ask again) Deny this time, ask next time Ask
Allow Remember for all Allow forever (all are checked) Allow forever
Deny Once Deny this time, ask next time Deny this time, ask next time Ask
Deny Remember for selected Deny forever Deny this time, ask next time Ask
Deny Remember for all Deny forever (all are checked) Deny forever

Shouldn't this be the latest record?

Earliest is deliberate. ... The overlap itself is flagged in the database settings as something to fix

So you're assuming an unintentional overlap?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FdoSecrets: client fingerprinting

2 participants