multi: add support for password recovery using seed

[ukane-philemon]

Closes #2475.

Changes in this PR:

• Support for password recovery using the app seed (new endpoint - /resetapppassword)
• Add ResetAppPass(...) to core.Core.
• Fix navigation menu display after logout (Minor)
• Fix the bug that prevents showing errors on the Export App Seed form (minor)
• Modify the Login Form and add a new button to the settings page to accommodate
  new password reset functionality.
• Hide unnecessary/unused btns on the settings page if the user is logged out. (Minor)
• Add a new form for App Pass Reset.
• Add toggling to the Restore Seed label on the 'init' page.

Implementation details:
I figured out we needed a way to verify that the provided seed was indeed the correct seed, so I opted to use part of the original seed to encrypt seed+pass. A user wanting to reset pass will just provide the exported seed, we derive the encryption password from it and get the original seed and password with which we can change to the new password.

This is a tested and working POC as in the video below and suggestions are welcomed.

PS: There's also a Reset App Password(apart from the forgot app pass btn) on the settings page that also opens the rest pass form, but I guess we'll have to keep only one?

UI changes and password reset flow(video):
https://github.com/decred/dcrdex/assets/57448127/92f31aee-0fe3-4de4-b525-99eecb5d7632

After logging out (before):

After logging out (now):

[norwnd]

Cool! I left some considerations (mostly from user perspective).

Several more comments on Forgot App password form:

• ideally, it should be as close as possible (in terminology, layout, etc.) to Change App password (including variable names in the code); additionally I noticed that in UI some buttons/views use "Application" while other buttons/views use "App" (and some forms don't even mention either), would be better to use one of those everywhere for consistency
• the name and description on Forgot App password form could emphasize that it's a password reset and not initial setup, which it currently sounds like: Set your app password. This password will protect your DEX account keys and connected wallets.
• description on Forgot App password form could mention that wallet data will be erased and that user will need to re-create all wallets
• I'd remove Remember my password flag from Forgot App password form for simplicity (it's already overloaded with info), it's non-critical functionality and it isn't clear that Submit will log you in right away; alternatively we could just redirect user to login page (but that's suboptimal since he'll need to enter password once again)

[norwnd]

There is a better way than displaying this warning, I think, it'll also address this concern you have:

There might be some security concerns as any "valid" 128-char hex string can be used as a seed and there's no way to verify the reset pass seed except that wallets created with it will be different.

Upon seed generation (and for existing seeds, we can do that upon login once, to preserve backward compatibility) we can hash seed and store that hash in plain text on disk.
We can remove this warning tooltip and instead just hash the seed user supplied and compare it against that "original hash" we have stored on disk. If they match - it means user entered correct seed (the one he was using DEX with in the past), if they don't match we should warn the user suggesting:

