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

test(encryption): add integration test for db encryption #2133

Merged
merged 3 commits into from
May 13, 2024
Merged
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
6 changes: 3 additions & 3 deletions packages/shared/lib/services/environment.service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as uuid from 'uuid';
import db from '../db/database.js';
import encryptionManager, { ENCRYPTION_KEY, pbkdf2 } from '../utils/encryption.manager.js';
import encryptionManager, { pbkdf2 } from '../utils/encryption.manager.js';
import type { Environment } from '../models/Environment.js';
import type { EnvironmentVariable } from '../models/EnvironmentVariable.js';
import type { Account } from '../models/Admin.js';
Expand Down Expand Up @@ -493,11 +493,11 @@ class EnvironmentService {
}

export async function hashSecretKey(key: string) {
if (!ENCRYPTION_KEY) {
if (!encryptionManager.getKey()) {
return key;
}

return (await pbkdf2(key, ENCRYPTION_KEY, 310000, 32, 'sha256')).toString('base64');
return (await pbkdf2(key, encryptionManager.getKey(), 310000, 32, 'sha256')).toString('base64');
}

export default new EnvironmentService();
82 changes: 82 additions & 0 deletions packages/shared/lib/utils/encryption.manager.integration.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import { beforeAll, describe, expect, it } from 'vitest';
import encryptionManager, { EncryptionManager } from './encryption.manager';
import db, { multipleMigrations } from '../db/database.js';
import { seedAccountEnvAndUser } from '../db/seeders/index.js';
import environmentService from '../services/environment.service';

describe('encryption', () => {
beforeAll(async () => {
await multipleMigrations();
});

describe('status', () => {
it('should report disabled if no key and no previous key', async () => {
const res = await new EncryptionManager('').encryptionStatus();
expect(res).toBe('disabled');
});

it('should report not_started if key and no previous key', async () => {
const res = await new EncryptionManager('aHcTnJX5yaDJHF/EJLc6IMFSo2+aiz1hPsTkpsufxa0=').encryptionStatus();
expect(res).toBe('not_started');
});

it('should report require_decryption if no key and one previous key', async () => {
const res = await new EncryptionManager('').encryptionStatus({ encryption_complete: true, encryption_key_hash: 'erer' });
expect(res).toBe('require_decryption');
});

it('should report require_rotation if different keys', async () => {
const res = await new EncryptionManager('aHcTnJX5yaDJHF/EJLc6IMFSo2+aiz1hPsTkpsufxa0=').encryptionStatus({
encryption_complete: true,
encryption_key_hash: 'erer'
});
expect(res).toBe('require_rotation');
});

it('should report incomplete if same key but not finished', async () => {
const res = await new EncryptionManager('aHcTnJX5yaDJHF/EJLc6IMFSo2+aiz1hPsTkpsufxa0=').encryptionStatus({
encryption_complete: false,
encryption_key_hash: 'sM+EkzNi7o4Crw3cVfg01jBbmSEAfDdmTzYWoxbryvk='
});
expect(res).toBe('incomplete');
});

it('should report done if same key and complete', async () => {
const res = await new EncryptionManager('aHcTnJX5yaDJHF/EJLc6IMFSo2+aiz1hPsTkpsufxa0=').encryptionStatus({
encryption_complete: true,
encryption_key_hash: 'sM+EkzNi7o4Crw3cVfg01jBbmSEAfDdmTzYWoxbryvk='
});
expect(res).toBe('done');
});
});

describe('encryption', () => {
it('should encrypt environment', async () => {
// we create a different schema because we have only one DB for all tests
db.knex.client.config.searchPath = 'nango_encrypt';
db.schema = () => 'nango_encrypt';

await multipleMigrations();

// Disable encryption manually since it's set by default
// @ts-expect-error Modify the key on the fly
encryptionManager.key = '';
await db.knex.from(`_nango_db_config`).del();

const { env } = await seedAccountEnvAndUser();
expect(env.secret_key_iv).toBeNull();
expect(env.secret_key_hashed).toBe(env.secret_key);

// Re-enable encryption
// @ts-expect-error Modify the key on the fly
encryptionManager.key = 'aHcTnJX5yaDJHF/EJLc6IMFSo2+aiz1hPsTkpsufxa0=';
await encryptionManager.encryptDatabaseIfNeeded();

const envAfterEnc = (await environmentService.getRawById(env.id))!;
expect(envAfterEnc.secret_key_iv).not.toBeNull();
expect(envAfterEnc.secret_key_tag).not.toBeNull();
expect(envAfterEnc.secret_key).not.toBe(env.secret_key);
expect(envAfterEnc.secret_key_hashed).not.toBe(env.secret_key);
});
});
});
66 changes: 46 additions & 20 deletions packages/shared/lib/utils/encryption.manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const logger = getLogger('Encryption.Manager');
export const pbkdf2 = utils.promisify(crypto.pbkdf2);
export const ENCRYPTION_KEY = process.env['NANGO_ENCRYPTION_KEY'] || '';

class EncryptionManager extends Encryption {
export class EncryptionManager extends Encryption {
private keySalt = 'X89FHEGqR3yNK0+v7rPWxQ==';

public shouldEncrypt(): boolean {
Expand Down Expand Up @@ -201,31 +201,57 @@ class EncryptionManager extends Encryption {
return keyBuffer.toString(this.encoding);
}

public async encryptDatabaseIfNeeded() {
const dbConfig: DBConfig | null = await db.knex.first().from<DBConfig>('_nango_db_config');
const previousEncryptionKeyHash = dbConfig?.encryption_key_hash;
const encryptionKeyHash = this.key ? await this.hashEncryptionKey(this.key, this.keySalt) : null;
/**
* Determine the Database encryption status
*/
public async encryptionStatus(
dbConfig?: DBConfig
): Promise<'disabled' | 'not_started' | 'require_rotation' | 'require_decryption' | 'done' | 'incomplete'> {
if (!dbConfig) {
if (!this.key) {
return 'disabled';
} else {
return 'not_started';
}
} else if (!this.key) {
return 'require_decryption';
}

const isEncryptionKeyNew = dbConfig == null && this.key;
const isEncryptionIncomplete = dbConfig != null && previousEncryptionKeyHash === encryptionKeyHash && !dbConfig.encryption_complete;
const previousEncryptionKeyHash = dbConfig.encryption_key_hash;
const encryptionKeyHash = await this.hashEncryptionKey(this.key, this.keySalt);
if (previousEncryptionKeyHash !== encryptionKeyHash) {
return 'require_rotation';
}
return dbConfig.encryption_complete ? 'done' : 'incomplete';
}

if (isEncryptionKeyNew || isEncryptionIncomplete) {
if (isEncryptionKeyNew) {
logger.info('🔐 Encryption key has been set. Encrypting database...');
await this.saveDbConfig({ encryption_key_hash: encryptionKeyHash, encryption_complete: false });
} else if (isEncryptionIncomplete) {
logger.info('🔐 Previously started database encryption is incomplete. Continuing encryption of database...');
}
public async encryptDatabaseIfNeeded() {
const dbConfig = await db.knex.select<DBConfig>('*').from<DBConfig>('_nango_db_config').first();
const status = await this.encryptionStatus(dbConfig);
const encryptionKeyHash = this.key ? await this.hashEncryptionKey(this.key, this.keySalt) : null;

await this.encryptDatabase();
await this.saveDbConfig({ encryption_key_hash: encryptionKeyHash, encryption_complete: true });
if (status === 'disabled') {
return;
}

const isEncryptionKeyChanged = dbConfig?.encryption_key_hash != null && previousEncryptionKeyHash !== encryptionKeyHash;
if (isEncryptionKeyChanged) {
throw new Error('You cannot edit or remove the encryption key once it has been set.');
if (status === 'done') {
return;
}
if (status === 'require_rotation') {
throw new Error('Rotation of NANGO_ENCRYPTION_KEY is not supported.');
}
if (status === 'require_decryption') {
throw new Error('A previously set NANGO_ENCRYPTION_KEY has been removed from your environment variables.');
}
if (status === 'not_started') {
logger.info('🔐 Encryption key has been set. Encrypting database...');
await this.saveDbConfig({ encryption_key_hash: encryptionKeyHash, encryption_complete: false });
}
if (status === 'incomplete') {
logger.info('🔐 Previously started database encryption is incomplete. Continuing encryption of database...');
}

await this.encryptDatabase();
await this.saveDbConfig({ encryption_key_hash: encryptionKeyHash, encryption_complete: true });
Copy link
Collaborator

Choose a reason for hiding this comment

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

unrelated to your changes but all the queries in encryptDatabase and saveDbConfig should probably be inside a transaction.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At first glance yes, I'm just wondering about the size of the transaction if you have a large db. I think the script assume it can fail at any time and try everything from the start every time which is nice imo

}

private async encryptDatabase() {
Expand Down
4 changes: 4 additions & 0 deletions packages/utils/lib/encryption.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ export class Encryption {
}
}

getKey() {
return this.key;
}

public encrypt(str: string): [string, string, string] {
const iv = crypto.randomBytes(12);
const cipher = crypto.createCipheriv(this.algo, Buffer.from(this.key, this.encoding), iv);
Expand Down
Loading