fix: unreliable HIBP caching strategy #2468
Conversation
| @@ -107,20 +111,20 @@ func lcsLength(a, b string) int { | |||
| return greatestLength | |||
| } | |||
|
|
|||
| func (s *DefaultPasswordValidator) fetch(hpw []byte, apiDNSName string) error { | |||
| func (s *DefaultPasswordValidator) fetch(hpw []byte, apiDNSName string) (int64, error) { | |||
There was a problem hiding this comment.
Returning the count here does not rely on the cache having the value in the recursive run.
| }) | ||
|
|
||
| // verify the fetch was done, i.e. channel is empty |
There was a problem hiding this comment.
We had some fishy cases, therefore we ensure here the fetch was done.
Especially the first two cases had the same password, therefore cache hit.
Codecov Report
@@ Coverage Diff @@
## master #2468 +/- ##
=======================================
Coverage 76.52% 76.53%
=======================================
Files 316 316
Lines 17596 17603 +7
=======================================
+ Hits 13465 13472 +7
Misses 3197 3197
Partials 934 934
Continue to review full report at Codecov.
|
| } | ||
| } | ||
|
|
||
| s.hashes.SetWithTTL(prefix+result[0], count, 1, hashCacheItemTTL) | ||
| if prefix+result[0] == b20(hpw) { |
There was a problem hiding this comment.
thisCount is the count of the actual password. We fetch a whole range of password hashes and add it to the cache. It could be that there is not a hit at all. In that case we want to return zero. But in case of a hit, we want to update thisCount before returning, which this if does.
zepatrik
force-pushed
the
fix/pw-hash-cache
branch
from
91840b5
to
1f0cf06
Compare
No description provided.