Skip to content

Conversation

@jubrad
Copy link
Contributor

@jubrad jubrad commented Nov 7, 2025

Move password hash iterations to a system var.

Defaults to 600_000 and uses 600_000 for
all system users.

Motivation

Tips for reviewer

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

@jubrad jubrad force-pushed the configurable-hash-iterations branch 2 times, most recently from 31cf233 to d414346 Compare November 7, 2025 16:43
Move password hash iterations to a system var.

Defaults to 600_000 and uses 600_000 for
all system users.
@jubrad jubrad force-pushed the configurable-hash-iterations branch from d414346 to 7ad5109 Compare November 7, 2025 17:01
@jubrad jubrad requested a review from DAlperin November 7, 2025 17:25
@jubrad jubrad marked this pull request as ready for review November 7, 2025 17:25
@jubrad jubrad requested a review from a team as a code owner November 7, 2025 17:25
@jubrad jubrad requested a review from SangJunBak November 7, 2025 17:25
- move password_hash_iterations to scram_iterations
- make iterations required in hash functions
- fix some typos
@jubrad jubrad force-pushed the configurable-hash-iterations branch from cae1ec5 to fbbc7bd Compare November 8, 2025 21:30
@jubrad jubrad requested review from DAlperin and SangJunBak November 8, 2025 21:43
password,
&attributes
.scram_iterations
.expect("If there's a password there must be hash iterations"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While this should be true, I'm open to other ways to do this. It seems like the attribute field needs to be Optional<NonZeroU32>, at least nothing else really makes sense, and this part of the code doesn't have access to dyncfg so it can't fetch a well known default. I suppose we could set to 600_000 with a soft panic?

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.

3 participants