Skip to content

[nexus] reduce MIN_EXPECTED_PASSWORD_VERIFY_TIME to 650ms#1987

Merged
sunshowers merged 1 commit intomainfrom
sunshowers/spr/nexus-reduce-min_expected_password_verify_time-to-650ms
Nov 29, 2022
Merged

[nexus] reduce MIN_EXPECTED_PASSWORD_VERIFY_TIME to 650ms#1987
sunshowers merged 1 commit intomainfrom
sunshowers/spr/nexus-reduce-min_expected_password_verify_time-to-650ms

Conversation

@sunshowers
Copy link
Copy Markdown
Contributor

On sufficiently fast machines like my Ryzen 7950x, we can go under the
900ms limit.

I tested this by running it a few times, and it appears to work.

Created using spr 1.3.4
@davepacheco
Copy link
Copy Markdown
Collaborator

Based on the output you pasted in chat (1.3s for two iterations), this seems pretty safe. For reference, here's my rationale from last time we reduced this. I think it still holds for a value of 650ms. This is basically saying: it should take at least 650ms on all development and test machines, which is still within the "0.5s-1s" target mentioned in RFD 321. But it's getting close. Much beyond this and I'd feel more comfortable bumping the Argon parameters instead (so it takes longer).

I'm not an expert in this area and if other folks do feel it's safer to bump the parameters now rather than reduce the minimum expected time, that'd work for me too.

@sunshowers
Copy link
Copy Markdown
Contributor Author

OK -- I looked at bumping the Argon params but I was worried that it would require passwords to be reset (depending on the design of the database) so I didn't touch it.

@sunshowers
Copy link
Copy Markdown
Contributor Author

Looking at that discussion, I guess the other option here would be to have separate minimum required times for dev machines and gimlets. But maybe let's get this landed for now and iterate on it.

@sunshowers sunshowers merged commit 75a7f55 into main Nov 29, 2022
@sunshowers sunshowers deleted the sunshowers/spr/nexus-reduce-min_expected_password_verify_time-to-650ms branch November 29, 2022 00:19
@davepacheco
Copy link
Copy Markdown
Collaborator

OK -- I looked at bumping the Argon params but I was worried that it would require passwords to be reset (depending on the design of the database) so I didn't touch it.

For the record, it shouldn't cause passwords to be reset (there are tests to ensure that), and even if it did, we're at a stage where that's okay.

Looking at that discussion, I guess the other option here would be to have separate minimum required times for dev machines and gimlets. But maybe let's get this landed for now and iterate on it.

True. I explained in that comment why I don't think that's the way to go. I don't think there's any particular reason to differentiate based on hardware. We could differentiate dev/prod, but I'm afraid that makes it easier to accidentally deploy insecure configuration to prod.

@sunshowers
Copy link
Copy Markdown
Contributor Author

Ah, got it -- thanks!

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