Merge branch 'master' into neilalexander/pinecone

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

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{
 			{

internal/version.go

@@ -17,7 +17,7 @@ var build string
 const (
 	VersionMajor = 0
 	VersionMinor = 3
-	VersionPatch = 10
+	VersionPatch = 11
 	VersionTag   = "" // example: "rc1"
 )
 

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

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{
+				res.KeyError(req.UserID, req.DeviceID, &api.KeyError{
-					Err: fmt.Sprintf("%s device %s: algorithm / key ID %s one-time key already exists", key.UserID, key.DeviceID, keyIDWithAlgo),
+					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{
+			res.KeyError(req.UserID, req.DeviceID, &api.KeyError{
-				Err: fmt.Sprintf("%s device %s : failed to store one-time keys: %s", key.UserID, key.DeviceID, err.Error()),
+				Err: fmt.Sprintf("%s device %s : failed to store one-time keys: %s", req.UserID, req.DeviceID, err.Error()),
 			})
 			continue
 		}

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
+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

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

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,

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.profiles.insertProfile(ctx, txn, localpart); err != nil {
-	if err := d.accountDatas.insertAccountData(ctx, txn, localpart, "", "m.push_rules", json.RawMessage(`{
+		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.

userapi/storage/accounts/sqlite3/constraint.go

@@ -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)
-}

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 account, err = d.accounts.insertAccount(ctx, txn, localpart, hash, appserviceID); err != nil {
-		if isConstraintError(err) {
+		return nil, sqlutil.ErrUserExists
-			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.