Closed huumn closed 6 months ago
Since the problem is similar to common "remember me" or "stay logged in" functionality except it's related to wallet credentials instead of login credentials, I think the solution is to have a setting that asks if this wallet should be "remembered". If this is not checked, we delete the wallet credentials on logout.
We can still prefix the storage keys with account ids so credentials from accounts don't conflict if the same device is used.
I think the solution is to have a setting that asks if this wallet should be "remembered"
I kind of dislike adding things a person has to think about to serve an edge case. Why not just delete them on logout?
Why not just delete them on logout?
I thought it would be annoying to configure wallets again if you logged out once. But we can simply delete on logout first and then see if it will be annoying (and prefix storage keys with account id) :)
How "dangerous" is this localStorage leak? if its not I guess just changing the name as a prefix is simple enough
const storageKey = `{$userID}__webln:provider:nwc`
const configStr = window.localStorage.getItem(storageKey).split("__")[1]
Otherwise would some encryption of the config json be worth the effort if we end up deciding deleting it is a hassle?
How "dangerous" is this localStorage leak?
It gives access to spend external user funds so pretty dangerous. That's why we recommend to set budgets in the wallet setup page:
Otherwise would some encryption of the config json be worth the effort if we end up deciding deleting it is a hassle?
I think encryption and deleting on logout are separate issues. If you're logging out on a device, I think it's a reasonable expectation that everything about your session is deleted (encrypted or not).
Regarding encryption: We're open for ideas! We definitely want to improve the security around these credentials. All we did so far was to add CSP (nonce-based strict policy) in #805 to make XSS more difficult.
For example, we've discussed storing the credentials on the server but that makes us more of a target. Also, I think it's a reasonable expectation from stackers that we don't store them in plain text. They should be encrypted in some way that we cannot decrypt. But I would consider them always a liability.
So the question comes down to: How should this encryption scheme look like?
Should it use a password or PIN from which we derive a key? Should it be more like 2FA where we store a decryption key on the server and the encrypted content is in their local storage which gets decrypted and stored in memory (never store decrypted credentials in local storage)? How safe is storing the decrypted credentials in memory? Should we not use local storage at all? How much can we trust the Web Crypto API? How safe is it against XSS? How much can we trust ourselves to develop a secure encryption scheme with it? See warning on the Web Crypto API
page:
Warning: This API provides a number of low-level cryptographic primitives. It's very easy to misuse them, and the pitfalls involved can be very subtle.
Even assuming you use the basic cryptographic functions correctly, secure key management and overall security system design are extremely hard to get right, and are generally the domain of specialist security experts.
Errors in security system design and implementation can make the security of the system completely ineffective.
Please learn and experiment, but don't guarantee or imply the security of your work before an individual knowledgeable in this subject matter thoroughly reviews it. The Crypto 101 Course can be a great place to start learning about the design and implementation of secure systems.
So imo there is a lot to consider but we should also avoid analysis paralysis.
Got a nostr DM from someone who logged into another account in the same browser and their new account had access to their other account's NWC.
Given we store NWC in localstorage this makes sense. Can we create authenticated localstorage? If not, we should at least clear localstorage when someone logs out.
I don't expect this to be a common problem, but it's worth not letting this happen.