Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Rotating the keys of a key-connector account, for each account version the vectors carry.
//
// A key-connector rotation authorizes itself with the key fetched from the connector rather than a
// derived master key, which is the one thing that differs from the other rotations. The shared
// assertion body lives in `tests/rotation-cases.ts`.

import type { InitUserCryptoMethod } from "@bitwarden/sdk-internal";

import { KEY_CONNECTOR_URL } from "../model-server/install";
import {
assertRotationHarnessClean,
expectRotationSucceeds,
ROTATION_TIMEOUT,
setupRotation,
UNLOCK_METHOD,
type RotationCase,
type RotationHarness,
} from "../rotation-cases";
import { loadUserVectors, userVector, type UserVector } from "../test-vectors/load";

const users = loadUserVectors();

/** Pulls the key-connector key out of the vector's own key-connector unlock method. */
function keyConnectorKeyOf(vector: UserVector): string {
const method = vector.unlockMethods.find((m) => "keyConnector" in m) as
Extract<InitUserCryptoMethod, { keyConnector: unknown }> | undefined;
if (method === undefined) {
throw new Error(`${vector.name} has no key-connector unlock method`);
}
return method.keyConnector.master_key.toString();
}

const cases: [string, RotationCase][] = [
[
"V1 key connector",
{
vector: userVector(users, "v1-pbkdf2-key-connector"),
method: () => ({ KeyConnector: { key_connector_url: KEY_CONNECTOR_URL } }),
expectedUnlockMethod: UNLOCK_METHOD.keyConnector,
keyConnectorKey: keyConnectorKeyOf(userVector(users, "v1-pbkdf2-key-connector")),
},
],
[
"V2 key connector",
{
vector: userVector(users, "v2-pbkdf2-key-connector"),
method: () => ({ KeyConnector: { key_connector_url: KEY_CONNECTOR_URL } }),
expectedUnlockMethod: UNLOCK_METHOD.keyConnector,
keyConnectorKey: keyConnectorKeyOf(userVector(users, "v2-pbkdf2-key-connector")),
},
],
];

