rust-teos icon indicating copy to clipboard operation
rust-teos copied to clipboard

Revert to old tower keys

Open mariocynicys opened this issue 2 years ago • 6 comments

Changes:

  • Renames --overwritekey flag to --generatenewkey.
  • Adds a new option --towerkey <keyindex>, which allows the usage of old tower keys based on their indices (first key have index 0, second has 1, and so on).
  • Makes the dbm::DBM::load_tower_key() method accept an optional key index to load.

Closes #49

mariocynicys avatar Apr 25 '22 13:04 mariocynicys

@meryacine loved the proactivity regarding this. However, there's a rationale for not having had this feature in the first place:

The tower identity is supposed to be long lived, that's the way of making it reputationally accountable (i.e. the longer a tower has been properly behaving, the better). I don't think we should encourage easily changing ids, since users expect the tower id not to change though-out their subscription (that's the public key they use to verify data coming from the tower).

There's an exception here, which is the tower key being compromised, in which case it should be re-generated (hence the option being offered).

I'm open to discuss this since it is, at the end of the day, a design decision. To me, this could make sense if we approach it from the perspective of being able to recover an old key if --overwritekey was called by mistake (that's mainly why the database stores old keys, mistakes happen), but I'm not too confident about allowing key rotation.

sr-gi avatar Apr 25 '22 17:04 sr-gi

Shouldn't changing the tower keys already disrepute the old keys since the response signature would be invalid when trying to verify it using the old pubkey (the tower failed to respond). This by itself would discourage towers to rotate their keys.

mariocynicys avatar Apr 28 '22 20:04 mariocynicys

Considering that reverting to old keys should only be used as a backup method for mistakes, one way to restrict using it for key rotation is by deleting all the keys past the key we are reverting to. This won't allow the tower to store many identities and swap between them freely.

mariocynicys avatar Apr 28 '22 20:04 mariocynicys

I'm reconsidering whether the key rotation is a good idea in the first place. Had a chat with @cdecker recently and CoreLN does not allow that in the first place. I'm trying to poll what node implementors do in this case and maybe go the same direction (i.e. only store the last key in the db and maybe offer a fresh start option that may wipe the whole thing).

https://twitter.com/sr_gi/status/1520741992975769602

Any thoughts on that?

sr-gi avatar May 01 '22 12:05 sr-gi

Putting more thought into this, I think key rotation (or at least bootstrapping with a fresh key from cmd) is a good idea, at least for testing purposes.

If we end up only allowing this for testing (and therefore discouraging this for normal use) we can simply remove the old key storage and just keep the latest key.

sr-gi avatar Jun 22 '22 09:06 sr-gi

Reading this again...

Considering that reverting to old keys should only be used as a backup method for mistakes, one way to restrict using it for key rotation is by deleting all the keys past the key we are reverting to. This won't allow the tower to store many identities and swap between them freely.

Gave me a though: what if we had only 2 slots (maximum of 2 stored keys at any given time). So the tower owner will have only one chance to retrieve the old key if they are overwritten (if you overwrite the key twice = it's gone forever). This is basically us deleting the third latest key instead of the second (and note that recovering the second key deletes the first, no recovery for the third key).

mariocynicys avatar Jun 22 '22 12:06 mariocynicys