Skip to content

Store the passphrase in a zeroizing buffer instead of a TextEdit#2

Open
domenkozar wants to merge 1 commit into
dsociative:mainfrom
domenkozar:feat/zeroizing-input
Open

Store the passphrase in a zeroizing buffer instead of a TextEdit#2
domenkozar wants to merge 1 commit into
dsociative:mainfrom
domenkozar:feat/zeroizing-input

Conversation

@domenkozar

Copy link
Copy Markdown

This is the first change proposed in #1 (zeroizing input), as a standalone, minimal PR that doesn't touch the design discussions (no grab, no rendering change).

Problem

The passphrase field uses TextEdit::singleline(&mut password).password(true) on a String, then SecretString::from(password.clone()) + password.clear(). So the plaintext:

  • lives in egui's retained widget state and undo history, so it isn't gone when the dialog closes;
  • gets an extra clone(), and clear() doesn't zeroize the buffer.

Change

  • PinDialogState.password is now a zeroize::Zeroizing<String>.
  • Keystrokes (Event::Text / Event::Paste / Backspace) are fed straight into it and a masked bullet view is drawn, instead of a TextEdit, so the plaintext never enters egui's retained state or undo history.
  • On submit the value is moved into the SecretString with mem::take (no clone; the buffer is zeroized on drop).
  • The widget-query tests are reworked to drive the field with synthetic input events (the masked view is no longer an accessible widget), plus a backspace test. The accesskit dev-dependency is dropped (no longer used).

No change to the Assuan protocol or the dialog UX. zeroize is added as a direct dependency; it is already in the tree transitively via secrecy.

Refs #1.

Feed keystrokes directly into a zeroize::Zeroizing<String> and draw a
masked view, rather than an egui TextEdit whose retained widget state and
undo history keep the plaintext around after the dialog closes. Move the
value into the SecretString on submit (no clone; the buffer is zeroized on
drop). Tests drive the field with synthetic input events since the masked
view is no longer an accessible widget.

@dsociative dsociative left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Verified end to end on Wayland/niri (sandboxed offline build, 7/7 tests, clean strace). The zeroizing approach is exactly right and I want to land this first — two items:

  1. (blocking) The masked field lost its caret and focus indication. The bullet view is a non-focusable Label, so egui draws neither a caret nor a focus outline — side by side with main the field looks dead even though keystrokes register. Could you add manual focus/caret visuals (e.g. a focus-styled Frame stroke plus a caret painted after the last bullet), or otherwise restore the affordance? You know egui internals better than I do — pick whatever shape you prefer.
  2. (nit) #3 pre-reserves the buffer (String::with_capacity(256)) so ordinary typing never reallocates mid-entry and leaves unzeroized copies behind; since this PR can land standalone, it would be nice to have that here too.

Note: Event::Paste works fine here on top of eframe, but stops firing in the #3 stack — tracked in the #3 review, nothing to do in this PR.

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.

2 participants