Add an opt-in x11 feature: X11 backend + keyboard grab#4
Conversation
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.
Replace the eframe/glow (OpenGL) backend with a pure-CPU pipeline: winit for windowing, egui_software_backend to rasterize egui frames on the CPU, and softbuffer to present them. No GPU, OpenGL, or Vulkan is required, so the dialog runs on machines without working GL drivers. - Bump egui, egui-winit, and egui_kittest to the 0.34 line (pinned by egui_software_backend 0.0.3, which pins egui 0.34 / winit 0.30). - Drive one process-wide winit EventLoop with run_app_on_demand so a single gpg-agent connection can show several GETPIN/CONFIRM dialogs. - Keep the Assuan protocol layer, the zeroizing masked passphrase buffer, and the show_dialog(state, want_pin) signature unchanged. - Port the egui_kittest UI tests to the 0.34 API.
The default build stays Wayland-only and links no libX11. Enabling `x11` turns on winit's X11 backend and adds an XGrabKeyboard anti-snoop grab (Drop-guarded, retry-until-viewable, no-op off X11), covering every dialog window.
dsociative
left a comment
dsociative
left a comment
There was a problem hiding this comment.
Design accepted, including bundling winit's X11 backend under the same x11 feature — the default build getting stricter than main ("Wayland-only" now literally true at link level) is a nice touch. Verified: x11-dl absent from the default tree, present with --features x11; tests pass both ways.
One non-blocking nit: in redraw(), GrabAttempt::Retry → window.request_redraw() retries unboundedly, so if another client holds the grab (AlreadyGrabbed) the dialog busy-loops redraws for its lifetime. A bounded retry count or a short timer between attempts would be kinder. Happy to take that as a follow-up instead of blocking this.
For transparency: I don't run X11 sessions, so the grab path was verified by review and feature-gate checks, not exercised live.
2 participants
Implements proposal #2 from #1 (X11 keyboard grab), made opt-in to respect the crate's "no X11 dependencies" default.
Adds an
x11Cargo feature, off by default:x11-dl).x11turns on winit's X11 windowing backend and anXGrabKeyboardanti-snoop grab (Drop-guarded, retry-until-viewable, no-op off X11), covering every dialog window (GETPINandCONFIRM/MESSAGE).On X11, other clients can keylog a passphrase prompt unless it grabs the keyboard (this is why GnuPG's own pinentry does
XGrabKeyboard).Verified:
x11-dlis absent from the defaultcargo treeand present with--features x11;cargo testpasses both ways, with zero warnings.Design note: because the grab is the only reason this crate needs X11 at all, I bundled winit's X11 display backend under the same
x11feature. So with the feature off the build is Wayland-only, which matches the crate's "Native Wayland pinentry" identity. If you would rather keep X11 display available without the grab, the feature can be split intox11(backend) and a separate grab toggle. Your call.Stacking: this is stacked on #3 (CPU rendering) and #2 (zeroizing input), so the diff below includes those until they merge. The commit specific to this PR is
1aef2ae("Add an opt-inx11feature ...").Opened per the discussion in #1. Happy to adjust, split the feature, or hold entirely depending on how you want to handle the X11 question.