8000 Recovery Code Validation Race Possible · Issue #26106 · keycloak/keycloak · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
Recovery Code Validation Race Possible #26106
Closed
@abstractj

Description

@abstractj

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.

References

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

    0