Skip to content
This repository has been archived by the owner on Mar 5, 2022. It is now read-only.

docs: Security considerations section #68

Merged
merged 1 commit into from
Apr 9, 2020

Conversation

Moopli
Copy link
Contributor

@Moopli Moopli commented Apr 9, 2020

Signed-off-by: Filip Burlacu [email protected]

Signed-off-by: Filip Burlacu <[email protected]>
@Moopli Moopli requested review from troyronda and llorllale April 9, 2020 07:10
@Moopli Moopli self-assigned this Apr 9, 2020
@cla-bot cla-bot bot added the cla-signed label Apr 9, 2020
@codecov
Copy link

codecov bot commented Apr 9, 2020

Codecov Report

Merging #68 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #68   +/-   ##
=======================================
  Coverage   91.15%   91.15%           
=======================================
  Files          11       11           
  Lines         452      452           
=======================================
  Hits          412      412           
  Misses         21       21           
  Partials       19       19           
Impacted Files Coverage Δ
pkg/vdri/trustbloc/config/consortium.go 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc80179...1c7bbf5. Read the comment docs.

@@ -26,6 +26,9 @@
"type": "integer",
"minimum": 0
},
"history-hash": {
Copy link
Contributor

Choose a reason for hiding this comment

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

underscore not dash

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed in #70

@troyronda troyronda requested a review from rolsonquadras April 9, 2020 13:03
Error cases which terminate the discovery process in a failure state:
- Consortium config unavailable: The consortium domain points to a server that isn't functional
- Insufficient endorsement: Insufficient stakeholders endorse the consortium
## Security Considerations
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any guidance we can add here to help clients using did:trustbloc when considering security implications? What do they need to do? What do they need to consider in their own implementations?

For example, what are (some of) the consequences of not checking the history files / signatures during discovery? How does a client check that the crypto algorithms meet their requirements before trusting the consortium? things like that.

The provided sections on crypto agility and TOFU are good.

Copy link
Contributor

Choose a reason for hiding this comment

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

#71


### Trust on First Use

The TrustBloc DID Method is designed to address the trust-on-first-use (TOFU) problem with respect to trusting DID method endpoints. The [bootstrapping](#bootstrapping-trust) section details several methods by which a client may trust and verify the configuration of a consortium, and from there, use a set of DID method operation endpoints (ie, Sidetree endpoints) which it can trust on the basis of their inclusion in the configurations of endorsed stakeholders.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add ##Privacy Considerations as well? Some ideas (and I don't think we need a lot here)

  • although the public DID traceability problem still exists, consortiums will have different DIDs for entities which decreases their risk of observability and linkability.
  • consortiums may have strict requirements on which clients an access a given DID registry, preventing DID resolution (or even consortium discovery) to unauthorized parties.
  • DID Documents are not recorded on a permanent ledger, and therefore can be purged from consortium registries (while their existence still remains for forensic purposes?)

Anything else?

Copy link
Contributor

Choose a reason for hiding this comment

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

Here are two generic ones from the DID Core spec.
https://www.w3.org/TR/did-core/#privacy-considerations

Keep Personally-Identifiable Information (PII) Private
Note that DID documents should not contain PII.

DID Correlation Risks and Pseudonymous DIDs
Note that public DIDs have risk of correlation and pairwise unique DIDs should be used where correlation is problematic. Also suggest did:key and did:peer as examples of pairwise identifiers.

Copy link
Contributor

@troyronda troyronda Apr 9, 2020

Choose a reason for hiding this comment

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

@mavarley

DID Documents are not recorded on a permanent ledger, and therefore can be purged from consortium registries (while their existence still remains for forensic purposes?)

It is true that the DID documents (and their history) are not in the ledger. However, (patch) operations for multiple DIDs are mixed together in a Chunk file that is stored in the off-ledger storage. We should not delete the chunk file because that would impact the history of the other mixed-together DID operations. https://identity.foundation/sidetree/docs/spec/#chunk-files

@sandrask anything to add?

Copy link
Contributor

Choose a reason for hiding this comment

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

Anchor files are anchored to the target ledger system via embedding a CAS URI in the ledger’s transactional history. Once an Anchor File, Map File, and associated Chunk Files have been assembled for a given set of operations, a reference to the Anchor File (called Anchor String) is embedded within the target ledger.
Anchor string:
numOfOperations + “.” + CID of the anchor file

Copy link
Contributor

Choose a reason for hiding this comment

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

If we need to re-process Sidetree transactions (or another party wants to do it) chunk files need to be available.

Copy link
Contributor

Choose a reason for hiding this comment

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

#69

@@ -6,7 +6,7 @@ This is version `0.1` of the TrustBloc DID Method Specification.
## Introduction
_This section is non-normative_

The `did:trustbloc` DID method allows groups of independent entities to share custody of a DID registry consisting of [Sidetree](https://identity.foundation/sidetree/spec/) over a permissioned ledger.
The `did:trustbloc` DID method allows groups of independent entities to share custody of a DID registry consisting of [Sidetree](https://identity.foundation/sidetree/docs/spec/) over a permissioned ledger.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be more explicit that this link is to the the Sidetree protocol spec.

Perhaps an additional sentence like:

For more information on the Sidetree protocol, please refer to the Sidetree protocol specification.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed in #70

@troyronda troyronda requested a review from fqutishat April 9, 2020 14:14
Copy link
Contributor

@troyronda troyronda left a comment

Choose a reason for hiding this comment

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

Comments to be addressed in followup.

@troyronda troyronda merged commit 3bee141 into trustbloc:master Apr 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants