Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign up[WIP] Collections support #1940
Conversation
|
A possible use case for collection key generation in more apps (i.e. add key generation to blockstack.js) would be to collections of data that are shared with a group of people across apps. |
|
Generally, I have read through the code, and can understand what is happening. There are two things I'd like to see before I would approve this:
|
| } | ||
|
|
||
| function writeCollectionKeysToAppStorage(appPrivateKey, hubConfig, keyFile) { | ||
| const compressedAppPrivateKey = `${appPrivateKey}01` |
This comment has been minimized.
This comment has been minimized.
hstove
Sep 11, 2019
Collaborator
Can you explain why you're doing it this way? I would think you would just use getPublicKeyFromPrivate from blockstack.js. This seems like you're doing some other mechanism for getting the public key.
This comment has been minimized.
This comment has been minimized.
yknl
Sep 13, 2019
Author
Collaborator
You're right, I missed that some how. I addressed it in a new commit.
|
@hstove testing can be performed using the basic test page here: https://blockstack.github.io/blockstack-collections/page_test/ And running the that same repo locally with |
| || settings.collections[collectionName].length > 0) { | ||
| const defaultCollectionSettings = DEFAULT_NEW_COLLECTION_SETTING_ARRAY | ||
| updateIdentityCollectionSettings(collectionName, defaultCollectionSettings) | ||
| return Promise.resolve(defaultCollectionSettings[0].encryptionKeyIndex) |
This comment has been minimized.
This comment has been minimized.
zone117x
Sep 16, 2019
Member
Trying to make sure I understand what's going on here -- is this encryptionKeyIndex intended to be used for bumping in order to perform a key revocation for a given collection type/scope?
This comment has been minimized.
This comment has been minimized.
yknl
Sep 17, 2019
Author
Collaborator
Yes that's correct. This gives us a deterministic way of generating encryption keys for collections.
|
I've been testing a local build of this in beta mode, with the Testing contacts collections using the demo apps:
Everything seems to be working as expected. The The collection key derivation logic is a bit hard to follow. It looks correct, but I left a comment where I was unclear on the inputs going into the collection node. |
| .update(`${collectionTypeName}${collectionIdentifier}${this.salt}`) | ||
| .digest('hex') | ||
| const collectionIndex = hashCode(hash) | ||
| const collectionNode = this.hdNode.deriveHardened(collectionIndex) |
This comment has been minimized.
This comment has been minimized.
zone117x
Oct 14, 2019
Member
We should probably using a longer derivation path here. All of these inputs (collection name, collection ident, salt) end up as only 32 bits of uniqueness.
This is the same problem with app-private-keys that PR #1496 was trying to solve.
This is the blockstack.js function that is able to perform better derivation, which we can leverage for collection HD nodes: https://github.com/blockstack/blockstack.js/blob/e4864ad41f376d963d4917b95996a6032f992789/src/wallet.ts#L289-L312
| .then(results => { | ||
| const collectionKeys = results[0] | ||
| const collectionHubConfigs = results[1] | ||
| return updateAppCollectionKeys( |
This comment has been minimized.
This comment has been minimized.
friedger
Nov 9, 2019
Contributor
@yknl This adds a hard requirement to the blockstack auth flow to be online. In the current flow it is possible to sign-in while offline (for android this works offline, for the web the manifest is needed when not cached).
yknl commentedJul 18, 2019
This PR implements handling of collection scopes, collections storage bucket and encryption key generation based on the design proposal. It makes use of the identity settings feature for storage of encryption key indexes. The key indexes are used to deterministically generate encryption keys for collection buckets. (Credits to @friedger for the suggestion) Key rotation is accomplished by incrementing the index. This reduces the impact of data corruption if randomly generated keys are used instead. This is changed from the original design where encryption keys are generated based on the list of apps granted access.
Some open questions are whether the key generation logic should exist in blockstack.js or in the browser.