-
Notifications
You must be signed in to change notification settings - Fork 11
Edit Privacy and Considerations sections #37
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
base: main
Are you sure you want to change the base?
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.
Minor nits, otherwise LGTM.
index.html
Outdated
permit tracing of a user in manner not understood or authorized by the | ||
user | ||
by some third party. | ||
would permit tracing of a user by a third party |
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.
would permit tracing of a user by a third party | |
would enable tracking of a user, without consent, by a third party |
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.
Change committed in 26b848b
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.
Discussing further in #37 (comment)
index.html
Outdated
DID method implementations are encouraged to rotate keys and | ||
identifiers as often as possible, | ||
to avoid correlation. |
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 know that this was already in the implementer guide, but it makes no sense. If you have a DID that is static, it makes no sense to rotate keys often in order to avoid correlation.
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.
Also, "rotating keys" generally does not avoid correlation. Using different identifiers for different contexts does, however.
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.
Agree, key rotation is about mitigating compromise... not mitigating correlation....
But the part about identifiers is correct, generating a new short lived DID and using it for 1 purpose, and then rotating away from it without a network observing seeing that rotation does mitigate correlation.
did key comes to mind in this regard, I think this section can be improved.... very good catch and comments.
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.
Changed: #37 (comment)
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.
Mostly minor language massage.
index.html
Outdated
@@ -1162,7 +1162,7 @@ <h2>Privacy Considerations</h2> | |||
|
|||
<p> | |||
Decentralized Identifiers, like any other technology, can be used to |
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.
Decentralized Identifiers, like any other technology, can be used to | |
Decentralized Identifiers, like many other technologies, can be used to |
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.
Committed: 5c7153e
index.html
Outdated
DID method specifications are encouraged to have a section dedicated to | ||
personal data, covering the extent to |
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.
DID method specifications are encouraged to have a section dedicated to | |
personal data, covering the extent to | |
DID method specifiers are encouraged to include a specification section | |
dedicated to personal data, covering the extent to |
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 think they are actually required to do this, by DID Core.... Privacy and Security considerations are requirements of a did method spec... maybe we can say some thing like "try and make these really impressive"... but still some folks will probably think they didn't go far enough.
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.
Committed suggested change in b5dc795.
I don't see "personal data" mentioned in these requirements:
- https://www.w3.org/TR/did-core/#security-requirements "requirements for all DID method specifications when authoring the Security Considerations section"
- https://www.w3.org/TR/did-core/#privacy-requirements "requirements for all DID method specifications when authoring the Privacy Considerations section"
But it is discussed in non-normative sections:
- https://www.w3.org/TR/did-core/#binding-to-physical-identity "A DID and DID document do not inherently carry any personal data and it is strongly advised that non-public entities do not publish personal data in DID documents."
- https://www.w3.org/TR/did-core/#encrypted-data-in-did-documents "placing encrypted data in a DID document is not an appropriate means to protect personal data."
- https://www.w3.org/TR/did-core/#keep-personal-data-private "If a DID method specification is written for a public-facing verifiable data registry where corresponding DIDs and DID documents might be made publicly available, it is critical that those DID documents contain no personal data." ...
We could say that the personal data section is encouraged in addition to the Privacy and Security considerations sections which are required. Or would personal data be a subsection of Privacy and/or Security considerations?
I agree that more encouragement to develop these sections would be good.
index.html
Outdated
would permit tracing of a user by a third party | ||
in manner not understood or authorized by the user. |
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.
would permit tracing of a user by a third party | |
in manner not understood or authorized by the user. | |
would enable a third party to track a user without the user's authorization, | |
in a manner the user may not understand. |
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 applied the similar earlier change #37 (comment) (permit -> enable, tracing -> tracking, consent).
I like that this change uses the active voice.
But is it okay to not mention "authorized" here - i.e. that it is implied by "consent"?
Could use this change but change "consent" to "consent/authorization"?
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.
[@clehner] Could use this change but change "consent" to "consent/authorization"?
Sure, s/consent/authorization/
applied above.
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.
@TallTed
This change suggestion (diff) doesn't apply cleanly now, due to the changes in #37 (comment) (26b848b).
I see your newer change suggestion on this sentence as updated, here: #37 (comment).
The newer suggested phrasing is different from the one here.
Does the newer change suggestion supersede this one?
index.html
Outdated
Review any applicable local law when considering developing or | ||
operating a decentralized identifier method. | ||
</p> | ||
|
||
<p>Consider <a href="https://gdpr.eu/">GDPR</a>.</p> | ||
|
||
<p>Consider <a href="https://oag.ca.gov/privacy/ccpa">CCPA</a>.</p> | ||
|
||
<p>Consider <a | ||
href="https://www.bis.doc.gov/index.php/policy-guidance/encryption">EAR</a>. | ||
</p> | ||
operating a decentralized identifier method. Consider the following: |
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.
Review any applicable local law when considering developing or | |
operating a decentralized identifier method. | |
</p> | |
<p>Consider <a href="https://gdpr.eu/">GDPR</a>.</p> | |
<p>Consider <a href="https://oag.ca.gov/privacy/ccpa">CCPA</a>.</p> | |
<p>Consider <a | |
href="https://www.bis.doc.gov/index.php/policy-guidance/encryption">EAR</a>. | |
</p> | |
operating a decentralized identifier method. Consider the following: | |
Review any applicable local laws when considering developing or | |
operating a decentralized identifier method. Such local laws may | |
include, but are not limited to, the following: |
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.
Committed: 5f088c6
index.html
Outdated
Avoid technologies declared <em>de facto</em> standard based on | ||
particular implementations but lacking open standards. |
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.
Avoid technologies declared <em>de facto</em> standard based on | |
particular implementations but lacking open standards. | |
Avoid technologies declared <em>de facto</em> standards based on | |
particular implementations but lacking open standard technical | |
specifications. |
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.
Committed: c9d2bc6
index.html
Outdated
store information other than a DID Document, which reduces the direct | ||
Avoid requiring credential schemas to be stored on ledgers. Many DID | ||
methods cannot | ||
store information other than a DID document, which reduces the direct | ||
interoperability, substitutability, and cost effectiveness of | ||
solutions that make use of rare or poorly supported features such as |
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.
solutions that make use of rare or poorly supported features such as | |
solutions that make use of rarely or poorly supported features such as |
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.
Committed: 287afe9
Looks good, thanks for investing time in improving this. |
index.html
Outdated
</p> | ||
|
||
<p class="advisement"> | ||
Support for legacy cryptography systems such as | ||
Consider support for legacy cryptography systems such as |
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.
Consider support for legacy cryptography systems such as | |
Consider support for widely adopted cryptography systems such as |
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 might be controversial, but i think legacy has the wrong connotation 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.
Consider support for legacy cryptography systems such as | |
Consider support for historically prevalent cryptography systems such as |
?
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.
Added both suggested phrasings, and removed the latter clause which I think is made redundant: c539bca
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.
@clehner this PR is excellent, please apply or dismiss the suggested changes and ping me for a re-review.
attacks have been performed in the wild due to improper selection of | ||
random values in key material and other aspects of cryptography. | ||
</section> | ||
|
||
<section class="informative"> | ||
<h3>Zero Knowledge Proofs</h3> | ||
<h3>Zero-Knowledge Proofs</h3> |
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 a comment on @clehner 's excellent editorial work.
Why does the DID Implementation Guide talk about zero-knowledge proofs at all? Perhaps to the extent that an issuer may wish to use a DID Document to publish the verification key for a zero-knowledge proof–capable VC, but beyond that, the information in this section seems to relate little to DIDs
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.
DID documents are to contain such crypto material as may be necessary to communicate with the entity identified by that DID. DID method specifiers and implementers may use ZKPs for these purposes, which possibility I think makes this section just as relevant as any of the others 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.
Really, my concern with the section is not that it talks about ZKPs and how they may be used with DIDs, my concern is that it doesn't.
The guidance here is to 1) use proper curves for ZKPs, 2) not use ZKP proof formats tied to specific ledger technology, and 3) not store schemas on ledgers.
What does this guidance have to do with DIDs?
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.
@brentzundel -- This should perhaps become a new issue of its own?
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 approve, once the suggestions already made by others have been accepted.
Co-authored-by: Manu Sporny <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Orie Steele <[email protected]> Co-authored-by: Ted Thibodeau Jr <[email protected]>
index.html
Outdated
identifiers as often as possible, | ||
to avoid correlation. |
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.
identifiers as often as possible, | |
to avoid correlation. | |
identifiers as often as practical, | |
to avoid compromise and correlation, respectively. |
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.
Applied in 45d8529.
Related earlier discussion: #37 (comment)
@TallTed would you mind resolving all change suggestions that have been addressed? or lmk if you feel this is good from your perspective? |
Co-authored-by: Ted Thibodeau Jr <[email protected]>
index.html
Outdated
</a> should be consulted when selecting curves for usage with zero | ||
knowledge proofs, | ||
href="https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-pairing-friendly-curves"> | ||
Pairing-Friendly Curves</a> |
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.
Pairing-Friendly Curves</a> | |
Pairing-Friendly Curves</a>, |
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.
Committed: 5b85935
I have no "resolve" button anywhere. Another downside to applying suggestions externally to the thread where they're suggested, instead of using the "commit" button that's right there for the PR author (and possibly others). |
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
I believe the requested changes have been resolved. |
Continue editing from #20 and #26, applying minor changes in sections 5 and 6.
Preview | Diff
Preview | Diff