talaia-labs / rust-teos

The Eye of Satoshi - Lightning Watchtower
https://talaia-labs.github.io/talaia.watch/
MIT License
135 stars 63 forks source link

Revert to old tower keys #50

Open mariocynicys opened 2 years ago

mariocynicys commented 2 years ago

Changes:

Closes #49

sr-gi commented 2 years ago

@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.

mariocynicys commented 2 years ago

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 commented 2 years ago

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.

sr-gi commented 2 years ago

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 commented 2 years ago

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.

mariocynicys commented 2 years ago

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).