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

feat: encrypt metadata feature #2059

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions apps/api/src/pkg/cache/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export function initCache(c: Context<HonoEnv>, metrics: Metrics): C<CacheNamespa
c.executionCtx,
defaultOpts,
),
encryptedMeta: new Namespace<CacheNamespaces["encryptedMeta"]>(c.executionCtx, defaultOpts),
});
}

Expand Down
1 change: 1 addition & 0 deletions apps/api/src/pkg/cache/namespaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export type CacheNamespaces = {
total: number;
};
identityByExternalId: Identity | null;
encryptedMeta: string | null;
};

export type CacheNamespace = keyof CacheNamespaces;
62 changes: 62 additions & 0 deletions apps/api/src/routes/v1_keys_createKey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,17 @@ When validating a key, we will return this back to you, so you can clearly ident
trialEnds: "2023-06-16T17:16:37.161Z",
},
}),
encryptedMeta: z
.record(z.unknown())
.optional()
.openapi({
description:
"This is a place for encrypted meta data, anything that feels useful for you should go here",
example: {
billingTier: "PRO",
trialEnds: "2023-06-16T17:16:37.161Z",
},
}),
roles: z
.array(z.string().min(1).max(512))
.optional()
Expand Down Expand Up @@ -336,6 +347,56 @@ export const registerV1KeysCreateKey = (app: App) =>
byteLength: req.byteLength ?? 16,
prefix: req.prefix,
}).toString();
let metaSecret = req.encryptedMeta ? JSON.stringify(req.encryptedMeta) : null;

if (metaSecret) {
const perm = rbac.evaluatePermissions(
buildUnkeyQuery(({ or }) => or("*", "api.*.encrypt_meta", `api.${api.id}.encrypt_meta`)),
auth.permissions,
);

if (perm.err) {
throw new UnkeyApiError({
code: "INTERNAL_SERVER_ERROR",
message: `unable to evaluate permissions: ${perm.err.message}`,
});
}

if (!perm.val.valid) {
throw new UnkeyApiError({
code: "INSUFFICIENT_PERMISSIONS",
message: `insufficient permissions to encrypt metadata: ${perm.val.message}`,
});
}

if (metaSecret) {
const encryptedMeta = await retry(
3,
async () => {
return await vault.encrypt(c, {
keyring: authorizedWorkspaceId,
data: metaSecret as string,
});
},
(attempt, err) =>
logger.warn("vault.encrypt failed", {
attempt,
err: err.message,
}),
);

metaSecret = encryptedMeta.encrypted;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're ignoring the returned encryption key id, that needs to be stored as well, otherwise we can't reroll the encryption keys

await db.primary.insert(schema.secrets).values({
id: newId("secret"),
workspaceId: authorizedWorkspaceId,
encrypted: encryptedMeta.encrypted,
// Using newId here because the id returned by the vault i.e. encryptionKeyId is a data agnostic id.
// So its going to give errors if we the encrypted Metadata is not unique.
encryptionKeyId: newId("enryptedMeta"),
});
}
}

