-
Notifications
You must be signed in to change notification settings - Fork 36
feat: ESSR protected client APIs #351
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: main
Are you sure you want to change the base?
Conversation
|
WebOfTrust/signify-ts#304 created on Signify side |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #351 +/- ##
==========================================
+ Coverage 93.80% 93.99% +0.18%
==========================================
Files 37 37
Lines 8462 8704 +242
==========================================
+ Hits 7938 8181 +243
+ Misses 524 523 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Updated PR to allow both signed headers and ESSR. I'm leaving this as draft as I still have the edge case I need to test for in the comment I left on the PR above, and I need to update my Signify PR. |
|
|
||
| if not self.authn.verify(req): | ||
| # @TODO - foconnor: Not sure if this should be here - Signify is not signing headers for passcode rotation. | ||
| if not self.authn.inbound(req): |
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.
@pfeairheller Passcode rotation (PUT /agent/{caid}) in Signify doesn't use signed headers (just has a signed body), like the rest of the /agent endpoints, so perhaps this shouldn't be checking for signed headers here. If not, I can update Signify to use the correct fetch, and adjust this to accept ESSR if needed.
Any idea @rodolfomiranda?
| if req.path.startswith(path): | ||
| return | ||
|
|
||
| req.path = quote(req.path) |
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.
The path is already encoded when sending over the wire. e.g. set 'alice' to 'alice test' in a test, and it will fail on main, as /alice%20test is being turned into /alice%2520test with this line (double encoded).
This PR adds an alternative mechanism to authenticate with the KERIA API from a Signify client, and leaves the current RFC-9421 signed headers in place too. Beyond confidentiality, it also resolves other issues from before such as unsigned query params and bodies. (#287)
The unsigned HTTP request from the client is converted to a HTTP bytestream and embedded in an ESSR payload. This payload becomes the body of a wrapper HTTP request for the
"/"path - so tunneled via ESSR. The wrapper could be handled by pure TCP but for now this works quite nicely in the Falcon middlewares with minimal changes.Because nothing else uses
"/"as the path, the authentication type is determined based on the path. A cleaner approach in a TCP world would just to have a CESR based command signed and optionally encrypted following ESSR, rather than complicating things with managing headers, or embedding HTTP requests within each other.I have written the corresponding code in Signify and all integration tests pass.
Flow for generating request from Signify:
POST /identifiers <body>Signify-Resourceheader (Encrypt Sender)crypto_box_seal(http_request_string, public_key_of_keria_agent)POST /where the body of the request is the sealed box bytes (application/octet-stream)Signify-Resource,Signify-TimestampandSignify-Receiverheaders (receiver is Sign Receiver)Note:
Like the keripy ESSR parser, the commitment to unsigned HTTP request is done by taking a digest:
A signature is created over this dict. I'm re-using the signature style of the signed headers.