feat: --guesses (zxcvbn) and --memory (argon2id) flags#8
Conversation
ParseMemory accepts the Argon2id memory parameter as 64m, 1g, or a bare integer (MB). KB intentionally omitted because Argon2id is never configured below MB granularity. ParseGuesses accepts the 'guesses' field from a zxcvbn report, including scientific notation (1.234e+10). Returns *big.Int because guess counts for long passwords routinely exceed int64. Both helpers are wired up in the next commit.
Argon2id is a memory-hard function: its actual attacker throughput depends almost as much on memory pressure as on time iterations. The previous CalculateHashRate formula divided only by workFactor (t), treating memory as a fixed baseline. That underestimated defender strength for any policy that bumps memory above the baseline. Adds memoryMB as a new parameter on CalculateHashRate and MaxLengthForBudget. The argon2id branch now divides by both the time factor and memoryMB / argon2BaselineMemoryMB (64MB), so doubling memory roughly halves attacker throughput. memoryMB <= baseline is clamped to the baseline so an unrealistic input never appears to speed the attacker up. This commit is just the signature/scaling change. All call sites pass 0 (= use baseline), preserving the prior behavior. The next commit wires up the --memory CLI flag.
--memory accepts an Argon2id memory parameter (e.g. 64m, 128m, 1g) that flows through CalculateHashRate and MaxLengthForBudget so defenders running heavier memory settings see the corresponding cost benefit. --guesses accepts an external guess count, typically the 'guesses' field from a zxcvbn report, and short-circuits the built-in character-class entropy estimator. This lets brtc act as a pure cost converter on top of a real strength meter: guesses=$(zxcvbn-cli 'P@ssw0rd!' --json | jq -r .guesses) brtc --guesses "$guesses" --algo bcrypt --cost 12 'P@ssw0rd!' When --guesses is set the password argument becomes optional (so it can be omitted entirely if the caller does not want to pass the literal string), and budget/max-length is skipped because the external estimator does not give us a charset to scale against. Adds calc.FromGuesses for the construction path and a log2 helper that falls back to bit length so log2(huge *big.Int) does not blow up float64. MemoryMB is exposed on OutputData JSON via 'memory_mb' (omitempty) so JSON consumers see the parameter.
Adds the two new flags to the options table, plus a worked example of the recommended brtc + zxcvbn combo (zxcvbn does pattern detection, brtc converts the resulting guess count into time and USD), and an Argon2id memory-sweep example showing how memory hardens against GPU attackers.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for Argon2id memory parameters and external entropy estimates via the new --memory and --guesses flags. The changes include updated hash rate calculation logic to account for memory-hard functions, a new parsing utility for memory units and scientific notation, and expanded documentation and tests. The review feedback identifies several opportunities to improve the robustness of the new parsing logic, specifically by making the memory unit regex more flexible, preventing potential integer overflows during unit conversion, and handling infinite values when parsing external guess counts.
| "strings" | ||
| ) | ||
|
|
||
| var memoryRe = regexp.MustCompile(`^(\d+)([mg])?$`) |
There was a problem hiding this comment.
The regular expression for parsing memory is quite restrictive. It doesn't allow for common variations like mb, gb, or spaces between the number and the unit (e.g., 64 mb). Improving the regex to be more flexible would enhance the user experience.
| var memoryRe = regexp.MustCompile(`^(\d+)([mg])?$`) | |
| var memoryRe = regexp.MustCompile(`^(\d+)\s*([mg]b?)?$`) |
| switch m[2] { | ||
| case "", "m": | ||
| return n, nil | ||
| case "g": | ||
| return n * 1024, nil | ||
| } | ||
| return 0, fmt.Errorf("unknown unit %s", m[2]) |
There was a problem hiding this comment.
The unit parsing logic should be updated to handle the more flexible units (like mb or gb) allowed by the suggested regex. Additionally, when converting GB to MB, there is a potential for integer overflow if a very large value is provided. Adding a check for this ensures robustness, especially on 32-bit architectures.
switch {
case m[2] == "" || strings.HasPrefix(m[2], "m"):
return n, nil
case strings.HasPrefix(m[2], "g"):
if n > 2097151 { // Limit to ~2TB to prevent overflow on 32-bit systems
return 0, fmt.Errorf("memory value too large: %d GB", n)
}
return n * 1024, nil
}
return 0, fmt.Errorf("unknown unit %s", m[2])| f, _, err := big.ParseFloat(s, 10, 256, big.ToNearestEven) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("guesses: cannot parse %q", s) | ||
| } |
There was a problem hiding this comment.
The big.ParseFloat function can return infinite values if the input string is "inf" or if the value exceeds the maximum representable exponent. Since f.Int(nil) has undefined behavior for infinite values, it is important to check for this condition explicitly.
| f, _, err := big.ParseFloat(s, 10, 256, big.ToNearestEven) | |
| if err != nil { | |
| return nil, fmt.Errorf("guesses: cannot parse %q", s) | |
| } | |
| f, _, err := big.ParseFloat(s, 10, 256, big.ToNearestEven) | |
| if err != nil { | |
| return nil, fmt.Errorf("guesses: cannot parse %q", s) | |
| } | |
| if f.IsInf() { | |
| return nil, fmt.Errorf("guesses: value cannot be infinite") | |
| } |
big.ParseFloat happily accepts 'inf' / '+inf' / '-inf'. The subsequent big.Float.Int(nil) on +Inf returns a nil *big.Int rather than an error, and that nil value would then panic downstream in TimeToCrack via big.Float.SetInt(nil). Adds an explicit IsInf check after sign validation, plus tests for 'inf' and '+inf'.
|
Addressed @gemini-code-assist's three suggestions:
|
|
Thanks for the update and for addressing the |
1 participant
Summary
Adds the two highest-leverage usability gaps from the post-merge review of #1:
--memoryaccepts an Argon2id memory parameter (64m,128m,1g, or bare MB) and routes it through the hashrate model. Argon2id is a memory-hard function — bumping memory is the single most effective defender lever against GPU attackers — but the previous code only divided byworkFactor(time iterations), silently underestimating defender strength for any policy above the 64MB baseline.--guessesaccepts an external guess count, typically theguessesfield from a zxcvbn report, and short-circuits the built-in naive entropy estimator. This finally lets brtc act as a pure cost converter on top of a real strength meter, addressing the README positioning.When
--guessesis set the password argument becomes optional and the budget/max-length feature is skipped (it needs a charset assumption that an external estimator does not provide).How to use
Verification (local)
go test -count=1 ./...✓golangci-lint run ./...✓ 0 issuesTest additions
TestParseMemory/TestParseGuesses— unit boundaries, scientific notation, rejection of malformed inputs.TestCalculateHashRate_Argon2MemoryScaling— baseline, doubling, sub-baseline clamp, multiplicative composition with time.TestFromGuessesandTestFromGuesses_HugeNumberDoesNotOverflow— bit-length fallback for guess counts beyond float64 range.