Skip to content
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

Add suggestion on how malicious issuers can be detected. #161

Merged
merged 4 commits into from
Apr 16, 2024

Conversation

msporny
Copy link
Member

@msporny msporny commented Mar 31, 2024

This PR attempts to address issue #143 by adding guidance on how holder software can detect malicious issuers that inject tracking identifiers into their credentials.

/cc @zoracon


Preview | Diff

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated
cryptographic key for every credential issued that is tracked in a status list.
This sort of collusion can be detected by [=holder=] software by detecting
if the global identifiers used within a [=verifiable credential=] are shared
by other credentials. [=Holders=] could then be warned when presenting a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry but I do not follow this. Assuming most holders only get a single credential from each issuer then how is this possible? Furthermore even if they obtain multiple credentials from one issuer, then it is unlikely they will all have the same unique identifiers in them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An individual holder (a user of holder software) is not expected to be able to do this, but holder software (that services multiple holders) potentially could. As an example, holder software could offer an opt-in tool that could perform this analysis across multiple holders and report back its findings.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And which users are going to trust holder software that is shared by multiple users and that scans the wallets of multiple users?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And which users are going to trust holder software that is shared by multiple users and that scans the wallets of multiple users?

Most of them, I'd expect. Most people accept this kind of analysis from the most popular Web browsers, probably often without knowing it. Hopefully wallet providers will do better, especially when it is easier to provide alternatives (that don't do it at all) than it is to compete in the browser market.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you saying that web browsers scan a user's PC and converse between themselves about the contents? (as this is the scenario that you are proposing for wallets). If so, it surprises me, as I was not aware of this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@David-Chadwick, @dlongley — Please pick up this thread below, following the suggestions I first made based on the thread above and then modified based on reactions from @David-Chadwick

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TallTed, the text below looks ok to me modulo the change from "unique" to "shared" ... I'll comment down there on that.

@David-Chadwick,

Are you saying that web browsers scan a user's PC and converse between themselves about the contents?

I'm not implying anything as specific as "scan a user's PC", but rather, that some Web browsers (and some search engines) collect information based on your behavior and various inputs, aggregate it with information from other users, and send it somewhere for analysis, yes.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Copy link
Contributor

@David-Chadwick David-Chadwick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definition of unique "being the only one of its kind; unlike anything else." If an identifier is shared it cannot be unique. Please change to the following

that contains some global identifier(s) that are supposed to be unique to that credential.

@TallTed
Copy link
Member

TallTed commented Apr 5, 2024

@msporny — Could you merge the suggestions that have no argument, so the remaining suggestions are clearer to review?

@msporny
Copy link
Member Author

msporny commented Apr 6, 2024

I think the updated language is fine. Requesting a re-review from @TallTed and @David-Chadwick just to be sure. I'll merge after I get confirmation from both of them that the new language meets their concerns.

index.html Outdated Show resolved Hide resolved
Copy link
Contributor

@David-Chadwick David-Chadwick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we can easily see the revised text, it makes sense.

Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editorial — minor language tweaks and a paragraph break.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Show resolved Hide resolved
@msporny
Copy link
Member Author

msporny commented Apr 16, 2024

Editorial, multiple reviews, changes requested and made, no objections, merging.

@msporny msporny merged commit c09e94b into main Apr 16, 2024
2 checks passed
@msporny msporny deleted the msporny-malicious-protections branch April 16, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants