EPIC: Operator key replacement

[mdyring]

Summary

This is a feature request to introduce a replacement mechanism for validator operator keys.

It seems like a sensible place to introduce this would be in the "edit validator" code paths.

Problem Definition

With the fairly recent introduction of multisig it seems likely that validators will want to strengthen their security post launch, which will require the ability to replace the valoper key.

If a valoper key compromise is suspected it also makes sense to replace it.

Proposal

From a validator perspective it should be as simple as:

gaiacli edit-validator ... --from <existing key> --next-from <new key>

TX should be signed by the existing key and, for bonus points and to reduce risk, could required a signature with the new key as well.

Replacement should only be allowed if the new valoper account adheres to the minimum self bond.

Couple of ideas to solve scenario where validator can't finance 2 x min-self-bond:

1. Respect a send message that happens in same TX, which moves >= min-self-bond from old account to new (my personal preference)
2. Introduce explicit TX type (so not edit-validator) for this, which includes the logic to sufficiently fund the new account to level of min-self-bond.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[alexanderbez]

Thanks @mdyring! This can be of great benefit to the validator community certainly. Unfortunately, this will not be so easy on the SDK as the validator operator key is used in many places throughout the SDK as a primary index key -- this will be a very costly tx.

[mdyring]

I wouldn't expect this to be a common occurence and only done if required. So high cost seem OK.

[zmanian]

One thing that would pair well with this is the idea of being able to move a bond between accounts without unbonding.

[rigelrozanski]

I'd like to hear @sunnya97's thoughts on key management strategies here

[sunnya97]

This seems like it could be relatively solved by use of SubKeys. Keep your primary key in the coldest of cold storages, and then you can rotate your "operator key" (the one you actually use) however often you want.

[mdyring]

@sunnya97 @cwgoes looked at the SubKeys proposal and it looks like a step in the right direction!

I like the idea of ACL/permissioned subkeys, but I think it should be improved slightly:

1. Enable key rotation of all keys, including the master key. I would suggest the modifications below to remove the need for a master key associated with SubKeyAccount while introducing a special wildcard entry ("*") for PermissionedRoutes, which signals the master permission. PubKeyIndex 0 consequentially has no special meaning.

2. Per-subkey sequence, as independent processes might want to sign for same account with different subkeys. Overhead seems negligible.

A signature would then reference the account instead of master pubkey and point to SubKeyIndex.

// StdSignature represents a sig
type StdSignature struct {
  Address       AccAddress
  Signature     []byte
  SubKeyIndex   uint // References SubKeys, 0 indexed
}

Master PubKey removed from SubKeyAccount along with Sequence:

type SubKeyAccount struct {
  Address       AccAddress
  AccountNumber uint64
  Coins         Coins
  SubKeys       []SubKeyMetadata
}

Sequence tracked for each SubKey:

type SubKeyMetadata struct {
  PubKey               sdk.AccPubKey
  PermissionedRoutes   []string  // Route of "*" implies Master permission
  DailyFeeAllowance    sdk.Coins
  DailyFeeUsed         sdk.Coins
  Sequence             uint64
  Revoked              bool
}

It would probably make sense to migrate existing accounts to the new account structure, this seems to be straight-forward migrate by taking existing PubKey and adding as SubKey with "*" PermissionedRoutes.

[sunnya97]

So the MasterKey is special because it is the key whose hash is the account address. This is important to enable account pruning in the future. See @zmanian's comments on this thread:
#4352

By the way, I reopened the SubKeys PR here:
#4380

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[zakarialounes]

Why this issue is closed? Could be very useful!

[alexanderbez]

Why this issue is closed? Could be very useful!

I think you commented on the wrong issue ;-)

[zakarialounes]

Oh yes, sorry. Can't wait to see this feature available <3

[onivalidator]

@zmanian @sunnya97 @alexanderbez Curious if you guys had any thoughts or updates on this front? I know it's been sometime since it was discussed, but seems like it could be a worthwhile implementation on the validator-security front. Not sure the development pain on implementing something like this, but is there a world where we might be able to see this feature in sdk 47?

[alexanderbez]

@onivalidator there hasn't been any implementation efforts on this front AFAIK, so it's definitely not slated in 0.47.

It hasn't been a huge priority for the core team. I'm not sure what the latest state of sub-keys is or if its even relevant? We're happy to consider a full proposal in an ADR and then work on it.

cc @tac0turtle

[tac0turtle]

I think @zmanian has an impl of this

[dylanschultzie]