describe("key connector key rotation", () => {
let harness: RotationHarness;

afterEach(() => assertRotationHarnessClean(harness));

describe.each(cases)("%s", (_label, rotationCase) => {
it(
"posts a V2 cryptographic state, the re-encrypted vault and the right unlock method",
async () => {
harness = setupRotation(rotationCase.vector, rotationCase.keyConnectorKey);
await expectRotationSucceeds(harness, rotationCase);
},
ROTATION_TIMEOUT,
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
// The four ways a key connector migration can go sideways, kept so none of them regresses.
//
// Ordering matters more here than in most operations. If the account were enrolled server-side while the
// key connector had not stored the key, the user would be permanently locked out β€” so a failure at the
// connector must abort before enrollment. The reverse order is the safe one, and the last test documents
// that the SDK does leave an orphaned key behind rather than risk the other way round.
//
// The connector model enforces the real verb constraint (`POST` with a key present is a 409, `PUT` with
// none a 404), so the first test only has to check that the migration completed at all.

import { LocalState, SETTINGS } from "../model-server/local-state";
import { syncToLocalState, unlockMethodFor } from "../model-server/sync";
import { KEY_CONNECTOR_URL } from "../model-server/install";
import {
assertMigrationHarnessClean,
KEY_CONNECTOR_KEY_BYTES,
MIGRATION_CASES,
MIGRATION_TIMEOUT,
setupMigration,
type MigrationHarness,
} from "./migration-support";
import { makePasswordManagerClient, makeStateBridge } from "../utils";

/** The cheapest vector in the set to unlock; none of these cases turns on the account version. */
const vector = MIGRATION_CASES[0][1];

describe("key connector migration", () => {
let harness: MigrationHarness;

afterEach(() => assertMigrationHarnessClean(harness));

it(
"updates the existing key with PUT when the connector already holds one",
async () => {
// The connector rejects a PUT for a key that does not exist and a POST for one that does, so
// picking the wrong verb here fails the migration outright.
harness = setupMigration(vector);
const { api, keyConnector } = harness;
const preexisting = Buffer.alloc(KEY_CONNECTOR_KEY_BYTES, 7).toString("base64");
keyConnector.seedKey(preexisting);

const local = new LocalState();
await syncToLocalState(api, vector.account.email, local);
const client = await local.unlock(unlockMethodFor(api, vector.account.email));
await client.user_crypto_management().migrate_to_key_connector(KEY_CONNECTOR_URL);

// The connector rejects a POST when a key already exists, so the migration completing at all is
// what shows the client read first and chose PUT. And the key it stores is its own new one, not
// the key the connector already had.
expect(keyConnector.key()).not.toBe(preexisting);
expect(keyConnector.key()).toBeTruthy();
},
MIGRATION_TIMEOUT,
);

it(
"sends nothing at all when the client is locked",
async () => {
// No `initialize_user_crypto`, so there is no user key to wrap. The migration must fail at
// step 2, before the freshly minted key connector key has been shown to anyone β€” otherwise a
// locked client would leave a usable key sitting at the connector.
harness = setupMigration();
const client = makePasswordManagerClient(makeStateBridge(), SETTINGS);

await expect(
client.user_crypto_management().migrate_to_key_connector(KEY_CONNECTOR_URL),
).rejects.toBeDefined();

expect(harness.servers.requests).toEqual([]);
expect(harness.keyConnector.key()).toBeUndefined();
},
MIGRATION_TIMEOUT,
);

it(
"does not enroll the account when the key connector rejects the key",
async () => {
// The invariant that protects against permanent lockout: if the server recorded the account as
// key-connector-unlocked while the connector had no key, nothing could ever unlock it again.
harness = setupMigration(vector);
const { api, keyConnector } = harness;
keyConnector.failWrites(500);

const local = new LocalState();
await syncToLocalState(api, vector.account.email, local);
const client = await local.unlock(unlockMethodFor(api, vector.account.email));
await expect(
client.user_crypto_management().migrate_to_key_connector(KEY_CONNECTOR_URL),
).rejects.toBeDefined();

// Neither side changed: no key stored, and β€” the invariant that matters β€” no enrolment recorded.
// If the account were enrolled while the connector held no key, nothing could unlock it again.
expect(keyConnector.key()).toBeUndefined();
expect(api.db.user(api.soleUserId()).keyConnectorKeyWrappedUserKey).toBeUndefined();
},
MIGRATION_TIMEOUT,
);

it(
"surfaces a failure to enroll, after the key connector has already stored the key",
async () => {
// Override just the enrolment endpoint to fail, leaving the rest of the model intact.
harness = setupMigration(vector, {
"POST /accounts/key-connector/enroll": () => ({
status: 500,
json: { message: "enrolment unavailable" },
}),
});
const { api, keyConnector } = harness;

const local = new LocalState();
await syncToLocalState(api, vector.account.email, local);
const client = await local.unlock(unlockMethodFor(api, vector.account.email));
await expect(
client.user_crypto_management().migrate_to_key_connector(KEY_CONNECTOR_URL),
).rejects.toBeDefined();

// Documenting real behaviour rather than endorsing it: the key is already at the connector at
// this point and is not rolled back. That is the safe way round β€” the account still unlocks by
// master password, and a retry overwrites the orphaned key via PUT.
expect(keyConnector.key()).toBeDefined();
expect(api.db.user(api.soleUserId()).keyConnectorKeyWrappedUserKey).toBeUndefined();
},
MIGRATION_TIMEOUT,
);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// The shared harness for `migrate_to_key_connector`, so the flow and the edge cases can each stand up
// the two servers without re-deriving the setup.
//
// A migration spans two independent origins β€” the API and the key connector β€” so every test needs both
// models installed together, and both are inspected afterwards.

import type { Routes } from "../http-mock";
import { ApiServer } from "../model-server/api-server";
import { installServers, type InstalledServers } from "../model-server/install";
import { KeyConnectorServer } from "../model-server/key-connector-server";
import { loadUserVectors, userVector, type UserVector } from "../test-vectors/load";

/** Unlocking to get a user key in memory pays the account's real KDF cost. */
export const MIGRATION_TIMEOUT = 120_000;

/** A key connector key is 32 raw bytes, sent base64. */
export const KEY_CONNECTOR_KEY_BYTES = 32;

const users = loadUserVectors();

/**
* A V1 and a V2 account, because the user key being wrapped differs in kind: V1 is an
* `Aes256CbcHmac` key, V2 an `XChaCha20Poly1305` COSE key, and `encrypt_user_key` encodes them
* differently. `v1-pbkdf2-min-iterations` is also the cheapest vector in the set to unlock.
*/
export const MIGRATION_CASES: [string, UserVector][] = [
["a V1 account", userVector(users, "v1-pbkdf2-min-iterations")],
["a V2 account", userVector(users, "v2-argon2id-blob")],
];

export interface MigrationHarness {
api: ApiServer;
keyConnector: KeyConnectorServer;
servers: InstalledServers;
}

/**
* Stands up an API model and a key connector model on their own origins.
*
* Passing no vector leaves the API with no account at all, which is what the locked-client case needs.
*/
export function setupMigration(vector?: UserVector, extraRoutes?: Routes): MigrationHarness {
const api = new ApiServer();
if (vector !== undefined) {
api.seedUser(vector);
}
const keyConnector = new KeyConnectorServer();
return { api, keyConnector, servers: installServers({ api, keyConnector, extraRoutes }) };
}

/** The assertions every migration suite makes in `afterEach`. */
export function assertMigrationHarnessClean(harness: MigrationHarness): void {
expect(harness.servers.unmatched.map((request) => request.route)).toEqual([]);
// No seeded account's password, user key, private key or master key may ever appear in a request
// body. Policed by the server on every request, so no individual test has to remember to look.
expect(harness.api.secretLeaks()).toEqual([]);
harness.servers.restore();
}
Loading
Loading