diff --git a/CHANGES.md b/CHANGES.md index 2bf7b11b5..c909c5715 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,13 @@ # Changelog +## Dendrite 0.3.11 (2021-03-02) + +### Fixes + +- **SECURITY:** A bug in SQLite mode which could cause the registration flow to complete unexpectedly for existing accounts has been fixed (PostgreSQL deployments are not affected) +- A panic in the federation sender has been fixed when shutting down destination queues +- The `/keys/upload` endpoint now correctly returns the number of one-time keys in response to an empty upload request + ## Dendrite 0.3.10 (2021-02-17) ### Features diff --git a/clientapi/routing/keys.go b/clientapi/routing/keys.go index ba03a352f..e22336428 100644 --- a/clientapi/routing/keys.go +++ b/clientapi/routing/keys.go @@ -38,7 +38,10 @@ func UploadKeys(req *http.Request, keyAPI api.KeyInternalAPI, device *userapi.De return *resErr } - uploadReq := &api.PerformUploadKeysRequest{} + uploadReq := &api.PerformUploadKeysRequest{ + DeviceID: device.ID, + UserID: device.UserID, + } if r.DeviceKeys != nil { uploadReq.DeviceKeys = []api.DeviceKeys{ { diff --git a/internal/version.go b/internal/version.go index 1ba71fc9a..0d3487799 100644 --- a/internal/version.go +++ b/internal/version.go @@ -17,7 +17,7 @@ var build string const ( VersionMajor = 0 VersionMinor = 3 - VersionPatch = 10 + VersionPatch = 11 VersionTag = "" // example: "rc1" ) diff --git a/keyserver/api/api.go b/keyserver/api/api.go index 442af8715..5cb287bc1 100644 --- a/keyserver/api/api.go +++ b/keyserver/api/api.go @@ -108,6 +108,8 @@ type OneTimeKeysCount struct { // PerformUploadKeysRequest is the request to PerformUploadKeys type PerformUploadKeysRequest struct { + UserID string // Required - User performing the request + DeviceID string // Optional - Device performing the request, for fetching OTK count DeviceKeys []DeviceKeys OneTimeKeys []OneTimeKeys // OnlyDisplayNameUpdates should be `true` if ALL the DeviceKeys are present to update diff --git a/keyserver/internal/internal.go b/keyserver/internal/internal.go index 53afe0a60..f53a07619 100644 --- a/keyserver/internal/internal.go +++ b/keyserver/internal/internal.go @@ -513,6 +513,23 @@ func (a *KeyInternalAPI) uploadLocalDeviceKeys(ctx context.Context, req *api.Per } func (a *KeyInternalAPI) uploadOneTimeKeys(ctx context.Context, req *api.PerformUploadKeysRequest, res *api.PerformUploadKeysResponse) { + if req.UserID == "" { + res.Error = &api.KeyError{ + Err: "user ID missing", + } + } + if req.DeviceID != "" && len(req.OneTimeKeys) == 0 { + counts, err := a.DB.OneTimeKeysCount(ctx, req.UserID, req.DeviceID) + if err != nil { + res.Error = &api.KeyError{ + Err: fmt.Sprintf("a.DB.OneTimeKeysCount: %s", err), + } + } + if counts != nil { + res.OneTimeKeyCounts = append(res.OneTimeKeyCounts, *counts) + } + return + } for _, key := range req.OneTimeKeys { // grab existing keys based on (user/device/algorithm/key ID) keyIDsWithAlgorithms := make([]string, len(key.KeyJSON)) @@ -521,9 +538,9 @@ func (a *KeyInternalAPI) uploadOneTimeKeys(ctx context.Context, req *api.Perform keyIDsWithAlgorithms[i] = keyIDWithAlgo i++ } - existingKeys, err := a.DB.ExistingOneTimeKeys(ctx, key.UserID, key.DeviceID, keyIDsWithAlgorithms) + existingKeys, err := a.DB.ExistingOneTimeKeys(ctx, req.UserID, req.DeviceID, keyIDsWithAlgorithms) if err != nil { - res.KeyError(key.UserID, key.DeviceID, &api.KeyError{ + res.KeyError(req.UserID, req.DeviceID, &api.KeyError{ Err: "failed to query existing one-time keys: " + err.Error(), }) continue @@ -531,8 +548,8 @@ func (a *KeyInternalAPI) uploadOneTimeKeys(ctx context.Context, req *api.Perform for keyIDWithAlgo := range existingKeys { // if keys exist and the JSON doesn't match, error out as the key already exists if !bytes.Equal(existingKeys[keyIDWithAlgo], key.KeyJSON[keyIDWithAlgo]) { - res.KeyError(key.UserID, key.DeviceID, &api.KeyError{ - Err: fmt.Sprintf("%s device %s: algorithm / key ID %s one-time key already exists", key.UserID, key.DeviceID, keyIDWithAlgo), + res.KeyError(req.UserID, req.DeviceID, &api.KeyError{ + Err: fmt.Sprintf("%s device %s: algorithm / key ID %s one-time key already exists", req.UserID, req.DeviceID, keyIDWithAlgo), }) continue } @@ -540,8 +557,8 @@ func (a *KeyInternalAPI) uploadOneTimeKeys(ctx context.Context, req *api.Perform // store one-time keys counts, err := a.DB.StoreOneTimeKeys(ctx, key) if err != nil { - res.KeyError(key.UserID, key.DeviceID, &api.KeyError{ - Err: fmt.Sprintf("%s device %s : failed to store one-time keys: %s", key.UserID, key.DeviceID, err.Error()), + res.KeyError(req.UserID, req.DeviceID, &api.KeyError{ + Err: fmt.Sprintf("%s device %s : failed to store one-time keys: %s", req.UserID, req.DeviceID, err.Error()), }) continue } diff --git a/sytest-blacklist b/sytest-blacklist index 601a3f705..b635c9f0e 100644 --- a/sytest-blacklist +++ b/sytest-blacklist @@ -66,4 +66,7 @@ A prev_batch token from incremental sync can be used in the v1 messages API Forgotten room messages cannot be paginated # Blacklisted due to flakiness -Can re-join room if re-invited \ No newline at end of file +Can re-join room if re-invited + +# Blacklisted due to flakiness after #1774 +Local device key changes get to remote servers with correct prev_id \ No newline at end of file diff --git a/sytest-whitelist b/sytest-whitelist index d53fa899d..1e4442a09 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -143,7 +143,6 @@ Local new device changes appear in v2 /sync Local update device changes appear in v2 /sync Get left notifs for other users in sync and /keys/changes when user leaves Local device key changes get to remote servers -Local device key changes get to remote servers with correct prev_id Server correctly handles incoming m.device_list_update If remote user leaves room, changes device and rejoins we see update in sync If remote user leaves room, changes device and rejoins we see update in /keys/changes diff --git a/userapi/internal/api.go b/userapi/internal/api.go index cf588a40c..4abf908d4 100644 --- a/userapi/internal/api.go +++ b/userapi/internal/api.go @@ -87,7 +87,7 @@ func (a *UserInternalAPI) PerformAccountCreation(ctx context.Context, req *api.P ServerName: a.ServerName, UserID: fmt.Sprintf("@%s:%s", req.Localpart, a.ServerName), } - return nil + return err } if err = a.AccountDB.SetDisplayName(ctx, req.Localpart, req.Localpart); err != nil { @@ -161,6 +161,7 @@ func (a *UserInternalAPI) deviceListUpdate(userID string, deviceIDs []string) er var uploadRes keyapi.PerformUploadKeysResponse a.KeyAPI.PerformUploadKeys(context.Background(), &keyapi.PerformUploadKeysRequest{ + UserID: userID, DeviceKeys: deviceKeys, }, &uploadRes) if uploadRes.Error != nil { @@ -217,6 +218,7 @@ func (a *UserInternalAPI) PerformDeviceUpdate(ctx context.Context, req *api.Perf // display name has changed: update the device key var uploadRes keyapi.PerformUploadKeysResponse a.KeyAPI.PerformUploadKeys(context.Background(), &keyapi.PerformUploadKeysRequest{ + UserID: req.RequestingUserID, DeviceKeys: []keyapi.DeviceKeys{ { DeviceID: dev.ID, diff --git a/userapi/storage/accounts/postgres/storage.go b/userapi/storage/accounts/postgres/storage.go index 870756d8b..e6adbfd83 100644 --- a/userapi/storage/accounts/postgres/storage.go +++ b/userapi/storage/accounts/postgres/storage.go @@ -170,8 +170,8 @@ func (d *Database) CreateAccount( func (d *Database) createAccount( ctx context.Context, txn *sql.Tx, localpart, plaintextPassword, appserviceID string, ) (*api.Account, error) { + var account *api.Account var err error - // Generate a password hash if this is not a password-less user hash := "" if plaintextPassword != "" { @@ -180,14 +180,16 @@ func (d *Database) createAccount( return nil, err } } - if err := d.profiles.insertProfile(ctx, txn, localpart); err != nil { + if account, err = d.accounts.insertAccount(ctx, txn, localpart, hash, appserviceID); err != nil { if sqlutil.IsUniqueConstraintViolationErr(err) { return nil, sqlutil.ErrUserExists } return nil, err } - - if err := d.accountDatas.insertAccountData(ctx, txn, localpart, "", "m.push_rules", json.RawMessage(`{ + if err = d.profiles.insertProfile(ctx, txn, localpart); err != nil { + return nil, err + } + if err = d.accountDatas.insertAccountData(ctx, txn, localpart, "", "m.push_rules", json.RawMessage(`{ "global": { "content": [], "override": [], @@ -198,7 +200,7 @@ func (d *Database) createAccount( }`)); err != nil { return nil, err } - return d.accounts.insertAccount(ctx, txn, localpart, hash, appserviceID) + return account, nil } // SaveAccountData saves new account data for a given user and a given room. diff --git a/userapi/storage/accounts/sqlite3/constraint.go b/userapi/storage/accounts/sqlite3/constraint.go deleted file mode 100644 index 32f96c8e4..000000000 --- a/userapi/storage/accounts/sqlite3/constraint.go +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2020 The Matrix.org Foundation C.I.C. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// +build !wasm - -package sqlite3 - -import ( - "errors" - - "github.com/mattn/go-sqlite3" -) - -func isConstraintError(err error) bool { - return errors.Is(err, sqlite3.ErrConstraint) -} diff --git a/userapi/storage/accounts/sqlite3/storage.go b/userapi/storage/accounts/sqlite3/storage.go index 92c1c669e..747be34f6 100644 --- a/userapi/storage/accounts/sqlite3/storage.go +++ b/userapi/storage/accounts/sqlite3/storage.go @@ -204,6 +204,7 @@ func (d *Database) createAccount( ctx context.Context, txn *sql.Tx, localpart, plaintextPassword, appserviceID string, ) (*api.Account, error) { var err error + var account *api.Account // Generate a password hash if this is not a password-less user hash := "" if plaintextPassword != "" { @@ -212,14 +213,13 @@ func (d *Database) createAccount( return nil, err } } - if err := d.profiles.insertProfile(ctx, txn, localpart); err != nil { - if isConstraintError(err) { - return nil, sqlutil.ErrUserExists - } + if account, err = d.accounts.insertAccount(ctx, txn, localpart, hash, appserviceID); err != nil { + return nil, sqlutil.ErrUserExists + } + if err = d.profiles.insertProfile(ctx, txn, localpart); err != nil { return nil, err } - - if err := d.accountDatas.insertAccountData(ctx, txn, localpart, "", "m.push_rules", json.RawMessage(`{ + if err = d.accountDatas.insertAccountData(ctx, txn, localpart, "", "m.push_rules", json.RawMessage(`{ "global": { "content": [], "override": [], @@ -230,7 +230,7 @@ func (d *Database) createAccount( }`)); err != nil { return nil, err } - return d.accounts.insertAccount(ctx, txn, localpart, hash, appserviceID) + return account, nil } // SaveAccountData saves new account data for a given user and a given room.