Sitelet https://github.com/dropwizard/dropwizard/pull/11348
Skip to content

Resolve BasicCredentials side channel / timing concern - #11348

Open
accwebs wants to merge 3 commits into
dropwizard:release/5.0.xfrom
accwebs:feature/basic-credentials-side-channel
Open

accwebs wants to merge 3 commits into
dropwizard:release/5.0.xfrom
accwebs:feature/basic-credentials-side-channel

Conversation

@accwebs

@accwebs accwebs commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor
Problem:

See #11334

Solution:
  1. Resurrect @insaf021's BasicCredentials changes from Security hardening: path-traversal guard, file-descriptor leak fix, and timing-oracle fixes #11233.
    1. Remove all changes unrelated to BasicCredentials (PR bundled together various unrelated things).
    2. Back out the exclusion of password from hashCode(). I don't believe this change is correct/desired. See further discussion below.
  2. Improve basic auth examples to not depict "secret".equals() which is timing unsafe and just all-around not a realistic example
    1. Auth docs
    2. dropwizard-example project
  3. Improve AuthUtil to 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-example build to correctly require Java 17.

Why I opted to continue including password in hashCode()

@insaf021's original PR dropped password from the hashCode() 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?

  • I'm gonna assume that generally speaking we can assume that CPUs will do multiplications on character code points (hashCode computation on a String) for a single character pretty constantly regardless of specific code's integer value.
  • Certainly, the length of the password (number of characters) affects the hashCode time.

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.

  • How would I 'search for' the correct length given that no matter what I supply as input, the correct password's length still affects the computation the same?

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.

Comment thread docs/source/manual/auth.rst Outdated
Comment thread docs/source/manual/auth.rst Outdated
… 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
accwebs force-pushed the feature/basic-credentials-side-channel branch from 7dbf599 to dd4cc3c Compare October 5, 2026 20:07

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants