Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle cross-signing key changes when old key is cached #12904

Open
dbkr opened this issue Mar 27, 2020 · 5 comments
Open

Handle cross-signing key changes when old key is cached #12904

dbkr opened this issue Mar 27, 2020 · 5 comments

Comments

@dbkr
Copy link
Member

dbkr commented Mar 27, 2020

Ensure we handle the user resetting their cross-signing keys and notice that the ones we have cached are no longer useful. We can do this by ensuring that we always revalidate the that the private key we use matches the public key stored on the account.

Presumably if we don't have the key cached, we should prompt for the SSSS (secure secret storage and sharing) passphrase?

@foldleft
Copy link
Contributor

foldleft commented Apr 3, 2020

we do this already for getCrossSigningKey, when we prompt the user:

        function validateKey(key) {
            if (!key) return;
            const signing = new global.Olm.PkSigning();
            const gotPubkey = signing.init_with_seed(key);
            if (gotPubkey === expectedPubkey) {
                return [gotPubkey, signing];
            }
            signing.free();
        }

        let privkey;
        if (this._cacheCallbacks.getCrossSigningKeyCache && shouldCache) {
            privkey = await this._cacheCallbacks
              .getCrossSigningKeyCache(type, expectedPubkey);
        }

        const cacheresult = validateKey(privkey);

however, it doesn't happen for key sharing. for that we just hand over whatever we have in the cache.

@foldleft
Copy link
Contributor

foldleft commented Apr 3, 2020

and then for requests, it looks like this:

const crossSigning = new CrossSigningInfo(
                    original.userId,
                    { getCrossSigningKey: async (type) => {
                        console.debug("VerificationBase.done: requesting secret",
                                      type, this.deviceId);
                        const { promise } = client.requestSecret(
                            `m.cross_signing.${type}`, [this.deviceId],
                        );
                        const result = await promise;
                        const decoded = decodeBase64(result);
                        return Uint8Array.from(decoded);
                    } },
                    original._cacheCallbacks,
                );

which is to say, the requesting side validates that it didn't get a bad key. the responding side, however, does happily send a bad key.

So I think there's no case where you might end up getting your keys poisoned: all the paths in question do check.

However, there's still one edge-case we should probably consider: if a bad key is sent as a response, pre-empting a correct response, then key sharing won't work properly

@dbkr
Copy link
Member Author

dbkr commented Apr 3, 2020

OK yeah - as long as we then fall back to getting the keys out of SSSS if our cached key is wrong then we are probably fine here, as even in the worst case if we do get stale keys from a keyshare request, it will just mean the user has to enter their passphrase. We can maybe worry about validating keys either before sharing or upon receiving at a later date if we want to.

@foldleft
Copy link
Contributor

foldleft commented Apr 8, 2020

should we close this?

@jryans
Copy link
Collaborator

jryans commented Apr 8, 2020

Sounds like we can at the very least defer, I'll leave to @dbkr on whether to close or not.

@jryans jryans added phase:4 and removed phase:3 labels Apr 8, 2020
t3chguy pushed a commit that referenced this issue Oct 17, 2024
Rename prettier config file to .cjs (staging version)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants