-
Notifications
You must be signed in to change notification settings - Fork 112
Add normative requirements regarding media type and proof #1014
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -5132,6 +5132,12 @@ <h2>The <code>application/credential+ld+json</code> Media Type</h2> | |||||||||||||
the use of a specific media type. | ||||||||||||||
</p> | ||||||||||||||
|
||||||||||||||
<p> | ||||||||||||||
This media type MUST NOT be used to describe a verifiable credential with an <dfn>embedded proof</dfn>. | ||||||||||||||
</p> | ||||||||||||||
Comment on lines
+5135
to
+5137
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. -1 to this. VCs and the three party model generally do not create a "control structure". Neither the issuer (nor this WG) get to tell a verifier what they can or cannot accept. I think media types are not the right way to accomplish the goals here. If an API does not want to accept a particular field, it should say so / have a schema that rejects it, etc. Those are the right tools for that job. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dlongley are you ok with "should not" language for inclusion of an embedded proof? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if we can flip the language around and say something like this:
I'm offering the text above not because I think it should be in the spec, but to do a temperature check on compromise language that might work. IOW, the statement above says: "Use this 'unsecured' media type and embedded proofs together at your peril... DO use a more specific media type for expressing that the content is secured." That said, we will probably also need language that effectively states: "DO NOT just trust media types... that's an easily exploitable attack vector". If it says the media type is secured, you MUST run algorithm XYZ to ensure that it is. Another way to look at this is when MUST an implementation throw an error. I can think of two instances where it definitely MUST throw an error:
... I'm sure there are other cases where we can all agree that an error MUST be thrown (at some layer). To see if other media type registrations contained language like this, I started with It does not contain any language about what properties MUST/SHOULD or MUST NOT/SHOULD NOT be included in the media type registration... so, that got me wondering... are there other media types that have this sort of language? Update: I just looked through the entire structured syntax suffix registry and found no language in each IANA registration section that prohibited certain content based on media type. What inspired the language in this PR? Was it from another media type registration? If so, which one? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No, I'm afraid not. I don't think it's the right use of media types. IMO, the language is unnecessarily restrictive, ineffectual, and likely to create more confusion and unnecessary complexity as a result. However, I am not against more specific media types (other than that I think we should have a strong reason for every new media type we create). Nor am I against recommending that more specific media types be used if there's a desire to communicate that some data meets some subclass constraints. But that just seems to be general media type advice that shouldn't need to be said in a particular media type registration.
I think language like that is unnecessarily fearful. There's no reason to bring concepts like "use this as your own peril" into this. We don't need to create situations where people are afraid to consume certain data because of the media type that accompanies it. That further proliferates the wrong idea about media types, how to decide whether data is authentic (and what to do / not do about it), and how to work with data in the ecosystem generally, IMO. In the three party model, there is no single party that tells other parties what to accept and then they just ... accept it. That's a core difference to some common two party models. There is no "Authorization Server" here. Instead, there is self-describing data that conforms to certain specs. Independent parties implement to those specs and when they see data that they understand -- they know how to consume it and what it means. If they don't understand it, they ignore it or reject it -- their choice. What they understand and are willing to accept is up to their business rules. They don't need to get together with other parties or have some special, elevated authorization party tell them what to do; that's just not how this technology works at all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @msporny This seems closer...
How about this variation:
To your point re the registration side - I think you are correct there, in that I don't think the registration block is the right place for this. I would like clear normative guidance about how to use the media types, and believe that the correct location for language to this effect does not need to be noted right inline below the table as it is done here, and probably could be located elsewhere in the spec. edit: to add a notes on few things that get into multiple representations that then point out to external specs for normative and interop purposes such as If we are going to have any guidance in this section it should likely be in one (or aspects of the guidance in both of ) the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The v1 context says Perhaps the mistake we have made was to define https://w3c.github.io/vc-data-model/#proofs-signatures
@msporny confusion over what a "credential" is... and wanting to define a media type so that the bytes for a representation of a "credential" can be secured and referred to consistently... As I noted above, perhaps there is no concept of In either case, this dialog is emphasizing a critical flaw in the spec... it is clear the current language is being interpreted in drastically different ways by readers. A few paths forward.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Strong +1 to this. It's the point I was trying to convey in #1044 (comment) AFAIK there is no normative definition of what a I am against 1, and in favor of doing 2 and 3. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not necessarily against what @andresuribe87 proposes (and the spirit of his comment) ... provided that we can make everything hang together. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I should also note that @andresuribe87's comment I think somewhat captures what we tried do to simplify things in the 1.1 work. |
||||||||||||||
<p> | ||||||||||||||
This media type MAY be used with <dfn>external proof</dfn>. | ||||||||||||||
OR13 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
</p> | ||||||||||||||
</section> | ||||||||||||||
|
||||||||||||||
|
||||||||||||||
|
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.
@OR13,
My objection was marked resolved without addressing my concern: #1014 (comment)
I see that some people support that statement. I object to it. We don't have consensus yet.
I currently think it's the wrong thing to do and I've asked for a technical basis for it and one or more clear concrete examples -- otherwise I think it should be removed.
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.
Agreed, but I raised this PR, and I can merge suggestions from WG members that I agree with.
You are welcome to raise a separate PR that implements the consensus you wish the WG to adopt.
Or request changes on this PR given it is heading a direction you do not agree with.
@dlongley I opened #1033 for your review, its like this PR but with your suggestion applied
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.
Technically speaking, a W3C verifiable credential is also a W3C credential. It does not make sense to me to say that you can't tag a VC with an embedded proof with the media type
application/credential+ld+json
.I've heard an argument that to do this would be similar to making a mistake like JOSE's
alg: none
, but this argument is not technically clear. I've also argued that perhaps the opposite is true:alg: none
argument is metaphorical, then I say that no one should depend on a media type to determine whether a credential is to be trusted. So if the statement banning using this media type with content with an embedded proof is meant to imply that a credential tagged with this media type can therefore be trusted -- that is bad security practice. To me, that bad practice would be metaphorically similar toalg: none
.alg: none
argument refers to a literal problem, then I say thatalg: none
itself should be prohibited regardless of anything we do here; therefore, this should be an orthogonal concern.I've heard an argument that using
application/credential+ld+json
"tells an implementer something important and to treat it differently". But this offers nothing specific nor does it surface any particular choices as to how things would be different if someone did happen to tag content with an embedded proof with that media type.In fact, as an implementer, I would not expect to change any behavior whether an embedded proof was present or not -- other than potentially having the option to try and verify such a credential. But, if my software never tries that anyway (which would have to be the case if the statement were accepted), I do not expect to start rejecting credentials just because they have embedded proofs on them in these circumstances. That seems harmful and like I would be missing out on processing otherwise useful credentials.
It also seems like there may be use cases that are made more complex (having to handle two different media types when it's otherwise unnecessary) if embedded proofs are disallowed. As an example, a use case that comes to mind involves signing VCs with multiple proofs via proof sets. The first time a proof is attached to the credential, a caller would need to use media type 1, and the second time, media type 2. But existing implementations today that attach such proofs do not need to care about this, so this adds unnecessary complexity. Given my experience with what developers tend to do in these situations, I'd expect some of them to just start ignoring the prohibition statement, leading to interoperability problems. So, this prohibition statement needs to have real value, IMO.
Now, if this statement is not present, we don't have to worry about any of that. If we're going to have such a restriction it should be clearly articulated why -- both the benefits and the drawbacks, and a technical discussion should proceed from that followed by arrival at consensus. Otherwise, it's not worth including it at the moment and doing so could be harmful, IMO.
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.
@dlongley can you please request changes so it is clear that this PR cannot be merged without addressing / discussing them?
To be clear, I don't intend to alter the PR, but I would like it to be clear to WG members that you are objecting to the proposed text they have suggested which I have merged here.
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.
You may have missed this -- I did that above as the first thing in this particular subthread: #1014 (review)
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.
@dlongley that is a suggestion. Under "add a review" there's a "request changes" button that is a clear "do not merge" signal
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.
@OR13 wrote:
decentralgabe wrote:
lols
@dlongley perhaps try firing a few flares into the sky? Waving a giant "I OBJECT" flag?
It's clear that "Request Changes" and repeatedly saying you don't agree with the PR and then having the conversation arbitrarily marked as resolved to "make reviews easier" has created some level of confusion about your position on the issue.
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.
I mean it's literally called "add a suggestion" - in every project I've worked on I haven't taken it to be a blocking request. only official reviews of which the options are [approve, comment, request changes]
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.
@OR13, @decentralgabe,
:) ... as I said above, that's the first thing I did here. To be clear, I submitted a review where I chose "Request Changes" and included specific suggested changes (as can be seen above). I do appreciate all of the screen shots of what I did though! :P
I think what you may not be expecting is that only code owners / required reviewers get the red change icon and big red merge blocking warnings. It doesn't matter how hard I clicked the "Request Changes" button (and I did), it only shows a gray icon and treats my objection as coming from a mere peon worthy of dismissal. :)
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.
Editor hat on -- it's typically bad form for any Editor to dismiss change requests OR suggestions in a way that hides the fact that there continues to be disagreement on a particular line in a PR.
@OR13 was acting in good faith in an attempt to make the PR easier to read, however, in dismissing the change request and the resulting debate, no one else coming into this PR can see that there was clearly disagreement on a particular line.
Additionally, as Editors, it's our job to ensure that if we are not to take suggestions from WG Members (and the public at large) that we have the consent of the individual that raised the change request in the first place... because if we don't have consent, the PR will just be blocked upon merge with a request to roll back the PR (causing extra and unnecessary churn).
As a general rule, as an Editor, I won't merge a PR that has any pending suggestions (note, NOT change requests) that have not explicitly been closed/resolved by the individual that raised them OR where there isn't clear agreement that they're fine w/ their change suggestion or request not being merged.
When you resolve controversial change requests without the consent of the person that raised the change request, it almost always leads to further conflict in the PR.
We have been able to merge PRs on this spec for 9+ years w/o resorting to using the "Resolve conversation" button on threads containing disagreement.
That said, different Editors have different PR management styles... and the WG empowers the Editors to manage PRs in the way they feel will lead to reaching consensus of the group.
Open standards are not open source, incubation, or corporate projects, they're managed differently because the bar for reaching consensus is much higher at W3C (much to the dismay of a number of our WG members) :P.