search

ssbc / ssb-peer-invites

A new ssb invite system to create invites without having a pub
MIT License
26 stars 2 forks source link

rm cap.js #7

Closed cryptix closed 5 years ago

cryptix commented 5 years ago

@dominictarr this isn't used by the tests as far as I could tell so I ripped it out. I'm not sure ssb-config supplies one yet, though?

mixmix commented 5 years ago

I think the question @cryptix is what our strategy is for where to put default config. I've been wondering if each plugin should have an explicit method like baseConfig or something so we know where to look? actually it would probably have to be a file ... echh this needs some concerted attention.

Anyway, how do we know if this is safe to rip out ?

dominictarr commented 5 years ago

yes we do need a default config for things like this. For questions like this, I'd first look to see what defacto patterns we are already using in other modules. off the top of my head: fairly sure each module is responsible for it's default caps... no that is wrong! ssb-server has the shs cap. ssb-keys has a cap for sign, but it defaults to no cap. (ssb-keys predates using caps)

dominictarr commented 5 years ago

requiring the cap to be in config means that now you need to keep ssb-peer-invites and ssb-config in sync... but ssb-peer-invites doesn't directly depend on ssb-config so you have a peer dependency. I think it's safer/saner to just let ssb-peer-invites own the default cap.

cryptix commented 5 years ago

how do we know if this is safe to rip out

I took this action just because the comment on the default sounded like it was a lost secret and shouldn’t be there in the first place. The unused requires didn’t help in figuring out what it is for either.

dominictarr commented 5 years ago

as a discussion PR this is excellent, but we need to have this somewhere - where should it go? Or should there just be a test that fails so that you arn't inclined to rip it out.

The unused requires didn’t help in figuring out what it is for either.

which unused requires are you refering to?

cryptix commented 5 years ago

which unused requires are you refering to?

apart from the changes in test/, types.js and valid.js stood out as being just unused variables.

dominictarr commented 5 years ago

I'm wouldn't be surprised if they contained some thing that became unused, I worked on this over a long time before it was ready, but both of those are used in index.js https://github.com/ssbc/ssb-peer-invites/blob/master/index.js#L3-L5

cryptix commented 5 years ago

but both of those are used in index.js

sorry that wasn't what I meant. types.js and valid.js - or rather all the files touched in this PR other than index.js - just required cap.js into an unused variable.

screen shot 2019-02-04 at 10 59 23

Again: I still don't understand what the purpose of this cap was and the comment on it sounded like it shouldn't go into a realse like this. Which is why I got the big axe and make a fire to find out what is going on here.

ps: I still don't know it's purpose.

dominictarr commented 5 years ago

@cryptix it's passed as hmac_key to signObj https://github.com/ssbc/ssb-peer-invites/blob/master/valid.js#L45-L51 see documentation of that method: https://github.com/ssbc/ssb-keys#signobjkeys-hmac_key-obj

dominictarr commented 5 years ago

on that note, we should change this...: https://github.com/ssbc/ssb-peer-invites/blob/master/cap.js#L9

cryptix commented 5 years ago

That comment is why I've made this PR. Should have made that clearer maybe.

dominictarr commented 5 years ago

@cryptix did you see: https://github.com/ssbc/ssb-peer-invites/pull/7#issuecomment-460420491 would you say you know what it's for now?

cryptix commented 5 years ago

I see mix already cleaned up the unused variables so let's close this.

cryptix commented 5 years ago

@dominictarr: I see now. I was afraid it's related to the seed creation. The way I understood it now, is that it changes the flavor of the signature (not plain hash but HMACed) but need to read up on chosen protocol attack why this is needed and or usefull.

I could see that you exchange it with a group first so only these invites are valid? The way I understood HMACs is that you don't share the secret with everyone so it seems a bit pointless to have a global default one but so it the main scuttlenet shs caps, I guess.

dominictarr commented 5 years ago

it's not about having a secret, it's just about gauranteenig a different value. sign(a) in one context could maybe be reused in another (unintended) context if a was also a valid value in that context. by doing sign(hmac(a, X)) then in the other context sign(hmac(a, Y)) the signature from one context cannot be reused in the other (unless there is a hash collision!)

It's just a way to create incompatible signatures, that won't be confused.