Resolve tension between Threat Model and Privacy & Security Considerations - #711
Resolve tension between Threat Model and Privacy & Security Considerations#711eric-schuh wants to merge 5 commits into
Conversation
…sideration sections into threats. Also modifying the privacy and security consideration sections to inform readers of the transition to using the threat model approach in place of these sections.
| </section> | ||
|
|
||
| <section> | ||
| <h3>Other Security Considerations</h3> |
There was a problem hiding this comment.
Maybe we can put this in the Threat Model section introduction?
| </p> | ||
|
|
||
| <section> | ||
| <h3>Secure Coding Practices</h3> |
There was a problem hiding this comment.
Maybe this goes in the conformance section? Or where we talk about conforming implementations?
kezike
left a comment
There was a problem hiding this comment.
@eric-schuh Thanks for extracting the threats from the security and privacy considerations. Feeling better about deleting those sections.
My review feedback contained the following types of comments:
- Suggestion for threat tags
- Suggestion for different STRIDE taxonomy class
- Response ID conflict resolution
- Minor renaming/capitalization/grammar updates
- Clarification questions
After these updates are applied and PR #709 is merged, we will also need to add the inline references (including tags) for each of the threats to the core spec doc. Happy to help with that if needed.
| var threat = { | ||
| id: "T30", | ||
| name: "Issuance request and/or delivery made by third party", | ||
| desc: "Verifiable credentials issuance has a request phase and a delivery phase. The request might be made by the subject or another role, and delivery can be to a client that might or might not be controlled by the subject.", |
There was a problem hiding this comment.
This threat is a bit confusing. For one, the terms “request phase” and “delivery phase” are a bit confusing. Secondly the notion of the subject requesting issuance is not common, unless you’re referring to the credential refresh flow. Could you reword this threat or explain the intention behind it in a comment?
There was a problem hiding this comment.
I gave it a try. This language came from the existing Security Considerations section this used to belong to so I am interpreting what was there. I do think that "requesting" the issuance of a credential is a fairly common pattern but agree that the "phase" terminology is likely from old language.
| (function () { | ||
| var threat = { | ||
| id: "T40", | ||
| name: "Improper credential validation leads to security vulnerabilities", |
There was a problem hiding this comment.
| name: "Improper credential validation leads to security vulnerabilities", | |
| name: "Improper Credential Validation Leads to Security Vulnerabilities", |
| name: "Improper credential validation leads to security vulnerabilities", | ||
| desc: "In most cases, simply verifying the proof might not be sufficient to properly handle the received data. Improper validation will often lead to security vulnerabilities.", | ||
| response: [ | ||
| { id: "R44", name: "Additional validation", type: "Reduce", desc: "Verifier services are expected to configure additional validation steps based on their use cases. To define such additional validations, implementers can refer to specifications such as Section 2.3: Resource Integrity and Section 2.4: Contexts and Vocabularies in the Verifiable Credential Data Integrity 1.0 specification where further information can be found about context handling and integrity verification. Additional validation steps can be accounted for when returning a verification response object, through the problem details." } |
There was a problem hiding this comment.
| { id: "R44", name: "Additional validation", type: "Reduce", desc: "Verifier services are expected to configure additional validation steps based on their use cases. To define such additional validations, implementers can refer to specifications such as Section 2.3: Resource Integrity and Section 2.4: Contexts and Vocabularies in the Verifiable Credential Data Integrity 1.0 specification where further information can be found about context handling and integrity verification. Additional validation steps can be accounted for when returning a verification response object, through the problem details." } | |
| { id: "R44", name: "Additional Credential Validation", type: "Reduce", desc: "Verifier services are expected to configure additional validation steps based on their use cases. To define such additional validations, implementers can refer to resources such as the <a data-cite="?VC-DATA-INTEGRITY#resource-integrity">Resource Integrity</a> and <a data-cite="?VC-DATA-INTEGRITY#contexts-and-vocabularies">Contexts and Vocabularies</a> sections in [[[?VC-DATA-INTEGRITY]]], where further information can be found about context handling and integrity verification. Additional validation steps can be reported when returning a verification response object, through the <a href=“#error-handling”>problem details</a>." } |
There was a problem hiding this comment.
Please confirm the following before applying this suggestion:
- The
data-citeanchor tag attributes properly link to the appropriate sections in the Data Integrity spec [[[?VC-DATA-INTEGRITY]]]properly renders the named link to the Data Integrity spec- The
#error-handlinganchor properly links to the Error Handling section of the VCALM spec (may need to use<a data-cite="VCALM#error-handling">problem details</a>instead)
| ], | ||
| elements: ["E1", "P5", "P4"], | ||
| taxonomyName: "STRIDE", | ||
| taxonomyClass: "Denial of Service" |
There was a problem hiding this comment.
| taxonomyClass: "Denial of Service" | |
| taxonomyClass: "Tampering" |
| </section> | ||
|
|
||
|
|
||
| <section class="appendix"> |
There was a problem hiding this comment.
Don’t know if it makes sense to have traditional security and privacy consideration sections in the threat model, but I agree with Manu about finding strategic places to place this information in the core spec (e.g., conformance, threat model, or some other appendix) or the new threat model document.
kezike
left a comment
There was a problem hiding this comment.
@eric-schuh Thanks for extracting the threats from the security and privacy considerations. Feeling better about deleting those sections.
My review feedback contained the following types of comments:
- Suggestion for threat tags
- Suggestion for different STRIDE taxonomy class
- Response ID conflict resolution
- Minor renaming/capitalization/grammar updates
- Clarification questions
After these updates are applied and PR #709 is merged, we will also need to add the inline references (including tags) for each of the threats to the core spec doc. Happy to help with that if needed.
Thanks for the review, I will go through the changes before the call tomorrow. I think the best path forward is to get #709 merged and then update this PR to handle any conflicts, include the new inline references for the new threats, and handle the suggestions made on the last call about the two security consideration sections I left as appendixes. |
@eric-schuh I have merged PR #709 into |
Co-authored-by: Kayode Ezike <kezike13@gmail.com>
Will do, I am having some issues with Github not responding this morning so will return to this tomorrow and finish up resolving your non-editorial comments/suggestions. As long as there is no push back from the group on the call today I will plan on merging once those are complete. |
| var threat = { | ||
| id: "T30", | ||
| name: "Issuance Request and/or Delivery Made by Third Party", | ||
| desc: "Verifiable credentials issuance has a request phase and a delivery phase. The request might be made by the subject or another role, and delivery can be to a client that might or might not be controlled by the subject.", |
There was a problem hiding this comment.
| desc: "Verifiable credentials issuance has a request phase and a delivery phase. The request might be made by the subject or another role, and delivery can be to a client that might or might not be controlled by the subject.", | |
| desc: "There are no guarantees that a request for the issuance of a verifiable credentials is made by the subject of the credential. Similarly there are no guarantees that the credential is being delivered to a wallet controlled directly by the subject of said credential. Workflows allowing these third party actors to act as, or on behalf of, the subject without their authorization can create unknown harms to the subject in question.", |
Co-authored-by: Kayode Ezike <kezike13@gmail.com> Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Kayode Ezike <kezike13@gmail.com>
| var threat = { | ||
| id: "T38", | ||
| name: "Mishandled Status Information Upon Credential Deletion Reveals Information", | ||
| desc: "When deleting a verifiable credential, handling of its status information needs to be considered. Some use cases might call for deletion of a particular verifiable credential to also set the revocation and suspension bits of that verifiable credential, such that any sort of status check for the deleted credential fails and use of the credential is halted.", |
There was a problem hiding this comment.
| desc: "When deleting a verifiable credential, handling of its status information needs to be considered. Some use cases might call for deletion of a particular verifiable credential to also set the revocation and suspension bits of that verifiable credential, such that any sort of status check for the deleted credential fails and use of the credential is halted.", | |
| desc: "When deleting a verifiable credential, handling of its status information needs to be considered. Some use cases might call for deletion of a particular verifiable credential to also set the revocation and suspension bits of that verifiable credential, such that any sort of status check for the deleted credential fails and use of the credential is halted without the knowledge of the holder. This can lead to frustration and confusion when the holder attempts to use the credential for what they deem to be a valid purpose.", |
| (function () { | ||
| var threat = { | ||
| id: "T38", | ||
| name: "Mishandled Status Information Upon Credential Deletion Reveals Information", |
There was a problem hiding this comment.
| name: "Mishandled Status Information Upon Credential Deletion Reveals Information", | |
| name: "Mishandled Status Information Upon Credential Deletion Lead to Holder Confusion", |
This PR is in response to: #687
Adding threats created from transforming the privacy and security consideration sections into threats. Also modifying the privacy and security consideration sections to inform readers of the transition to using the threat model approach in place of these sections.
Any sections from the security considerations that did not have threats that directly applied to this threat model but still wanted to make a recommendation were move to appendixes in the threat model.
Preview | Diff