-
Notifications
You must be signed in to change notification settings - Fork 112
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
Protected term errors when supporting v1 and v2 #1150
Comments
excellent catch @OR13 |
🔥 Don't be evil. We need to discuss some security issuers here. Probably a better resolution to the above error const multiPresentationSupport = {
'@context': {'@vocab': 'https://www.w3.org/2018/credentials#'},
type: ['VerifiablePresentation'],
id: `urn:uuid:${uuid.v4()}`,
holder: holder,
verifiableCredential: verifiablePresentations,
} ![]() Notice this time the graphs are joined properly, because Also notice that "issuer dependent terms" are defined relative to w3c, not w3id.org. This is important since those terms might leak claim vocabulary metadata into the network logs of w3id.org. For example, users who click https://w3id.org/security#proof leak interest of a particular credential vocabulary being relevant to a referrer, to w3id.org... Goto to microsoft.com, open console, past: (async ()=>{
// comment this code out to remove an undefined term error
// and demonstrate that w3id.org sees traffic from referring websites
// and might be attacked for facilitating term definitions by threat actors that don't like the term being defined
var term = property.undefined
await fetch('https://w3id.org/security#proof')
})() ![]() Notice the referrer in the request headers, and the location in the response headers: ![]() ![]() ![]() Because of w3id.org, GitHub learns of microsoft.com interest in security vocabulary... and w3id.org learns term /vocabulary resolution timing related to microsoft.com and github. You can click this link https://w3id.org/security#proof to make w3id.org care even more about this particular term. Consider also the dangers, and costs of: (async ()=>{
// comment this code out to remove an undefined term error
// and demonstrate that w3id.org sees traffic from referring websites
// and might be attacked for facilitating term definitions by threat actors that don't like the term being defined
var term = property.undefined
while(true){
await fetch('https://w3id.org/security#proof')
}
})() Very quickly this becomes a cost center for the vocabulary service provider.... and the redirection service... (don't run the above code, it will denial of service attack the operators of https://github.com/perma-id/w3id.org The problem with URLs, is that developers use them... and not always per the specs, despite our best efforts. We could mitigate some of these issues by using term definitions that are not resolvable, but that will create a different set of usability challenges, since someone would need to look up where the URNs were defined to understand what they mean, for example: urn:ietf:params:oauth:jwk-thumbprint:sha-256:NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs Is not resolvable, but but: https://w3id.org/security#jwk-thumbprint is... even though the link above provides no human readable definitions. |
Yeah, we made breaking changes in v2. However, I think we can address this by adding This approach works in the JSON-LD playground: https://tinyurl.com/29yye6vc |
@OR13 — Please put this and similar warnings before the code that should not be run, and optimally break such code making it unexecutable without some fix(es), and include some comment(s) saying that it has been broken, and if you must include some comment(s) explicitly describing how a bad actor could change the code to make it executable. Note that various crawlers may act on any URLs they encounter, whether or not they're "live" on the page where they're encountered, so you may already be causing trouble for various entities. |
@TallTed how would you suggest disabling the above code? Should I make the URL invalid so fetch doesn't run properly? |
Related to #1158 |
Must be done before CR, because of the context being normative, see also #1149 |
By inserting characters or words or comment delimiters or the like, such that the code won't run, optimally at all, but minimally before it does the thing causing you to warn people not to run the code.
Sure, that might work. Making it invalid early, such that no |
The issue was discussed in a meeting on 2023-06-28
View the transcript2.15. Protected term errors when supporting v1 and v2 (issue vc-data-model#1150)See github issue vc-data-model#1150. Brent Zundel: # 1150 Orie raised this a few weeks ago. Needs someone assigned and a label. Orie Steele: If you make v2 presentation and use a v1 you get an error.
Brent Zundel: assigned to dllongley. Orie Steele: correction on scribes part if you make a v2 presentation and us a v1 context you'll get an error. Brent Zundel: pending close issues be aware! |
@TallTed thanks! I updated the examples to throw undefined term errors that halt execution, in honor of how JSON-LD processors also throw errors when terms are not properly defined in a context. |
This solution to this is in this comment. We should apply this when we make other context changes and rev the normative context hash. Related PRs for potentially adding or modifying context terms: #938 There may be more than that. We might be able to save some effort by making the context updates at once. |
The issue was discussed in a meeting on 2023-08-16
View the transcript2.3. Protected term errors when supporting v1 and v2 (issue vc-data-model#1150)See github issue vc-data-model#1150. Kristina Yasuda: protected term errors when doing v1 and v2, dave has pointed to multiple PRs in his latest comment, what's plan here? Dave Longley: I don't plan to do PR unless it's done as a final pass before CR over all of the other issues we have -- we have a number of things that will need tomake changes to v2 context (other specs, etc) -- we need to make pass at context at the end, tagged this item and others to make sure we get everything in there. Kristina Yasuda: waiting for v2 context to be "final final". Dave Longley: Not "final final", just right before we go to CR. |
1 similar comment
The issue was discussed in a meeting on 2023-08-16
View the transcript2.3. Protected term errors when supporting v1 and v2 (issue vc-data-model#1150)See github issue vc-data-model#1150. Kristina Yasuda: protected term errors when doing v1 and v2, dave has pointed to multiple PRs in his latest comment, what's plan here? Dave Longley: I don't plan to do PR unless it's done as a final pass before CR over all of the other issues we have -- we have a number of things that will need tomake changes to v2 context (other specs, etc) -- we need to make pass at context at the end, tagged this item and others to make sure we get everything in there. Kristina Yasuda: waiting for v2 context to be "final final". Dave Longley: Not "final final", just right before we go to CR. |
This will be addressed by #1259. |
PR #1259 is merged, closing. |
Consider:
The text was updated successfully, but these errors were encountered: