Skip to content

Commit

Permalink
core: app seed and HD keys
Browse files Browse the repository at this point in the history
This overhauls the client credentials system and introduces
deterministic account key generation.


Registration

Old way: Random private key generated for each new DEX account. Clients 
can export the key through the settings screen.

New way: An app seed is generated during client initialization. When a 
new dex account is created, the server's pubkey is used to 
deterministically generate a private key for the account. Users will 
save a copy of their app seed. In a simple world, that's all they need. 
During subsequent registrations, the same key will be generated, the 
server will report the key as already paid, and the registration fee 
payment will be skipped, the account ready for immediate use. The option 
to export individual account keys is still there though, as users may 
have a need to move accounts a la carte to e.g. other machines. Account 
keys can be imported as well. Upgraded (legacy) account keys and 
imported keys are treated identically, but differently than hierarchical 
keys.


client/db

Old way: A single set of Crypter parameters linked to the app password 
was stored using the Store method. All DEX account private keys and 
wallet passwords were encrypted with that Crypter.

New way: Store is gone. There is now an inner key, which acts as a 
single static password for the client. The inner key is generated once 
and only once during initialization. The inner key is itself encrypted 
by the outer key that changes with the user password. This means that 
when a user updates their password, no account keys or wallet passwords 
need to be re-encrypted. The keys, encrypted app seed, and the relevant 
Crypter parameters are embodied by the PrimaryCredentials struct, and 
the DB interface has new methods for upgrading to, initializing, and 
updating the PrimaryCredentials.


UI

The app password initialization form now has an option to specify the 
app seed.

Since the password can now be cached for registration, and the /register 
view may start the user on any number of forms, I've added a password 
field to the one form that didn't already have one, the DEX address 
form. The field is displayed conditionally based on whether we already 
have the password cached. To make this work, I combined the two forms 
from /register and /settings into a single template for use on both 
pages. The form has also been given a corresponding javascript class 
DEXAddressForm.

When the user first registers, if not supplying a seed, they will be 
notified to go to the settings page and save their app seed asap. This 
is in lieu of a more proactive solution such as requiring the user to 
immediately echo back some portion of the seed. While I do recognize the 
value of such a flow, I would want to do that substantial work in a 
separate effort.

Instead of creating a full-fledged system for authenticated upgrades,
perform an upgrade of the database structure via a standard DB upgrade,
but with a sparsely populated skeleton for the stored PrimaryCredentials.
Core recognizes this condition during Login and updates the DB.
We are relying on an assumption that even if future DB upgrades are
expecting a particular structure, they won't necessarily require that
the PrimaryCredentials fields themselves are populated. This is
probably safe, since the PrimaryCredentials themselves are pretty
worthless without a user password in hand.

client/core

Creates the PrimaryCredentials during initialization, or populates during 
Login if upgrading. Decrypts the inner key or seed as necessary. Updates 
the PrimaryCredentials during app password changes. Deterministically 
generates DEX private keys (and soon wallet seeds for built-ins). When 
registering, recognizes new msgjson errors for account exists or account 
suspended, and act accordingly. For suspended account, the solution now 
is to just generate a random key, treat it as imported, and repay the 
fee. The implications are that if a client wants to get their account 
unsuspended via operator diplomacy, they need to do that before 
restoring the account. PreRegister method combines fetching of the DEX 
configuration and sending the /register request. This allows the client 
to know as soon as they've submitted the DEX URL whether we were able to 
restore the account with the HD key, solving the bad UX mentioned here 
and here.


server

db: Return special errors during registration when the client's key 
already exists or is suspended. Return the fee coin as a courtesy.

auth: Translate db errors during registration to msgjson errors.
  • Loading branch information
buck54321 committed Aug 3, 2021
1 parent bc60036 commit b66a35f
Show file tree
Hide file tree
Showing 43 changed files with 2,028 additions and 762 deletions.
10 changes: 6 additions & 4 deletions client/core/account.go
Expand Up @@ -84,7 +84,6 @@ func (c *Core) AccountExport(pw []byte, host string) (*Account, error) {
return nil, codedError(acctKeyErr, err)
}
dc.acct.keyMtx.RLock()
accountID := dc.acct.id.String()
privKey := hex.EncodeToString(dc.acct.privKey.Serialize())
dc.acct.keyMtx.RUnlock()

