diff --git a/backend/src/db/migrations/20250528183744_remove-encrypted-salt-from-shared-secret.ts b/backend/src/db/migrations/20250528183744_remove-encrypted-salt-from-shared-secret.ts new file mode 100644 index 000000000..5ccd0f631 --- /dev/null +++ b/backend/src/db/migrations/20250528183744_remove-encrypted-salt-from-shared-secret.ts @@ -0,0 +1,27 @@ +import { Knex } from "knex"; + +import { TableName } from "../schemas"; + +export async function up(knex: Knex): Promise { + if (await knex.schema.hasTable(TableName.SecretSharing)) { + const hasEncryptedSalt = await knex.schema.hasColumn(TableName.SecretSharing, "encryptedSalt"); + + if (hasEncryptedSalt) { + await knex.schema.alterTable(TableName.SecretSharing, (t) => { + t.dropColumn("encryptedSalt"); + }); + } + } +} + +export async function down(knex: Knex): Promise { + if (await knex.schema.hasTable(TableName.SecretSharing)) { + const hasEncryptedSalt = await knex.schema.hasColumn(TableName.SecretSharing, "encryptedSalt"); + + if (!hasEncryptedSalt) { + await knex.schema.alterTable(TableName.SecretSharing, (t) => { + t.binary("encryptedSalt").nullable(); + }); + } + } +} diff --git a/backend/src/db/schemas/secret-sharing.ts b/backend/src/db/schemas/secret-sharing.ts index 7de34708c..7a7bf17bb 100644 --- a/backend/src/db/schemas/secret-sharing.ts +++ b/backend/src/db/schemas/secret-sharing.ts @@ -28,7 +28,6 @@ export const SecretSharingSchema = z.object({ encryptedSecret: zodBuffer.nullable().optional(), identifier: z.string().nullable().optional(), type: z.string().default("share"), - encryptedSalt: zodBuffer.nullable().optional(), authorizedEmails: z.unknown().nullable().optional() }); diff --git a/backend/src/server/routes/v1/secret-sharing-router.ts b/backend/src/server/routes/v1/secret-sharing-router.ts index e712ee138..6118d2753 100644 --- a/backend/src/server/routes/v1/secret-sharing-router.ts +++ b/backend/src/server/routes/v1/secret-sharing-router.ts @@ -62,9 +62,7 @@ export const registerSecretSharingRouter = async (server: FastifyZodProvider) => }), body: z.object({ hashedHex: z.string().min(1).optional(), - password: z.string().optional(), - email: z.string().optional(), - hash: z.string().optional() + password: z.string().optional() }), response: { 200: z.object({ @@ -91,8 +89,7 @@ export const registerSecretSharingRouter = async (server: FastifyZodProvider) => hashedHex: req.body.hashedHex, password: req.body.password, orgId: req.permission?.orgId, - email: req.body.email, - hash: req.body.hash + actorId: req.permission?.id }); if (sharedSecret.secret?.orgId) { diff --git a/backend/src/services/secret-sharing/secret-sharing-service.ts b/backend/src/services/secret-sharing/secret-sharing-service.ts index 702078364..24739b01b 100644 --- a/backend/src/services/secret-sharing/secret-sharing-service.ts +++ b/backend/src/services/secret-sharing/secret-sharing-service.ts @@ -115,8 +115,6 @@ export const secretSharingServiceFactory = ({ const encryptWithRoot = kmsService.encryptWithRootKey(); - let salt: string | undefined; - let encryptedSalt: Buffer | undefined; const orgEmails = []; if (emails && emails.length > 0) { @@ -133,10 +131,6 @@ export const secretSharingServiceFactory = ({ }); } } - - // Generate salt for signing email hashes (if emails are provided) - salt = crypto.randomBytes(32).toString("hex"); - encryptedSalt = encryptWithRoot(Buffer.from(salt)); } const encryptedSecret = encryptWithRoot(Buffer.from(secretValue)); @@ -158,14 +152,13 @@ export const secretSharingServiceFactory = ({ userId: actorId, orgId, accessType, - authorizedEmails: emails && emails.length > 0 ? JSON.stringify(emails) : undefined, - encryptedSalt + authorizedEmails: emails && emails.length > 0 ? JSON.stringify(emails) : undefined }); const idToReturn = `${Buffer.from(newSharedSecret.identifier!, "hex").toString("base64url")}`; // Loop through recipients and send out emails with unique access links - if (emails && salt) { + if (emails) { const user = await userDAL.findById(actorId); if (!user) { @@ -174,9 +167,6 @@ export const secretSharingServiceFactory = ({ for await (const email of emails) { try { - const hmac = crypto.createHmac("sha256", salt).update(email); - const hash = hmac.digest("hex"); - // Only show the username to emails which are part of the organization const respondentUsername = orgEmails.includes(email) ? user.username : undefined; @@ -186,7 +176,7 @@ export const secretSharingServiceFactory = ({ substitutions: { name, respondentUsername, - secretRequestUrl: `${appCfg.SITE_URL}/shared/secret/${idToReturn}?email=${encodeURIComponent(email)}&hash=${hash}` + secretRequestUrl: `${appCfg.SITE_URL}/shared/secret/${idToReturn}` }, template: SmtpTemplates.SecretRequestCompleted }); @@ -474,9 +464,8 @@ export const secretSharingServiceFactory = ({ sharedSecretId, hashedHex, orgId, - password, - email, - hash + actorId, + password }: TGetActiveSharedSecretByIdDTO) => { const sharedSecret = isUuidV4(sharedSecretId) ? await secretSharingDAL.findOne({ @@ -506,6 +495,17 @@ export const secretSharingServiceFactory = ({ throw new ForbiddenRequestError(); } + // If the secret was shared with specific emails, verify that the current user's session email is authorized + if (sharedSecret.authorizedEmails && (sharedSecret.authorizedEmails as string[]).length > 0) { + if (!actorId) throw new UnauthorizedError(); + + const user = await userDAL.findById(actorId); + if (!user || !user.email) throw new UnauthorizedError(); + + if (!(sharedSecret.authorizedEmails as string[]).includes(user.email)) + throw new UnauthorizedError({ message: "Email not authorized to view secret" }); + } + // all secrets pass through here, meaning we check if its expired first and then check if it needs verification // or can be safely sent to the client. if (expiresAt !== null && expiresAt < new Date()) { @@ -524,31 +524,6 @@ export const secretSharingServiceFactory = ({ }); } - const decryptWithRoot = kmsService.decryptWithRootKey(); - - if (sharedSecret.authorizedEmails && sharedSecret.encryptedSalt) { - // Verify both params were passed - if (!email || !hash) { - throw new BadRequestError({ - message: "This secret is email protected. Parameters must include email and hash." - }); - - // Verify that email is authorized to view shared secret - } else if (!(sharedSecret.authorizedEmails as string[]).includes(email)) { - throw new UnauthorizedError({ message: "Email not authorized to view secret" }); - - // Verify that hash matches - } else { - const salt = decryptWithRoot(sharedSecret.encryptedSalt).toString(); - const hmac = crypto.createHmac("sha256", salt).update(email); - const rebuiltHash = hmac.digest("hex"); - - if (rebuiltHash !== hash) { - throw new UnauthorizedError({ message: "Email not authorized to view secret" }); - } - } - } - // Password checks const isPasswordProtected = Boolean(sharedSecret.password); const hasProvidedPassword = Boolean(password); @@ -561,6 +536,8 @@ export const secretSharingServiceFactory = ({ } } + const decryptWithRoot = kmsService.decryptWithRootKey(); + // If encryptedSecret is set, we know that this secret has been encrypted using KMS, and we can therefore do server-side decryption. let decryptedSecretValue: Buffer | undefined; if (sharedSecret.encryptedSecret) { diff --git a/backend/src/services/secret-sharing/secret-sharing-types.ts b/backend/src/services/secret-sharing/secret-sharing-types.ts index 049dbb913..3d968edf3 100644 --- a/backend/src/services/secret-sharing/secret-sharing-types.ts +++ b/backend/src/services/secret-sharing/secret-sharing-types.ts @@ -37,11 +37,8 @@ export type TGetActiveSharedSecretByIdDTO = { sharedSecretId: string; hashedHex?: string; orgId?: string; + actorId?: string; password?: string; - - // For secrets shared with specific emails - email?: string; - hash?: string; }; export type TValidateActiveSharedSecretDTO = TGetActiveSharedSecretByIdDTO & { diff --git a/frontend/src/hooks/api/secretSharing/queries.ts b/frontend/src/hooks/api/secretSharing/queries.ts index cfd505ff0..13867aa05 100644 --- a/frontend/src/hooks/api/secretSharing/queries.ts +++ b/frontend/src/hooks/api/secretSharing/queries.ts @@ -11,13 +11,10 @@ export const secretSharingKeys = { allSecretRequests: () => ["secretRequests"] as const, specificSecretRequests: ({ offset, limit }: { offset: number; limit: number }) => [...secretSharingKeys.allSecretRequests(), { offset, limit }] as const, - getSecretById: (arg: { - id: string; - hashedHex: string | null; - password?: string; - email?: string; - hash?: string; - }) => ["shared-secret", arg], + getSecretById: (arg: { id: string; hashedHex: string | null; password?: string }) => [ + "shared-secret", + arg + ], getSecretRequestById: (arg: { id: string }) => ["secret-request", arg] as const }; @@ -73,34 +70,24 @@ export const useGetSecretRequests = ({ export const useGetActiveSharedSecretById = ({ sharedSecretId, hashedHex, - password, - email, - hash + password }: { sharedSecretId: string; hashedHex: string | null; password?: string; - - // For secrets shared to specific emails (optional) - email?: string; - hash?: string; }) => { return useQuery({ queryKey: secretSharingKeys.getSecretById({ id: sharedSecretId, hashedHex, - password, - email, - hash + password }), queryFn: async () => { const { data } = await apiRequest.post( `/api/v1/secret-sharing/shared/public/${sharedSecretId}`, { ...(hashedHex && { hashedHex }), - password, - email, - hash + password } ); diff --git a/frontend/src/pages/public/ViewSharedSecretByIDPage/ViewSharedSecretByIDPage.tsx b/frontend/src/pages/public/ViewSharedSecretByIDPage/ViewSharedSecretByIDPage.tsx index 389aee68f..406dee4c3 100644 --- a/frontend/src/pages/public/ViewSharedSecretByIDPage/ViewSharedSecretByIDPage.tsx +++ b/frontend/src/pages/public/ViewSharedSecretByIDPage/ViewSharedSecretByIDPage.tsx @@ -38,14 +38,6 @@ export const ViewSharedSecretByIDPage = () => { from: ROUTE_PATHS.Public.ViewSharedSecretByIDPage.id, select: (el) => el.key }); - const email = useSearch({ - from: ROUTE_PATHS.Public.ViewSharedSecretByIDPage.id, - select: (el) => el.email - }); - const hash = useSearch({ - from: ROUTE_PATHS.Public.ViewSharedSecretByIDPage.id, - select: (el) => el.hash - }); const [password, setPassword] = useState(); const { hashedHex, key } = extractDetailsFromUrl(urlEncodedKey); @@ -57,9 +49,7 @@ export const ViewSharedSecretByIDPage = () => { } = useGetActiveSharedSecretById({ sharedSecretId: id, hashedHex, - password, - email, - hash + password }); const navigate = useNavigate(); @@ -94,6 +84,8 @@ export const ViewSharedSecretByIDPage = () => { navigate({ to: "/login" }); + + return; } if (error) { diff --git a/frontend/src/pages/public/ViewSharedSecretByIDPage/route.tsx b/frontend/src/pages/public/ViewSharedSecretByIDPage/route.tsx index 7cbcb59a2..7b98dded3 100644 --- a/frontend/src/pages/public/ViewSharedSecretByIDPage/route.tsx +++ b/frontend/src/pages/public/ViewSharedSecretByIDPage/route.tsx @@ -7,9 +7,7 @@ import { authKeys, fetchAuthToken } from "@app/hooks/api/auth/queries"; import { ViewSharedSecretByIDPage } from "./ViewSharedSecretByIDPage"; const SharedSecretByIDPageQuerySchema = z.object({ - key: z.string().catch(""), - email: z.string().optional(), - hash: z.string().optional() + key: z.string().catch("") }); export const Route = createFileRoute("/shared/secret/$secretId")({