feat(security): add password verifier formats#8251
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for secure password verifiers, including PBKDF2-SHA256 and MySQL Native Password, to protect credentials at rest. It also adds a frontend_auth configuration option for flownodes to authenticate internal gRPC requests to the frontend. The review feedback suggests enforcing a strict 32-byte length check on decoded PBKDF2-SHA256 hashes to prevent truncation issues, and optimizing the verification path by using a stack-allocated array instead of a heap-allocated vector.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
This PR extends GreptimeDB authentication configuration by introducing explicit password verifier formats (e.g., plain:, pbkdf2_sha256:..., mysql_native_password:...) for the static user provider, and adds a dedicated frontend_auth config for flownode-to-frontend internal gRPC authentication to avoid reusing user_provider credentials.
Changes:
- Add
PasswordVerifierparsing/verification to supportplain,pbkdf2_sha256, andmysql_native_passwordverifier strings in static credentials. - Add
frontend_auth(basic auth) to flownode options and prefer it for internal requests, keepinguser_provideras a fallback. - Update example configs and add tests for new verifier formats and
frontend_authbehavior.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/flow/src/server.rs | Prefer frontend_auth for internal auth header construction; add validation + tests. |
| src/flow/src/engine.rs | Add from_basic() constructor (and keep from_user_pwd() via delegation). |
| src/flow/src/adapter.rs | Add frontend_auth options struct with password-redacting Debug. |
| src/cmd/tests/load_config_test.rs | Update expected FlownodeOptions to include frontend_auth. |
| src/auth/src/user_provider/static_user_provider.rs | Support non-plain verifiers in auth flow; reject exporting non-plain verifier as plaintext; add tests. |
| src/auth/src/user_provider.rs | Introduce PasswordVerifier parsing/verification and extend credential parsing + tests. |
| src/auth/src/common.rs | Factor MySQL native password auth to allow verifying against a precomputed stage-2 hash. |
| src/auth/Cargo.toml | Add dependencies needed for verifier parsing/verification (pbkdf2, sha2, subtle, hex). |
| config/standalone.example.toml | Document supported password verifier formats. |
| config/frontend.example.toml | Document supported password verifier formats. |
| config/flownode.example.toml | Document verifier formats and add frontend_auth example. |
| Cargo.lock | Lockfile updates for new dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0b5b982 to
0a61fdd
Compare
|
I suggest to keep this file provider simple. It's not designed for security or any production usage. Users with this concern should switch to the enterprise edition. |
This seems more like a baseline security improvement than a commercial-only feature. Support for non-plaintext password verifiers and separation of internal auth from user credential handling should be part of the open-source core, while enterprise editions can differentiate with SSO, LDAP, audit logs, and advanced access control, which we already have. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6e3b9f806
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
- Reject pbkdf2_sha256 verifiers whose hash is not 32 bytes and bound the salt length, preventing short-hash verifiers from matching on a prefix. - Verify pbkdf2_sha256 with a stack-allocated buffer. - Report only the length, not the bytes, when a mysql native password verifier has an illegal length. - Map empty frontend_auth credentials to an invalid-config error instead of an internal error. Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
Pick the first plain-text credential instead of failing when the first user happens to hold a hashed verifier. Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
c6e3b9f to
0618645
Compare
|
|
I'll implement a command like |
Internal flownode-to-frontend communication no longer authenticates (see GreptimeTeam#8244), so the plain-text credential export path is dead code. Drop get_one_user_pwd, its now-orphan as_plain_text helper, and the related tests. Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
|
Is it a breaking change? |
Yes, but it won't break anything actually unless your password starts with |
Signed-off-by: Dennis Zhuang <killme2008@gmail.com>
I hereby agree to the terms of the GreptimeDB CLA.
What's changed and what's your intention?
Stop requiring plaintext passwords at rest in the static/watch file user provider.
A credential password now accepts an explicit verifier format:
plain:<password>— plaintext (the default when no prefix is given)pbkdf2_sha256:<iterations>:<hex_salt>:<hex_hash>— at-rest hash, used by cleartext-capable paths (HTTP/gRPC Basic, PostgreSQL cleartext, MySQL clear password)mysql_native_password:<hex_sha1_sha1_password>— at-rest verifier that still serves the MySQLmysql_native_passwordhandshakeParsing is hardened:
get_one_user_pwd().Capability note
A single verifier format does not serve every wire protocol:
mysql_native_passwordplain:<password>or legacyuser=passwordpbkdf2_sha256:<iterations>:<hex_salt>:<hex_hash>mysql_native_password:<hex_sha1_sha1_password>pbkdf2_sha256protects passwords at rest. It does not change wire security; cleartext-capable protocols still need TLS in production.Breaking change
Passwords are now prefix-parsed. A legacy plaintext password that literally starts with
plain:,pbkdf2_sha256:, ormysql_native_password:changes meaning.Use
plain:to make that intent explicit. For example, the literal passwordplain:secretshould be configured as:PR Checklist