Expand All @@ -101,8 +100,11 @@ func (c *Core) AccountExport(pw []byte, host string) (*Account, error) {

// Account ID is exported for informational purposes only, it is not used during import.
acct := &Account{
Host: host,
AccountID: accountID,
Host: host,
AccountID: dc.acct.id.String(),
// PrivKey: Note that we don't differentiate between legacy and
// hierarchical private keys here. On import, all keys are treated as
// legacy keys.
PrivKey: privKey,
DEXPubKey: hex.EncodeToString(dc.acct.dexPubKey.SerializeCompressed()),
Cert: hex.EncodeToString(dc.acct.cert),
Expand Down Expand Up @@ -149,7 +151,7 @@ func (c *Core) AccountImport(pw []byte, acct Account) error {
if err != nil {
return codedError(decodeErr, err)
}
accountInfo.EncKey, err = crypter.Encrypt(privKey)
accountInfo.LegacyEncKey, err = crypter.Encrypt(privKey)
if err != nil {
return codedError(encryptionErr, err)
}
Expand Down
16 changes: 8 additions & 8 deletions client/core/account_test.go
Expand Up @@ -218,8 +218,8 @@ func TestAccountExportAccountKeyError(t *testing.T) {
host := tCore.conns[tDexHost].acct.host
rig.crypter.(*tCrypter).decryptErr = tErr
_, err := tCore.AccountExport(tPW, host)
if !errorHasCode(err, acctKeyErr) {
t.Fatalf("expected account key error, actual error: '%v'", err)
if !errorHasCode(err, passwordErr) {
t.Fatalf("expected password error, actual error: '%v'", err)
}
}

Expand Down Expand Up @@ -264,22 +264,22 @@ func TestAccountImport(t *testing.T) {
if !rig.db.verifyCreateAccount {
t.Fatalf("expected execution of db.CreateAccount")
}
if rig.db.accountInfoPersisted.Host != host {
t.Fatalf("unexprected accountInfo Host")
if rig.db.acct.Host != host {
t.Fatalf("unexpected accountInfo Host")
}
DEXpubKey, _ := hex.DecodeString(account.DEXPubKey)
if !bytes.Equal(rig.db.accountInfoPersisted.DEXPubKey.SerializeCompressed(), DEXpubKey) {
if !bytes.Equal(rig.db.acct.DEXPubKey.SerializeCompressed(), DEXpubKey) {
t.Fatal("unexpected DEXPubKey")
}
feeCoin, _ := hex.DecodeString(account.FeeCoin)
if !bytes.Equal(rig.db.accountInfoPersisted.FeeCoin, feeCoin) {
if !bytes.Equal(rig.db.acct.FeeCoin, feeCoin) {
t.Fatal("unexpected FeeCoin")
}
cert, _ := hex.DecodeString(account.Cert)
if !bytes.Equal(rig.db.accountInfoPersisted.Cert, cert) {
if !bytes.Equal(rig.db.acct.Cert, cert) {
t.Fatal("unexpected Cert")
}
if !rig.db.accountInfoPersisted.Paid {
if !rig.db.acct.Paid {
t.Fatal("unexpected Paid value")
}
if rig.db.accountProofPersisted.Host != host {
Expand Down
13 changes: 13 additions & 0 deletions client/core/bookie.go
Expand Up @@ -16,6 +16,7 @@ import (
"decred.org/dcrdex/dex/encode"
"decred.org/dcrdex/dex/msgjson"
"decred.org/dcrdex/dex/order"
"github.com/decred/dcrd/dcrec/secp256k1/v3"
)

var (
Expand Down Expand Up @@ -677,6 +678,18 @@ func (dc *dexConnection) refreshServerConfig() error {
dc.assets = assets
dc.assetsMtx.Unlock()

// If we're fetching config and the server sends the pubkey in config, set
// the dexPubKey now. We also know that we're fetching the config for the
// first time (via connectDEX), and the dexConnection has not been assigned
// to dc.conns yet, so we can still update the acct.dexPubKey field without
// a data race.
if dc.acct.dexPubKey == nil && len(cfg.DEXPubKey) > 0 {
dc.acct.dexPubKey, err = secp256k1.ParsePubKey(cfg.DEXPubKey)
if err != nil {
return fmt.Errorf("error decoding secp256k1 PublicKey from bytes: %w", err)
}
}

dc.epochMtx.Lock()
dc.epoch = epochs
dc.epochMtx.Unlock()
Expand Down

0 comments on commit b66a35f

Please sign in to comment.