Conversation
accwebs
commented
Sep 29, 2026
accwebs
commented
Sep 29, 2026
pstackle
approved these changes
Sep 29, 2026
… is @insaf021's changes for MessageDigest.isEqual() but password is no longer removed from hashCode(). Also fix AuthUtil to eliminate the "secret".equals() pattern. Doesn't really matter for tests, but might as well encourage proper usage.
…e to require Java 17 which is the current minimum
…cret".equals() which is timing unsafe and just all-around not a realistic example
accwebs
force-pushed
the
feature/basic-credentials-side-channel
branch
from
October 5, 2026 20:07
7dbf599 to
dd4cc3c
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem:
See #11334
Solution:
BasicCredentialschanges from Security hardening: path-traversal guard, file-descriptor leak fix, and timing-oracle fixes #11233.BasicCredentials(PR bundled together various unrelated things).passwordfromhashCode(). I don't believe this change is correct/desired. See further discussion below."secret".equals()which is timing unsafe and just all-around not a realistic exampledropwizard-exampleprojectAuthUtilto eliminate the"secret".equals()pattern. Doesn't really matter for tests, but might as well encourage proper usage.Cleanups along the way:
4. Fix
dropwizard-examplebuild to correctly require Java 17.Why I opted to continue including
passwordin hashCode()@insaf021's original PR dropped
passwordfrom thehashCode()computation. I think the mentality is that 'including secret values into the comparison can leak info about them'. Thus 'we should not include the password in hashCode at all'. This is a probably generally-correct rule of thumb in most cases.But, if you read my analysis on #11334 the inclusion in hashCode today is what makes me believe the current lack of timing-constant comparison is not exploitable in the "use POJO in Map key" usage pattern. Hence why I'm skeptical to now adopt a stance that we should not include it in hashCode() as the 'fix'.
What leakage, exactly, comes out of the hash code?
So a length leakage is possible, but not data leakage. So let's talk about how exactly a length leakage would work?
In the case of equals() - the reason equals() is vulnerable is because the attacker just keeps trying prefixes and does TONS of measurements.
Suppose I try the following input passwords:
aa
ba
ca
da
ea
fa
...
za
Aa
Ba
...
Za
0a
...
9a
a
...
I try each of these 1000 times. The slightly slower one is the 'right prefix'. Suppose H is correct. Now search letter #2:
Haa
Hba
Hca
...
Hza
etc.
Within <100,000 requests I can probably figure out the password.
But, how exactly would a hashCode() timing measurement be used in this way? I can't do a prefix search, so I can at best find the correct length.
I'd have to know objectively the absolute timing of the remote CPU to somehow understand the request total time -> on this CPU it's a few NS slower so this password is longer. That's even assuming I can trigger a hashCode computation at will; String caches it once computed the first time.
In sum, I'm not sure hashCode() being operated on password is the same threat as String.equals(). In a sense, hashCode is guarding the String.equals() measurement today (more accurately: bucket segregation by hash is what guards equals from being called) so even once we fix .equals to use MessageDigest.isEqual I am of the opinion hashCode on the password should be retained.
I could be wrong about this.