Add payload hash to signer JWT claims #356
Conversation
…he source" This reverts commit 58c6117.
| })?; | ||
|
|
||
| // Make sure the request contains a hash of the payload in its claims | ||
| if !body.is_empty() { |
There was a problem hiding this comment.
Won't this always be true?
There was a problem hiding this comment.
No, the body is empty for GET requests like /signer/v1/get_pubkeys - since they're still JWT-authenticated, this has to be there to handle them.
api/signer-api.yml
Outdated
| @@ -593,6 +633,185 @@ paths: | |||
| schema: | |||
| type: string | |||
| example: "OK" | |||
| /reload: | |||
There was a problem hiding this comment.
We discussed some time ago if we should include non Signer API endpoints here or not. We can discuss it again though, if you have another point of view
Ref: #231 (comment)
There was a problem hiding this comment.
Ah thank you for the link - I hadn't seen that. I added it since /status was in the docs and that's technically a non-signer route, but I've removed all of the non-signer routes now. We should probably make another doc to cover "root" routes like this. If you want to treat them separately from /status then we should change them to something like /admin/reload instead and have a third doc for them (admin-api.yml maybe?).
| The token **must include** the following claims: | ||
| - `exp` (integer): Expiration timestamp | ||
| - `module` (string): The ID of the module making the request, which must match a module ID in the Commit-Boost configuration file. | ||
| - `payload_hash` (string): The Keccak-256 hash of the JSON-encoded request body, with optional `0x` prefix. This is required to prevent JWT replay attacks. |
There was a problem hiding this comment.
Json is not an ideal serialization format to hash, let's use ssz with the nonce and object root. As an optimization, we could re use that root directly when providing the signature
There was a problem hiding this comment.
Ok, but note that this isn't exclusive to signing requests - any request made to the server with a body (e.g., any request that uses POST) needs to do this, which means we need to define SSZ types for every request and have the user conform to those.
Co-authored-by: ltitanb <[email protected]>
Co-authored-by: ltitanb <[email protected]>
This is part 2 of the update on CBST2-01, following #354 and #353. This solves one of the issues found within the audit by making all of the routes with a request body (e.g., the
POSTroutes) encode the Keccak256 hash of the payload body into the JWT claims for the request's auth header. Doing so means JWTs can't be intercepted and reused for unrelated requests, such as for signing different things other than what the original request was for. This affects all routes, including the new/revoke_jwtand/reloadones.