Derive a PIN, and give the BIP-85 secrets a list to be chosen from - #159
Merged
Merged
Conversation
…ct style Two checks this branch was failing. The first refuses a string conversion anywhere under src/, because that is how a secret leaks through a trace statement, and it is enforced by grep rather than by reading -- so it holds for a screen's format string exactly as it does for a PRINTF. Two labels were composed with one: the BIP-85 result header, which joins a header and a derivation path, and the review's application value, which appends a language. Both are built by appending now, through a bounded helper whose result is checked at each step. That check is not decoration: the result header carries the derivation path, and a path silently cut short does not lead back to the secret, so a label that cannot be composed whole is not shown at all. The second is clang-format. The project sets a 100-column limit and aligns consecutive macros; these two files were written to 80 and unaligned. Only alignment and continuation lines move -- no comment is reflowed.
Two pieces the DICE application was missing, both of them pure arithmetic and
both of them what a PIN is made of.
The path formatter is the fourth of a set of three. Every other application
has one that sits beside its derivation and takes the same arguments, so that
the path a review displays and the path the derivation walks come from one
builder call; DICE had the builder and no formatter, which left the only way
to show its path being to assemble the components again somewhere else. A
BIP-85 path wrong in one component still derives a perfectly well-formed
secret -- just not the one on screen, and with nothing to say so.
bip85_dice_rolls_to_digits() is the whole of what turns DICE into a PIN: with
a ten-sided die each roll is a decimal digit, so the digits are the rolls,
written down in order. Nothing is hashed, reduced or folded, because anything
that were could not be reproduced by another implementation of the
specification -- which is the only reason to derive a PIN rather than invent
one. It writes characters and never converts to an integer: strtol("0934") is
934, and a leading zero lost that way leaves a four-digit PIN looking like a
three-digit one. Everything is checked before anything is written, so a
refusal leaves an empty string rather than the digits that were valid before
the one that was not.
The stack cost of bip85_dice_roll() is now written where it is paid. Its
digest buffer is 2048 bytes, allocated whether four rolls are asked for or
five hundred, and the measured free stack at the deepest point of a
derivation is 1848 bytes on nanos against 23403 or more everywhere else. The
application is safe today only because BIP-85 has no screen on the two-button
stack at all: the function is compiled on all six targets and called from
none of them but the touch flow. Speculos dropped nanos, so nothing would
show the overflow -- whoever exposes DICE there has to shrink the buffer
first, rather than only adding a screen.
The unit tests pin the DICE path against the decimal numbers in the
specification and the digits against the properties that have no other
symptom: leading zeros kept, a short buffer refused whole, a roll above 9
refused before any digit is written.
A PIN is offered as a fourth secret to derive, and it is not a fourth
primitive: PIN(length, index) is DICE(sides = 10, rolls = length, index),
path m/83696968'/89101'/10'/{length}'/{index}'. That is what the review shows
before anything is derived, and it is what lets the same digits be derived
again by any other implementation of the specification. The enumeration
records DICE, which BIP-85 defines, and a separate field records the use it
is being put to -- the PIN today, generic rolls later -- so that the label on
a screen never becomes the thing the derivation is named after.
The roll count gets a field of its own rather than sharing the existing
length. That variable already means "how much data the buffer holds" and is
what the password screens collect; a third meaning would be read by whichever
screen asked first, and the review builds the path it announces out of
exactly these fields.
Nothing is derived before the long press, and nothing partial is ever drawn.
The derivation reports how many rolls it actually produced -- the DRNG stream
can run out under rejection sampling -- so the count is compared against what
was asked for before a digit reaches a screen; anything else erases both
buffers and says so. A PIN one digit short looks exactly like a PIN, which
makes it the one failure the user cannot see.
The length is asked with three buttons rather than a keypad: 4, 6 or 8 digits
is a choice, and a keypad would put a number pad in front of someone about to
be shown a number, with a range error waiting behind every other value.
The secret list is the SDK's own now. Four hand-drawn buttons stacked upward
from the bottom of the screen did not fit: the fourth reached into the title,
and Flex drew "PIN" across the second line of the question while clipping the
first. Nothing in a test could see it -- every text event is still reported at
full height with its full text. nbgl_useCaseGenericConfiguration() draws a
titled header with a back arrow and one touchable bar per secret, and it
paginates itself, so the fifth entry is an entry rather than another silent
overlap. Each bar carries its own token: the index the SDK reports is a
position on the page it drew, which stops meaning "which secret" the moment
the list paginates.
The list reads top-down where the buttons read bottom-up, so the functional
tests that walked it now use the SDK's list component and count from the top.
The constants naming those positions sit beside the menu's own, for the same
reason: a bare number on a screen is a claim nothing else checks.
The journey end to end on the three touch devices, compared against vectors derived offline with bipsea from the seed BIP-85 itself publishes -- the same seed the BIP39 test already uses, and the same oracle that reproduces both the specification's six-sided dice vector and the mnemonic that test expects. So the oracle agrees with this repository everywhere the two can be compared before being asked for anything new. Four digits at index 1 derive 0934, and that case is why the file exists: a PIN read through an integer anywhere would draw "934" and look perfectly healthy doing it. Eight digits at index 3 are there because the roll count and the index sit in adjacent path components, so a flow that swapped them would still produce eight perfectly good digits. The other half is that refusing produces nothing, and it is asserted by the destination rather than by an absence: a screen that failed to draw for an unrelated reason would satisfy "the PIN is not on screen", while arriving home is only possible through the branch that erases and goes home. Leaving the length screen and the index keypad both return to the secret list, which is what asked the question; closing the result and walking the journey again finds nothing carried over. Values are read off the screen with the wrapping removed rather than matched against one drawn line. A derivation path has no space in it, so it wraps on characters -- Flex draws it as two events -- and wait_for_text_on_screen() matches with re.match(), which turns the parentheses of "PIN (Index #0)" into a group that matches something else.
The fuzzing build stopped configuring the moment the builder image was republished: SeedParsers.cmake named `unit-tests/mock/src/cx_crc.c` inside the SDK, and that directory is gone from it. Every ClusterFuzzLite job now fails at CMake generate, on a file no commit in this repository ever touched. That file was there because the SDK's own cx_crc32() computes nothing -- it delegates to the cx_crc_hw() syscall, so a host build needs a stand-in, and using the SDK's own was preferred over writing a third CRC-32. With it gone, the remaining choice is between writing that third implementation and using the one this repository already has, and the second is better: the cmocka suite and the fuzzers then agree on the checksum by construction rather than by review. So the fuzzers link tests/unit/lib/bolos/cx_crc.c, and cx_crc32() joins the other host stand-ins in extra/host_syscalls.c -- byte-for-byte what tests/unit/lib/testutils.c defines for the cmocka build, which is the rule that file already states for the three stand-ins it carries. Linking that file needed one more thing: it included cx.h, which reaches cx_ed25519.h, which includes <openssl/bn.h>. A CRC-32 depending on OpenSSL is invisible to the cmocka suite, which builds OpenSSL anyway, and fatal to the fuzzer build, which deliberately does not -- the whole reason SeedParsers.cmake leaves the big-number stand-in out. The include is now cx_crc.h, which is what those two functions actually need. Verified in the image ClusterFuzzLite builds: the four targets compile, link and run, and the unit suite still passes.
bip85_password_length_set() and bip85_get_get() are declared in bip85_app.h and defined nowhere, in this repository or outside it. Nothing calls them either, so neither has ever been more than a line of a header: a promise the compiler was never asked to check and the linker never had to keep. The second one is also a rename that stopped halfway -- "bip85_get_get" is not a name, and the comment above it says "Gets the BIP85 child password length", which is bip85_length_get() one screen down and already used. A header is the part of a module a caller is entitled to believe. Two entries of this one described functions that do not exist, and the only way to find that out was to go looking for their definitions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A PIN can now be derived from the seed, as BIP-85's DICE application with a
ten-sided die. The list of secrets it joins is rebuilt on the SDK's own list
component, because a fourth hand-drawn button did not fit.
Why
BIP-85 defines DICE, and a PIN is what most people would actually use it for:
PIN(length, index) = DICE(sides = 10, rolls = length, index = index), whichis
m/83696968'/89101'/10'/{length}'/{index}'. Each roll of a ten-sided dieis a decimal digit, so the digits are the rolls, in order, untouched — and
that is the whole point. A PIN that went through a hash, a modulo or an
integer on the way to the screen could not be derived again by any other
implementation of the specification, which is the only reason to derive one
rather than invent one.
The device already had
bolos_ux_bip85_dice(). It had no caller, no pathformatter, and no way to reach it.
What changed
PIN is a preset, not a primitive. The application type recorded is DICE,
which BIP-85 defines; a separate field records what the derivation is being
used for. That keeps a fifth secret — generic rolls, any number of sides —
from having to fight the label "PIN" for the same enumeration slot, and it is
why the review takes its Application row from the use rather than from the
list's own table.
Nothing is derived before the long press, and nothing partial is ever
drawn. The derivation reports how many rolls it actually produced — the DRNG
stream can run out under rejection sampling — so that count is compared
against what was asked for before a digit reaches a screen. Anything else
erases both buffers and says so. A PIN one digit short looks exactly like a
PIN, which makes it the one failure a user cannot see.
Leading zeros are kept, because they are digits and not the absence of
one: four rolls at index 1 of the specification's seed are
0, 9, 3, 4, andthat PIN is
0934. Nothing converts to an integer anywhere.The length is asked with three buttons, not a keypad. 4, 6 or 8 digits is
a choice; a keypad would put a number pad in front of someone about to be
shown a number, with a range error waiting behind every other value they
could type.
The roll count gets its own field rather than sharing the existing length,
which already means "how much data the app buffer holds" and is what the
password screens collect. A third meaning would be read by whichever screen
asked first — and the review builds the path it announces out of exactly these
fields.
The secret list is the SDK's own now. Four buttons stacked upward from the
bottom of the screen did not fit: the fourth reached into the title, and Flex
drew "PIN" across the second line of the question while clipping the first.
Nothing in a test could see it — every text event is still reported at full
height, with its full text, so every assertion on the wording passes.
nbgl_useCaseGenericConfiguration()draws a titled header with a back arrowand one touchable bar per secret, and it paginates itself: the fifth entry is
an entry rather than another silent overlap. Each bar carries its own token,
because the index the SDK reports is a position on the page it drew, which
stops meaning "which secret" the moment the list paginates.
The same screen, on Stax and Apex:
The review shows the path before anything is derived, built by the
formatter that sits beside the derivation and takes the same arguments, so the
screen cannot announce a path the derivation does not walk. The warning names
a PIN rather than "a secret": these digits open whatever they open.
The result carries its path in the label, folded in rather than given a
pair of its own, so a page break can never separate a secret from the only
thing that reproduces it:
A stack cost, written where it is paid
bip85_dice_roll()allocates 2 048 bytes of stack for its digest, whicheverattempt is reached and whether four rolls are asked for or five hundred.
Measured free stack at the deepest point of a derivation: 1 848 bytes on
nanos, against 23 403 on nanox, 35 691 on nanos2, 29 086 on stax, 29 146 on
flex and 33 242 on apex_p. Only nanos does not fit, and it does not fit by a
margin no rearrangement of that function closes.
Nothing is exposed there today — BIP-85 has no screen on the two-button stack,
and its entry points are not even compiled for it — so this is a note for
whoever adds one, in the function that would overflow. Speculos dropped nanos,
so no emulator would show it.
Cost
Measured against this branch's base, six clean builds each. The two-button
targets pay nothing: the new helpers compile there but nothing calls them, so
the linker drops them. RAM is
_ebss - _bss; thebssfiguresizereportsis a fixed-size region and says nothing.
Testing
Six targets build with no warnings. 83 unit tests, one file added: the
rolls-to-digits rendering, pinned on the properties that have no other symptom
— leading zeros kept, a short buffer refused whole with nothing written, a
roll above 9 refused before any digit reaches the buffer. The DICE path is
pinned against the decimal numbers in the specification rather than against
what the builder happens to produce.
The functional campaign passes on the five emulated devices: 32 tests on stax,
flex and apex_p, 13 on nanox and nanosp. Eight of those are new and walk the
PIN journey to the digits and to nothing at all — the refusal asserted by the
destination rather than by an absence, since a screen that failed to draw
would satisfy "the PIN is not on screen".
The vectors were derived offline with
bipseafrom the seed BIP-85 itselfpublishes, and checked both ways: the same oracle reproduces the
specification's own six-sided dice vector and the mnemonic the BIP39 test
already expects, so it agrees with this repository everywhere the two can be
compared before being asked for anything new.
039262,0934and61615716are what the device draws.
Also in this branch: the style commit from the previous pull request, which
was merged while it was being written and so missed the two checks it fixes.