Repository navigation
Write min PIN length RP IDs before the other setMinPINLength state - #806
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request reorders the operations in process_set_min_pin_length to write the RP IDs first, ensuring that if this potentially failing step fails, the minimum PIN length and force-change flag are not left in an updated state. A new unit test has been added to verify this behavior under failure conditions. I have no feedback to provide as the changes are correct and well-tested.
The setMinPINLength subcommand persists the force-change flag and the new minimum PIN length before writing the RP ID list. That list is the only input of the command that can still fail at storage time, for example when its serialized form exceeds the maximum value size of the persistent store. In that case the platform gets an error, but the minimum PIN length was already updated (and the PIN already flagged for change), leaving the authenticator in a state that does not match any completed command. Write the RP IDs first, so a failing subcommand leaves the other state untouched. This mirrors the ordering fix that was applied to the large blob parameters.
264644e to
e2d4aae
Compare
The
setMinPINLengthsubcommand persists the force-change flag and the new minimum PIN length before writing the RP ID list. That list is the only input of the subcommand that can still fail at storage time, for example when its serialized form exceeds the maximum value size of the persistent store.In that case the platform gets an error, but the minimum PIN length was already updated and the PIN was already flagged for change, leaving the authenticator in a state that does not correspond to any completed command.
Write the RP IDs first, so a failing subcommand leaves the other state untouched. This mirrors the ordering fix that was applied to the large blob parameters in #797.