Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hsmtool (and hsm_secret encryption) refactorings #4307

Merged
merged 9 commits into from
Jan 6, 2021

Conversation

darosior
Copy link
Collaborator

@darosior darosior commented Jan 3, 2021

This is a bunch of cleanups on top of #4303 .

Mainly, this makes the hsm_encryption contained into a common/ module, shared by hsmd, lightningd (options and hsm_control) and hsmtool. This then allows us to fuzz a roundtrip sanity check of our encryption.

Also:

  • Cleanup errors in hsmtool
  • Confirm password on encrypt in hsmtool
  • Be stricter on hsm_secret validity (only accept 32-bytes (plaintext) and 73-bytes (encrypted) seeds)

There are still some cleanups left to be done (the channel seed derivation function is still duplicated, we could use opt for argument parsing).

errx() was printing the confusing errno as well ("Error could not [...] :Success")

Signed-off-by: Antoine Poinsot <[email protected]>
Changelog-changed: lightningd: the `--encrypted-hsm` now asks you to confirm your password when first set
Changelog-changed: hsmtool: the `encrypt` now asks you to confirm your password
Signed-off-by: Antoine Poinsot <[email protected]>
This avoids duplication of both logic and error-prone values, such as
the salt. Grouping all hsm encryption logic into a public API will also
allow us to fuzz it.

Signed-off-by: Antoine Poinsot <[email protected]>
This makes use of the constant defined in the previous commits to more
accurately detect plaintext, encrypted, and invalid seeds. We now error
on invalid seeds.

Changelog-changed: hsmd: we now error at startup on invalid hsm_secret
Changelog-changed: hsmtool: all commands now error on invalid hsm_secret
Signed-off-by: Antoine Poinsot <[email protected]>
It's more useful if we actually want to use the output as, well, a
string..

Signed-off-by: Antoine Poinsot <[email protected]>
@cdecker
Copy link
Member

cdecker commented Jan 6, 2021

Rebased on top of master after merging #4303

ACK dc5c2db

@cdecker cdecker merged commit a4f07a3 into ElementsProject:master Jan 6, 2021
@darosior darosior deleted the hsmtool_refactor branch January 6, 2021 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants