lightning
lightning copied to clipboard
hsmd: make get_per_commitment_point unconditionally safe by not returning secret
Removes returned old_secret from get_per_commitment_point making it safe to call on all commits
Motivation: When get_per_commitment_point returns the old secret it implies a revocation of index - 2. This causes a crash when VLS has validated a commitment but not seen the following revoke of the prior and the channel is asynchronously restarted. See https://gitlab.com/lightning-signer/validating-lightning-signer/-/issues/469
The last two commits in this PR are somewhat optional: by increasing the HSM_VERSION to 6 we can inform the signer that it should never return an old_secret (and therefore can't fail). The native hsmd implementation doesn't have the safety issue and it would be ok to leave it at HSM_VERSION 5. But it's cleaner to have consistent semantics.
All but the last two commits in the PR sequence are cleanup and reduction refactoring and worth considering even if an HSM_VERSION change is not considered.
Cleanup and refactoring Includes:
- prune unreachable pre HSM_VERSION 5 code
- factor out unneeded make_revocation_msg_from_secret
- split get_per_commitment_point uses into separate functions (one for point, one for secret)
- make the negotiated hsmd version available to libhsmd
The short-term fix for the VLS crash is merged on our side as in https://gitlab.com/lightning-signer/validating-lightning-signer/-/merge_requests/643
The matching "clean fix" on the VLS side to be matched w/ the last two commits of this PR is https://gitlab.com/lightning-signer/validating-lightning-signer/-/merge_requests/641
@rustyrussell
Ack 1b98ea88caf830743b167e32c7270000fd31ad5b