diff --git a/auth/auth.go b/auth/auth.go index 49a2c9a..1648eac 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -13,8 +13,9 @@ type UserId int32 type Email string type DeviceId string type Password string -type KDFKey string // KDF output -type Salt string +type KDFKey string // KDF output +type ClientSaltSeed string // part of client-side KDF input along with root password +type ServerSalt string // server-side KDF input for accounts type TokenString string type AuthScope string @@ -65,7 +66,8 @@ func (at *AuthToken) ScopeValid(required AuthScope) bool { return at.Scope == ScopeFull || at.Scope == required } -const SaltLength = 8 +const ServerSaltLength = 16 +const ClientSaltSeedLength = 32 // https://words.filippo.io/the-scrypt-parameters/ func passwordScrypt(p Password, saltBytes []byte) ([]byte, error) { @@ -79,15 +81,15 @@ func passwordScrypt(p Password, saltBytes []byte) ([]byte, error) { // Given a password (in the same format submitted via request), generate a // random salt, run the password and salt thorugh the KDF, and return the salt // and kdf output. The result generally goes into a database. -func (p Password) Create() (key KDFKey, salt Salt, err error) { - saltBytes := make([]byte, SaltLength) +func (p Password) Create() (key KDFKey, salt ServerSalt, err error) { + saltBytes := make([]byte, ServerSaltLength) if _, err := rand.Read(saltBytes); err != nil { return "", "", fmt.Errorf("Error generating salt: %+v", err) } keyBytes, err := passwordScrypt(p, saltBytes) if err == nil { key = KDFKey(hex.EncodeToString(keyBytes[:])) - salt = Salt(hex.EncodeToString(saltBytes[:])) + salt = ServerSalt(hex.EncodeToString(saltBytes[:])) } return } @@ -97,7 +99,7 @@ func (p Password) Create() (key KDFKey, salt Salt, err error) { // whether the result kdf output matches the kdf test output. // The salt and test kdf output generally come out of the database, and is used // to check a submitted password. -func (p Password) Check(checkKey KDFKey, salt Salt) (match bool, err error) { +func (p Password) Check(checkKey KDFKey, salt ServerSalt) (match bool, err error) { saltBytes, err := hex.DecodeString(string(salt)) if err != nil { return false, fmt.Errorf("Error decoding salt from hex: %+v", err) diff --git a/auth/auth_test.go b/auth/auth_test.go index 31a753d..800319c 100644 --- a/auth/auth_test.go +++ b/auth/auth_test.go @@ -68,7 +68,7 @@ func TestCreatePassword(t *testing.T) { if len(key1) != 64 { t.Error("Key has wrong length", key1) } - if len(salt1) != 16 { + if len(salt1) != 32 { t.Error("Salt has wrong length", salt1) } @@ -86,8 +86,8 @@ func TestCreatePassword(t *testing.T) { func TestCheckPassword(t *testing.T) { const password = Password("password 1") - const key = KDFKey("b9a3669973fcd2da3625e84da9d9a2da87bd280bcb02586851e1cb5bee1efa10") - const salt = Salt("080cbdf6d247c665") + const key = KDFKey("83a832b55ba28616c91e0b514d3f297bc12d43fbc69ff7e7a72ec15f90613858") + const salt = ServerSalt("080cbdf6d247c665080cbdf6d247c665") match, err := password.Check(key, salt) if err != nil { @@ -97,7 +97,7 @@ func TestCheckPassword(t *testing.T) { t.Error("Expected password to match correct key and salt") } - const wrongKey = KDFKey("0000000073fcd2da3625e84da9d9a2da87bd280bcb02586851e1cb5bee1efa10") + const wrongKey = KDFKey("000000000ba28616c91e0b514d3f297bc12d43fbc69ff7e7a72ec15f90613858") match, err = password.Check(wrongKey, salt) if err != nil { t.Error("Error checking password") @@ -106,7 +106,7 @@ func TestCheckPassword(t *testing.T) { t.Error("Expected password to not match incorrect key") } - const wrongSalt = Salt("00000000d247c665") + const wrongSalt = ServerSalt("00000000d247c66500000000d247c665") match, err = password.Check(key, wrongSalt) if err != nil { t.Error("Error checking password") @@ -115,7 +115,7 @@ func TestCheckPassword(t *testing.T) { t.Error("Expected password to not match incorrect salt") } - const invalidSalt = Salt("Whoops") + const invalidSalt = ServerSalt("Whoops") match, err = password.Check(key, invalidSalt) if err == nil { // It does a decode of salt inside the function but not the key so we won't diff --git a/server/account.go b/server/account.go index 2dfb8df..7108c8d 100644 --- a/server/account.go +++ b/server/account.go @@ -9,20 +9,22 @@ import ( "lbryio/lbry-id/store" ) -// TODO email verification cycle - type RegisterRequest struct { - Email auth.Email `json:"email"` - Password auth.Password `json:"password"` + Email auth.Email `json:"email"` + Password auth.Password `json:"password"` + ClientSaltSeed auth.ClientSaltSeed `json:"clientSaltSeed"` } func (r *RegisterRequest) validate() error { if !validateEmail(r.Email) { - return fmt.Errorf("Invalid 'email'") + return fmt.Errorf("Invalid or missing 'email'") } if r.Password == "" { return fmt.Errorf("Missing 'password'") } + if !validateClientSaltSeed(r.ClientSaltSeed) { + return fmt.Errorf("Invalid or missing 'clientSaltSeed'") + } return nil } @@ -32,7 +34,7 @@ func (s *Server) register(w http.ResponseWriter, req *http.Request) { return } - err := s.store.CreateAccount(registerRequest.Email, registerRequest.Password) + err := s.store.CreateAccount(registerRequest.Email, registerRequest.Password, registerRequest.ClientSaltSeed) if err != nil { if err == store.ErrDuplicateEmail || err == store.ErrDuplicateAccount { diff --git a/server/account_test.go b/server/account_test.go index ddec3b4..4c4b2b1 100644 --- a/server/account_test.go +++ b/server/account_test.go @@ -17,7 +17,7 @@ func TestServerRegisterSuccess(t *testing.T) { testStore := TestStore{} s := Server{&testAuth, &testStore} - requestBody := []byte(`{"email": "abc@example.com", "password": "123"}`) + requestBody := []byte(`{"email": "abc@example.com", "password": "123", "clientSaltSeed": "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234" }`) req := httptest.NewRequest(http.MethodPost, PathRegister, bytes.NewBuffer(requestBody)) w := httptest.NewRecorder() @@ -49,7 +49,7 @@ func TestServerRegisterErrors(t *testing.T) { name: "validation error", // missing email address email: "", expectedStatusCode: http.StatusBadRequest, - expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Request failed validation: Invalid 'email'", + expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Request failed validation: Invalid or missing 'email'", // Just check one validation error (missing email address) to make sure the // validate function is called. We'll check the rest of the validation @@ -81,7 +81,7 @@ func TestServerRegisterErrors(t *testing.T) { server := Server{&testAuth, &testStore} // Make request - requestBody := fmt.Sprintf(`{"email": "%s", "password": "123"}`, tc.email) + requestBody := fmt.Sprintf(`{"email": "%s", "password": "123", "clientSaltSeed": "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234"}`, tc.email) req := httptest.NewRequest(http.MethodPost, PathAuthToken, bytes.NewBuffer([]byte(requestBody))) w := httptest.NewRecorder() @@ -96,35 +96,53 @@ func TestServerRegisterErrors(t *testing.T) { } func TestServerValidateRegisterRequest(t *testing.T) { - registerRequest := RegisterRequest{Email: "joe@example.com", Password: "aoeu"} + registerRequest := RegisterRequest{Email: "joe@example.com", Password: "aoeu", ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234"} if registerRequest.validate() != nil { - t.Fatalf("Expected valid RegisterRequest to successfully validate") + t.Errorf("Expected valid RegisterRequest to successfully validate") } - registerRequest = RegisterRequest{Email: "joe-example.com", Password: "aoeu"} + registerRequest = RegisterRequest{Email: "joe-example.com", Password: "aoeu", ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234"} err := registerRequest.validate() if !strings.Contains(err.Error(), "email") { - t.Fatalf("Expected RegisterRequest with invalid email to not successfully validate") + t.Errorf("Expected RegisterRequest with invalid email to return an appropriate error") } // Note that Golang's email address parser, which I use, will accept // "Joe " so we need to make sure to avoid accepting it. See // the implementation. - registerRequest = RegisterRequest{Email: "Joe ", Password: "aoeu"} + registerRequest = RegisterRequest{Email: "Joe ", Password: "aoeu", ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234"} err = registerRequest.validate() if !strings.Contains(err.Error(), "email") { - t.Fatalf("Expected RegisterRequest with email with unexpected formatting to not successfully validate") + t.Errorf("Expected RegisterRequest with email with unexpected formatting to return an appropriate error") } - registerRequest = RegisterRequest{Password: "aoeu"} + registerRequest = RegisterRequest{Password: "aoeu", ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234"} err = registerRequest.validate() if !strings.Contains(err.Error(), "email") { - t.Fatalf("Expected RegisterRequest with missing email to not successfully validate") + t.Errorf("Expected RegisterRequest with missing email to return an appropriate error") } - registerRequest = RegisterRequest{Email: "joe@example.com"} + registerRequest = RegisterRequest{Email: "joe@example.com", ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234"} err = registerRequest.validate() if !strings.Contains(err.Error(), "password") { - t.Fatalf("Expected RegisterRequest with missing password to not successfully validate") + t.Errorf("Expected RegisterRequest with missing password to return an appropriate error") + } + + registerRequest = RegisterRequest{Email: "joe@example.com", Password: "aoeu"} + err = registerRequest.validate() + if !strings.Contains(err.Error(), "clientSaltSeed") { + t.Errorf("Expected RegisterRequest with missing clientSaltSeed to return an appropriate error") + } + + registerRequest = RegisterRequest{Email: "joe@example.com", Password: "aoeu", ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234"} + err = registerRequest.validate() + if !strings.Contains(err.Error(), "clientSaltSeed") { + t.Errorf("Expected RegisterRequest with clientSaltSeed of wrong length to return an appropriate error") + } + + registerRequest = RegisterRequest{Email: "joe@example.com", Password: "aoeu", ClientSaltSeed: "xxxx1234xxxx1234xxxx1234xxxx1234xxxx1234xxxx1234xxxx1234xxxx1234"} + err = registerRequest.validate() + if !strings.Contains(err.Error(), "clientSaltSeed") { + t.Errorf("Expected RegisterRequest with clientSaltSeed with a non-hex string to return an appropriate error") } } diff --git a/server/auth_test.go b/server/auth_test.go index 14aa498..2947577 100644 --- a/server/auth_test.go +++ b/server/auth_test.go @@ -116,7 +116,7 @@ func TestServerAuthHandlerErrors(t *testing.T) { func TestServerValidateAuthRequest(t *testing.T) { authRequest := AuthRequest{DeviceId: "dId", Email: "joe@example.com", Password: "aoeu"} if authRequest.validate() != nil { - t.Fatalf("Expected valid AuthRequest to successfully validate") + t.Errorf("Expected valid AuthRequest to successfully validate") } tt := []struct { diff --git a/server/client.go b/server/client.go new file mode 100644 index 0000000..2424176 --- /dev/null +++ b/server/client.go @@ -0,0 +1,81 @@ +package server + +import ( + "encoding/base64" + "encoding/json" + "fmt" + "net/http" + + "lbryio/lbry-id/auth" + "lbryio/lbry-id/store" +) + +// Thanks to Standard Notes. See: +// https://docs.standardnotes.com/specification/encryption/ + +type ClientSaltSeedResponse struct { + ClientSaltSeed auth.ClientSaltSeed `json:"clientSaltSeed"` +} + +// TODO - There's probably a struct-based solution here like with POST/PUT. +// We could put that struct up top as well. +// TODO - maybe common code with getWalletParams? +func getClientSaltSeedParams(req *http.Request) (email auth.Email, err error) { + emailSlice, hasEmailSlice := req.URL.Query()["email"] + + if !hasEmailSlice || emailSlice[0] == "" { + err = fmt.Errorf("Missing email parameter") + } + + if err == nil { + decodedEmail, err := base64.StdEncoding.DecodeString(emailSlice[0]) + if err == nil { + email = auth.Email(decodedEmail) + } + } + + if !validateEmail(email) { + email = "" + err = fmt.Errorf("Invalid email") + } + + return +} + +func (s *Server) getClientSaltSeed(w http.ResponseWriter, req *http.Request) { + if !getGetData(w, req) { + return + } + + email, paramsErr := getClientSaltSeedParams(req) + + if paramsErr != nil { + // In this specific case, the error is limited to values that are safe to + // give to the user. + errorJson(w, http.StatusBadRequest, paramsErr.Error()) + return + } + + seed, err := s.store.GetClientSaltSeed(email) + if err == store.ErrWrongCredentials { + errorJson(w, http.StatusNotFound, "No match for email") + return + } + if err != nil { + internalServiceErrorJson(w, err, "Error getting client salt seed") + return + } + + clientSaltSeedResponse := ClientSaltSeedResponse{ + ClientSaltSeed: seed, + } + + response, err := json.Marshal(clientSaltSeedResponse) + + if err != nil { + internalServiceErrorJson(w, err, "Error generating client salt seed response") + return + } + + fmt.Fprintf(w, string(response)) +} diff --git a/server/client_test.go b/server/client_test.go new file mode 100644 index 0000000..2e7dfe7 --- /dev/null +++ b/server/client_test.go @@ -0,0 +1,103 @@ +package server + +import ( + "encoding/base64" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "net/http/httptest" + "testing" + + "lbryio/lbry-id/auth" + "lbryio/lbry-id/store" +) + +func TestServerGetClientSalt(t *testing.T) { + tt := []struct { + name string + emailGetParam string + emailCallExpected auth.Email + + expectedStatusCode int + expectedErrorString string + + storeErrors TestStoreFunctionsErrors + }{ + { + name: "success", + emailGetParam: base64.StdEncoding.EncodeToString([]byte("good@example.com")), + emailCallExpected: "good@example.com", + expectedStatusCode: http.StatusOK, + }, + { + name: "invalid email", + emailGetParam: base64.StdEncoding.EncodeToString([]byte("bad-example.com")), + + expectedStatusCode: http.StatusBadRequest, + expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Invalid email", + }, + { + name: "account not found", + emailGetParam: base64.StdEncoding.EncodeToString([]byte("nonexistent@example.com")), + emailCallExpected: "nonexistent@example.com", + expectedStatusCode: http.StatusNotFound, + expectedErrorString: http.StatusText(http.StatusNotFound) + ": No match for email", + + storeErrors: TestStoreFunctionsErrors{GetClientSaltSeed: store.ErrWrongCredentials}, + }, + { + name: "db error getting client salt", + emailGetParam: base64.StdEncoding.EncodeToString([]byte("good@example.com")), + emailCallExpected: "good@example.com", + expectedStatusCode: http.StatusInternalServerError, + expectedErrorString: http.StatusText(http.StatusInternalServerError), + + storeErrors: TestStoreFunctionsErrors{GetClientSaltSeed: fmt.Errorf("Some random DB Error!")}, + }, + } + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + + testAuth := TestAuth{} + testStore := TestStore{ + TestClientSaltSeed: auth.ClientSaltSeed("abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234"), + + Errors: tc.storeErrors, + } + + s := Server{&testAuth, &testStore} + + req := httptest.NewRequest(http.MethodGet, PathClientSaltSeed, nil) + q := req.URL.Query() + q.Add("email", string(tc.emailGetParam)) + req.URL.RawQuery = q.Encode() + w := httptest.NewRecorder() + + s.getClientSaltSeed(w, req) + + body, _ := ioutil.ReadAll(w.Body) + + expectStatusCode(t, w, tc.expectedStatusCode) + expectErrorString(t, body, tc.expectedErrorString) + + // In this case, a salt seed is expected iff there is no error string + expectSaltSeed := len(tc.expectedErrorString) == 0 + + if !expectSaltSeed { + return // The rest of the test does not apply + } + + var result ClientSaltSeedResponse + err := json.Unmarshal(body, &result) + + if err != nil || result.ClientSaltSeed != testStore.TestClientSaltSeed { + t.Errorf("Expected client salt seed response to have the test client salt secret: result: %+v err: %+v", string(body), err) + } + + if testStore.Called.GetClientSaltSeed != tc.emailCallExpected { + t.Errorf("Expected Store.GetClientSaltSeed to be called with %s got %s", tc.emailCallExpected, testStore.Called.GetClientSaltSeed) + } + }) + } +} diff --git a/server/integration_test.go b/server/integration_test.go index bbaf7a1..ab5a357 100644 --- a/server/integration_test.go +++ b/server/integration_test.go @@ -2,6 +2,7 @@ package server import ( "bytes" + "encoding/base64" "encoding/json" "fmt" "io/ioutil" @@ -105,7 +106,7 @@ func TestIntegrationWalletUpdates(t *testing.T) { s.register, PathRegister, ®isterResponse, - `{"email": "abc@example.com", "password": "123"}`, + `{"email": "abc@example.com", "password": "123", "clientSaltSeed": "1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd"}`, ) checkStatusCode(t, statusCode, responseBody, http.StatusCreated) @@ -271,11 +272,33 @@ func TestIntegrationChangePassword(t *testing.T) { s.register, PathRegister, ®isterResponse, - `{"email": "abc@example.com", "password": "123"}`, + `{"email": "abc@example.com", "password": "123", "clientSaltSeed": "1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd"}`, ) checkStatusCode(t, statusCode, responseBody, http.StatusCreated) + //////////////////// + t.Log("Request: Get client salt seed") + //////////////////// + + var clientSaltSeedResponse ClientSaltSeedResponse + responseBody, statusCode = request( + t, + http.MethodGet, + s.getClientSaltSeed, + fmt.Sprintf("%s?email=%s", PathClientSaltSeed, base64.StdEncoding.EncodeToString([]byte("abc@example.com"))), + &clientSaltSeedResponse, + "", + ) + + checkStatusCode(t, statusCode, responseBody) + + if clientSaltSeedResponse.ClientSaltSeed != "1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd" { + t.Fatalf("Unexpected client salt seed. want: %+v got: %+v", + "1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd", + clientSaltSeedResponse.ClientSaltSeed) + } + //////////////////// t.Log("Request: Get auth token") //////////////////// @@ -315,11 +338,30 @@ func TestIntegrationChangePassword(t *testing.T) { s.changePassword, PathPassword, &changePasswordResponse, - `{"email": "abc@example.com", "oldPassword": "123", "newPassword": "456"}`, + `{"email": "abc@example.com", "oldPassword": "123", "newPassword": "456", "clientSaltSeed": "8678def95678def98678def95678def98678def95678def98678def95678def9"}`, ) checkStatusCode(t, statusCode, responseBody) + //////////////////// + t.Log("Request: Get client salt seed") + //////////////////// + + responseBody, statusCode = request( + t, + http.MethodGet, + s.getClientSaltSeed, + fmt.Sprintf("%s?email=%s", PathClientSaltSeed, base64.StdEncoding.EncodeToString([]byte("abc@example.com"))), + &clientSaltSeedResponse, + "", + ) + + checkStatusCode(t, statusCode, responseBody) + + if clientSaltSeedResponse.ClientSaltSeed != "8678def95678def98678def95678def98678def95678def98678def95678def9" { + t.Fatalf("Unexpected client salt seed. want: %+v got: %+v", "8678def95678def98678def95678def98678def95678def98678def95678def9", clientSaltSeedResponse.ClientSaltSeed) + } + //////////////////// t.Log("Request: Put first wallet - fail because token invalidated on password change") //////////////////// @@ -404,12 +446,32 @@ func TestIntegrationChangePassword(t *testing.T) { "hmac": "my-hmac-2", "email": "abc@example.com", "oldPassword": "456", - "newPassword": "789" + "newPassword": "789", + "clientSaltSeed": "0000ffff0000ffff0000ffff0000ffff0000ffff0000ffff0000ffff0000ffff" }`), ) checkStatusCode(t, statusCode, responseBody) + //////////////////// + t.Log("Request: Get client salt seed") + //////////////////// + + responseBody, statusCode = request( + t, + http.MethodGet, + s.getClientSaltSeed, + fmt.Sprintf("%s?email=%s", PathClientSaltSeed, base64.StdEncoding.EncodeToString([]byte("abc@example.com"))), + &clientSaltSeedResponse, + "", + ) + + checkStatusCode(t, statusCode, responseBody) + + if clientSaltSeedResponse.ClientSaltSeed != "0000ffff0000ffff0000ffff0000ffff0000ffff0000ffff0000ffff0000ffff" { + t.Fatalf("Unexpected client salt seed. want: %+v got: %+v", "0000ffff0000ffff0000ffff0000ffff0000ffff0000ffff0000ffff0000ffff", clientSaltSeedResponse.ClientSaltSeed) + } + //////////////////// t.Log("Request: Get wallet - fail because token invalidated on password change") //////////////////// diff --git a/server/password.go b/server/password.go index c43506f..5f1a308 100644 --- a/server/password.go +++ b/server/password.go @@ -17,6 +17,7 @@ type ChangePasswordRequest struct { Email auth.Email `json:"email"` OldPassword auth.Password `json:"oldPassword"` NewPassword auth.Password `json:"newPassword"` + ClientSaltSeed auth.ClientSaltSeed `json:"clientSaltSeed"` } func (r *ChangePasswordRequest) validate() error { @@ -25,7 +26,7 @@ func (r *ChangePasswordRequest) validate() error { walletAbsent := (r.EncryptedWallet == "" && r.Hmac == "" && r.Sequence == 0) if !validateEmail(r.Email) { - return fmt.Errorf("Invalid 'email'") + return fmt.Errorf("Invalid or missing 'email'") } if r.OldPassword == "" { return fmt.Errorf("Missing 'oldPassword'") @@ -33,9 +34,13 @@ func (r *ChangePasswordRequest) validate() error { if r.NewPassword == "" { return fmt.Errorf("Missing 'newPassword'") } + // Too bad we can't do this so easily with clientSaltSeed if r.OldPassword == r.NewPassword { return fmt.Errorf("'oldPassword' and 'newPassword' should not be the same") } + if !validateClientSaltSeed(r.ClientSaltSeed) { + return fmt.Errorf("Invalid or missing 'clientSaltSeed'") + } if !walletPresent && !walletAbsent { return fmt.Errorf("Fields 'encryptedWallet', 'sequence', and 'hmac' should be all non-empty and non-zero, or all omitted") } @@ -54,6 +59,7 @@ func (s *Server) changePassword(w http.ResponseWriter, req *http.Request) { changePasswordRequest.Email, changePasswordRequest.OldPassword, changePasswordRequest.NewPassword, + changePasswordRequest.ClientSaltSeed, changePasswordRequest.EncryptedWallet, changePasswordRequest.Sequence, changePasswordRequest.Hmac) @@ -66,6 +72,7 @@ func (s *Server) changePassword(w http.ResponseWriter, req *http.Request) { changePasswordRequest.Email, changePasswordRequest.OldPassword, changePasswordRequest.NewPassword, + changePasswordRequest.ClientSaltSeed, ) if err == store.ErrUnexpectedWallet { errorJson(w, http.StatusConflict, "Wallet exists; need an updated wallet when changing password") diff --git a/server/password_test.go b/server/password_test.go index d7346e5..4dec5ec 100644 --- a/server/password_test.go +++ b/server/password_test.go @@ -106,7 +106,7 @@ func TestServerChangePassword(t *testing.T) { }, { name: "validation error", expectedStatusCode: http.StatusBadRequest, - expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Request failed validation: Invalid 'email'", + expectedErrorString: http.StatusText(http.StatusBadRequest) + ": Request failed validation: Invalid or missing 'email'", // Just check one validation error (missing email address) to make sure // the validate function is called. We'll check the rest of the @@ -155,16 +155,18 @@ func TestServerChangePassword(t *testing.T) { const oldPassword = "old password" const newPassword = "new password" + const clientSaltSeed = "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234" requestBody := []byte( fmt.Sprintf(`{ "encryptedWallet": "%s", - "sequence": %d, - "hmac": "%s", - "email": "%s", - "oldPassword": "%s", - "newPassword": "%s" - }`, tc.newEncryptedWallet, tc.newSequence, tc.newHmac, tc.email, oldPassword, newPassword), + "sequence": %d , + "hmac": "%s", + "email": "%s", + "oldPassword": "%s", + "newPassword": "%s", + "clientSaltSeed": "%s" + }`, tc.newEncryptedWallet, tc.newSequence, tc.newHmac, tc.email, oldPassword, newPassword, clientSaltSeed), ) req := httptest.NewRequest(http.MethodPost, PathPassword, bytes.NewBuffer(requestBody)) @@ -191,6 +193,7 @@ func TestServerChangePassword(t *testing.T) { Email: tc.email, OldPassword: oldPassword, NewPassword: newPassword, + ClientSaltSeed: clientSaltSeed, }), testStore.Called.ChangePasswordWithWallet; want != got { t.Errorf("Store.ChangePasswordWithWallet called with: expected %+v, got %+v", want, got) } @@ -202,9 +205,10 @@ func TestServerChangePassword(t *testing.T) { } else { // Called ChangePasswordNoWallet with the expected parameters if want, got := (ChangePasswordNoWalletCall{ - Email: tc.email, - OldPassword: oldPassword, - NewPassword: newPassword, + Email: tc.email, + OldPassword: oldPassword, + NewPassword: newPassword, + ClientSaltSeed: clientSaltSeed, }), testStore.Called.ChangePasswordNoWallet; want != got { t.Errorf("Store.ChangePasswordNoWallet called with: expected %+v, got %+v", want, got) } @@ -234,15 +238,17 @@ func TestServerValidateChangePasswordRequest(t *testing.T) { Email: "abc@example.com", OldPassword: "123", NewPassword: "456", + ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234", } if changePasswordRequest.validate() != nil { t.Errorf("Expected valid ChangePasswordRequest with wallet fields to successfully validate") } changePasswordRequest = ChangePasswordRequest{ - Email: "abc@example.com", - OldPassword: "123", - NewPassword: "456", + Email: "abc@example.com", + OldPassword: "123", + NewPassword: "456", + ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234", } if changePasswordRequest.validate() != nil { t.Errorf("Expected valid ChangePasswordRequest without wallet fields to successfully validate") @@ -261,9 +267,10 @@ func TestServerValidateChangePasswordRequest(t *testing.T) { Email: "abc-example.com", OldPassword: "123", NewPassword: "456", + ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234", }, "email", - "Expected WalletRequest with invalid email to not successfully validate", + "Expected ChangePasswordRequest with invalid email to return an appropriate error", }, { // Note that Golang's email address parser, which I use, will accept // "Abc " so we need to make sure to avoid accepting it. See @@ -275,9 +282,10 @@ func TestServerValidateChangePasswordRequest(t *testing.T) { Email: "Abc ", OldPassword: "123", NewPassword: "456", + ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234", }, "email", - "Expected WalletRequest with email with unexpected formatting to not successfully validate", + "Expected ChangePasswordRequest with email with unexpected formatting to return an appropriate error", }, { ChangePasswordRequest{ EncryptedWallet: "my-encrypted-wallet", @@ -285,9 +293,10 @@ func TestServerValidateChangePasswordRequest(t *testing.T) { Sequence: 2, OldPassword: "123", NewPassword: "456", + ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234", }, "email", - "Expected WalletRequest with missing email to not successfully validate", + "Expected ChangePasswordRequest with missing email to return an appropriate error", }, { ChangePasswordRequest{ EncryptedWallet: "my-encrypted-wallet", @@ -295,9 +304,10 @@ func TestServerValidateChangePasswordRequest(t *testing.T) { Sequence: 2, Email: "abc@example.com", NewPassword: "456", + ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234", }, "oldPassword", - "Expected WalletRequest with missing old password to not successfully validate", + "Expected ChangePasswordRequest with missing old password to return an appropriate error", }, { ChangePasswordRequest{ EncryptedWallet: "my-encrypted-wallet", @@ -305,19 +315,56 @@ func TestServerValidateChangePasswordRequest(t *testing.T) { Sequence: 2, Email: "abc@example.com", OldPassword: "123", + ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234", }, "newPassword", - "Expected WalletRequest with missing new password to not successfully validate", + "Expected ChangePasswordRequest with missing new password to return an appropriate error", }, { ChangePasswordRequest{ - Hmac: "my-hmac", - Sequence: 2, - Email: "abc@example.com", - OldPassword: "123", - NewPassword: "456", + EncryptedWallet: "my-encrypted-wallet", + Hmac: "my-hmac", + Sequence: 2, + Email: "abc@example.com", + OldPassword: "123", + NewPassword: "456", + }, + "clientSaltSeed", + "Expected ChangePasswordRequest with missing clientSaltSeed to return an appropriate error", + }, { + ChangePasswordRequest{ + EncryptedWallet: "my-encrypted-wallet", + Hmac: "my-hmac", + Sequence: 2, + Email: "abc@example.com", + OldPassword: "123", + NewPassword: "456", + ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234", + }, + "clientSaltSeed", + "Expected ChangePasswordRequest with clientSaltSeed of wrong length to return an appropriate error", + }, { + ChangePasswordRequest{ + EncryptedWallet: "my-encrypted-wallet", + Hmac: "my-hmac", + Sequence: 2, + Email: "abc@example.com", + OldPassword: "123", + NewPassword: "456", + ClientSaltSeed: "xxxx1234xxxx1234xxxx1234xxxx1234xxxx1234xxxx1234xxxx1234xxxx1234", + }, + "clientSaltSeed", + "Expected ChangePasswordRequest with clientSaltSeed with a non-hex string to return an appropriate error", + }, { + ChangePasswordRequest{ + Hmac: "my-hmac", + Sequence: 2, + Email: "abc@example.com", + OldPassword: "123", + NewPassword: "456", + ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234", }, "'encryptedWallet', 'sequence', and 'hmac'", // More likely to fail when we change the error message but whatever - "Expected WalletRequest with missing encrypted wallet (but with other wallet fields present) to not successfully validate", + "Expected ChangePasswordRequest with missing encrypted wallet (but with other wallet fields present) to return an appropriate error", }, { ChangePasswordRequest{ EncryptedWallet: "my-encrypted-wallet", @@ -325,9 +372,10 @@ func TestServerValidateChangePasswordRequest(t *testing.T) { Email: "abc@example.com", OldPassword: "123", NewPassword: "456", + ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234", }, "'encryptedWallet', 'sequence', and 'hmac'", // More likely to fail when we change the error message but whatever - "Expected WalletRequest with missing hmac (but with other wallet fields present) to not successfully validate", + "Expected ChangePasswordRequest with missing hmac (but with other wallet fields present) to return an appropriate error", }, { ChangePasswordRequest{ EncryptedWallet: "my-encrypted-wallet", @@ -336,9 +384,10 @@ func TestServerValidateChangePasswordRequest(t *testing.T) { Email: "abc@example.com", OldPassword: "123", NewPassword: "456", + ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234", }, "'encryptedWallet', 'sequence', and 'hmac'", // More likely to fail when we change the error message but whatever - "Expected WalletRequest with sequence < 1 (but with other wallet fields present) to not successfully validate", + "Expected ChangePasswordRequest with sequence < 1 (but with other wallet fields present) to return an appropriate error", }, { ChangePasswordRequest{ EncryptedWallet: "my-encrypted-wallet", @@ -347,9 +396,10 @@ func TestServerValidateChangePasswordRequest(t *testing.T) { Email: "abc@example.com", OldPassword: "123", NewPassword: "123", + ClientSaltSeed: "abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234", }, "should not be the same", - "Expected WalletRequest with password that does not change to not successfully validate", + "Expected ChangePasswordRequest with password that does not change to return an appropriate error", }, } for _, tc := range tt { @@ -357,6 +407,5 @@ func TestServerValidateChangePasswordRequest(t *testing.T) { if !strings.Contains(err.Error(), tc.expectedErrorSubstr) { t.Errorf(tc.failureDescription) } - } } diff --git a/server/server.go b/server/server.go index 3dc6d52..9596afe 100644 --- a/server/server.go +++ b/server/server.go @@ -1,6 +1,7 @@ package server import ( + "encoding/hex" "encoding/json" "fmt" "log" @@ -13,13 +14,14 @@ import ( // TODO proper doc comments! -const ApiVersion = "2" +const ApiVersion = "3" const PathPrefix = "/api/" + ApiVersion const PathAuthToken = PathPrefix + "/auth/full" const PathRegister = PathPrefix + "/signup" const PathPassword = PathPrefix + "/password" const PathWallet = PathPrefix + "/wallet" +const PathClientSaltSeed = PathPrefix + "/client-salt-seed" const PathUnknownEndpoint = PathPrefix + "/" const PathWrongApiVersion = "/api/" @@ -177,6 +179,12 @@ func validateEmail(email auth.Email) bool { return string(email) == e.Address } +func validateClientSaltSeed(clientSaltSeed auth.ClientSaltSeed) bool { + _, err := hex.DecodeString(string(clientSaltSeed)) + const seedHexLength = auth.ClientSaltSeedLength * 2 + return len(clientSaltSeed) == seedHexLength && err == nil +} + // TODO - both wallet and token requests should be PUT, not POST. // PUT = "...creates a new resource or replaces a representation of the target resource with the request payload." @@ -195,6 +203,7 @@ func (s *Server) Serve() { http.HandleFunc(PathWallet, s.handleWallet) http.HandleFunc(PathRegister, s.register) http.HandleFunc(PathPassword, s.changePassword) + http.HandleFunc(PathClientSaltSeed, s.getClientSaltSeed) http.HandleFunc(PathUnknownEndpoint, s.unknownEndpoint) http.HandleFunc(PathWrongApiVersion, s.wrongApiVersion) diff --git a/server/server_test.go b/server/server_test.go index 09a01ed..fc02365 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -36,9 +36,10 @@ type SetWalletCall struct { } type ChangePasswordNoWalletCall struct { - Email auth.Email - OldPassword auth.Password - NewPassword auth.Password + Email auth.Email + OldPassword auth.Password + NewPassword auth.Password + ClientSaltSeed auth.ClientSaltSeed } type ChangePasswordWithWalletCall struct { @@ -48,6 +49,7 @@ type ChangePasswordWithWalletCall struct { Email auth.Email OldPassword auth.Password NewPassword auth.Password + ClientSaltSeed auth.ClientSaltSeed } // Whether functions are called, and sometimes what they're called with @@ -60,6 +62,7 @@ type TestStoreFunctionsCalled struct { GetWallet bool ChangePasswordWithWallet ChangePasswordWithWalletCall ChangePasswordNoWallet ChangePasswordNoWalletCall + GetClientSaltSeed auth.Email } type TestStoreFunctionsErrors struct { @@ -71,6 +74,7 @@ type TestStoreFunctionsErrors struct { GetWallet error ChangePasswordWithWallet error ChangePasswordNoWallet error + GetClientSaltSeed error } type TestStore struct { @@ -86,6 +90,8 @@ type TestStore struct { TestEncryptedWallet wallet.EncryptedWallet TestSequence wallet.Sequence TestHmac wallet.WalletHmac + + TestClientSaltSeed auth.ClientSaltSeed } func (s *TestStore) SaveToken(authToken *auth.AuthToken) error { @@ -103,7 +109,7 @@ func (s *TestStore) GetUserId(auth.Email, auth.Password) (auth.UserId, error) { return 0, s.Errors.GetUserId } -func (s *TestStore) CreateAccount(auth.Email, auth.Password) error { +func (s *TestStore) CreateAccount(auth.Email, auth.Password, auth.ClientSaltSeed) error { s.Called.CreateAccount = true return s.Errors.CreateAccount } @@ -133,6 +139,7 @@ func (s *TestStore) ChangePasswordWithWallet( email auth.Email, oldPassword auth.Password, newPassword auth.Password, + clientSaltSeed auth.ClientSaltSeed, encryptedWallet wallet.EncryptedWallet, sequence wallet.Sequence, hmac wallet.WalletHmac, @@ -144,6 +151,7 @@ func (s *TestStore) ChangePasswordWithWallet( Email: email, OldPassword: oldPassword, NewPassword: newPassword, + ClientSaltSeed: clientSaltSeed, } return s.Errors.ChangePasswordWithWallet } @@ -152,15 +160,26 @@ func (s *TestStore) ChangePasswordNoWallet( email auth.Email, oldPassword auth.Password, newPassword auth.Password, + clientSaltSeed auth.ClientSaltSeed, ) (err error) { s.Called.ChangePasswordNoWallet = ChangePasswordNoWalletCall{ - Email: email, - OldPassword: oldPassword, - NewPassword: newPassword, + Email: email, + OldPassword: oldPassword, + NewPassword: newPassword, + ClientSaltSeed: clientSaltSeed, } return s.Errors.ChangePasswordNoWallet } +func (s *TestStore) GetClientSaltSeed(email auth.Email) (seed auth.ClientSaltSeed, err error) { + s.Called.GetClientSaltSeed = email + err = s.Errors.GetClientSaltSeed + if err == nil { + seed = s.TestClientSaltSeed + } + return +} + // expectStatusCode: A helper to call in functions that test that request // handlers responded with a certain status code. Cuts down on noise. func expectStatusCode(t *testing.T, w *httptest.ResponseRecorder, expectedStatusCode int) { diff --git a/server/wallet_test.go b/server/wallet_test.go index 72aa1ce..55c092c 100644 --- a/server/wallet_test.go +++ b/server/wallet_test.go @@ -277,7 +277,7 @@ func TestServerPostWallet(t *testing.T) { func TestServerValidateWalletRequest(t *testing.T) { walletRequest := WalletRequest{Token: "seekrit", EncryptedWallet: "my-encrypted-wallet", Hmac: "my-hmac", Sequence: 2} if walletRequest.validate() != nil { - t.Fatalf("Expected valid WalletRequest to successfully validate") + t.Errorf("Expected valid WalletRequest to successfully validate") } tt := []struct { diff --git a/store/account_test.go b/store/account_test.go index 6cd1daf..9601b36 100644 --- a/store/account_test.go +++ b/store/account_test.go @@ -9,13 +9,13 @@ import ( "lbryio/lbry-id/auth" ) -func expectAccountMatch(t *testing.T, s *Store, email auth.Email, password auth.Password) { +func expectAccountMatch(t *testing.T, s *Store, email auth.Email, password auth.Password, seed auth.ClientSaltSeed) { var key auth.KDFKey - var salt auth.Salt + var salt auth.ServerSalt err := s.db.QueryRow( - `SELECT key, salt from accounts WHERE email=?`, - email, + `SELECT key, server_salt from accounts WHERE email=? AND client_salt_seed=?`, + email, seed, ).Scan(&key, &salt) if err != nil { t.Fatalf("Error finding account for: %s %s - %+v", email, password, err) @@ -53,29 +53,29 @@ func TestStoreCreateAccount(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - email, password := auth.Email("abc@example.com"), auth.Password("123") + email, password, seed := auth.Email("abc@example.com"), auth.Password("123"), auth.ClientSaltSeed("abcd1234abcd1234") // Get an account, come back empty expectAccountNotExists(t, &s, email) // Create an account - if err := s.CreateAccount(email, password); err != nil { + if err := s.CreateAccount(email, password, seed); err != nil { t.Fatalf("Unexpected error in CreateAccount: %+v", err) } // Get and confirm the account we just put in - expectAccountMatch(t, &s, email, password) + expectAccountMatch(t, &s, email, password, seed) newPassword := auth.Password("xyz") // Try to create a new account with the same email and different password, // fail because email already exists - if err := s.CreateAccount(email, newPassword); err != ErrDuplicateAccount { + if err := s.CreateAccount(email, newPassword, seed); err != ErrDuplicateAccount { t.Fatalf(`CreateAccount err: wanted "%+v", got "%+v"`, ErrDuplicateAccount, err) } // Get the email and same *first* password we successfully put in - expectAccountMatch(t, &s, email, password) + expectAccountMatch(t, &s, email, password, seed) } // Test GetUserId for nonexisting email @@ -96,7 +96,7 @@ func TestStoreGetUserIdAccountExists(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - createdUserId, email, password := makeTestUser(t, &s) + createdUserId, email, password, _ := makeTestUser(t, &s) // Check that there's now a user id for the email and password if userId, err := s.GetUserId(email, password); err != nil || userId != createdUserId { @@ -112,17 +112,25 @@ func TestStoreGetUserIdAccountExists(t *testing.T) { func TestStoreAccountEmptyFields(t *testing.T) { // Make sure expiration doesn't get set if sanitization fails tt := []struct { - name string - email auth.Email - password auth.Password + name string + email auth.Email + clientSaltSeed auth.ClientSaltSeed + password auth.Password }{ { - name: "missing email", - email: "", - password: "xyz", + name: "missing email", + email: "", + clientSaltSeed: "abcd1234abcd1234", + password: "xyz", + }, + { + name: "missing client salt seed", + email: "a@example.com", + clientSaltSeed: "", + password: "xyz", }, // Not testing empty key and salt because they get generated to something - // non-empty in the method + // non-empty in the method, even if email is empty } for _, tc := range tt { @@ -132,7 +140,7 @@ func TestStoreAccountEmptyFields(t *testing.T) { var sqliteErr sqlite3.Error - err := s.CreateAccount(tc.email, tc.password) + err := s.CreateAccount(tc.email, tc.password, tc.clientSaltSeed) if errors.As(err, &sqliteErr) { if errors.Is(sqliteErr.ExtendedCode, sqlite3.ErrConstraintCheck) { return // We got the error we expected @@ -142,3 +150,29 @@ func TestStoreAccountEmptyFields(t *testing.T) { }) } } + +// Test GetClientSaltSeed for existing account +func TestStoreGetClientSaltSeedAccountSuccess(t *testing.T) { + s, sqliteTmpFile := StoreTestInit(t) + defer StoreTestCleanup(sqliteTmpFile) + + _, email, _, createdSeed := makeTestUser(t, &s) + + // Check that there's now a user id for the email + if seed, err := s.GetClientSaltSeed(email); err != nil || seed != createdSeed { + t.Fatalf("Unexpected error in GetClientSaltSeed: err: %+v seed: %v", err, seed) + } +} + +// Test GetClientSaltSeed for nonexisting email +func TestStoreGetClientSaltSeedAccountNotExists(t *testing.T) { + s, sqliteTmpFile := StoreTestInit(t) + defer StoreTestCleanup(sqliteTmpFile) + + email := auth.Email("abc@example.com") + + // Check that there's no user id for email and password first + if seed, err := s.GetClientSaltSeed(email); err != ErrWrongCredentials || seed != "" { + t.Fatalf(`GetClientSaltSeed error for nonexistant account: wanted "%+v", got "%+v. seed: %v"`, ErrWrongCredentials, err, seed) + } +} diff --git a/store/auth_test.go b/store/auth_test.go index bfa016c..e41b658 100644 --- a/store/auth_test.go +++ b/store/auth_test.go @@ -77,7 +77,7 @@ func TestStoreInsertToken(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, _, _ := makeTestUser(t, &s) + userId, _, _, _ := makeTestUser(t, &s) // created for addition to the DB (no expiration attached) authToken1 := auth.AuthToken{ @@ -123,7 +123,7 @@ func TestStoreUpdateToken(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, _, _ := makeTestUser(t, &s) + userId, _, _, _ := makeTestUser(t, &s) // created for addition to the DB (no expiration attached) authTokenUpdate := auth.AuthToken{ @@ -181,7 +181,7 @@ func TestStoreSaveToken(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, _, _ := makeTestUser(t, &s) + userId, _, _, _ := makeTestUser(t, &s) // Version 1 of the token for both devices // created for addition to the DB (no expiration attached) @@ -274,7 +274,7 @@ func TestStoreGetToken(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, _, _ := makeTestUser(t, &s) + userId, _, _, _ := makeTestUser(t, &s) // created for addition to the DB (no expiration attached) authToken := auth.AuthToken{ @@ -327,7 +327,7 @@ func TestStoreTokenUTC(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, _, _ := makeTestUser(t, &s) + userId, _, _, _ := makeTestUser(t, &s) authToken := auth.AuthToken{ Token: "seekrit-1", @@ -394,7 +394,7 @@ func TestStoreTokenEmptyFields(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - tc.authToken.UserId, _, _ = makeTestUser(t, &s) + tc.authToken.UserId, _, _, _ = makeTestUser(t, &s) var sqliteErr sqlite3.Error diff --git a/store/password_test.go b/store/password_test.go index 05cae92..32de0e5 100644 --- a/store/password_test.go +++ b/store/password_test.go @@ -15,7 +15,7 @@ func TestStoreChangePasswordSuccess(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, email, oldPassword := makeTestUser(t, &s) + userId, email, oldPassword, _ := makeTestUser(t, &s) token := auth.TokenString("my-token") _, err := s.db.Exec( @@ -35,15 +35,16 @@ func TestStoreChangePasswordSuccess(t *testing.T) { } newPassword := oldPassword + auth.Password("_new") + newSeed := auth.ClientSaltSeed("edf98765edf98765edf98765edf98765edf98765edf98765edf98765edf98765") encryptedWallet := wallet.EncryptedWallet("my-enc-wallet-2") sequence := wallet.Sequence(2) hmac := wallet.WalletHmac("my-hmac-2") - if err := s.ChangePasswordWithWallet(email, oldPassword, newPassword, encryptedWallet, sequence, hmac); err != nil { + if err := s.ChangePasswordWithWallet(email, oldPassword, newPassword, newSeed, encryptedWallet, sequence, hmac); err != nil { t.Errorf("ChangePasswordWithWallet: unexpected error: %+v", err) } - expectAccountMatch(t, &s, email, newPassword) + expectAccountMatch(t, &s, email, newPassword, newSeed) expectWalletExists(t, &s, userId, encryptedWallet, sequence, hmac) expectTokenNotExists(t, &s, token) } @@ -96,7 +97,7 @@ func TestStoreChangePasswordErrors(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, email, oldPassword := makeTestUser(t, &s) + userId, email, oldPassword, oldSeed := makeTestUser(t, &s) expiration := time.Now().UTC().Add(time.Hour * 24 * 14) authToken := auth.AuthToken{ Token: auth.TokenString("my-token"), @@ -132,9 +133,10 @@ func TestStoreChangePasswordErrors(t *testing.T) { submittedEmail := email + tc.emailSuffix // Possibly make it the wrong email submittedOldPassword := oldPassword + tc.oldPasswordSuffix // Possibly make it the wrong password - newPassword := oldPassword + auth.Password("_new") // Possibly make the new password different (as it should be) + newPassword := oldPassword + auth.Password("_new") // Make the new password different (as it should be) + newSeed := auth.ClientSaltSeed("edf98765edf98765edf98765edf98765edf98765edf98765edf98765edf98765") - if err := s.ChangePasswordWithWallet(submittedEmail, submittedOldPassword, newPassword, newEncryptedWallet, tc.sequence, newHmac); err != tc.expectedError { + if err := s.ChangePasswordWithWallet(submittedEmail, submittedOldPassword, newPassword, newSeed, newEncryptedWallet, tc.sequence, newHmac); err != tc.expectedError { t.Errorf("ChangePasswordWithWallet: unexpected value for err. want: %+v, got: %+v", tc.expectedError, err) } @@ -142,7 +144,7 @@ func TestStoreChangePasswordErrors(t *testing.T) { // This tests the transaction rollbacks in particular, given the errors // that are at a couple different stages of the txn, triggered by these // tests. - expectAccountMatch(t, &s, email, oldPassword) + expectAccountMatch(t, &s, email, oldPassword, oldSeed) if tc.hasWallet { expectWalletExists(t, &s, userId, oldEncryptedWallet, oldSequence, oldHmac) } else { @@ -157,7 +159,7 @@ func TestStoreChangePasswordNoWalletSuccess(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, email, oldPassword := makeTestUser(t, &s) + userId, email, oldPassword, _ := makeTestUser(t, &s) token := auth.TokenString("my-token") _, err := s.db.Exec( @@ -169,12 +171,13 @@ func TestStoreChangePasswordNoWalletSuccess(t *testing.T) { } newPassword := oldPassword + auth.Password("_new") + newSeed := auth.ClientSaltSeed("edf98765edf98765edf98765edf98765edf98765edf98765edf98765edf98765") - if err := s.ChangePasswordNoWallet(email, oldPassword, newPassword); err != nil { + if err := s.ChangePasswordNoWallet(email, oldPassword, newPassword, newSeed); err != nil { t.Errorf("ChangePasswordNoWallet: unexpected error: %+v", err) } - expectAccountMatch(t, &s, email, newPassword) + expectAccountMatch(t, &s, email, newPassword, newSeed) expectWalletNotExists(t, &s, userId) expectTokenNotExists(t, &s, token) } @@ -213,7 +216,7 @@ func TestStoreChangePasswordNoWalletErrors(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, email, oldPassword := makeTestUser(t, &s) + userId, email, oldPassword, oldSeed := makeTestUser(t, &s) expiration := time.Now().UTC().Add(time.Hour * 24 * 14) authToken := auth.AuthToken{ Token: auth.TokenString("my-token"), @@ -249,8 +252,9 @@ func TestStoreChangePasswordNoWalletErrors(t *testing.T) { submittedEmail := email + tc.emailSuffix // Possibly make it the wrong email submittedOldPassword := oldPassword + tc.oldPasswordSuffix // Possibly make it the wrong password newPassword := oldPassword + auth.Password("_new") // Possibly make the new password different (as it should be) + newSeed := auth.ClientSaltSeed("edf98765edf98765edf98765edf98765edf98765edf98765edf98765edf98765") - if err := s.ChangePasswordNoWallet(submittedEmail, submittedOldPassword, newPassword); err != tc.expectedError { + if err := s.ChangePasswordNoWallet(submittedEmail, submittedOldPassword, newPassword, newSeed); err != tc.expectedError { t.Errorf("ChangePasswordNoWallet: unexpected value for err. want: %+v, got: %+v", tc.expectedError, err) } @@ -258,7 +262,7 @@ func TestStoreChangePasswordNoWalletErrors(t *testing.T) { // deleted. This tests the transaction rollbacks in particular, given the // errors that are at a couple different stages of the txn, triggered by // these tests. - expectAccountMatch(t, &s, email, oldPassword) + expectAccountMatch(t, &s, email, oldPassword, oldSeed) if tc.hasWallet { expectWalletExists(t, &s, userId, encryptedWallet, sequence, hmac) } else { diff --git a/store/store.go b/store/store.go index 4c3d4b3..5cca61e 100644 --- a/store/store.go +++ b/store/store.go @@ -39,9 +39,10 @@ type StoreInterface interface { SetWallet(auth.UserId, wallet.EncryptedWallet, wallet.Sequence, wallet.WalletHmac) error GetWallet(auth.UserId) (wallet.EncryptedWallet, wallet.Sequence, wallet.WalletHmac, error) GetUserId(auth.Email, auth.Password) (auth.UserId, error) - CreateAccount(auth.Email, auth.Password) error - ChangePasswordWithWallet(auth.Email, auth.Password, auth.Password, wallet.EncryptedWallet, wallet.Sequence, wallet.WalletHmac) error - ChangePasswordNoWallet(auth.Email, auth.Password, auth.Password) error + CreateAccount(auth.Email, auth.Password, auth.ClientSaltSeed) error + ChangePasswordWithWallet(auth.Email, auth.Password, auth.Password, auth.ClientSaltSeed, wallet.EncryptedWallet, wallet.Sequence, wallet.WalletHmac) error + ChangePasswordNoWallet(auth.Email, auth.Password, auth.Password, auth.ClientSaltSeed) error + GetClientSaltSeed(auth.Email) (auth.ClientSaltSeed, error) } type Store struct { @@ -112,12 +113,14 @@ func (s *Store) Migrate() error { CREATE TABLE IF NOT EXISTS accounts( email TEXT NOT NULL UNIQUE, key TEXT NOT NULL, - salt TEXT NOT NULL, + client_salt_seed TEXT NOT NULL, + server_salt TEXT NOT NULL, user_id INTEGER PRIMARY KEY AUTOINCREMENT, CHECK ( email <> '' AND key <> '' AND - salt <> '' + client_salt_seed <> '' AND + server_salt <> '' ) ); ` @@ -331,9 +334,9 @@ func (s *Store) SetWallet(userId auth.UserId, encryptedWallet wallet.EncryptedWa func (s *Store) GetUserId(email auth.Email, password auth.Password) (userId auth.UserId, err error) { var key auth.KDFKey - var salt auth.Salt + var salt auth.ServerSalt err = s.db.QueryRow( - `SELECT user_id, key, salt from accounts WHERE email=?`, + `SELECT user_id, key, server_salt from accounts WHERE email=?`, email, ).Scan(&userId, &key, &salt) if err == sql.ErrNoRows { @@ -354,17 +357,16 @@ func (s *Store) GetUserId(email auth.Email, password auth.Password) (userId auth // Account // ///////////// -func (s *Store) CreateAccount(email auth.Email, password auth.Password) (err error) { +func (s *Store) CreateAccount(email auth.Email, password auth.Password, seed auth.ClientSaltSeed) (err error) { key, salt, err := password.Create() if err != nil { return } // userId auto-increments _, err = s.db.Exec( - "INSERT INTO accounts (email, key, salt) VALUES(?,?,?)", - email, key, salt, + "INSERT INTO accounts (email, key, server_salt, client_salt_seed) VALUES(?,?,?,?)", + email, key, salt, seed, ) - var sqliteErr sqlite3.Error if errors.As(err, &sqliteErr) { if errors.Is(sqliteErr.ExtendedCode, sqlite3.ErrConstraintUnique) { @@ -387,6 +389,7 @@ func (s *Store) ChangePasswordWithWallet( email auth.Email, oldPassword auth.Password, newPassword auth.Password, + clientSaltSeed auth.ClientSaltSeed, encryptedWallet wallet.EncryptedWallet, sequence wallet.Sequence, hmac wallet.WalletHmac, @@ -395,6 +398,7 @@ func (s *Store) ChangePasswordWithWallet( email, oldPassword, newPassword, + clientSaltSeed, encryptedWallet, sequence, hmac, @@ -411,11 +415,13 @@ func (s *Store) ChangePasswordNoWallet( email auth.Email, oldPassword auth.Password, newPassword auth.Password, + clientSaltSeed auth.ClientSaltSeed, ) (err error) { return s.changePassword( email, oldPassword, newPassword, + clientSaltSeed, wallet.EncryptedWallet(""), wallet.Sequence(0), wallet.WalletHmac(""), @@ -427,6 +433,7 @@ func (s *Store) changePassword( email auth.Email, oldPassword auth.Password, newPassword auth.Password, + clientSaltSeed auth.ClientSaltSeed, encryptedWallet wallet.EncryptedWallet, sequence wallet.Sequence, hmac wallet.WalletHmac, @@ -451,10 +458,10 @@ func (s *Store) changePassword( defer endTxn() var oldKey auth.KDFKey - var oldSalt auth.Salt + var oldSalt auth.ServerSalt err = tx.QueryRow( - `SELECT user_id, key, salt from accounts WHERE email=?`, + `SELECT user_id, key, server_salt from accounts WHERE email=?`, email, ).Scan(&userId, &oldKey, &oldSalt) if err == sql.ErrNoRows { @@ -477,8 +484,8 @@ func (s *Store) changePassword( } res, err := tx.Exec( - "UPDATE accounts SET key=?, salt=? WHERE user_id=?", - newKey, newSalt, userId, + "UPDATE accounts SET key=?, server_salt=?, client_salt_seed=? WHERE user_id=?", + newKey, newSalt, clientSaltSeed, userId, ) if err != nil { return @@ -533,3 +540,14 @@ func (s *Store) changePassword( _, err = tx.Exec("DELETE FROM auth_tokens WHERE user_id=?", userId) return } + +func (s *Store) GetClientSaltSeed(email auth.Email) (seed auth.ClientSaltSeed, err error) { + err = s.db.QueryRow( + `SELECT client_salt_seed from accounts WHERE email=?`, + email, + ).Scan(&seed) + if err == sql.ErrNoRows { + err = ErrWrongCredentials + } + return +} diff --git a/store/store_test.go b/store/store_test.go index 0e9feda..9e31d04 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -33,16 +33,18 @@ func StoreTestCleanup(tmpFile *os.File) { } } -func makeTestUser(t *testing.T, s *Store) (userId auth.UserId, email auth.Email, password auth.Password) { +func makeTestUser(t *testing.T, s *Store) (userId auth.UserId, email auth.Email, password auth.Password, seed auth.ClientSaltSeed) { email, password = auth.Email("abc@example.com"), auth.Password("123") key, salt, err := password.Create() if err != nil { t.Fatalf("Error creating password") } + seed = auth.ClientSaltSeed("abcd1234abcd1234") + rows, err := s.db.Query( - "INSERT INTO accounts (email, key, salt) values(?,?,?) returning user_id", - email, key, salt, + "INSERT INTO accounts (email, key, server_salt, client_salt_seed) values(?,?,?,?) returning user_id", + email, key, salt, seed, ) if err != nil { t.Fatalf("Error setting up account: %+v", err) diff --git a/store/wallet_test.go b/store/wallet_test.go index 40953ab..01d7d77 100644 --- a/store/wallet_test.go +++ b/store/wallet_test.go @@ -79,14 +79,14 @@ func expectWalletNotExists(t *testing.T, s *Store, userId auth.UserId) { return // found nothing, we're good } -// Test insertFirstWallet, using GetWallet, CreateAccount and GetUserID as a helpers +// Test insertFirstWallet // Try insertFirstWallet twice with the same user id, error the second time func TestStoreInsertWallet(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) // Get a valid userId - userId, _, _ := makeTestUser(t, &s) + userId, _, _, _ := makeTestUser(t, &s) // Get a wallet, come back empty expectWalletNotExists(t, &s, userId) @@ -108,7 +108,7 @@ func TestStoreInsertWallet(t *testing.T) { expectWalletExists(t, &s, userId, wallet.EncryptedWallet("my-enc-wallet"), wallet.Sequence(1), wallet.WalletHmac("my-hmac")) } -// Test updateWalletToSequence, using GetWallet, CreateAccount, GetUserID, and insertFirstWallet as helpers +// Test updateWalletToSequence, using insertFirstWallet as a helper // Try updateWalletToSequence with no existing wallet, err for lack of anything to update // Try updateWalletToSequence with a preexisting wallet but the wrong sequence, fail // Try updateWalletToSequence with a preexisting wallet and the correct sequence, succeed @@ -118,7 +118,7 @@ func TestStoreUpdateWallet(t *testing.T) { defer StoreTestCleanup(sqliteTmpFile) // Get a valid userId - userId, _, _ := makeTestUser(t, &s) + userId, _, _, _ := makeTestUser(t, &s) // Try to update a wallet, fail for nothing to update if err := s.updateWalletToSequence(userId, wallet.EncryptedWallet("my-enc-wallet-a"), wallet.Sequence(1), wallet.WalletHmac("my-hmac-a")); err != ErrNoWallet { @@ -174,7 +174,7 @@ func TestStoreSetWallet(t *testing.T) { defer StoreTestCleanup(sqliteTmpFile) // Get a valid userId - userId, _, _ := makeTestUser(t, &s) + userId, _, _, _ := makeTestUser(t, &s) // Sequence 2 - fails - out of sequence (behind the scenes, tries to update but there's nothing there yet) if err := s.SetWallet(userId, wallet.EncryptedWallet("my-enc-wallet-a"), wallet.Sequence(2), wallet.WalletHmac("my-hmac-a")); err != ErrWrongSequence { @@ -221,7 +221,7 @@ func TestStoreGetWallet(t *testing.T) { defer StoreTestCleanup(sqliteTmpFile) // Get a valid userId - userId, _, _ := makeTestUser(t, &s) + userId, _, _, _ := makeTestUser(t, &s) // GetWallet fails when there's no wallet encryptedWallet, sequence, hmac, err := s.GetWallet(userId) @@ -264,7 +264,7 @@ func TestStoreWalletEmptyFields(t *testing.T) { s, sqliteTmpFile := StoreTestInit(t) defer StoreTestCleanup(sqliteTmpFile) - userId, _, _ := makeTestUser(t, &s) + userId, _, _, _ := makeTestUser(t, &s) var sqliteErr sqlite3.Error diff --git a/test_client/README.md b/test_client/README.md index 2ad4690..ae58836 100644 --- a/test_client/README.md +++ b/test_client/README.md @@ -11,27 +11,39 @@ For this example we will be working with a locally running server so that we don ``` >>> from test_client import Client >>> c1 = Client("joe2@example.com", "123abc2", 'test_wallet_1', local=True) -Generating keys... -Done generating keys +Connecting to Wallet API at http://localhost:8090 >>> c2 = Client("joe2@example.com", "123abc2", 'test_wallet_2', local=True) -Generating keys... -Done generating keys +Connecting to Wallet API at http://localhost:8090 ``` -Register the account on the server with one of the clients. +Register the account on the server with one of the clients. See the salt seed it generated and sent to the server along with registration. ``` >>> c1.register() +Generating keys... +Done generating keys Registered +>>> c1.salt_seed +'fd7bcc62d7334fbf07aca5791783cb173e3aaef91e228f000a69e3ec8eef123e' +``` + +Set up the other client. See that it got the same salt seed from the server in the process, which it needs to make sure we have the correct encryption key and login password. + +``` +>>> c2.update_secrets() +Generating keys... +Done generating keys +>>> c2.salt_seed +'fd7bcc62d7334fbf07aca5791783cb173e3aaef91e228f000a69e3ec8eef123e' ``` Now that the account exists, grab an auth token with both clients. ``` >>> c1.get_auth_token() -Got auth token: e244aae31bb2070d9269c14706a3a352ddda5e090fefc315bd4d28b8bc787278 +Got auth token: 310077f33a9b8de99ee6c45ffbe4a06a7178683e4eb65500fc5ae26513f80928 >>> c2.get_auth_token() -Got auth token: c77afe32288a9bf45e55b8fba5aca2b9d3388a277ad89db6fff179337afbd62b +Got auth token: cdd18033dc48aeefedc278d116a6abbef7f0fb525d7ddfb2e1804a817a212c4a ``` ## Syncing @@ -43,7 +55,7 @@ Create a new wallet + metadata (we'll wrap it in a struct we'll call `WalletStat >>> c1.update_remote_wallet() Successfully updated wallet state on server Synced walletState: -WalletState(sequence=1, encrypted_wallet='czo4MTkyOjE2OjE6+oSc2AE5FY971fW2kQqFvXnen5RD8RU9pMjaKEnvFE8XrdXXogVooiu9Q/099eT8Y9UePoER/aphmzJBb/fwNTOWanFsPCdEObmwfuL1OLPJ+FuAJ07am8TUSJEy12yuMqtQSj6kVF2aMa4oABthKaZ00sx98HkkdUo6sWedY0o=') +WalletState(sequence=1, encrypted_wallet='czo4MTkyOjE2OjE6KY46kZ0oRC9W8g/LVCe3V3sLdCHk0sWEBqPjzcKykl3dDJpQvRXtz8HFXlD+bgvs8M5jHw7KjJ9ODUOEq3VoSawrKyZpgc8AYIx+vC4w+q6cKC3LToxr7FlfyAoQKo9dCothik/90ySVMAPY1BBrBmQ8H46eFEoMWZ4nG2OWGew=') 'Success' ``` @@ -55,7 +67,7 @@ Now, call `init_wallet_state` with the other client. Then, we call `get_remote_w >>> c2.init_wallet_state() >>> c2.get_remote_wallet() Got latest walletState: -WalletState(sequence=1, encrypted_wallet='czo4MTkyOjE2OjE6+oSc2AE5FY971fW2kQqFvXnen5RD8RU9pMjaKEnvFE8XrdXXogVooiu9Q/099eT8Y9UePoER/aphmzJBb/fwNTOWanFsPCdEObmwfuL1OLPJ+FuAJ07am8TUSJEy12yuMqtQSj6kVF2aMa4oABthKaZ00sx98HkkdUo6sWedY0o=') +WalletState(sequence=1, encrypted_wallet='czo4MTkyOjE2OjE6KY46kZ0oRC9W8g/LVCe3V3sLdCHk0sWEBqPjzcKykl3dDJpQvRXtz8HFXlD+bgvs8M5jHw7KjJ9ODUOEq3VoSawrKyZpgc8AYIx+vC4w+q6cKC3LToxr7FlfyAoQKo9dCothik/90ySVMAPY1BBrBmQ8H46eFEoMWZ4nG2OWGew=') 'Success' ``` @@ -67,12 +79,12 @@ Push a new version, GET it with the other client. Even though we haven't edited >>> c2.update_remote_wallet() Successfully updated wallet state on server Synced walletState: -WalletState(sequence=2, encrypted_wallet='czo4MTkyOjE2OjE6mTAOkeMQKuJWirqfyDBjzvneKqcDdNO7UzZ2EdGFs1iW89WMU5UxL//hetnIcXLFFh0SqUjCfj5heyLKEvYY5wJQ0cmIJZEAiPFIZWUjju8J8UEeRl5JWW89x3qhUNrog5a7PnIi/AIRAm6tl7gfzMoujHBWiLPM4xKOO8wX9dw=') +WalletState(sequence=2, encrypted_wallet='czo4MTkyOjE2OjE6pNxAvxzGvZZvlSuYhSdGFmHGvWImYJlEewOnp6iUBbqTo899MrCfgvvlOzBOuKwk4vpJYcQgEkb3u2+j7bnJ18OCFEUeDzq88JuoKNw6ppdAbpw8D7MIDZP4Tf+5O8LmjxKtbiMy/ztW0nUxi4Ls8uuJ436CdF0UwaevHOAvlOE=') 'Success' >>> c1.get_remote_wallet() Nothing to merge. Taking remote walletState as latest walletState. Got latest walletState: -WalletState(sequence=2, encrypted_wallet='czo4MTkyOjE2OjE6mTAOkeMQKuJWirqfyDBjzvneKqcDdNO7UzZ2EdGFs1iW89WMU5UxL//hetnIcXLFFh0SqUjCfj5heyLKEvYY5wJQ0cmIJZEAiPFIZWUjju8J8UEeRl5JWW89x3qhUNrog5a7PnIi/AIRAm6tl7gfzMoujHBWiLPM4xKOO8wX9dw=') +WalletState(sequence=2, encrypted_wallet='czo4MTkyOjE2OjE6pNxAvxzGvZZvlSuYhSdGFmHGvWImYJlEewOnp6iUBbqTo899MrCfgvvlOzBOuKwk4vpJYcQgEkb3u2+j7bnJ18OCFEUeDzq88JuoKNw6ppdAbpw8D7MIDZP4Tf+5O8LmjxKtbiMy/ztW0nUxi4Ls8uuJ436CdF0UwaevHOAvlOE=') 'Success' ``` @@ -98,12 +110,12 @@ The wallet is synced between the clients. The client with the changed preference >>> c1.update_remote_wallet() Successfully updated wallet state on server Synced walletState: -WalletState(sequence=3, encrypted_wallet='czo4MTkyOjE2OjE6uTrpDaroi9aQ0D5rtu8kietZspbFSlyQyEqqfRKA+bMp4Ob7VK3lznxByGs67IpPm2Z0ZorMzaNzkuCghXh/N6YDjQFhZTUWxVo9N10M1bi++2rq2tK4iagARbWPar+Ju8zba2UcknOLZKzphYU1t8EXPykpZUonXO894ljOb2kKEs7eltudGvdRB2DqNgH2') +WalletState(sequence=3, encrypted_wallet='czo4MTkyOjE2OjE6FvtBsBMaRvnY6+SnQiBab/FWjzC9mgu3OHFXRTcJm3MsByZWbrzkFz6y4FrPN1cC+/Rcw11oJNydyys2ZaPl6zNYP/uxV6anEmET1hTt1+E2NzUJ2K/K4BLN7AgLBLQDM8zAwVzCTeaT6MZvjTV/slYc7NEqlfCMgeId2WpBJp+BCbTLo0SjPWhHCsQvo3Hf') 'Success' >>> c2.get_remote_wallet() Nothing to merge. Taking remote walletState as latest walletState. Got latest walletState: -WalletState(sequence=3, encrypted_wallet='czo4MTkyOjE2OjE6uTrpDaroi9aQ0D5rtu8kietZspbFSlyQyEqqfRKA+bMp4Ob7VK3lznxByGs67IpPm2Z0ZorMzaNzkuCghXh/N6YDjQFhZTUWxVo9N10M1bi++2rq2tK4iagARbWPar+Ju8zba2UcknOLZKzphYU1t8EXPykpZUonXO894ljOb2kKEs7eltudGvdRB2DqNgH2') +WalletState(sequence=3, encrypted_wallet='czo4MTkyOjE2OjE6FvtBsBMaRvnY6+SnQiBab/FWjzC9mgu3OHFXRTcJm3MsByZWbrzkFz6y4FrPN1cC+/Rcw11oJNydyys2ZaPl6zNYP/uxV6anEmET1hTt1+E2NzUJ2K/K4BLN7AgLBLQDM8zAwVzCTeaT6MZvjTV/slYc7NEqlfCMgeId2WpBJp+BCbTLo0SjPWhHCsQvo3Hf') 'Success' >>> c2.get_preferences() {'animal': 'cow', 'car': ''} @@ -130,7 +142,7 @@ One client POSTs its change first. >>> c1.update_remote_wallet() Successfully updated wallet state on server Synced walletState: -WalletState(sequence=4, encrypted_wallet='czo4MTkyOjE2OjE6QQcktx8tncvrkjGJZk7o37IZ26AsGJnNLif2JiPIZnyRkINakzeU57cryvom9pG0qVdFFdDTAKKIreEj//yJt4pj40rhdsQ8nX6qCuN0nkcHtnpCNcTSmXlRfC/4WDfL5Mq5/HWYVVeQ54GlPp3n2Fj9910TlXVRibp6RO2P98f6cEP8kHM7s+efgLtCRmVK') +WalletState(sequence=4, encrypted_wallet='czo4MTkyOjE2OjE6rKE6TWvZxBp0No0S1CRrjW55i6w9pE6obS+9bwR76qaBBQ40lz0Ajd/2vXO1KBAQhxEHDJ6WJLPs15SgqhVspaNXmdwR1dYEHmJ8M+PW0KLv+vZoxxeGQ/5EBdrAZIfBhmI50SPF4RzmTzKTyw3VlSdqqhCutgi6FcXP+CLlsnH6qaLgjLDLISjwSMIwBd4y') 'Success' ``` @@ -142,7 +154,7 @@ Eventually, the client will be responsible (or at least more responsible) for me >>> c2.get_remote_wallet() Merging local changes with remote changes to create latest walletState. Got latest walletState: -WalletState(sequence=4, encrypted_wallet='czo4MTkyOjE2OjE6QQcktx8tncvrkjGJZk7o37IZ26AsGJnNLif2JiPIZnyRkINakzeU57cryvom9pG0qVdFFdDTAKKIreEj//yJt4pj40rhdsQ8nX6qCuN0nkcHtnpCNcTSmXlRfC/4WDfL5Mq5/HWYVVeQ54GlPp3n2Fj9910TlXVRibp6RO2P98f6cEP8kHM7s+efgLtCRmVK') +WalletState(sequence=4, encrypted_wallet='czo4MTkyOjE2OjE6rKE6TWvZxBp0No0S1CRrjW55i6w9pE6obS+9bwR76qaBBQ40lz0Ajd/2vXO1KBAQhxEHDJ6WJLPs15SgqhVspaNXmdwR1dYEHmJ8M+PW0KLv+vZoxxeGQ/5EBdrAZIfBhmI50SPF4RzmTzKTyw3VlSdqqhCutgi6FcXP+CLlsnH6qaLgjLDLISjwSMIwBd4y') 'Success' >>> c2.get_preferences() {'animal': 'horse', 'car': 'Audi'} @@ -154,12 +166,12 @@ Finally, the client with the merged wallet pushes it to the server, and the othe >>> c2.update_remote_wallet() Successfully updated wallet state on server Synced walletState: -WalletState(sequence=5, encrypted_wallet='czo4MTkyOjE2OjE6t9OMFtRl0D4E4YJoE8zR0VuteEroiRyOUgXEhjUBuG0stbwqO/WoNuydNxmRtVMLWgHV5DUlUGZKlTBsuf/fJ6svMdUU7R34uYsSve5ioJw+FBY/w25CYRpa49YZfNhu5YOtmeLHF7AuTMBoc2kkyJj0Jg0IhjqfORIQiifW0YwaWh/eEch9Kzxi+d5DGMaL') +WalletState(sequence=5, encrypted_wallet='czo4MTkyOjE2OjE62Tub/EfwCMdYpZ9N9wCRwTB3Di+eA0oVpn44v/n1UgOB8jNEIEtQptCfBtBE7yfIJP8pw544SkhxAfR2Zy8/UrLIhKMUSVeCl8bJP78AoJCPpeJEQo4GOqPvluWYS2eOh1urZojn5yqB5nGRnK4hYhQ6lOwgg4jfRFtTzMKPYb263ONb3mx1SkeoCwmBeRoF') 'Success' >>> c1.get_remote_wallet() Nothing to merge. Taking remote walletState as latest walletState. Got latest walletState: -WalletState(sequence=5, encrypted_wallet='czo4MTkyOjE2OjE6t9OMFtRl0D4E4YJoE8zR0VuteEroiRyOUgXEhjUBuG0stbwqO/WoNuydNxmRtVMLWgHV5DUlUGZKlTBsuf/fJ6svMdUU7R34uYsSve5ioJw+FBY/w25CYRpa49YZfNhu5YOtmeLHF7AuTMBoc2kkyJj0Jg0IhjqfORIQiifW0YwaWh/eEch9Kzxi+d5DGMaL') +WalletState(sequence=5, encrypted_wallet='czo4MTkyOjE2OjE62Tub/EfwCMdYpZ9N9wCRwTB3Di+eA0oVpn44v/n1UgOB8jNEIEtQptCfBtBE7yfIJP8pw544SkhxAfR2Zy8/UrLIhKMUSVeCl8bJP78AoJCPpeJEQo4GOqPvluWYS2eOh1urZojn5yqB5nGRnK4hYhQ6lOwgg4jfRFtTzMKPYb263ONb3mx1SkeoCwmBeRoF') 'Success' >>> c1.get_preferences() {'animal': 'horse', 'car': 'Audi'} @@ -190,7 +202,7 @@ We try to POST both of them to the server. The second one fails because of the c >>> c2.update_remote_wallet() Successfully updated wallet state on server Synced walletState: -WalletState(sequence=6, encrypted_wallet='czo4MTkyOjE2OjE6/mE/7xT6rZ8h11dWwHMB8K+XhqNVnzgkLEx6mFntRC/HKPGbRaqeHWiQrIPUZk+Y8eJlA4FrkI/snDyO4Gbo8OI2kef7PaPV1tiL9GVYbwPoD+/KQsb1RwMVkNMHiRhJyerMzX2e5DHOBZ8a9/gtY5QROKq17OF9I6WAbW4Kt+oyAMvwPhvr53K3PAgkUZZO') +WalletState(sequence=6, encrypted_wallet='czo4MTkyOjE2OjE6ZNErNr5SrgjRMOBmK2pKtU2wu+jdwR8WO/thAf+VrGJ9237sKTjNX0aQILuj9dOzY836xYk2vB1Niypgf4PvlnXEAZ64pHO2FV8aR/0JcjsufkdUXUIJH2hxDhT5Ui8kS2tXPAuo0xDxfqQgqiJaVNfgyCo2fzqz5m5V3jBzivm7fN8TpuNaT94koI2GFPc3') 'Success' >>> c1.update_remote_wallet() Submitted wallet is out of date. @@ -206,14 +218,14 @@ The client that is out of date will then call `get_remote_wallet`, which GETs an >>> c1.get_remote_wallet() Merging local changes with remote changes to create latest walletState. Got latest walletState: -WalletState(sequence=6, encrypted_wallet='czo4MTkyOjE2OjE6/mE/7xT6rZ8h11dWwHMB8K+XhqNVnzgkLEx6mFntRC/HKPGbRaqeHWiQrIPUZk+Y8eJlA4FrkI/snDyO4Gbo8OI2kef7PaPV1tiL9GVYbwPoD+/KQsb1RwMVkNMHiRhJyerMzX2e5DHOBZ8a9/gtY5QROKq17OF9I6WAbW4Kt+oyAMvwPhvr53K3PAgkUZZO') +WalletState(sequence=6, encrypted_wallet='czo4MTkyOjE2OjE6ZNErNr5SrgjRMOBmK2pKtU2wu+jdwR8WO/thAf+VrGJ9237sKTjNX0aQILuj9dOzY836xYk2vB1Niypgf4PvlnXEAZ64pHO2FV8aR/0JcjsufkdUXUIJH2hxDhT5Ui8kS2tXPAuo0xDxfqQgqiJaVNfgyCo2fzqz5m5V3jBzivm7fN8TpuNaT94koI2GFPc3') 'Success' >>> c1.get_preferences() {'animal': 'beaver', 'car': 'Toyota'} >>> c1.update_remote_wallet() Successfully updated wallet state on server Synced walletState: -WalletState(sequence=7, encrypted_wallet='czo4MTkyOjE2OjE6Xph2n4tSYT7iRhBsLn99bykQNuFI8oWckzuWcF5nUbl2GcJ53n32YnMSNtQLfuyt0oCjSSXS7BBq9uQSPQKWANBAN6MynSQzQ3UIEsxq6ExtdE1Ua22umxmxeo8vn/xYN6CaLnl0Bji1V7HlOztzRpZSml7ZVoNtbMf8iwThdOj4XR3EMElcHowQY2zd+Tzn') +WalletState(sequence=7, encrypted_wallet='czo4MTkyOjE2OjE6csXFcg5GaaIXauqORyfoSo3rcKiWzmTE2M/YBIJ1wmdBafqtUnKSO8DE/3EeKA35ow8iXJy5mowe4Ar8R+7m6FHxDblkDohjoeP9uZ5ziEirMSPu4eZsOcpXLdBsHp/qcGpNKAFnwGqeSdrxjvyFDQCyjl204mduE/X9mh6mlyYIei1IkK08rSTmc7mCuxIj') 'Success' ``` @@ -231,10 +243,17 @@ Generating keys... Done generating keys Successfully updated password and wallet state on server Synced walletState: -WalletState(sequence=8, encrypted_wallet='czo4MTkyOjE2OjE6xxIydbWzxcZ2e7OivUevFO98qzS/Fy/bag0IN5/Ecm8GDEgEY84deAli9YiVxCTbwuMM1qAaL9wuC/Rj8fU9FykmAa8YEghEfIiuOTPyaySgSvDp2JY6gdZ+N+fx7qkJfzXshz5q5TuMdztWCouh4sCoaV2c+Gl7ieijq6A0c4lccOTUur+LX1mrEC5KP9Zs') +WalletState(sequence=8, encrypted_wallet='czo4MTkyOjE2OjE6p5qwc7BWPjZE6sDI6To/dS5wVDMvzb6BA9oezrp3ecVryouLorggVTmnQLpcLdskBDNjE7/S5P4T4LT6hJCjAby2LCgeHuatDrySson8RHcs9rozLFoaPIQCDMkCPC8EGTN/g0aOAnDGvB6jaW/IsSLpMyXeth2OjloOtj2caT1hrihpThFrLPp4glBRLGL5') 'Success' ``` +We generate a new salt seed when we change the password + +``` +>>> c1.salt_seed +'9968ede44e397d7875c057e4ebe50fd1118dc43a3e404e134353be6224947aad' +``` + This operation invalidates all of the user's auth tokens. This prevents other clients from accidentally pushing a wallet encrypted with the old password. ``` @@ -248,19 +267,21 @@ b'{"error":"Unauthorized: Token Not Found"}\n' 'Failed to get remote wallet' ``` -The client that changed its password can easily get a new token because it has the new password saved locally. The other client needs to update its local password first. +The client that changed its password can easily get a new token because it has the new password saved locally. The other client needs to update its local password first, which again includes getting the updated salt seed from the server. ``` >>> c1.get_auth_token() -Got auth token: 3d5227f7873a43fecb991e5026d413142541720f33ca92898acf2c8b1cdeb20d +Got auth token: 796ea0575fe1ba5d6a43afec016f6ed2c9225a5180e76e744aad5b8857c8702b >>> c2.get_auth_token() Error 401 b'{"error":"Unauthorized: No match for email and password"}\n' >>> c2.set_local_password("eggsandwich") Generating keys... Done generating keys +>>> c2.salt_seed +'9968ede44e397d7875c057e4ebe50fd1118dc43a3e404e134353be6224947aad' >>> c2.get_auth_token() -Got auth token: d062d33d5692f7466f5560560fecc8e17fb903f13f5be8e4289a48a395a4306b +Got auth token: 61932e198643b6f8bf1cd42fd0d296ce01b9813b6c1f312826baca3d50f95d47 ``` We don't allow password changes if we have pending wallet changes to push. This is to prevent a situation where the user has to merge local and remote changes in the middle of a password change. @@ -276,13 +297,13 @@ Local changes found. Update remote wallet before changing password. >>> c1.update_remote_wallet() Successfully updated wallet state on server Synced walletState: -WalletState(sequence=9, encrypted_wallet='czo4MTkyOjE2OjE6i5NbYdtHfFjeIBfN1EL2nmOGlCr6hFbbPI5Y8Eq2JNeWDDy4UXTGJRNMA0SamvxneDb09RpwrW6+ffEo931rdZx0dozHCkEjKTeV5gthzbdoA7FXbiyDpJnx8DDyw2wyV/PjDKbH3dL2ojr/EgfiFivLq3FLXzopclXlL9zSipdKL3qgzN7PfRWuqiiNoY8q') +WalletState(sequence=9, encrypted_wallet='czo4MTkyOjE2OjE6i9/lJoTH+8okCoczA7Q0o/X/X8MfVulO3qAq2GtKEKW9m4JH7Fcup62BOhwHtsNPOHIiMv9er5SpOx9pGBq3s9Bei4k2fNq4RXmXEPZX66p1T26VboJ0o53etIhnfQ9Q3pdLssiURjkK4OXpDDzbw1KsYXAPnJS50Nb8/A7+14BTgLoyJmGjW3nwjTxFjzRt') 'Success' >>> c1.change_password("starboard") Generating keys... Done generating keys Successfully updated password and wallet state on server Synced walletState: -WalletState(sequence=10, encrypted_wallet='czo4MTkyOjE2OjE6GCvOav9loezTMQiq9KD7eQ834lIcOsVum6+zakt/GAX7t527dYbQ8HFWSB2O0CGuD4R5j4P2AJC7tIhBmhNbWGeAXjDtxlDFDRE//9BsFkZLAUyxGcMaOPz/obFXNrO0lFGM456fSS1E6EX17gmkDT1T6DKPQd9oNwx8UteEBLNz8V2Cw8Aa/eBrtzlgCSMf') +WalletState(sequence=10, encrypted_wallet='czo4MTkyOjE2OjE6D2TVSTbVugAqf8VSDmiIbArhQypf6o1LkQxAphJFlcbHHDtCQ4aqpmhsdWrkaf0Jd7+L2J12aWNf+XQGwS/kddTi9pplCwlAJB/RkTl4xeFPIRT5g5iVHrJcvCU2/gNa1BpRGtROn0fFb0SscS3WPxdQ5QA6jIGEblu1OY0KF/phpQt1DtNcuOS6rOz3NCmV') 'Success' ``` diff --git a/test_client/gen-readme.py b/test_client/gen-readme.py index b4bc917..5585ce9 100644 --- a/test_client/gen-readme.py +++ b/test_client/gen-readme.py @@ -47,11 +47,21 @@ c2 = Client("joe2@example.com", "123abc2", 'test_wallet_2', local=True) """) print(""" -Register the account on the server with one of the clients. +Register the account on the server with one of the clients. See the salt seed it generated and sent to the server along with registration. """) code_block(""" c1.register() +c1.salt_seed +""") + +print(""" +Set up the other client. See that it got the same salt seed from the server in the process, which it needs to make sure we have the correct encryption key and login password. +""") + +code_block(""" +c2.update_secrets() +c2.salt_seed """) print(""" @@ -215,6 +225,14 @@ code_block(""" c1.change_password("eggsandwich") """) +print(""" +We generate a new salt seed when we change the password +""") + +code_block(""" +c1.salt_seed +""") + print(""" This operation invalidates all of the user's auth tokens. This prevents other clients from accidentally pushing a wallet encrypted with the old password. """) @@ -225,13 +243,14 @@ c2.get_remote_wallet() """) print(""" -The client that changed its password can easily get a new token because it has the new password saved locally. The other client needs to update its local password first. +The client that changed its password can easily get a new token because it has the new password saved locally. The other client needs to update its local password first, which again includes getting the updated salt seed from the server. """) code_block(""" c1.get_auth_token() c2.get_auth_token() c2.set_local_password("eggsandwich") +c2.salt_seed c2.get_auth_token() """) diff --git a/test_client/test_client.py b/test_client/test_client.py index 828939c..0c140cc 100755 --- a/test_client/test_client.py +++ b/test_client/test_client.py @@ -2,7 +2,8 @@ from collections import namedtuple import base64, json, uuid, requests, hashlib, hmac from pprint import pprint -from hashlib import scrypt # TODO - audit! Should I use hazmat `Scrypt` instead for some reason? +from hashlib import scrypt, sha256 # TODO - audit! Should I use hazmat `Scrypt` instead for some reason? +import secrets WalletState = namedtuple('WalletState', ['sequence', 'encrypted_wallet']) @@ -68,23 +69,29 @@ class LBRYSDK(): class WalletSync(): def __init__(self, local): - self.API_VERSION = 2 + self.API_VERSION = 3 if local: BASE_URL = 'http://localhost:8090' else: BASE_URL = 'https://dev.lbry.id:8091' + + # Avoid confusion. I sometimes forget, at any rate. + print ("Connecting to Wallet API at " + BASE_URL) + API_URL = BASE_URL + '/api/%d' % self.API_VERSION self.AUTH_URL = API_URL + '/auth/full' self.REGISTER_URL = API_URL + '/signup' self.PASSWORD_URL = API_URL + '/password' self.WALLET_URL = API_URL + '/wallet' + self.CLIENT_SALT_SEED_URL = API_URL + '/client-salt-seed' - def register(self, email, password): + def register(self, email, password, salt_seed): body = json.dumps({ 'email': email, 'password': password, + 'clientSaltSeed': salt_seed, }) response = requests.post(self.REGISTER_URL, body) if response.status_code != 201: @@ -107,6 +114,23 @@ class WalletSync(): return response.json()['token'] + def get_salt_seed(self, email): + params = { + 'email': base64.encodestring(bytes(email.encode('utf-8'))), + } + response = requests.get(self.CLIENT_SALT_SEED_URL, params=params) + + if response.status_code == 404: + print ('Account not found') + raise Exception("Account not found") + + if response.status_code != 200: + print ('Error', response.status_code) + print (response.content) + raise Exception("Unexpected status code") + + return response.json()['clientSaltSeed'] + def get_wallet(self, token): params = { 'token': token, @@ -151,7 +175,7 @@ class WalletSync(): print (response.content) raise Exception("Unexpected status code") - def change_password_with_wallet(self, wallet_state, hmac, email, old_password, new_password): + def change_password_with_wallet(self, wallet_state, hmac, email, old_password, new_password, salt_seed): body = json.dumps({ "encryptedWallet": wallet_state.encrypted_wallet, "sequence": wallet_state.sequence, @@ -159,6 +183,7 @@ class WalletSync(): "email": email, "oldPassword": old_password, "newPassword": new_password, + 'clientSaltSeed': salt_seed, }) response = requests.post(self.PASSWORD_URL, body) @@ -174,11 +199,12 @@ class WalletSync(): print (response.content) raise Exception("Unexpected status code") - def change_password_no_wallet(self, email, old_password, new_password): + def change_password_no_wallet(self, email, old_password, new_password, salt_seed): body = json.dumps({ "email": email, "oldPassword": old_password, "newPassword": new_password, + 'clientSaltSeed': salt_seed, }) response = requests.post(self.PASSWORD_URL, body) @@ -194,18 +220,29 @@ class WalletSync(): print (response.content) raise Exception("Unexpected status code") -def derive_secrets(root_password, salt): - # TODO - Audit me audit me audit me! I don't know if these values are - # optimal. - # - # TODO - wallet_id in the salt? (with domain etc if we go that way) - # But, we probably want random salt anyway for each domain, who cares - # +# Thanks to Standard Notes. See: +# https://docs.standardnotes.com/specification/encryption/ + +# Sized in bytes +SALT_SEED_LENGTH = 32 +SALT_LENGTH = 16 + +def generate_salt_seed(): + return secrets.token_hex(SALT_SEED_LENGTH) + +def generate_salt(email, seed): + hash_input = (email + ":" + seed).encode('utf-8') + hash_output = sha256(hash_input).hexdigest().encode('utf-8') + return bytes(hash_output[:(SALT_LENGTH * 2)]) + +def derive_secrets(root_password, email, salt_seed): # 2017 Scrypt parameters: https://words.filippo.io/the-scrypt-parameters/ # # There's recommendations for interactive use, and stronger recommendations # for sensitive storage. Going with the latter since we're storing # encrypted stuff on a server. + # + # Auditors double check. scrypt_n = 1<<20 scrypt_r = 8 scrypt_p = 1 @@ -213,6 +250,8 @@ def derive_secrets(root_password, salt): key_length = 32 num_keys = 3 + salt = generate_salt(email, salt_seed) + print ("Generating keys...") kdf_output = scrypt( bytes(root_password, 'utf-8'), @@ -264,27 +303,51 @@ class Client(): return True def __init__(self, email, root_password, wallet_id='default_wallet', local=False): + self.wallet_sync_api = WalletSync(local=local) + # Represents normal client behavior (though a real client will of course save device id) self.device_id = str(uuid.uuid4()) self.auth_token = 'bad token' self.synced_wallet_state = None self.email = email + self.root_password = root_password - # TODO - generate randomly CLIENT SIDE and post to server with - # registration. And maybe get it to new clients along with the auth token. - # But is there an attack vector if we don't trust the salt? See how others - # do it. Since the same server sees one of the outputs of the KDF. Huh. - self.salt = b'I AM A SALT' - - self.set_local_password(root_password) self.wallet_id = wallet_id - self.wallet_sync_api = WalletSync(local=local) + def register(self): + # Note that for each registration, i.e. for each domain, we generate a + # different salt seed. + + self.salt_seed = generate_salt_seed() + self.lbry_id_password, self.sync_password, self.hmac_key = derive_secrets( + self.root_password, self.email, self.salt_seed) + + success = self.wallet_sync_api.register( + self.email, + self.lbry_id_password, + self.salt_seed + ) + if success: + print ("Registered") def set_local_password(self, root_password): + """ + For clients to catch up to another client that just changed the password. + """ # TODO - is UTF-8 appropriate for root_password? based on characters used etc. - self.lbry_id_password, self.sync_password, self.hmac_key = derive_secrets(root_password, self.salt) + self.root_password = root_password + self.update_secrets() + + def update_secrets(self): + """ + For clients other than the one that most recently registered or changed the + password, use this to get the salt seed from the server and generate keys + locally. + """ + self.salt_seed = self.wallet_sync_api.get_salt_seed(self.email) + self.lbry_id_password, self.sync_password, self.hmac_key = derive_secrets( + self.root_password, self.email, self.salt_seed) # TODO - This does not deal with the question of tying accounts to wallets. # Does a new wallet state mean a we're creating a new account? What happens @@ -330,14 +393,6 @@ class Client(): # TODO - actually set the right hash self.mark_local_changes_synced_to_empty() - def register(self): - success = self.wallet_sync_api.register( - self.email, - self.lbry_id_password, - ) - if success: - print ("Registered") - def get_auth_token(self): token = self.wallet_sync_api.get_auth_token( self.email, @@ -459,7 +514,12 @@ class Client(): # update that as well so that the sync password and hmac key are derived # from the same root password as the lbry id password. - new_lbry_id_password, new_sync_password, new_hmac_key = derive_secrets(new_root_password, self.salt) + self.salt_seed = generate_salt_seed() + new_lbry_id_password, new_sync_password, new_hmac_key = derive_secrets( + new_root_password, self.email, self.salt_seed) + # TODO - Think of failure sequence in case of who knows what. We + # could just get the old salt seed back from the server? + # We can't lose it though. Keep the old one around? Kinda sucks. if self.synced_wallet_state and self.synced_wallet_state.sequence > 0: # Don't allow it to change if we have local changes to push. This @@ -489,7 +549,7 @@ class Client(): hmac = create_hmac(submitted_wallet_state, new_hmac_key) # Update our password and submit our wallet. - updated = self.wallet_sync_api.change_password_with_wallet(submitted_wallet_state, hmac, self.email, self.lbry_id_password, new_lbry_id_password) + updated = self.wallet_sync_api.change_password_with_wallet(submitted_wallet_state, hmac, self.email, self.lbry_id_password, new_lbry_id_password, self.salt_seed) if updated: # We updated it. Now it's synced and we mark it as such. Update everything in one command to keep local changes in sync! @@ -501,7 +561,7 @@ class Client(): return "Success" else: # Update our password. - updated = self.wallet_sync_api.change_password_no_wallet(self.email, self.lbry_id_password, new_lbry_id_password) + updated = self.wallet_sync_api.change_password_no_wallet(self.email, self.lbry_id_password, new_lbry_id_password, self.salt_seed) if updated: # We updated it. Now we mark it as such. Update everything in one command to keep local changes in sync!