-
Notifications
You must be signed in to change notification settings - Fork 12
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
0.6 protocol migration: Signed Data Model #56
Merged
rrix
merged 19 commits into
consumer-reports-innovation-lab:main
from
rrix:signed-data-model
Dec 1, 2022
Merged
0.6 protocol migration: Signed Data Model #56
rrix
merged 19 commits into
consumer-reports-innovation-lab:main
from
rrix:signed-data-model
Dec 1, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kevinr
reviewed
Oct 26, 2022
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.
Overall this looks good. I've made some tweaks and suggestions but I think the meat is here and correct.
Co-authored-by: Kevin Riggle <[email protected]>
this ensures that the `kid` an the `iss` claim match Co-authored-by: Kevin Riggle <[email protected]>
has it been a year already
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This protocol revision moves towards a more robust security model briefly described in #54, the first part of a slow migration towards integrating some of our oldest feedback in #4 having come full circle on OIDC, OAuth2, and back to asymmetric signatures for the entire Data Rights Request. This builds on the research that @kevinr has been developing with us and the partner companies over the last few months.
In short, the entire Data Rights Request submitted in
POST /exercise
will be wrapped in a signed body. For now that envelope will continue to be an RFC 7515 JWT, but we are evaluating a move to https://doc.libsodium.org/ in coming protocol revisions. The core of this diff is thus a signed data model of a flattened Data Rights Request described in section 2.02 wrapped in JWT signed by asymmetric keysThe up-shot of this data model is that all
POST
requests to the API eventually will be signed by a private key, and the "service directory" we have been mulling over can simply be a mapping of "authorized agent key id" (thekid
parameter inPOST /exercise
) to the public key of the authorized agent, and there is no need for pair-wise API key exchange.In line with work that @dazzaji is preparing for the initial "public" version of the system rules, i've taken some liberties to clarify the Identity Verification systems within the technical specification -- to say that we haven't actually spent much time on the dynamic
need_user_verification
flow and that the initial IDv pipeline will be much simpler.There is a short list of work that needs to be addressed before a 0.6 is ready for full consideration:
I plan to tackle these in this diff:
POST /revoke
should be re-written to accept signed requestGET /status
authentication needs to be modeled, at least discussed. This is the major shortcoming of the signed data model in my eyes but maybe we can come up with a model that works for this. We could make this a POST but that doesn't feel "rest-y" but i am not sure i care.These will be added before 0.6 is tagged:
I did a pretty brash pass-through of this document and nipped-and-tucked some other confusing stuff like the distinction between the actors and the interfaces (#55)... i should have done this in a number of diffs, but stacking markdown diffs on github is worse than a visit to the dentist so this is what we get for the first revision.