enkilee opened a new pull request, #5134:
URL: https://github.com/apache/rocketmq-dashboard/pull/5134
<!-- Make sure the base branch is `master`: that is the RocketMQ Studio
trunk. -->
### Which Issue(s) This PR Fixes
<!-- Link the issue with a keyword so it closes on merge. Trivial fixes need
no issue.
https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue
-->
- Fixes #<issue-id>
### Brief Description
<!-- What changes and why. Keep it short — the diff already shows how. -->
## Summary
- Close a username-enumeration timing side channel in
`AuthService.loginDatabaseUser`.
When the supplied username did not exist, the method threw 401 immediately
without
running any PBKDF2 derivation. The "user exists but disabled" and "user
exists but
wrong password" paths both run a full 210 k-iteration PBKDF2 match before
returning
401, so an unauthenticated caller could distinguish "user not found"
(fast) from
"user found but wrong password" (slow) by measuring response time and
enumerate
valid usernames.
## Changes
- `AuthService.loginDatabaseUser`: when `findUserByUsername` returns empty,
execute
`passwordHasher.matches(request.getPassword(), DUMMY_PASSWORD_HASH)`
before throwing
401, so every 401 path pays the same PBKDF2 cost. This mirrors the
existing dummy-hash
guard already present on the "disabled account" branch.
- Add regression test
`unknownUserLoginBurnsDummyHashToPreventTimingSideChannelTest` in
`AuthServiceDatabaseTest` that uses a mock `PasswordHasher` to verify the
dummy
derivation runs exactly once when the user is not found.
## Root Cause
The original `orElseThrow` lambda returned the exception without performing
any work.
The disabled-account branch (added in the same PR #2313) already burned a
dummy hash
with an explicit comment explaining the timing-equalization intent, but the
user-not-found branch was overlooked, leaving a detectable timing gap.
### How Did You Test This Change?
<!-- Paste the commands you ran and what they printed. Typical verification:
backend `cd server && mvn -B -ntp test` (integration tests need
MySQL 8, see CONTRIBUTING.md)
frontend `cd web && npm test && npm run lint && npm run build`
A pull request with no verification will not be merged. -->
### Checklist
- [ ] One coherent change; unrelated modifications are not bundled in
- [ ] Commit subject follows Conventional Commits (`feat:` / `fix:` /
`refactor:` / `chore:` / `docs:` / `perf:`)
- [ ] Tests added or updated for non-trivial changes, test methods named
`...Test`
- [ ] New UI text has both Chinese and English entries under `web/src/i18n/`
- [ ] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks)
- [ ] New source files carry the ASF license header
- [ ] Documentation touched where behaviour changed (README / `docs/` /
in-app help)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
