-
Notifications
You must be signed in to change notification settings - Fork 58
CBST2-01: Fix JWT Replay Attack Vulnerability #297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: sigp-audit-fixes
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the current implementation, the nonce is effectively the total number of requests from a given module. This means that whenever any of server / module is restarted, everything has to be restarted, otherwise the client / server nonces would be out of sync. It also means that JWTs are regenerated for each request.
A different approach could be:
- only include the request hash in the JWT, this would allow separate restarting but not protect against replay
- use separate JWT with shorter expiration, say 30 seconds, and a separate header for replay protection with nonce + hash, with the nonce reset once the JWT is renewed
Cargo.toml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these seem changes due to a different formatter you're using? we have a taplo.toml
in the root which should take care of it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I run taplo fmt Cargo.toml
and that was the output. Do you have other results? For context, my installed version is taplo 0.10.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing a bit more, the only difference between my editor's format and the taplo
command is the indentation (my editor use 4 spaces versus the 2 spaces used by the command). This can be solved setting the indent_string
param in the taplo.toml
config with the desired value
|
||
Ok(()) | ||
fn build_jwt(&self) -> Result<Jwt, SignerClientError> { | ||
create_jwt(&self.module_id, self.nonce, &self.jwt_secret).map_err(SignerClientError::from) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not increasing the nonce here directly?
jti
claim as a nonce to avoid reply attacks.