Skip to content

feat: expose and validate the scrypt recipient work factor#548

Merged
msfjarvis merged 2 commits into
android-password-store:mainfrom
ElCruncharino:feat/scrypt-work-factor
Jun 22, 2026
Merged

feat: expose and validate the scrypt recipient work factor#548
msfjarvis merged 2 commits into
android-password-store:mainfrom
ElCruncharino:feat/scrypt-work-factor

Conversation

@ElCruncharino

Copy link
Copy Markdown
Contributor

Two small changes to ScryptRecipient, both from building on kage.

The default scrypt work factor was hardcoded as 18 inside wrap(). This pulls it out into a public ScryptRecipient.DEFAULT_WORK_FACTOR so callers can read or display the value kage uses instead of repeating the literal.

The recipient also never checked the work factor it was handed. wrap() does 1 shl workFactor, and an Int shift on the JVM masks the count to 5 bits, so workFactor = 33 quietly becomes 1 shl 1, which is 2, and 31 or 32 give a negative or zero N. You end up with a file encrypted at near-zero scrypt cost and no error. I added require(workFactor in 1..30) in an init block, the same range ScryptIdentity already enforces on the decrypt side. I also renamed the private decrypt-side constant from DEFAULT_WORK_FACTOR to DEFAULT_MAX_WORK_FACTOR, since it's the ceiling accepted when decrypting and read confusingly next to the new encrypt-side default.

The test checks that Int.MIN_VALUE, -1, 0, 31, 32, 33, Int.MAX_VALUE are rejected and that 1 and 30 still construct.

Notes for review:

  • The new DEFAULT_WORK_FACTOR constant is in the kage.api diff. The rename is private and doesn't touch the surface.
  • Verified with ./gradlew test checkKotlinAbi animalsnifferMain spotlessCheck; pitest left to CI.

Promote the default scrypt work factor to a public const so callers can
reference the value kage applies by default (e.g. to display it or derive
a relative factor) instead of hard-coding 18. The salt/stanza/label
constants stay internal.
ScryptRecipient.wrap computes `1 shl workFactor`, an Int shift whose
count is masked to its low 5 bits, so a work factor >= 31 silently wraps
(e.g. 33 -> N=2) and would emit a uselessly weak file with no error.
Bound the constructor argument to 1..30 in an init block, the same range
ScryptIdentity already enforces on decryption and the range the reference
age implementation accepts.

Also rename ScryptIdentity's private DEFAULT_WORK_FACTOR to
DEFAULT_MAX_WORK_FACTOR: it is the default ceiling on the work factor
accepted when decrypting, not an applied cost, and the old name collided
with ScryptRecipient.DEFAULT_WORK_FACTOR. Internal only; no API change.

@msfjarvis msfjarvis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@msfjarvis
msfjarvis merged commit 8e15a5f into android-password-store:main Jun 22, 2026
5 checks passed
@ElCruncharino
ElCruncharino deleted the feat/scrypt-work-factor branch July 12, 2026 17:46
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