From 9e4618000e0347741eac1279bf6c94c3b9980785 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 28 Jul 2021 10:25:45 +0100 Subject: [PATCH] Alias key backup endpoints onto /unstable, fix key backup bugs (#1947) * Default /unstable requests to stable endpoints if not overridden specifically with a custom route * Rewrite URL * Try something different * Fix routing manually * Fix selectLatestVersionSQL * Don't return 0 if no backup version exists * Log more useful error * fix up replace keys check * Don't enforce uniqueness on e2e_room_keys_versions_idx Co-authored-by: kegsay --- clientapi/routing/routing.go | 292 +++++++++--------- userapi/api/api.go | 10 +- .../accounts/postgres/key_backup_table.go | 2 +- .../postgres/key_backup_version_table.go | 17 +- userapi/storage/accounts/postgres/storage.go | 4 +- .../accounts/sqlite3/key_backup_table.go | 2 +- .../sqlite3/key_backup_version_table.go | 17 +- userapi/storage/accounts/sqlite3/storage.go | 4 +- 8 files changed, 187 insertions(+), 161 deletions(-) diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index cfc6c47fb..541c76282 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -898,157 +898,171 @@ func Setup( // Key Backup Versions (Metadata) - r0mux.Handle("/room_keys/version/{version}", - httputil.MakeAuthAPI("get_backup_keys_version", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { - vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) - if err != nil { - return util.ErrorResponse(err) - } - return KeyBackupVersion(req, userAPI, device, vars["version"]) - }), - ).Methods(http.MethodGet, http.MethodOptions) - r0mux.Handle("/room_keys/version", - httputil.MakeAuthAPI("get_latest_backup_keys_version", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { - return KeyBackupVersion(req, userAPI, device, "") - }), - ).Methods(http.MethodGet, http.MethodOptions) - r0mux.Handle("/room_keys/version/{version}", - httputil.MakeAuthAPI("put_backup_keys_version", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { - vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) - if err != nil { - return util.ErrorResponse(err) - } - return ModifyKeyBackupVersionAuthData(req, userAPI, device, vars["version"]) - }), - ).Methods(http.MethodPut) - r0mux.Handle("/room_keys/version/{version}", - httputil.MakeAuthAPI("delete_backup_keys_version", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { - vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) - if err != nil { - return util.ErrorResponse(err) - } - return DeleteKeyBackupVersion(req, userAPI, device, vars["version"]) - }), - ).Methods(http.MethodDelete) - r0mux.Handle("/room_keys/version", - httputil.MakeAuthAPI("post_new_backup_keys_version", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { - return CreateKeyBackupVersion(req, userAPI, device) - }), - ).Methods(http.MethodPost, http.MethodOptions) + getBackupKeysVersion := httputil.MakeAuthAPI("get_backup_keys_version", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { + vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) + if err != nil { + return util.ErrorResponse(err) + } + return KeyBackupVersion(req, userAPI, device, vars["version"]) + }) + + getLatestBackupKeysVersion := httputil.MakeAuthAPI("get_latest_backup_keys_version", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { + return KeyBackupVersion(req, userAPI, device, "") + }) + + putBackupKeysVersion := httputil.MakeAuthAPI("put_backup_keys_version", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { + vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) + if err != nil { + return util.ErrorResponse(err) + } + return ModifyKeyBackupVersionAuthData(req, userAPI, device, vars["version"]) + }) + + deleteBackupKeysVersion := httputil.MakeAuthAPI("delete_backup_keys_version", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { + vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) + if err != nil { + return util.ErrorResponse(err) + } + return DeleteKeyBackupVersion(req, userAPI, device, vars["version"]) + }) + + postNewBackupKeysVersion := httputil.MakeAuthAPI("post_new_backup_keys_version", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { + return CreateKeyBackupVersion(req, userAPI, device) + }) + + r0mux.Handle("/room_keys/version/{version}", getBackupKeysVersion).Methods(http.MethodGet, http.MethodOptions) + r0mux.Handle("/room_keys/version", getLatestBackupKeysVersion).Methods(http.MethodGet, http.MethodOptions) + r0mux.Handle("/room_keys/version/{version}", putBackupKeysVersion).Methods(http.MethodPut) + r0mux.Handle("/room_keys/version/{version}", deleteBackupKeysVersion).Methods(http.MethodDelete) + r0mux.Handle("/room_keys/version", postNewBackupKeysVersion).Methods(http.MethodPost, http.MethodOptions) + + unstableMux.Handle("/room_keys/version/{version}", getBackupKeysVersion).Methods(http.MethodGet, http.MethodOptions) + unstableMux.Handle("/room_keys/version", getLatestBackupKeysVersion).Methods(http.MethodGet, http.MethodOptions) + unstableMux.Handle("/room_keys/version/{version}", putBackupKeysVersion).Methods(http.MethodPut) + unstableMux.Handle("/room_keys/version/{version}", deleteBackupKeysVersion).Methods(http.MethodDelete) + unstableMux.Handle("/room_keys/version", postNewBackupKeysVersion).Methods(http.MethodPost, http.MethodOptions) // Inserting E2E Backup Keys // Bulk room and session - r0mux.Handle("/room_keys/keys", - httputil.MakeAuthAPI("put_backup_keys", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { - version := req.URL.Query().Get("version") - if version == "" { - return util.JSONResponse{ - Code: 400, - JSON: jsonerror.InvalidArgumentValue("version must be specified"), - } + putBackupKeys := httputil.MakeAuthAPI("put_backup_keys", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { + version := req.URL.Query().Get("version") + if version == "" { + return util.JSONResponse{ + Code: 400, + JSON: jsonerror.InvalidArgumentValue("version must be specified"), } - var reqBody keyBackupSessionRequest - resErr := clientutil.UnmarshalJSONRequest(req, &reqBody) - if resErr != nil { - return *resErr - } - return UploadBackupKeys(req, userAPI, device, version, &reqBody) - }), - ).Methods(http.MethodPut) + } + var reqBody keyBackupSessionRequest + resErr := clientutil.UnmarshalJSONRequest(req, &reqBody) + if resErr != nil { + return *resErr + } + return UploadBackupKeys(req, userAPI, device, version, &reqBody) + }) + // Single room bulk session - r0mux.Handle("/room_keys/keys/{roomID}", - httputil.MakeAuthAPI("put_backup_keys_room", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { - vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) - if err != nil { - return util.ErrorResponse(err) + putBackupKeysRoom := httputil.MakeAuthAPI("put_backup_keys_room", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { + vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) + if err != nil { + return util.ErrorResponse(err) + } + version := req.URL.Query().Get("version") + if version == "" { + return util.JSONResponse{ + Code: 400, + JSON: jsonerror.InvalidArgumentValue("version must be specified"), } - version := req.URL.Query().Get("version") - if version == "" { - return util.JSONResponse{ - Code: 400, - JSON: jsonerror.InvalidArgumentValue("version must be specified"), - } - } - roomID := vars["roomID"] - var reqBody keyBackupSessionRequest - reqBody.Rooms = make(map[string]struct { - Sessions map[string]userapi.KeyBackupSession `json:"sessions"` - }) - reqBody.Rooms[roomID] = struct { - Sessions map[string]userapi.KeyBackupSession `json:"sessions"` - }{ - Sessions: map[string]userapi.KeyBackupSession{}, - } - body := reqBody.Rooms[roomID] - resErr := clientutil.UnmarshalJSONRequest(req, &body) - if resErr != nil { - return *resErr - } - reqBody.Rooms[roomID] = body - return UploadBackupKeys(req, userAPI, device, version, &reqBody) - }), - ).Methods(http.MethodPut) + } + roomID := vars["roomID"] + var reqBody keyBackupSessionRequest + reqBody.Rooms = make(map[string]struct { + Sessions map[string]userapi.KeyBackupSession `json:"sessions"` + }) + reqBody.Rooms[roomID] = struct { + Sessions map[string]userapi.KeyBackupSession `json:"sessions"` + }{ + Sessions: map[string]userapi.KeyBackupSession{}, + } + body := reqBody.Rooms[roomID] + resErr := clientutil.UnmarshalJSONRequest(req, &body) + if resErr != nil { + return *resErr + } + reqBody.Rooms[roomID] = body + return UploadBackupKeys(req, userAPI, device, version, &reqBody) + }) + // Single room, single session - r0mux.Handle("/room_keys/keys/{roomID}/{sessionID}", - httputil.MakeAuthAPI("put_backup_keys_room_session", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { - vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) - if err != nil { - return util.ErrorResponse(err) + putBackupKeysRoomSession := httputil.MakeAuthAPI("put_backup_keys_room_session", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { + vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) + if err != nil { + return util.ErrorResponse(err) + } + version := req.URL.Query().Get("version") + if version == "" { + return util.JSONResponse{ + Code: 400, + JSON: jsonerror.InvalidArgumentValue("version must be specified"), } - version := req.URL.Query().Get("version") - if version == "" { - return util.JSONResponse{ - Code: 400, - JSON: jsonerror.InvalidArgumentValue("version must be specified"), - } - } - var reqBody userapi.KeyBackupSession - resErr := clientutil.UnmarshalJSONRequest(req, &reqBody) - if resErr != nil { - return *resErr - } - roomID := vars["roomID"] - sessionID := vars["sessionID"] - var keyReq keyBackupSessionRequest - keyReq.Rooms = make(map[string]struct { - Sessions map[string]userapi.KeyBackupSession `json:"sessions"` - }) - keyReq.Rooms[roomID] = struct { - Sessions map[string]userapi.KeyBackupSession `json:"sessions"` - }{ - Sessions: make(map[string]userapi.KeyBackupSession), - } - keyReq.Rooms[roomID].Sessions[sessionID] = reqBody - return UploadBackupKeys(req, userAPI, device, version, &keyReq) - }), - ).Methods(http.MethodPut) + } + var reqBody userapi.KeyBackupSession + resErr := clientutil.UnmarshalJSONRequest(req, &reqBody) + if resErr != nil { + return *resErr + } + roomID := vars["roomID"] + sessionID := vars["sessionID"] + var keyReq keyBackupSessionRequest + keyReq.Rooms = make(map[string]struct { + Sessions map[string]userapi.KeyBackupSession `json:"sessions"` + }) + keyReq.Rooms[roomID] = struct { + Sessions map[string]userapi.KeyBackupSession `json:"sessions"` + }{ + Sessions: make(map[string]userapi.KeyBackupSession), + } + keyReq.Rooms[roomID].Sessions[sessionID] = reqBody + return UploadBackupKeys(req, userAPI, device, version, &keyReq) + }) + + r0mux.Handle("/room_keys/keys", putBackupKeys).Methods(http.MethodPut) + r0mux.Handle("/room_keys/keys/{roomID}", putBackupKeysRoom).Methods(http.MethodPut) + r0mux.Handle("/room_keys/keys/{roomID}/{sessionID}", putBackupKeysRoomSession).Methods(http.MethodPut) + + unstableMux.Handle("/room_keys/keys", putBackupKeys).Methods(http.MethodPut) + unstableMux.Handle("/room_keys/keys/{roomID}", putBackupKeysRoom).Methods(http.MethodPut) + unstableMux.Handle("/room_keys/keys/{roomID}/{sessionID}", putBackupKeysRoomSession).Methods(http.MethodPut) // Querying E2E Backup Keys - r0mux.Handle("/room_keys/keys", - httputil.MakeAuthAPI("get_backup_keys", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { - return GetBackupKeys(req, userAPI, device, req.URL.Query().Get("version"), "", "") - }), - ).Methods(http.MethodGet, http.MethodOptions) - r0mux.Handle("/room_keys/keys/{roomID}", - httputil.MakeAuthAPI("get_backup_keys_room", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { - vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) - if err != nil { - return util.ErrorResponse(err) - } - return GetBackupKeys(req, userAPI, device, req.URL.Query().Get("version"), vars["roomID"], "") - }), - ).Methods(http.MethodGet, http.MethodOptions) - r0mux.Handle("/room_keys/keys/{roomID}/{sessionID}", - httputil.MakeAuthAPI("get_backup_keys_room_session", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { - vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) - if err != nil { - return util.ErrorResponse(err) - } - return GetBackupKeys(req, userAPI, device, req.URL.Query().Get("version"), vars["roomID"], vars["sessionID"]) - }), - ).Methods(http.MethodGet, http.MethodOptions) + getBackupKeys := httputil.MakeAuthAPI("get_backup_keys", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { + return GetBackupKeys(req, userAPI, device, req.URL.Query().Get("version"), "", "") + }) + + getBackupKeysRoom := httputil.MakeAuthAPI("get_backup_keys_room", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { + vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) + if err != nil { + return util.ErrorResponse(err) + } + return GetBackupKeys(req, userAPI, device, req.URL.Query().Get("version"), vars["roomID"], "") + }) + + getBackupKeysRoomSession := httputil.MakeAuthAPI("get_backup_keys_room_session", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { + vars, err := httputil.URLDecodeMapValues(mux.Vars(req)) + if err != nil { + return util.ErrorResponse(err) + } + return GetBackupKeys(req, userAPI, device, req.URL.Query().Get("version"), vars["roomID"], vars["sessionID"]) + }) + + r0mux.Handle("/room_keys/keys", getBackupKeys).Methods(http.MethodGet, http.MethodOptions) + r0mux.Handle("/room_keys/keys/{roomID}", getBackupKeysRoom).Methods(http.MethodGet, http.MethodOptions) + r0mux.Handle("/room_keys/keys/{roomID}/{sessionID}", getBackupKeysRoomSession).Methods(http.MethodGet, http.MethodOptions) + + unstableMux.Handle("/room_keys/keys", getBackupKeys).Methods(http.MethodGet, http.MethodOptions) + unstableMux.Handle("/room_keys/keys/{roomID}", getBackupKeysRoom).Methods(http.MethodGet, http.MethodOptions) + unstableMux.Handle("/room_keys/keys/{roomID}/{sessionID}", getBackupKeysRoomSession).Methods(http.MethodGet, http.MethodOptions) // Deleting E2E Backup Keys diff --git a/userapi/api/api.go b/userapi/api/api.go index b0d918560..75d06dd69 100644 --- a/userapi/api/api.go +++ b/userapi/api/api.go @@ -72,13 +72,11 @@ func (a *KeyBackupSession) ShouldReplaceRoomKey(newKey *KeyBackupSession) bool { // "if the keys have different values for is_verified, then it will keep the key that has is_verified set to true" if newKey.IsVerified && !a.IsVerified { return true - } - // "if they have the same values for is_verified, then it will keep the key with a lower first_message_index" - if newKey.FirstMessageIndex < a.FirstMessageIndex { + } else if newKey.FirstMessageIndex < a.FirstMessageIndex { + // "if they have the same values for is_verified, then it will keep the key with a lower first_message_index" return true - } - // "and finally, is is_verified and first_message_index are equal, then it will keep the key with a lower forwarded_count" - if newKey.ForwardedCount < a.ForwardedCount { + } else if newKey.ForwardedCount < a.ForwardedCount { + // "and finally, is is_verified and first_message_index are equal, then it will keep the key with a lower forwarded_count" return true } return false diff --git a/userapi/storage/accounts/postgres/key_backup_table.go b/userapi/storage/accounts/postgres/key_backup_table.go index ec6518267..0a2a26550 100644 --- a/userapi/storage/accounts/postgres/key_backup_table.go +++ b/userapi/storage/accounts/postgres/key_backup_table.go @@ -36,7 +36,7 @@ CREATE TABLE IF NOT EXISTS account_e2e_room_keys ( session_data TEXT NOT NULL ); CREATE UNIQUE INDEX IF NOT EXISTS e2e_room_keys_idx ON account_e2e_room_keys(user_id, room_id, session_id, version); -CREATE UNIQUE INDEX IF NOT EXISTS e2e_room_keys_versions_idx ON account_e2e_room_keys(user_id, version); +CREATE INDEX IF NOT EXISTS e2e_room_keys_versions_idx ON account_e2e_room_keys(user_id, version); ` const insertBackupKeySQL = "" + diff --git a/userapi/storage/accounts/postgres/key_backup_version_table.go b/userapi/storage/accounts/postgres/key_backup_version_table.go index aca575df2..51a462b32 100644 --- a/userapi/storage/accounts/postgres/key_backup_version_table.go +++ b/userapi/storage/accounts/postgres/key_backup_version_table.go @@ -146,12 +146,19 @@ func (s *keyBackupVersionStatements) selectKeyBackup( ) (versionResult, algorithm string, authData json.RawMessage, etag string, deleted bool, err error) { var versionInt int64 if version == "" { - err = txn.Stmt(s.selectLatestVersionStmt).QueryRowContext(ctx, userID).Scan(&versionInt) + var v *int64 // allows nulls + if err = txn.Stmt(s.selectLatestVersionStmt).QueryRowContext(ctx, userID).Scan(&v); err != nil { + return + } + if v == nil { + err = sql.ErrNoRows + return + } + versionInt = *v } else { - versionInt, err = strconv.ParseInt(version, 10, 64) - } - if err != nil { - return + if versionInt, err = strconv.ParseInt(version, 10, 64); err != nil { + return + } } versionResult = strconv.FormatInt(versionInt, 10) var deletedInt int diff --git a/userapi/storage/accounts/postgres/storage.go b/userapi/storage/accounts/postgres/storage.go index 9d6fd13a1..6bddbfc3f 100644 --- a/userapi/storage/accounts/postgres/storage.go +++ b/userapi/storage/accounts/postgres/storage.go @@ -479,7 +479,7 @@ func (d *Database) UpsertBackupKeys( err = d.keyBackups.updateBackupKey(ctx, txn, userID, version, newKey) changed = true if err != nil { - return err + return fmt.Errorf("d.keyBackups.updateBackupKey: %w", err) } } // if we shouldn't replace the key we do nothing with it @@ -490,7 +490,7 @@ func (d *Database) UpsertBackupKeys( err = d.keyBackups.insertBackupKey(ctx, txn, userID, version, newKey) changed = true if err != nil { - return err + return fmt.Errorf("d.keyBackups.insertBackupKey: %w", err) } } diff --git a/userapi/storage/accounts/sqlite3/key_backup_table.go b/userapi/storage/accounts/sqlite3/key_backup_table.go index c1a698e66..675093514 100644 --- a/userapi/storage/accounts/sqlite3/key_backup_table.go +++ b/userapi/storage/accounts/sqlite3/key_backup_table.go @@ -36,7 +36,7 @@ CREATE TABLE IF NOT EXISTS account_e2e_room_keys ( session_data TEXT NOT NULL ); CREATE UNIQUE INDEX IF NOT EXISTS e2e_room_keys_idx ON account_e2e_room_keys(user_id, room_id, session_id, version); -CREATE UNIQUE INDEX IF NOT EXISTS e2e_room_keys_versions_idx ON account_e2e_room_keys(user_id, version); +CREATE INDEX IF NOT EXISTS e2e_room_keys_versions_idx ON account_e2e_room_keys(user_id, version); ` const insertBackupKeySQL = "" + diff --git a/userapi/storage/accounts/sqlite3/key_backup_version_table.go b/userapi/storage/accounts/sqlite3/key_backup_version_table.go index 9a58fee75..a9e7bf5db 100644 --- a/userapi/storage/accounts/sqlite3/key_backup_version_table.go +++ b/userapi/storage/accounts/sqlite3/key_backup_version_table.go @@ -144,12 +144,19 @@ func (s *keyBackupVersionStatements) selectKeyBackup( ) (versionResult, algorithm string, authData json.RawMessage, etag string, deleted bool, err error) { var versionInt int64 if version == "" { - err = txn.Stmt(s.selectLatestVersionStmt).QueryRowContext(ctx, userID).Scan(&versionInt) + var v *int64 // allows nulls + if err = txn.Stmt(s.selectLatestVersionStmt).QueryRowContext(ctx, userID).Scan(&v); err != nil { + return + } + if v == nil { + err = sql.ErrNoRows + return + } + versionInt = *v } else { - versionInt, err = strconv.ParseInt(version, 10, 64) - } - if err != nil { - return + if versionInt, err = strconv.ParseInt(version, 10, 64); err != nil { + return + } } versionResult = strconv.FormatInt(versionInt, 10) var deletedInt int diff --git a/userapi/storage/accounts/sqlite3/storage.go b/userapi/storage/accounts/sqlite3/storage.go index 728ae9011..d752e3db8 100644 --- a/userapi/storage/accounts/sqlite3/storage.go +++ b/userapi/storage/accounts/sqlite3/storage.go @@ -520,7 +520,7 @@ func (d *Database) UpsertBackupKeys( err = d.keyBackups.updateBackupKey(ctx, txn, userID, version, newKey) changed = true if err != nil { - return err + return fmt.Errorf("d.keyBackups.updateBackupKey: %w", err) } } // if we shouldn't replace the key we do nothing with it @@ -531,7 +531,7 @@ func (d *Database) UpsertBackupKeys( err = d.keyBackups.insertBackupKey(ctx, txn, userID, version, newKey) changed = true if err != nil { - return err + return fmt.Errorf("d.keyBackups.insertBackupKey: %w", err) } }