Repository navigation
Harden Basic Auth credential buffers and verify A25 privacy - #192
Conversation
Code review (xhigh) — head 88b1490 (master)Static review of the diff and What looks right: status codes and decision order are unchanged, raw
Remote CI and native builds are unqualified, as stated in the PR. |
Follow-up review (xhigh) — head 65b64df (master)All five earlier findings are fixed. Verified by reading the code; I ran the Basic-auth and scheme test classes on master (26 tests, all passed).
One new minor point, not blocking: Fresh CI and native builds are still unqualified, as stated in the PR. |
Follow-up review (xhigh) — head 6d3b1ff (master)The charset-buffer finding is fixed. No new findings. Encoding and decoding now use a I ran the Basic-auth and scheme test classes on master (28 tests, all passed). Fresh CI and native builds are still unqualified, as stated in the PR. |
Closes #191
Basic authentication created immutable decoded credential/password Strings and left comparison arrays uncleared (A20, Excel row 98). Local authentication now uses request-owned buffers cleared on exit. The existing LDAP String API and extension signature remain unchanged and are documented as a residual limit.
The scheme is removed at a character boundary, preserving Unicode case-insensitive scheme compatibility. Base64-decoded malformed UTF-8 retains the former replacement behavior for both local comparison and LDAP, using cleared character/byte buffers instead of immutable credential Strings. Basic and shared scheme checks use one trim-equivalent scanner without creating credential substrings.
A25 rows 90–97 were already mitigated in both branch baselines; the initial TRACE privacy regression passed against each unmodified implementation. No password/header logging behavior change is needed. Regression checks now include distinct password prefix/suffix fragments and the Unicode suffix, full encoded credentials, log arguments/MDC/throwables and returned status content. Configured username/path diagnostics remain.
Validation on each branch: 34 focused tests passed, zero failures/errors/skips; pre-commit hooks and whitespace checks passed. A package-local parser boundary lets tests observe the actual credential buffers used by handleBasicAuth without replacing its logic; tests verify cleanup after success, invalid/empty/reserved/unknown credentials, malformed input, denied paths, and LDAP returns/exceptions. Compatibility tests cover Unicode scheme variants and malformed UTF-8.
Negative controls independently confirmed tests fail for byte-offset scheme stripping, missing UTF-8 replacement normalization, omitted middleware try-with-resources, and password-prefix logging. Earlier controls also caught full password logging and omitted buffer cleanup. Source/test/doc changes match across branches.
mvn -o clean test -Dtest=BasicAuthMiddlewareTest,BasicAuthCredentialsTest,BasicAuthPrivacyTest,AuthorizationSchemeCredentialsTest,BasicAuthMiddlewareLifecycleTest,BasicAnonymousBearerTest,BasicBearerNoUserTestThe request header, configured password, decoder/JVM internals and LDAP library may retain credentials; this does not promise full heap erasure. No live LDAP bind, AWS operation, native-image build or deployment was performed. Fresh remote CI on the updated head is required.
Charset-buffer follow-up: UTF-8 conversion uses the incremental encoder/decoder API with explicit output arrays sized from maximum expansion, and clears their full capacity in finally blocks. This avoids output resizing and discarded earlier conversion buffers. Comparison encoding uses the same owned-buffer path. New mixed-width Unicode and worst-case malformed-byte expansion cases pass on both branches. Fresh remote CI and native builds remain unqualified.