feat(confirm): page long bodies instead of promising to and not - #488
Conversation
Closes #485. The Cut Off screen told the user: This text is too long for the screen. Only part of it is shown. Hold to view it anyway. Holding showed nothing new. confirm_sm.c re-drew the SAME truncated body -- request_body unchanged -- so the remainder stayed unreachable and the second hold bought the user precisely nothing. Confirmed on hardware during the rc30 round: 118, 119 and 255-character bodies all redraw identically after the warning. A consent screen that says it is about to disclose something and then does not is worse than no warning at all: a user who reads it carefully is misled about what they have seen. So actually page it. RENDER overflow -- the body reached the renderer intact, so every character is still in hand. Split it across as many screens as it needs, titled "n/m". No warning screen: nothing is being hidden any more. SOURCE truncation -- characters were lost in vsnprintf() before the renderer ever saw them. They cannot be paged because they no longer exist. Keep a warning, but stop lying in it: "The rest cannot be displayed." Paging is a click, approving is a hold. Intermediate pages pass immediate=true, which sets the confirm timer to 1ms (confirm_sm.c swap_layout), so a short press advances them; only the LAST page takes the caller's real hold. Reading what you are being shown should not cost the same effort as consenting to it. Preserves the #482 invariant -- one required press, one ButtonRequest. Every page after the first writes its own request and clears button_request_acked, so a host that answers every request it is told about never waits on a press it never heard of. Page boundaries come from page_take(), a binary search over confirm_body_fits(), which is draw_string()'s own loop with the pixel writes switched off. Measuring and drawing cannot disagree because they are the same code -- the property 7e07b2d established and the reason there is no second model of the screen to get out of sync. Only layout_standard_notification is measured, as before. Custom layouts and layout_constant_power_notification place their own bodies and are untouched. BEHAVIOUR CHANGE for tests: a body over the fit boundary (118 characters for a plain body, measured on hardware) now produces numbered pages rather than a "Cut Off" screen followed by the same clipped text. The ButtonRequest count is unchanged for a two-page body; the titles and the press durations are not.
Hardware validation: PASS (5/5)Flashed
The Q3/hold pair is the security property and it is measured, not reported: Tester's words on the rendering: "that pagination is perfect", and separately The blank
|
Closes #485. Confirmed on hardware during the rc30 round.
The defect
The Cut Off screen says:
Holding shows nothing new.
confirm_sm.cre-draws the same truncated body —request_bodyunchanged — so the remainder stays unreachable and the second holdbuys the user nothing. Verified on device at 118, 119 and 255 characters: the
post-warning screen is byte-identical to the pre-warning one.
A consent screen that says it is about to disclose something and then doesn't is
worse than no warning at all — a user who reads it carefully is misled about what
they have seen.
The change
Page it, for real.
n/m, all of itvsnprintf()before the rendererThe split matters: source-truncated characters no longer exist and cannot be
paged, so that path keeps a warning — but an honest one.
Click to page, hold to approve
Intermediate pages pass
immediate=true, which sets the confirm timer to 1 ms(
swap_layout), so a short click advances them. Only the last page takesthe caller's real hold, because only the last page is the approval.
Reading what you are being shown shouldn't cost the same effort as consenting to
it — and on a single-button device, making the user hold 1.2 s per page to read
an address is how people learn to hold without reading.
Invariants preserved
#482 — one required press, one ButtonRequest. Every page after the first
writes its own request and clears
button_request_acked, so a host that answersevery request it is told about never waits on a press it never heard of.
7e07b2d — one model of the screen. Page boundaries come from
page_take(),a binary search over
confirm_body_fits(), which isdraw_string()'s own loopwith the pixel writes switched off. Measuring and drawing cannot disagree because
they are the same code. No second model to drift — the seam #428 was reopened
through three times.
Only
layout_standard_notificationis measured, as before. Custom layouts andlayout_constant_power_notificationplace their own bodies and are untouched.Behaviour change for tests
A body over the fit boundary (118 characters for a plain body, measured on
hardware in the rc30 round) now produces numbered pages rather than a
Cut Offscreen followed by the same clipped text.
The ButtonRequest count is unchanged for a two-page body. The titles change
(
FOO 1/2) and the press durations change (click, click, …, hold). Any testasserting the literal
Cut Offstring on a render-overflow body needs updating;tests asserting it on a source-truncated body still hold.
Not yet verified
CI compiles this; I could not build ARM locally. The press-duration behaviour and
the page titles want a hardware pass before rc31 — that is genuinely in the class
of things the emulator cannot check, since it has no physical button.