From 38999c54e1f0e3aebc812f5a56e41c9d60936558 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 10 Oct 2017 10:40:52 +0100 Subject: [PATCH] Generate new devices for each new /login (#281) --- .../dendrite/clientapi/auth/auth.go | 26 +++++++---- .../clientapi/auth/storage/devices/storage.go | 43 ++++++++++++++----- .../dendrite/clientapi/readers/login.go | 9 ++-- .../dendrite/clientapi/writers/register.go | 2 +- .../dendrite/cmd/create-account/main.go | 2 +- 5 files changed, 56 insertions(+), 26 deletions(-) diff --git a/src/github.com/matrix-org/dendrite/clientapi/auth/auth.go b/src/github.com/matrix-org/dendrite/clientapi/auth/auth.go index 833cf544..b6a3efc2 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/auth/auth.go +++ b/src/github.com/matrix-org/dendrite/clientapi/auth/auth.go @@ -29,17 +29,13 @@ import ( "github.com/matrix-org/util" ) -// UnknownDeviceID is the default device id if one is not specified. -// This deviates from Synapse which generates a new device ID if one is not specified. -// It's preferable to not amass a huge list of valid access tokens for an account, -// so limiting it to 1 unknown device for now limits the number of valid tokens. -// Clients should be giving us device IDs. -var UnknownDeviceID = "unknown-device" - // OWASP recommends at least 128 bits of entropy for tokens: https://www.owasp.org/index.php/Insufficient_Session-ID_Length // 32 bytes => 256 bits var tokenByteLength = 32 +// The length of generated device IDs +var deviceIDByteLength = 6 + // DeviceDatabase represents a device database. type DeviceDatabase interface { // Look up the device matching the given access token. @@ -62,8 +58,8 @@ func VerifyAccessToken(req *http.Request, deviceDB DeviceDatabase) (device *auth if err != nil { if err == sql.ErrNoRows { resErr = &util.JSONResponse{ - Code: 403, - JSON: jsonerror.Forbidden("Invalid access token"), + Code: 401, + JSON: jsonerror.UnknownToken("Unknown token"), } } else { resErr = &util.JSONResponse{ @@ -86,6 +82,18 @@ func GenerateAccessToken() (string, error) { return base64.RawURLEncoding.EncodeToString(b), nil } +// GenerateDeviceID creates a new device id. Returns an error if failed to generate +// random bytes. +func GenerateDeviceID() (string, error) { + b := make([]byte, deviceIDByteLength) + _, err := rand.Read(b) + if err != nil { + return "", err + } + // url-safe no padding + return base64.RawURLEncoding.EncodeToString(b), nil +} + // extractAccessToken from a request, or return an error detailing what went wrong. The // error message MUST be human-readable and comprehensible to the client. func extractAccessToken(req *http.Request) (string, error) { diff --git a/src/github.com/matrix-org/dendrite/clientapi/auth/storage/devices/storage.go b/src/github.com/matrix-org/dendrite/clientapi/auth/storage/devices/storage.go index df8bf4f3..ea7d8738 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/auth/storage/devices/storage.go +++ b/src/github.com/matrix-org/dendrite/clientapi/auth/storage/devices/storage.go @@ -18,6 +18,7 @@ import ( "context" "database/sql" + "github.com/matrix-org/dendrite/clientapi/auth" "github.com/matrix-org/dendrite/clientapi/auth/authtypes" "github.com/matrix-org/dendrite/common" "github.com/matrix-org/gomatrixserverlib" @@ -55,20 +56,42 @@ func (d *Database) GetDeviceByAccessToken( // If there is already a device with the same device ID for this user, that access token will be revoked // and replaced with the given accessToken. If the given accessToken is already in use for another device, // an error will be returned. +// If no device ID is given one is generated. // Returns the device on success. func (d *Database) CreateDevice( - ctx context.Context, localpart, deviceID, accessToken string, + ctx context.Context, localpart string, deviceID *string, accessToken string, ) (dev *authtypes.Device, returnErr error) { - returnErr = common.WithTransaction(d.db, func(txn *sql.Tx) error { - var err error - // Revoke existing token for this device - if err = d.devices.deleteDevice(ctx, txn, deviceID, localpart); err != nil { - return err - } + if deviceID != nil { + returnErr = common.WithTransaction(d.db, func(txn *sql.Tx) error { + var err error + // Revoke existing token for this device + if err = d.devices.deleteDevice(ctx, txn, *deviceID, localpart); err != nil { + return err + } - dev, err = d.devices.insertDevice(ctx, txn, deviceID, localpart, accessToken) - return err - }) + dev, err = d.devices.insertDevice(ctx, txn, *deviceID, localpart, accessToken) + return err + }) + } else { + // We generate device IDs in a loop in case its already taken. + // We cap this at going round 5 times to ensure we don't spin forever + var newDeviceID string + for i := 1; i <= 5; i++ { + newDeviceID, returnErr = auth.GenerateDeviceID() + if returnErr != nil { + return + } + + returnErr = common.WithTransaction(d.db, func(txn *sql.Tx) error { + var err error + dev, err = d.devices.insertDevice(ctx, txn, newDeviceID, localpart, accessToken) + return err + }) + if returnErr == nil { + return + } + } + } return } diff --git a/src/github.com/matrix-org/dendrite/clientapi/readers/login.go b/src/github.com/matrix-org/dendrite/clientapi/readers/login.go index de6ecf5c..ddbac12c 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/readers/login.go +++ b/src/github.com/matrix-org/dendrite/clientapi/readers/login.go @@ -46,6 +46,7 @@ type loginResponse struct { UserID string `json:"user_id"` AccessToken string `json:"access_token"` HomeServer gomatrixserverlib.ServerName `json:"home_server"` + DeviceID string `json:"device_id"` } func passwordLogin() loginFlows { @@ -113,15 +114,12 @@ func Login( token, err := auth.GenerateAccessToken() if err != nil { - return util.JSONResponse{ - Code: 500, - JSON: jsonerror.Unknown("Failed to generate access token"), - } + httputil.LogThenError(req, err) } // TODO: Use the device ID in the request dev, err := deviceDB.CreateDevice( - req.Context(), acc.Localpart, auth.UnknownDeviceID, token, + req.Context(), acc.Localpart, nil, token, ) if err != nil { return util.JSONResponse{ @@ -136,6 +134,7 @@ func Login( UserID: dev.UserID, AccessToken: dev.AccessToken, HomeServer: cfg.Matrix.ServerName, + DeviceID: dev.ID, }, } } diff --git a/src/github.com/matrix-org/dendrite/clientapi/writers/register.go b/src/github.com/matrix-org/dendrite/clientapi/writers/register.go index 84227d15..a405f5ef 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/writers/register.go +++ b/src/github.com/matrix-org/dendrite/clientapi/writers/register.go @@ -303,7 +303,7 @@ func completeRegistration( } // // TODO: Use the device ID in the request. - dev, err := deviceDB.CreateDevice(ctx, username, auth.UnknownDeviceID, token) + dev, err := deviceDB.CreateDevice(ctx, username, nil, token) if err != nil { return util.JSONResponse{ Code: 500, diff --git a/src/github.com/matrix-org/dendrite/cmd/create-account/main.go b/src/github.com/matrix-org/dendrite/cmd/create-account/main.go index d031afc2..3d5c3587 100644 --- a/src/github.com/matrix-org/dendrite/cmd/create-account/main.go +++ b/src/github.com/matrix-org/dendrite/cmd/create-account/main.go @@ -87,7 +87,7 @@ func main() { } device, err := deviceDB.CreateDevice( - context.Background(), *username, "create-account-script", *accessToken, + context.Background(), *username, nil, *accessToken, ) if err != nil { fmt.Println(err.Error())