-
Notifications
You must be signed in to change notification settings - Fork 99
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
Define the JSON representation as JSON (without excluding directives) #396
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.
This PR is far less complicated than the alternative proposed in #394.
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.
This is not the purpose of the JSON section as written. If a new representation is desired, it should be defined in its own section with its own mime type.
This proposal is compatible with the proposals outlined in: #398 |
@jricher I am open to doing this, would you oppose |
It may come as a surprise to the WG that the intent of the JSON section was to remove processing directives that are valid JSON that leads to loss of data when converting between formats. It may also come as a surprise to the WG that representations must now list "processing directives" that are to be eliminated when converting to the abstract data model. It will certainly come as a surprise to the JSON-LD implementers that they now have to look at the registry and construct the If the intent of the section you wrote on JSON was the above, I expect there to be vigorous debate on the two sentences that this PR modifies. Primarily because there are implementers that are going to support both JSON and JSON-LD representations, and some of those implementers do not agree with the two statements this PR is modifying. My suggestion is that if a DID Method would like to remove known properties such as |
@msporny your argument for preserving the context data speaks to it not being a property of the did document but of the resolution metadata that processing the representation into the abstract document would produce. If the context data has to be preserved, then "context" needs to be defined as a property on the DID document itself. As I recall, there was significant pushback against that. |
DID Core defines what DID JSON is....if it differs widely from how Google, Microsoft and 99% of developers use JSON, we failed.... there is nothing stopping DID Method Authors from using JSON Schema / whatever they want to massively restrict what can be presented in a did document... in fact... any DID method that does not do this, is creating security issues for everyone who uses it.... If a DID Method author wants to return a DID Document with |
If you ignore properties that add security, that creates security problems, no? These arguments of "it's just JSON" and "this special property needs to be preserved" contradict each other. |
which property "adds security" ? The argument is "JSON does not delete properties that people don't understand.... it ignores them". |
I'm not saying delete the property, I'm saying that you should never produce the property so it's an error to consume it. |
if the property is if we are headed towards restricting JSON representation to "only known good values", I support that, but I expect @selfissued will object. |
I expect many in the group to object to that, myself included -- it eliminates any possibility of local extensibility and forces all extensibility to happen through a centralized registry. |
That's not the argument. The argument is "it's just JSON, and therefore all properties are preserved." Eliminating arbitrary properties is something that JSON does not do. It will come as a surprise to developers. There is a lot of talking past each other on this topic. I'm going to try and summarize some assertions I've heard on the calls/discussion threads to see if it changes anyone's understanding of the issues at hand:
|
This resonates with me. There is a lot of talk about "dropping" or "removing" or "preserving" |
My point is that telling JSON developers what they can or can't produce, in their JSON... is the path to rampant conformance test failure... and remains a terrible idea. I agree, a JSON developer who intends to not support any other representation SHOULD NOT produce an It's also the case that if they do, they still have a JSON document.... we can decide to throw an error, and treat DID JSON differently than vanilla JSON.... or we can not throw an error. I am in favor of not throwing extra errors, especially when they are completely not expected... I am in favor of all these representations being valid: https://github.com/transmute-industries/did-core/tree/master/packages/did-representation-examples/json AND all these representations being valid: https://github.com/transmute-industries/did-core/tree/master/packages/did-representation-examples/jsonld Note this one in particular: https://github.com/transmute-industries/did-core/blob/master/packages/did-representation-examples/jsonld/didDocument.xml
https://sucuri.net/guides/owasp-top-10-security-vulnerabilities-2020/ How to Prevent Insecure DeserializationsThe best way to protect your web application from this type of risk is not to accept serialized objects from untrusted sources. If you can’t do this, OWASP security provides more technical recommendations that you (or your developers) can try to implement:
The point I am making is that NO EXCEPTION is thrown when parsing JSON with property members that are allowed in JSON. If we decide that we are going to throw errors for JSON membership we are going to enter a state where some developers do the extra unnecessary work that did core suggests and most don't.... thats called attack surface... and we are creating it by not understanding the default behavior of JSON, which is to not throw errors when "unknown" properties are present.... to a JSON developer who has never read the did core spec or JSON-LD...
@peacekeeper will you be adding a Consider:
With response:
Any yet, no
Why not just return Asking for JSON should yield JSON.... asking for JSON-LD should yield JSON-LD.... JSON DID Documents SHOULD NOT have an My recommendation for any DID Method authors reading this in the future trying to support JSON is review OWASP, use something like JSON Schema, don't allow arbitrary unknown JSON members (this can lead to arbitrary code execution), and most of all, don't use a |
I think I agree with this and also agree with merging the PR, but for different reasons than @OR13 . If I understand correctly, @OR13 wants to be able to add Therefore, since the production rules for JSON do not add an |
@OR13 we should discuss Universal Resolver and other concrete implementations elsewhere, but yes the plan is of course to support multiple representations (JSON-LD, JSON, ...), and yes you're right that there's a missing |
This PR is still under debate... we had a special topic call to try to resolve some things: |
With PR #469 and #455 merged, and with #454 on track to being merged, those three PRs address what this PR was attempting to define. There is no way this PR is going to go into the spec at this point and it has been marked pending close for 15 days. Closing this PR given that it has been overtaken by events. |
Producer
application/did+json
Consumer
application/did+json
The JSON section should not reference the JSON-LD representation at all.
https://github.com/microsoft/api-guidelines/blob/vNext/Guidelines.md#61-ignore-rule
https://developers.google.com/knowledge-graph/reference/rest/v1?apix_params=%7B%22query%22%3A%22chuck%20norris%22%7D#http_request
Google Knowledge Graph return JSON-LD with
content-type: application/json; charset=UTF-8
https://developers.google.com/knowledge-graph/reference/rest/v1
Azure Digital Twin API returns JSON-LD with
content-type
application/json
https://docs.microsoft.com/en-us/azure/digital-twins/how-to-manage-routes-apis-cli
JSON Schema has this to say: https://json-schema.org/draft-06/json-schema-core.html
http://json-schema.org/understanding-json-schema/about.html
https://developer.mozilla.org/en-US/docs/Learn/JavaScript/Objects/JSON
https://tools.ietf.org/html/rfc4627
Preview | Diff