We at Lavender.Five would like to see this happen. I recognize it'd be a fairly notable lift, such as the self bond requirement as noted in the original post, but would be a huge boon to validators.

A short list of benefits this would provide:

• keys being compromised due to operator negligence/hostile actors
• rotating keys from simple hot-mnemonics (as is the default method recommended for new operators) to utilizing a hardware wallet
• rotating keys to a multisig for operators that grow large enough to justify that actions

[mikedotexe]

For what it's worth, I found this issue today because I'd use it for safety reasons if it were available. (No attacks, but to be thorough.)

[PFC-developer]

we @ PFC would like to see some form of key rotation available.

lack of key rotation is a security issue imho.

[faddat]

We would use this and we're interested in developing it. @tac0turtle -- interested in a call maybe with @zmanian to plan it out.

@PFC-developer I agree.

@mikedotexe I agree.

We recently came up with what we consider to be an ideal way for how to run validators across an organization like notional.

It's kind of like if we had it to do over again, how would we do it?

• Multisig for the operator account
• Authz for signing governance transactions

There are countless security reasons that this is better than what is common practice today.

If this feature were implemented, it would be possible to create a guide that allows teams operating validators to do effective planning.

Currently, I think that's just not possible, though kudos to teams who had the foresight to go multisig from the start.

Since we don't have any way to transfer the operator status, to my knowledge many validator teams have come up with their own solutions that to some degree emulate this, but are ultimately not as good.

If anybody else would be interested in joining this call I am happy to have you, and I think that this is an important one. Thank you.

[tac0turtle]

personally I don't think we should do this in the current staking module. We should rewrite the staking module with a variety of new features, the current module is over 4+ years old and there have been countless learnings from then

[PFC-developer]

I think there should be 2 tracks.
1 to fix a immediate concern (rotation of keys)
And another to build a better staking module with what we've learned over the last 4 years

[dylanschultzie]

@PFC-developer Depends on the lift for adding the rotation of keys, imo. If it requires a large rewrite in order to implement the feature, it stands to reason the module should just be rewritten cleanly with the new findings.

edit: At first glance at the code, I agree with your take. I don't think it'll be a huge enough lift to prioritize a full rewrite before adding a key-swapping feature.

Then again, it'll likely take a while for the module to be added to chains. Maybe the focus should be on a module rewrite?

[alexanderbez]

I think there should be 2 tracks. 1 to fix a immediate concern (rotation of keys) And another to build a better staking module with what we've learned over the last 4 years

I sort of agree here actually.

Operator (not consensus) key rotation should actually be pretty trivial and not add too much to the current staking module. Just spit balling here:

• Define a new message to rotate a validator's operator key
• Key rotation takes places at T + UnbondingPeriod, where T is when the message was committed, i.e. put it into a queue
• A validator can have only 1 key rotation in the queue at a time
• Rotate all "mature" keys from the key rotation queue during EndBlock

That should honestly pretty much be it. This allows use of multi-sigs too. We might want to explore using group accounts instead of multi-sig though @PFC-developer as they're more flexible and provides better on-chain UX.

Now I do agree x/staking needs a larger overhaul in the form of a new module. Would be happy to work on this if there's consensus.

[tac0turtle]

sounds good, we can do it. Let's add it to our stretch items and then q2. I think we may be able to start it this quarter

[tac0turtle]

iqlusion has a pr for consensus key rotation, dont think it would be too hard to also add operator key replacement as well

[alexanderbez]

Yeah I think my proposal #3863 (comment) is pretty sound. Just needs to be implemented :p

[danbryan]

Yeah,we need key rotation for the operator accounts. For example, when a partner leaves the company, and both partners had access to the key material. There should be a way to update the account for the remaining partner. The current options are
1.) risk that the old partner will not be malicious with the key material.
2.) Ask everyone to redelegate to the new account, which is a huge PITA on networks like Kujira, where that involves thousands of delegators to take action.

I would love to see a 3rd option inline with the ideas above.

[jonathanpberger]

1.) risk that the old partner will not be malicious with the key material.

It's uglier than "hope Old Partner is virtuous"; Old Partner is at risk here. God forbid something happens (and the old partner is indeed innocent); now they get dragged into an investigation they oughtta be clear of (with the attendant cost, hassle, repetitional risk, etc). There should be a clean mechanism for flushing stale signers.

[tac0turtle]

picking this up.

[tac0turtle]

diving into this, i think its more of a headache to do this from within staking and instead we can do key rotation/ sub keys with the new accounts module