Description
Description
Recovery Codes are meant to be valid only once. The application logic deletes any Recovery Code used successfully. The foremost important method for this is org.keycloak.credential.RecoveryAuthnCodesCredentialProvider#isValid The high level actions performed by this method are:
- Fetch and construct the RecoveryAuthnCodesCredentialModel object from the database (wrapper class for the Recovery Codes defined for the user).
- If all Recovery Codes have not been used; Compare the user-supplied Recovery Code against the next one to be used (the user-supplied one is hashed with SHA256 and base-64 encoded before comparison) .
- If both Recovery Codes match; Remove the used Recovery Code from RecoveryAuthnCodesCredentialModel, persist the changes in the database, and return true.
Locking is not used to mark that method as a critical section to be accessed only by a thread at the time. Therefore, two requests can be processed in parallel by the isValid() method.
If both requests are processed in parallel, then the Recovery Codes for the user cannot be updated and persisted between the processing of requests. Said another way; Both requests will have the same Recovery Code to compare against if the remaining Recovery Codes are not yet persisted by the first request when the second request processing picks the Recovery Code it must compare with the user input.
Two different requests, originating from different HTTP clients but using the same credentials including the very same valid Recovery Code, emitted at the same time can both pass the Recovery Code validation phase of the authentication process.
Although mostly academic (given the timings necessary, limited impact, and prerequisites), this issue violates the expectation of a Recovery Code being valid only once.
Version
>= 23.0.4
Recommendations
Consider defining the isValid() method as a critical section to prevent parallel execution of it for the same user.