microsoft / CCF

Confidential Consortium Framework
https://microsoft.github.io/CCF/
Apache License 2.0
770 stars 207 forks source link

Add test that adds/removes members with share encryption keys in public recovery #4573

Open achamayou opened 1 year ago

achamayou commented 1 year ago

Notes from today's experiments:

  1. It's possible to add/remove members with share encryption keys in public recovery
  2. That's because it is possible to re-key and re-share the recovery ledger secrets
  3. However, the at-recovery secrets themselves cannot be re-keyed, and so this behaviour is unhelpful, despite being technically correct

As discussed with @eddyashton, there is known and deliberate member re-keying during recoveries (#4253), so forbidding any and all membership change isn't desirable.

Conclusion: strengthen *_member validation to disallow any changes to members that have an encryption key during recovery: it's very unlikely to do what users expect, and add an FAQ entry explaining this limitation.

achamayou commented 1 year ago

Further notes, from discussion with @jumaffre:

Transitioning recovery_members to Active during recovery should fail/return an error/rollback in /ack. Separately, set_member and remove_member should fail during both validate and apply if either:

set_recovery_threshold should be similarly gated, in validate and apply and native code (js_node_trigger_recovery_shares_refresh).

The resulting invariants are: