Skip to content
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

Can't sync with project keys that aren't 64 characters or a valid hex string. #440

Closed
okdistribute opened this issue Oct 8, 2020 · 3 comments

Comments

@okdistribute
Copy link
Contributor

okdistribute commented Oct 8, 2020

Describe the bug
If your project key is malformed somehow, Mapeo will fail to find other devices.

Can you do the same thing twice?

  1. Use a project key configuration that is not a 32 byte buffer converted to hex.
  2. You won't see any devices.
  3. Look in the log error file, you'll see
2020-10-08T23:50:41.604Z [electron-background] error: syncJoin Error: projectKey must be undefined or a 32-byte Buffer, or a hex string encoding a 32-byte buffer
    at discoveryKey (/home/okdistribute/dev/mapeo-desktop/node_modules/@mapeo/core/sync.js:569:11)
    at Sync.join (/home/okdistribute/dev/mapeo-desktop/node_modules/@mapeo/core/sync.js:297:17)
    at MapeoRPC.syncJoin (/home/okdistribute/dev/mapeo-desktop/src/background/mapeo-core/mapeo.js:149:22)
    at Object.handlers.sync-join (/home/okdistribute/dev/mapeo-desktop/src/background/mapeo-core/index.js:161:17)
    at Server.<anonymous> (/home/okdistribute/dev/mapeo-desktop/node_modules/electron-rabbit/src/background-ipc.js:27:35)
    at Server.emit (/home/okdistribute/dev/mapeo-desktop/node_modules/event-pubsub/es5.js:74:21)
    at Server.gotData (/home/okdistribute/dev/mapeo-desktop/node_modules/node-ipc/dao/socketServer.js:194:14)
    at Socket.emit (events.js:223:5)
    at Socket.EventEmitter.emit (domain.js:475:20)
    at addChunk (_stream_readable.js:309:12)
    at readableAddChunk (_stream_readable.js:286:13)
    at Socket.Readable.push (_stream_readable.js:224:10)

Expected behavior

Project keys don't have to be a certain length, it should sync with any device that has the same key no matter what the key is.

Is it necessary that project keys have to be this particular length? This is arbitrary and puts up an unnecessary barrier to adoption.

For security once we have mapeo web, ideally all project keys will be at least a certain number of characters long, but there is no necessity for it to be exactly 64 characters.

Screenshots
I opened a pull request that updates the errors for fatal mapeo core errors so that Mapeo will tell the user that something is wrong at least.

image

Mapeo Version:
Mapeo Core v8, affecting all Mapeo Mobile and Mapeo Desktop versions for the past couple of years.

@jencastrodoesstuff
Copy link

Related to mapeo-config-amarakaeri/issues/5

@gmaclennan
Copy link
Member

I agree that sync errors due to issues with project keys leads to a bad user experience. I think the PR to show the error to the user is really helpful. I'm wary about writing code to accept different lengths of project key, since that will require changes to the code that uses it for encryption, and if it is shorter then it introduces a security risk because of lack of entropy.

We do need to address project keys and how they are created and managed. I think ideally they are created internally by Mapeo and the user never sees them.

As a short term measure I added a quick fix to mapeo-settings-builder that validates the project key as part of building the config. This way a user like @jencastrodoesstuff can see the error with the key without needing to install the settings and test sync. I tried to write clear error messages if the key is invalid:

image

I also added a command to generate a project key, which you can run with:

npx mapeo-settings-builder generate-key

It will output a random project key (note this currently runs very slowly sometimes due to large files being downloaded).

To update existing config to use the latest mapeo-settings-builder, from terminal, within the folder with the config, run the command:

npm install mapeo-settings-builder@latest

@okdistribute
Copy link
Contributor Author

Thanks for this @gmaclennan! I'm going to update the User Guide on how to use this tool with your notes here, at https://docs.mapeo.app. Let's make sure to continue keeping this up to date! It can be a really useful resource and having everyone's help to update it will make new user's life easier and help scale mapeo to the 1,000 projects it wants to see!

I'm wary about writing code to accept different lengths of project key, since that will require changes to the code that uses it for encryption, and if it is shorter then it introduces a security risk because of lack of entropy.

Just checked in with @noffle, and hyperswarm does require a 32 byte buffer for encryption purpses, but that's the only place. She noted that it would be possible to create shorter project keys and then expand those project keys into 32 byte buffers for encryption purposes using hashing, but that would be a breaking protocol change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants