error in keys handle: one-time key already exists

r3k2 commented 3 years ago

Background information

Dendrite version or git SHA: latest from master I updated every time there is a push to master.
Monolith or Polylith?: Monolith
SQLite3 or Postgres?:
Postgresql
Running in Docker?: no way!
go version: go version go1.17 linux/arm64

Description

I am reporting an error that may or may not be an issue. but better safe than sorry :)

Steps to reproduce

Installed go-neb a golang bot for matrix. seems to work so far. but I get this error in the dendrite logs related to keys.. and since is a new feature want to make sure is known.

time="2021-08-30T17:54:18.038892320Z" level=error msg="Failed to upload one or more keys" func=github.com/matrix-org/dendrite/clientapi/routing.UploadKeys file="github.com/matrix-org/dendrite/clientapi/routing/keys.go:71" key_errors="map[@hispabot:hispagatos.org:map[HRNsLRF9:@hispabot:hispagatos.org device HRNsLRF9: algorithm / key ID signed_curve25519:AAAMHA one-time key already exists]]" req.id=U6cZPFL2OUmT req.method=POST req.path=/_matrix/client/r0/keys/upload user_id="@hispabot:hispagatos.org"

if this is a bot issue let me know and delete this. Thanks

kegsay commented 2 years ago

Dendrite is pretty strict and validates E2EE keys. Synapse does not and treats the entire upload as a random JSON blob. I suspect this means E2EE bugs in clients will appear as Dendrite issues. The relevant code here is:

        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(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
            }
        }

The key ID itself is pretty small and looks like AAAMHA in this example. I suspect it's possible for duplicates to exist (with different signatures) if the client is forgetting which keys it has uploaded and it resets its key generation ID number. I'm unsure if this is a client bug or if servers should just overwrite OTKs in this case. Any input from @BillCarsonFr or @uhoreg would be welcome.

uhoreg commented 2 years ago

Synapse also checks if a one-time key has already been uploaded: https://github.com/matrix-org/synapse/blob/f8cf02b200dc56c3de857b05ee7ef4a58d23d254/synapse/handlers/e2e_keys.py#L638-L648 so this should also fail on Synapse.

kegsay commented 2 years ago

This feels like a client bug then.

matrix-org / dendrite