feat(signer): add "Sign voluntary exit" permission - #107
Conversation
Voluntary exits were authorized solely by source IP address (server.rules.admin-ips), which is unusable in environments with dynamic client addresses such as Kubernetes. Introduce a dedicated "Sign voluntary exit" operation checked against the client certificate, matching how all other actions are authorized. Clients holding the permission (explicitly or via "All") can sign voluntary exits from any address; clients without it fall back to the generic "Sign" permission gated by admin-ips, preserving existing configurations. Closes attestantio#105
AntiD2ta
left a comment
There was a problem hiding this comment.
The new "Voluntary exit" permission (that allows signing an exit gated on domain/certs rather than on IPs) is neatly wired and its implementation is sound. A concern I have is that the feature is opt-out, i.e you have to do the following if you want to use "All" and still rely on admin-ips as a safety measure:
[~Sign voluntary exit, All]
This breaks the current contract we have, where "All" is safe to use and it allows exits from admin-ips only (which is opt-in in a way: you must set the IPs). People using "All" and not opting out from "Sign voluntary exit" may find themselves in a position where clients who couldn't previously get an exit signed, now can.
We are brainstorming internally if we are fine releasing this as a breaking change or requesting changes to make it opt-in only. Do you have a strong opinion on this?
While we discuss this, I raised some minor items I will be grateful if you can address/challenge.
I also thought about that and implementation of it based on these:
So I decided to implement it like it was mentioned in the docs. But maybe it's a good idea not to exclude P.S.: Or let me think about that again... |
- carry out the duplicate public key check across the entire Multisign batch, as the rules now run separately per action - fall back to the generic Sign permission only on an authorisation denial, not on system failures - guard against nil request data in OnSignVoluntaryExit - clarify in the permissions documentation that an explicit denial of "Sign voluntary exit" only removes the IP-free path
Voluntary exits were authorized solely by source IP address (server.rules.admin-ips), which is unusable in environments with dynamic client addresses such as Kubernetes.
Introduce a dedicated "Sign voluntary exit" operation checked against the client certificate, matching how all other actions are authorized. Clients holding the permission (explicitly or via "All") can sign voluntary exits from any address; clients without it fall back to the generic "Sign" permission gated by admin-ips, preserving existing configurations.
Closes #105