Skip to content
This repository has been archived by the owner on Jul 23, 2023. It is now read-only.

Don't log secrets in cleartext #10

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Legogris
Copy link
Contributor

@Legogris Legogris commented Jan 20, 2021

Thanks for your work on this @fiatjaf ! Having an integrated wallet with lightningd is great.

When setting this up, I have some thoughts on how to improve the security story for sparko.

This one I hope is obvious:

  • login password omitted
  • omit generated access-key
  • access keys replaced with first characters of sha256 hash

Following up in #11

* login password omitted
* omit generated access-key
* access keys replaced with first characters of sha256 hash
@fiatjaf
Copy link
Owner

fiatjaf commented Jan 21, 2021

Logging a hash is confusing, it's better to not log anything.

But following up on #11 I think this works if we add a new optional option, sparko-keyhashes=, that will work just like sparko-keys=, but for people who don't want to type the key on their config file, they can just type a hash.

And on memory we will just store the hashes. For sparko-keys we hash them all. For sparko-keyhashes we just use the hash the user has provided.

Then when someone does a call attempt we hash the key they sent and check against the key hashes we have in memory. What do you think?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants