Key storage detecting #438
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #438 +/- ##
==========================================
+ Coverage 90.82% 90.89% +0.06%
==========================================
Files 126 128 +2
Lines 4209 4283 +74
Branches 666 678 +12
==========================================
+ Hits 3823 3893 +70
- Misses 251 254 +3
- Partials 135 136 +1
☔ View full report in Codecov by Sentry. |
babenek
force-pushed
the
jks
branch
2 times, most recently
from
72d307f to
344f94d
Compare
babenek
force-pushed
the
jks
branch
2 times, most recently
from
83e86d0 to
56c680e
Compare
| for pw_probe in ["", "changeit", "changeme"]: | ||
| try: | ||
| keystore = jks.KeyStore.loads(data_provider.data, pw_probe, try_decrypt_keys=True) | ||
| if keystore.private_keys or keystore.secret_keys: |
There was a problem hiding this comment.
This part is not clear.
You check for private keys in the keystore. They are returned in encrypted form (if your password guess is wrong) or decrypted format (if password matches). This is the first part of IF.
Then ELSE section is used for cases when there are no private keys at all. But you marked jks as candidate as well.
It seems there is a trouble out here. 4 cases should be checked independently:
- pkey present, default password, issue.
- pkey present, unknown password, no issue.
- no pkey, default password, no issue (?)
- no pkey, unknown password, no issue.
There was a problem hiding this comment.
In any case - a storage with default password should not appear.
IF-ELSE block just decide which text will be in info for user.
Unknown password will raise an exception and no issue is created.
There was a problem hiding this comment.
https://pyjks.readthedocs.io/en/latest/_modules/jks/jks.html#KeyStore.loads
if try_decrypt_keys:
try:
entry.decrypt(store_password)
except DecryptionFailureException:
pass # ok, let user call decrypt() manually
No exception
There was a problem hiding this comment.
In common (wrong) case the password of keys is the same like for the storage.
Suppose, if the key has good encryption - else branch will be taken.
Anyway, the scanner finds unencrypted(weak password) storage.
There was a problem hiding this comment.
Whether it is used? Cannot see traces in the code.
There was a problem hiding this comment.
in tests/data/doc.json above - the storage is encrypted with different password, so it is not found
There was a problem hiding this comment.
See comment.
The code does not match jks source code behavior https://pyjks.readthedocs.io/en/latest/_modules/jks/jks.html#KeyStore.loads
Description
Please include a summary of the change and which is fixed.
based on #441
How has this been tested?
Please describe the tests that you ran to verify your changes.