const start = secret.slice(0, (req.prefix?.length ?? 0) + 5);
const kId = newId("key");
const hash = await sha256(secret.toString());
Expand All @@ -347,6 +408,7 @@ export const registerV1KeysCreateKey = (app: App) =>
start,
ownerId: externalId,
meta: req.meta ? JSON.stringify(req.meta) : null,
encryptedMeta: metaSecret ? metaSecret : null,
workspaceId: authorizedWorkspaceId,
forWorkspaceId: null,
expires: req.expires ? new Date(req.expires) : null,
Expand Down
9 changes: 9 additions & 0 deletions apps/api/src/routes/v1_keys_verifyKey.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ describe("with temporary key", () => {
expires: now,
environment: "prod",
meta: JSON.stringify({ hello: "world" }),
encryptedMeta: JSON.stringify({ encrypted: "data" }),
};

await h.db.primary.insert(schema.keys).values(key);
Expand Down Expand Up @@ -163,6 +164,7 @@ describe("with temporary key", () => {
expect(res.body.valid).toBe(false);
expect(res.body.code).toBe("EXPIRED");
expect(res.body.meta).toMatchObject({ hello: "world" });
expect(res.body.encryptedMeta).toMatchObject({ encrypted: "data" });
expect(res.body.expires).toBe(key.expires.getTime());
expect(res.body.environment).toBe(key.environment);
expect(res.body.name).toBe(key.name);
Expand All @@ -189,6 +191,9 @@ describe("with metadata", () => {
meta: JSON.stringify({
disabledReason: "cause I can",
}),
encryptedMeta: JSON.stringify({
disabledReason: "cause I can",
}),
enabled: false,
});

Expand All @@ -205,6 +210,7 @@ describe("with metadata", () => {
expect(res.status, `expected 200, received: ${JSON.stringify(res, null, 2)}`).toBe(200);
expect(res.body.valid).toBe(false);
expect(res.body.meta).toMatchObject({ disabledReason: "cause I can" });
expect(res.body.encryptedMeta).toMatchObject({ disabledReason: "cause I can" });
},
{ timeout: 20000 },
);
Expand Down Expand Up @@ -266,6 +272,9 @@ describe("with identity", () => {
meta: {
hello: "world",
},
encryptedMeta: {
encrypted: "data",
},
};
await h.db.primary.insert(schema.identities).values(identity);

Expand Down
51 changes: 48 additions & 3 deletions apps/api/src/routes/v1_keys_verifyKey.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { rootKeyAuth } from "@/pkg/auth/root_key";
import { UnkeyApiError, openApiErrorResponses } from "@/pkg/errors";
import type { App } from "@/pkg/hono/app";
import { DisabledWorkspaceError, MissingRatelimitError } from "@/pkg/keys/service";
import { retry } from "@/pkg/util/retry";
import { createRoute, z } from "@hono/zod-openapi";
import { SchemaError } from "@unkey/error";
import { permissionQuerySchema } from "@unkey/rbac";
import { buildUnkeyQuery, permissionQuerySchema } from "@unkey/rbac";

const route = createRoute({
tags: ["keys"],
Expand Down Expand Up @@ -176,6 +178,17 @@ A key could be invalid for a number of reasons, for example if it has expired, h
stripeCustomerId: "cus_1234",
},
}),
encryptedMeta: z
.record(z.unknown())
.optional()
.openapi({
description:
"Encrypted metadata that might contain secrets that you want to store with the key",
example: {
roles: ["admin", "user"],
stripeCustomerId: "cus_1234",
},
}),
expires: z.number().int().optional().openapi({
description:
"The unix timestamp in milliseconds when the key will expire. If this field is null or undefined, the key is not expiring.",
Expand Down Expand Up @@ -286,8 +299,7 @@ export type V1KeysVerifyKeyResponse = z.infer<
export const registerV1KeysVerifyKey = (app: App) =>
app.openapi(route, async (c) => {
const req = c.req.valid("json");
const { keyService, analytics } = c.get("services");

const { keyService, analytics, vault, cache } = c.get("services");
const { val, err } = await keyService.verifyKey(c, {
key: req.key,
apiId: req.apiId,
Expand Down Expand Up @@ -327,12 +339,45 @@ export const registerV1KeysVerifyKey = (app: App) =>
});
}

let metaSecret: string | undefined | null;
if (val.key.encryptedMeta) {
try {
await rootKeyAuth(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what rootKey are you using here?
verifying keys does not require a root key. I'm very surprised this didn't cause issues when you tested it.

c,
buildUnkeyQuery(({ or }) =>
or("*", "api.*.decrypt_meta", `api.${val.api.id}.decrypt_meta`),
),
);
const { val: vaultRes } = await cache.encryptedMeta.swr(
val.key.encryptedMeta!,
async () => {
const encryptedMeta =
typeof val.key.encryptedMeta === "string"
? val.key.encryptedMeta
: JSON.stringify(val.key.encryptedMeta);

const decryptRes = await retry(3, () =>
vault.decrypt(c, {
keyring: val.key.workspaceId,
encrypted: encryptedMeta,
}),
);
return decryptRes.plaintext;
},
);
metaSecret = vaultRes;
} catch {
metaSecret = undefined;
}
}

const responseBody = {
keyId: val.key?.id,
valid: val.valid,
name: val.key?.name ?? undefined,
ownerId: val.key?.ownerId ?? undefined,
meta: val.key?.meta ? JSON.parse(val.key?.meta) : undefined,
encryptedMeta: metaSecret ? JSON.parse(metaSecret) : undefined,
expires: val.key?.expires?.getTime(),
remaining: val.remaining ?? undefined,
ratelimit: val.ratelimit ?? undefined,
Expand Down
12 changes: 12 additions & 0 deletions apps/api/src/routes/v1_migrations_createKey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,17 @@ When validating a key, we will return this back to you, so you can clearly ident
trialEnds: "2023-06-16T17:16:37.161Z",
},
}),
encryptedMeta: z
.record(z.unknown())
.optional()
.openapi({
description:
"This is a place for dynamic encrypted meta data, anything that feels useful for you should go here",
example: {
billingTier: "PRO",
trialEnds: "2023-06-16T17:16:37.161Z",
},
}),
roles: z
.array(z.string().min(1).max(512))
.optional()
Expand Down Expand Up @@ -402,6 +413,7 @@ export const registerV1MigrationsCreateKeys = (app: App) =>
ownerId: key.ownerId ?? null,
identityId: null,
meta: key.meta ? JSON.stringify(key.meta) : null,
encryptedMeta: key.encryptedMeta ? JSON.stringify(key.encryptedMeta) : null,
workspaceId: authorizedWorkspaceId,
forWorkspaceId: null,
expires: key.expires ? new Date(key.expires) : null,
Expand Down
Loading