bitflicker64 commented on code in PR #3334:
URL: https://github.com/apache/hugegraph/pull/3334#discussion_r4258770789
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java:
##########
@@ -43,12 +52,27 @@
public class StandardAuthenticator implements HugeAuthenticator {
private static final String INITING_STORE = "initing_store";
+ private static final int MAX_ADMIN_PA_FILE_BYTES = 4096;
Review Comment:
Minor: the reader accepts up to 4096 bytes, but
`StringEncoding.hashPassword` hashes with jBCrypt, which uses only the first 72
bytes of the password. A file holding a longer secret (for example the 96
characters of `openssl rand -hex 48`) creates an admin that also accepts any
password sharing those first 72 bytes. Checked against jbcrypt 0.4: for the
hash of 100 `a` characters, `checkpw` returns true for 72 `a` and for 72 `a`
followed by `ZZZ`, and false for 71 `a`. The truncation is in the existing
hashing and applies to `auth.admin_pa` too, so this PR does not make it worse,
but `testOversizedFileFails` now asserts that a 4096-byte password is accepted
as-is. Requested change: either refuse a password longer than 72 UTF-8 bytes in
`readAdminPasswordFile` with a `ConfigException` that names the option, or keep
the 4096-byte read bound and state the 72-byte effective length in the option
description and `docker/README.md`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