• either to check and re-enter his seed again
• or use a different flow to hard-reset his DEX app entirely (this use-case is also a separate valid one, say if I downloaded DEX to just try it out, but didn't back up the seed and forgot the password I used ... I have no way to reset it, unless I delete DB manually)

[ukane-philemon]

We can remove this warning tooltip and instead just hash the seed user supplied and compare it against that "original hash" we have stored on disk. If they match - it means user entered correct seed (the one he was using DEX with in the past), if they don't match we should warn the user suggesting:

In eb467a4, I opted to stick to our existing encryption pattern which requires a password to create a "crypter" and storing the necessary details to DB.

use a different flow to hard-reset his DEX app entirely (this use-case is also a separate valid one, say if I downloaded DEX to just try it out, but didn't back up the seed and forgot the password I used ... I have no way to reset it, unless I delete DB manually)

I think in this scenario they will have to do the 'advanced stuff'. We could have a guide in place for this.

[norwnd]

In eb467a4, I opted to stick to our existing encryption pattern which requires a password to create a "crypter" and storing the necessary details to DB.

Your approach is even better!` perhaps I didn't quite understand how the seed encryption-decryption works,

I think in this scenario they will have to do the 'advanced stuff'. We could have a guide in place for this.

In general, users don't read tutorials/text, not anymore (when they have a lot of apps that do stuff for them), so I'd treat a tutorial as a measure of last resort and I frankly think things like #2468 is mostly a waste of time (might be useful for a couple of nerds, but that's it). I mean, it's nice to have tech spec/article but to force your users read it before using an app ... is where you lose them.
Perhaps me advocating this topic is also a waste of time, I'll stop now.

That said, this scenario of DEX re-creation is probably a rare one - hence doesn't deserve much attention any time soon (perhaps could be a separate issue lying around for someone, perhaps new contributor to the repo, to pick up down the line).

[norwnd]

Reading downloading/uploading seed as file - it isn't something I personally would ever use, and it kinda promotes bad practice of storing uncyphered/unencrypted seed on your PC, or am I missing something ?

[ukane-philemon]

it kinda promotes bad practice of storing uncyphered/unencrypted seed on your PC

I agree. I was playing with the thought that downloading would be better than copying them(we currently don't restrict this). Manually writing out the seed hex can be a pita considering that they are not valid words even as they appear in chunks.

[norwnd]

I was playing with the thought that downloading would be better than copying them(we currently don't restrict this).

I think @martonp implemented some kind of warning for using clipboard when dealing with private key for Ethereum, perhaps we can add similar warning for seed. But, "proper solution", as per your comment

Manually writing out the seed hex can be a pita considering that they are not valid words even as they appear in chunks.

would be to move to 12/24 word seed phrase, I have no idea why it hasn't been done yet.

[ukane-philemon]

Thanks for your input. I think these suggestions can go into an issue page for better attention.

[buck54321]

Yeah, this one's a pita because we have 64-bit seeds. Solution is needed. Keep clipboard available for now. OK to add a warning.

[norwnd]

View Application Seed form doesn't display error when I enter wrong password there (haven't checked whether it's an issue on master):

[norwnd]

Once everything is ready, we might want to test how this reset-app-with-seed feature works when we have some open/executing orders (to see 1) whether it allows seed reset - which it should and 2) whether wallet re-sync from scratch will allow for re-discovering and completing outstanding trades - both for redeem and refund),

just leaving this note here as a reminder.

[ukane-philemon]

Thanks for the review @norwnd, I'll convert this to draft until it's ready.

[ukane-philemon]

alternatively we could just redirect user to login page (but that's suboptimal since he'll need to enter password once again)

It's standard practice to log in again after a password reset to ensure the new password (If we don't want the auto login).

[JoeGruffins]

I have not tested this yet but I thought you would have to recreate all internal wallets and Reconfigure all external wallets? The passwords for those is encrypted and based on the user's password initially?

I really think you should have waited for buck or chap to comment on the issue, this is a lot of work and not a bug fix.

[ukane-philemon]

Thanks for your input @JoeGruffins. I have a tested POC that follows the same part as password change and won't require recreating or reconfiguring wallets. I'd appreciate more suggestions on this.

[ukane-philemon]

View Application Seed form doesn't display error when I enter wrong password there (haven't checked whether it's an issue on master):

Yes, it is and since it's minor I've included a fix for it here. @buck54321, do you want me to PR the fix separately?

[ukane-philemon]

Once everything is ready, we might want to test how this reset-app-with-seed feature works when we have some open/executing orders (to see 1) whether it allows seed reset - which it should and 2) whether wallet re-sync from scratch will allow for re-discovering and completing outstanding trades - both for redeem and refund),

just leaving this note here as a reminder.

With the new suggestion, I think these issues won’t be a concern anymore.

[buck54321]

View Application Seed form doesn't display error when I enter wrong password there (haven't checked whether it's an issue on master):

Yes, it is and since it's minor I've included a fix for it here. @buck54321, do you want me to PR the fix separately?

No. It's fine here.

[buck54321]

I liked @norwnd's original suggestion to use a hash of the seed. Please consider buck54321/dcrdex@dbeaf39...buck54321:dcrdex:simpler-seed-hash.

edit: Nevermind. Is bad suggestion.

[JoeGruffins]

@buck54321 I may be wrong but your diff replaces the inner key and so for any current wallets we will not be able to decrypt their passwords any longer?

[martonp]

Not sure if it's a pre-existing issue, but if I go to the settings page when I'm logged in and refresh the page, only the settings option will be displayed in the drop down menu. (This is after I changed .UserInfo.Initialized to IsInitialized in settings.tmpl).

Personally I would like auto-login, but I'm just lazy :).

Also, @JoeGruffins is right about the encrypted db passwords. I think it's just as secure to encrypt the password with the seed as storing the hash anyways.

[buck54321]

@buck54321 I may be wrong but your diff replaces the inner key and so for any current wallets we will not be able to decrypt their passwords any longer?

Ooooh. Right.

[ukane-philemon]

Not sure if it's a pre-existing issue, but if I go to the settings page when I'm logged in and refresh the page, only the settings option will be displayed in the drop down menu. (This is after I changed .UserInfo.Initialized to IsInitialized in settings.tmpl).

Thanks for testing @martonp. I'll look into this.

[ukane-philemon]

@buck54321 thanks for reviewing. I'll make changes soon.

[ukane-philemon]

Not sure if it's a pre-existing issue, but if I go to the settings page when I'm logged in and refresh the page, only the settings option will be displayed in the drop down menu

I think this was fixed by #2481

[buck54321]

I don't think we should add this functionality if it requires messing with our existing security.

[ukane-philemon]

I don't think we should add this functionality if it requires messing with our existing security.

Do you think this approach is reasonable instead?

pass protects seed (we have this already)
and
64 byte seed protects pass (proposed) instead of 32 byte seed protecting pass?

[buck54321]

@ukane-philemon What do you think about this upgrade buck54321/dcrdex@024aa50...buck54321:dcrdex:seed-derived-inner-key ?

[ukane-philemon]

@ukane-philemon What do you think about this upgrade buck54321/[email protected]:dcrdex:seed-derived-inner-key ?

Thanks. Your suggestion will allow users to recover with their seed without the additional encryption and recovery seed in this PR.

[ukane-philemon]

Rebased.

[buck54321]

Per brief conversation with wizards @jrick and @davecgh, I think we should append some constant domain bytes to the seed before hashing into the innerKey. So instead of

innerKey := blake256.Sum256(seed)

we would do something like

const keyParameter = []byte{0xaa, 0xbb, 0xcc}
innerKey := blake256.Sum256(append(seed, keyParameter))

But write a function for it

func seedKey(seed []byte) []byte { .. }

since it's used in multiple places

[davecgh]

we would do something like

const keyParameter = []byte{0xaa, 0xbb, 0xcc}
innerKey := blake256.Sum256(append(seed, keyParameter))

...

I would suggest using the result of hashing a known value that clearly describes its purpose so that it is a publicly verifiable value that is clear exactly where it came from and that it is a nothing-up-my-sleeve number as opposed to a potential attempt to insert a backdoor.

Something like the following:

func seedKey(seed []byte) []byte {
	// keyParam is a domain-specific value to ensure the resulting key is unique
	// for the specific use case of deriving an inner encryption key from the
	// seed.  Any other uses of derivation from the seed should similarly create
	// their own domain-specific value to ensure uniqueness.
	//
	// It is equal to BLAKE-256([]byte("DCRDEX-InnerKey-v0")).
	keyParam := [32]byte{
		0x75, 0x25, 0xb1, 0xb6, 0x53, 0x33, 0x9e, 0x33,
		0xbe, 0x11, 0x61, 0x45, 0x1a, 0x88, 0x6f, 0x37,
		0xe7, 0x74, 0xdf, 0xca, 0xb4, 0x8a, 0xee, 0x0e,
		0x7c, 0x84, 0x60, 0x01, 0xed, 0xe5, 0xf6, 0x97,
	}
	key := make([]byte, len(seed)+len(keyParam))
	copy(key, seed)
	copy(key[len(seed):], keyParam[:])
	innerKey := blake256.Sum256(key)
	return innerKey[:]
}

[ukane-philemon]

Thanks for the suggestion @buck54321, @davecgh. I used your suggestion as is after verifying with....

fmt.Printf("%x", blake256.Sum256([]byte("DCRDEX-InnerKey-v0")))
7525b1b653339e33be1161451a886f37e774dfcab48aee0e7c846001ede5f697