ssbc / ssb-keys

keyfile operations for ssb
36 stars 26 forks source link

loadOrCreateSync can clobber existing keys in certain circumstances #44

Closed iameli closed 6 years ago

iameli commented 6 years ago

I'm still playing around with the exact repro case but it goes something like this. I have my local secret symlinked to kbfs so that it's backed up and secure and stuff.

▶︎ ls -l ~/.ssb/secret
lrwxr-xr-x 1 elinostreams staff 34 Apr 25 21:49 /Users/elinostreams/.ssb/secret -> /keybase/private/iameli/ssb/secret

The trouble arises when kbfs is temporarily unavailable. This could be for a variety of reasons — I think it happened a couple times when I was coming back from sleep mode using Patchwork. It then proceeds to try and create a new key, which succeeds? And clobbers my existing one in keybase.

I've got the file backed up plenty of places, nbd there, but this seems like something that should be avoided at all costs. Two thoughts:

  1. I'm going to throw in some error logging right here on my local Patchwork instances so I can figure out what is probably an issue specific to kbfs symlinks.
  2. Should createSync perhaps be a bit more conservative in not overwriting existing files if they exist?
dominictarr commented 6 years ago

yes I think it would be entirely reasonable if createSync just threw an error if something was already there, but invalid. If you made a PR I'd be happy to merge it. Please include tests though, because it's a somewhat weird usecase (you are the first person to ask for this) so if there arn't tests someone might change it.

iameli commented 6 years ago

right on, will do exactly that once I repro

iameli commented 6 years ago

the more I look in to this, the more I think this is mostly a result of weird optimistic syncing behavior on the part of kbfs

we'll call this good if #45 or something like it gets merged