fix: security advisory (#297)

* fix(encryption): change encryption method to AES-GCM

* fix(sec): remove dummy postgres addresses from compose files

* fix(sec): add .env to gitignore

* fix(sec): put docker-compose specific env files in .env.docker

* fix(sec): add encryption tag to db

Author: arielweinberger

.env

@@ -43,7 +43,9 @@ Thumbs.db
 **/@generated
 .nx-container
 
+.env
 .env.local
+.env.docker
 schema.graphql
 
 # temp

.gitignore

@@ -0,0 +1,8 @@
+/*
+  Warnings:
+
+  - Added the required column `encryptionTag` to the `ProviderApiKey` table without a default value. This is not possible if the table is not empty.
+
+*/
+-- AlterTable
+ALTER TABLE "ProviderApiKey" ADD COLUMN     "encryptionTag" TEXT NOT NULL;

apps/server/prisma/migrations/20240428124644_add_tag_to_provider_api_key_encryption/migration.sql

@@ -174,6 +174,8 @@ model ProviderApiKey {
   encryptedData    String
   /// @HideField({ input: true, output: true })
   encryptedDataKey String
+  /// @HideField({ input: true, output: true })
+  encryptionTag    String
   censoredValue    String
   provider         String
   organizationId   String

apps/server/prisma/schema.prisma

@@ -2,7 +2,6 @@ import { Injectable } from "@nestjs/common";
 import { PrismaService } from "../prisma.service";
 import { EncryptionService } from "../encryption/encryption.service";
 import { ProviderApiKey } from "@prisma/client";
-import { StringFilter } from "../../@generated/prisma/string-filter.input";
 
 @Injectable()
 export class ProviderApiKeysService {
@@ -21,7 +20,8 @@ export class ProviderApiKeysService {
   async decryptProviderApiKey(key: ProviderApiKey): Promise<string> {
     const decrypted = await this.encryptionService.decrypt(
       key.encryptedData,
-      key.encryptedDataKey
+      key.encryptedDataKey,
+      key.encryptionTag
     );
     return decrypted;
   }
@@ -42,7 +42,7 @@ export class ProviderApiKeysService {
       where: { provider, organizationId },
     });
 
-    const { encryptedData, encryptedDataKey } =
+    const { encryptedData, encryptedDataKey, encryptionTag } =
       await this.encryptionService.encrypt(value);
 
     const censoredValue = this.censorApiKey(value);
@@ -55,6 +55,7 @@ export class ProviderApiKeysService {
         data: {
           encryptedData,
           encryptedDataKey,
+          encryptionTag,
           censoredValue,
         },
       });
@@ -67,6 +68,7 @@ export class ProviderApiKeysService {
         provider,
         encryptedData,
         encryptedDataKey,
+        encryptionTag,
         censoredValue,
         organizationId,
       },

apps/server/src/app/credentials/provider-api-keys.service.ts

@@ -48,35 +48,40 @@ export class EncryptionService {
   async encrypt(data: string): Promise<{
     encryptedData: string;
     encryptedDataKey: string;
+    encryptionTag: string;
   }> {
     this.logger.info("Encrypting data");
 
     const dataKey = await this.generateDataKey();
-    const iv = crypto.randomBytes(16);
-    const cipher = crypto.createCipheriv("aes-256-cbc", dataKey.plaintext, iv);
-    const encrypted = Buffer.concat([
-      cipher.update(data, "utf8"),
-      cipher.final(),
-    ]);
-    const encryptedData = Buffer.concat([iv, encrypted]);
+    const iv = crypto.randomBytes(12); // 12 bytes is recommended for GCM
+    const cipher = crypto.createCipheriv("aes-256-gcm", dataKey.plaintext, iv);
+    let encrypted = cipher.update(data, "utf8");
+    encrypted = Buffer.concat([encrypted, cipher.final()]);
 
     return {
-      encryptedData: encryptedData.toString("hex"),
+      encryptedData: Buffer.concat([iv, encrypted]).toString("hex"),
       encryptedDataKey: Buffer.from(dataKey.ciphertext).toString("base64"),
+      encryptionTag: cipher.getAuthTag().toString("hex"), // Store the tag for verification during decryption
     };
   }
 
-  async decrypt(encryptedData: string, dataKeyBase64: string): Promise<string> {
+  async decrypt(
+    encryptedData: string,
+    dataKeyBase64: string,
+    tagHex: string
+  ): Promise<string> {
     this.logger.info("Decrypting data");
 
     const encryptedDataBuffer = Buffer.from(encryptedData, "hex");
     const { Plaintext: dataKey } = await this.kms.decrypt({
       CiphertextBlob: Buffer.from(dataKeyBase64, "base64"),
     });
 
-    const iv = encryptedDataBuffer.slice(0, 16);
-    const data = encryptedDataBuffer.slice(16);
-    const decipher = crypto.createDecipheriv("aes-256-cbc", dataKey, iv);
+    const iv = encryptedDataBuffer.slice(0, 12);
+    const data = encryptedDataBuffer.slice(12);
+    const decipher = crypto.createDecipheriv("aes-256-gcm", dataKey, iv);
+    decipher.setAuthTag(Buffer.from(tagHex, "hex")); // Set the authentication tag for verification
+
     return decipher.update(data) + decipher.final("utf8");
   }
 }

apps/server/src/app/encryption/encryption.service.ts

@@ -24,8 +24,8 @@ services:
       dockerfile: ./apps/server/Dockerfile
     entrypoint: /bin/sh
     command: -c "npx prisma migrate deploy"
-    environment:
-      - DATABASE_URL=postgres://postgres:postgres@postgres:5432/pezzo
+    env_file:
+      - ./.env.docker
     depends_on:
       postgres:
         condition: service_healthy
@@ -72,8 +72,10 @@ services:
     depends_on:
       postgres:
         condition: service_healthy
+    env_file:
+      - ./.env.docker
     environment:
-      POSTGRES_CONNECTION_URI: postgres://postgres:postgres@postgres:5432/supertokens
+      POSTGRES_CONNECTION_URI: "${SUPERTOKENS_DATABASE_URL}"
     healthcheck:
       test: >
         bash -c 'exec 3<>/dev/tcp/127.0.0.1/3567 && echo -e "GET /hello HTTP/1.1\r\nhost: 127.0.0.1:3567\r\nConnection: close\r\n\r\n" >&3 && cat <&3 | grep "Hello"'

docker-compose.infra.yaml

@@ -11,10 +11,10 @@ services:
     env_file:
       - ./.env
       - ./.env.local
+      - ./.env.docker
       - ./apps/server/.env
       - ./apps/server/.env.local
     environment:
-      - DATABASE_URL=postgres://postgres:postgres@postgres:5432/pezzo
       - SUPERTOKENS_CONNECTION_URI=http://supertokens:3567
       - CLICKHOUSE_HOST=clickhouse
       - REDIS_URL=redis://redis-stack-server:6379