Replies: 1 comment 6 replies
-
|
I don't think it would be a bad idea to have the codes salted, but I also think that in most cases a database breach would also lead to leaking the secret used in OTP generation, so an attacker could use it directly. I assume the secret cannot be hashed since you have to use to to derive the current value for the OTP. |
Beta Was this translation helpful? Give feedback.
6 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Background
Me and my team recently use new preview feature 'Recovery Authn Code' in our product. But my security team indicates that the way Recovery Code being hashed is not quite safe under rainbow-table-attack since it isn't hash with salt, but we found that something like passowrd(in keycloak) has salt into it when it is being hashed.
Below is the current recovery code hash logic, only hash by SHA-512, and then those hashed codes(rawCodeHashedAsBytes) will be saved into Database.
Proposal
We now have customized the code by adding salt(128 bit) to the 12 recovery codes when doing hash(SHA-512 + salt = HS512) for security concern.
Below is the key change, we added a new param 'salt' to column SECRET_DATA in table FED_USER_CREDENTIAL by customizing a few classes.
{ "FED_USER_CREDENTIAL": [ { "ID": "***", "SALT": null, "TYPE": "recovery-authn-codes", "CREATED_DATE": 1695195076512, "USER_ID": "***", "REALM_ID": "***", "STORAGE_PROVIDER_ID": "***", "USER_LABEL": "Recovery Authentication Codes", "SECRET_DATA": "{\"salt\":\"CEwUvh1o+60piNjUitoFFw==\",\"codes\":[{\"number\":12,\"encodedHashedValue\":\"f4e75faddab0a70be912617e02eaae18c353baf0a4df65c36bf32d038249627aebed9ca7f8490315854e86e5966f0fc1f05da287dd5c81afc2911e97d8c5b8bc\"}]}", "CREDENTIAL_DATA": "{\"hashIterations\":1,\"algorithm\":\"HS512\",\"totalCodes\":12,\"remainingCodes\":1}", "PRIORITY": 20 } ] }Others
I have read about the design Discussion, it seems like salt or security concern wasn't mentioned about in that context.
Questions
I want to know how does the other developer or maintainer think about this. 🤔 Will it be possible(or neccessary) to enhance the hash logic? Or consider about my proposal?
Beta Was this translation helpful? Give feedback.
All reactions