Resolve tension between Threat Model and Privacy & Security Considerations - #711
Resolve tension between Threat Model and Privacy & Security Considerations#711eric-schuh wants to merge 2 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
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?
| (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)
| 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." } | ||
| ], | ||
| elements: ["E1", "P5", "P4"], |
There was a problem hiding this comment.
| elements: ["E1", "P5", "P4"], | |
| elements: ["E1", "P5", "P4"], | |
| tags: ["security"], |
| ], | ||
| 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
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. |
| name: "Malicious entity acts on behalf of incapable party", | ||
| desc: "Some subjects such as a new born baby, a pet, or a person with dementia, are incapable of acting on their own behalf. In these cases requests may be made on behalf of a subject with no cryptographic delegation possible.", | ||
| name: "Malicious Entity Acts on Behalf of Incapable Party", | ||
| desc: "Some subjects such as a new born baby, a pet, or a person with dementia, are incapable of acting on their own behalf. In these cases requests may be made on behalf of a subject with no cryptographic delegation possible. This can be problematic for guardians with malicious intent.", |
There was a problem hiding this comment.
| desc: "Some subjects such as a new born baby, a pet, or a person with dementia, are incapable of acting on their own behalf. In these cases requests may be made on behalf of a subject with no cryptographic delegation possible. This can be problematic for guardians with malicious intent.", | |
| desc: "Some subjects such as a new born baby, a pet, or a person with dementia, are incapable of acting on their own behalf. In these cases requests may be made on behalf of a subject with no cryptographic delegation possible. This can be problematic when guardians have malicious intent.", |
5 participants
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