Skip to content

Commit

Permalink
Merge branch 'feat-secure-workspace-egress' into tre-bugfixes
Browse files Browse the repository at this point in the history
  • Loading branch information
Tim Nguyen committed Aug 9, 2021
2 parents 201e2cb + 146406f commit 3f0649c
Show file tree
Hide file tree
Showing 6 changed files with 232 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ const settingKeys = {
studyDataKmsKeyArn: 'studyDataKmsKeyArn',
studyDataKmsPolicyWorkspaceSid: 'studyDataKmsPolicyWorkspaceSid',
enableEgressStore: 'enableEgressStore',
egressStoreBucketName: 'egressStoreBucketName',
egressStoreKmsKeyArn: 'egressStoreKmsKeyArn',
egressStoreKmsPolicyWorkspaceSid: 'egressStoreKmsPolicyWorkspaceSid',
};
Expand Down Expand Up @@ -310,16 +309,18 @@ class EnvironmentResourceService extends Service {
// @private
async addToKmsKeyPolicy(requestContext, memberAccountId) {
await this.updateKMSPolicy(environmentStatement => addAccountToStatement(environmentStatement, memberAccountId));
await this.addToEgressKmsKeyPolicy(memberAccountId);
// Write audit event
await this.audit(requestContext, { action: 'add-to-KmsKey-policy', body: memberAccountId });
}

async addToEgressKmsKeyPolicy(memberAccountId) {
const enableEgressStore = this.settings.get(settingKeys.enableEgressStore);
if (enableEgressStore && enableEgressStore.toUpperCase() === 'TRUE') {
await this.updateEgressKMSPolicy(environmentStatement =>
addAccountToStatement(environmentStatement, memberAccountId),
);
}

// Write audit event
await this.audit(requestContext, { action: 'add-to-KmsKey-policy', body: memberAccountId });
}

// @private
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ jest.mock('../../../data-source-bucket-service');
jest.mock('../application-role-service');
jest.mock('../filesystem-role-service');

const AWSMock = require('aws-sdk-mock');
const Aws = require('@aws-ee/base-services/lib/aws/aws-service');
const Logger = require('@aws-ee/base-services/lib/logger/logger-service');
const LockService = require('@aws-ee/base-services/lib/lock/lock-service');
Expand Down Expand Up @@ -86,6 +87,7 @@ describe('EnvironmentResourceService', () => {
let lockService;
let fsRoleService;
let envService;
let aws;

beforeEach(async () => {
// Initialize services container and register dependencies
Expand All @@ -112,6 +114,12 @@ describe('EnvironmentResourceService', () => {
lockService = await container.find('lockService');
fsRoleService = await container.find('roles-only/filesystemRoleService');
envService = await container.find('environmentScService');
aws = await service.service('aws');
AWSMock.setSDKInstance(aws.sdk);
});

afterEach(() => {
AWSMock.restore();
});

it('provides env role policy', async () => {
Expand Down Expand Up @@ -209,4 +217,91 @@ describe('EnvironmentResourceService', () => {
expect(fsRoleService.deallocateRole).toHaveBeenCalledTimes(1);
expect(envService.updateStudyRoles).toHaveBeenCalledTimes(1);
});

it('allocates egress resources only', async () => {
service._settings = {
get: settingName => {
if (settingName === 'enableEgressStore') {
return 'true';
}
if (settingName === 'egressStoreKmsKeyArn') {
return 'test-egressStoreKmsKeyArn';
}
if (settingName === 'egressStoreKmsPolicyWorkspaceSid') {
return 'test-egressStoreKmsPolicyWorkspaceSid';
}
return undefined;
},
};
const requestContext = createAdminContext();
const study = createStudy();
const studies = [study];
const memberAccountId = '1234';

lockService.tryWriteLockAndRun = jest.fn((_id, fn) => {
return fn();
});

service.addToEgressKmsKeyPolicy = jest.fn();
service.audit = jest.fn();

await service.updateKMSPolicyForEgress(requestContext, { studies, memberAccountId });

expect(service.addToEgressKmsKeyPolicy).toHaveBeenCalledTimes(1);
expect(service.addToEgressKmsKeyPolicy).toHaveBeenCalledWith(memberAccountId);
});

it('should update Egress KMS Policy', async () => {
service._settings = {
get: settingName => {
if (settingName === 'enableEgressStore') {
return 'true';
}
if (settingName === 'egressStoreKmsKeyArn') {
return 'test-egressStoreKmsKeyArn';
}
if (settingName === 'egressStoreKmsPolicyWorkspaceSid') {
return 'test-egressStoreKmsPolicyWorkspaceSid';
}
return undefined;
},
};
AWSMock.mock('KMS', 'describeKey', (params, callback) => {
expect(params).toMatchObject({
KeyId: 'test-egressStoreKmsKeyArn',
});
callback(null, { KeyMetadata: { KeyId: 'kmsStudyKeyId' } });
});
AWSMock.mock('KMS', 'getKeyPolicy', (params, callback) => {
expect(params).toMatchObject({
KeyId: 'kmsStudyKeyId',
PolicyName: 'default',
});
callback(null, { Policy: '{}' });
});
const expectedKMSPolicy = {
Statement: [
{
Sid: 'test-egressStoreKmsPolicyWorkspaceSid',
Effect: 'Allow',
Principal: {
AWS: ['arn:aws:iam::accountId1:root'],
},
Action: ['kms:Encrypt', 'kms:Decrypt', 'kms:ReEncrypt*', 'kms:GenerateDataKey*', 'kms:DescribeKey'],
Resource: '*',
},
],
};
const putKeyPolicyMock = jest.fn((params, callback) => {
expect(params).toMatchObject({
KeyId: 'kmsStudyKeyId',
PolicyName: 'default',
Policy: JSON.stringify(expectedKMSPolicy),
});
callback(null, {});
});
AWSMock.mock('KMS', 'putKeyPolicy', putKeyPolicyMock);
await service.addToEgressKmsKeyPolicy('accountId1');
expect(putKeyPolicyMock).toHaveBeenCalledTimes(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,18 @@ const Service = require('@aws-ee/base-services-container/lib/service');
const { getSystemRequestContext } = require('@aws-ee/base-services/lib/helpers/system-context');
const { processInBatches } = require('@aws-ee/base-services/lib/helpers/utils');

const { addAccountToStatement } = require('../../../helpers/utils');

/**
* This service is responsible for allocating and de-allocating AWS resources for the environment so that
* the environment can access what it needs, such as studies. This service implements the roles only
* study access strategy.
*/
const settingKeys = {
enableEgressStore: 'enableEgressStore',
egressStoreKmsKeyArn: 'egressStoreKmsKeyArn',
egressStoreKmsPolicyWorkspaceSid: 'egressStoreKmsPolicyWorkspaceSid',
};
class EnvironmentResourceService extends Service {
constructor() {
super();
Expand Down Expand Up @@ -254,6 +261,71 @@ class EnvironmentResourceService extends Service {
return s3Mounts;
}

async updateKMSPolicyForEgress(requestContext, { studies: allStudies, memberAccountId }) {
// Legacy access strategy is only applicable for studies that have resources attributes
const kmsKeyAlias = this.settings.get(settingKeys.egressStoreKmsKeyArn);
const studies = _.filter(allStudies, study => !_.isEmpty(study.resources));
const lockService = await this.service('lockService');
const lockId = `kms-policy-access-${kmsKeyAlias}`;

if (_.isEmpty(studies)) {
await lockService.tryWriteLockAndRun({ id: lockId }, async () => {
await this.addToEgressKmsKeyPolicy(memberAccountId);
});
return;
}
await this.audit(requestContext, { action: 'add-to-egress-kms-policy', body: kmsKeyAlias });
}

async addToEgressKmsKeyPolicy(memberAccountId) {
const enableEgressStore = this.settings.get(settingKeys.enableEgressStore);
if (enableEgressStore && enableEgressStore.toUpperCase() === 'TRUE') {
await this.updateEgressKMSPolicy(environmentStatement =>
addAccountToStatement(environmentStatement, memberAccountId),
);
}
}

async updateEgressKMSPolicy(updateStatementFn) {
const kmsClient = await this.getKMS();
const kmsKeyAlias = this.settings.get(settingKeys.egressStoreKmsKeyArn);
const keyId = (await kmsClient.describeKey({ KeyId: kmsKeyAlias }).promise()).KeyMetadata.KeyId;

// Get existing policy
const kmsPolicy = JSON.parse(
(await kmsClient.getKeyPolicy({ KeyId: keyId, PolicyName: 'default' }).promise()).Policy,
);

// Get statement
const sid = this.settings.get(settingKeys.egressStoreKmsPolicyWorkspaceSid);
if (!kmsPolicy.Statement) {
kmsPolicy.Statement = [];
}
let environmentStatement = kmsPolicy.Statement.find(statement => statement.Sid === sid);
if (!environmentStatement) {
// Create new statement if it doesn't already exist
environmentStatement = {
Sid: sid,
Effect: 'Allow',
Principal: { AWS: [] },
Action: ['kms:Encrypt', 'kms:Decrypt', 'kms:ReEncrypt*', 'kms:GenerateDataKey*', 'kms:DescribeKey'],
Resource: '*', // Only refers to this key since it's a resource policy
};
}

// Update policy
environmentStatement = await updateStatementFn(environmentStatement);

// remove the old statement from KMS policy
kmsPolicy.Statement = kmsPolicy.Statement.filter(statement => statement.Sid !== sid);

// add the revised statement if it contains principals (otherwise leave it out)
if (environmentStatement.Principal.AWS.length > 0) {
kmsPolicy.Statement.push(environmentStatement);
}
await kmsClient.putKeyPolicy({ KeyId: keyId, PolicyName: 'default', Policy: JSON.stringify(kmsPolicy) }).promise();
}

async audit(requestContext, auditEvent) {
const auditWriterService = await this.service('auditWriterService');
// Calling "writeAndForget" instead of "write" to allow main call to continue without waiting for audit logging
Expand All @@ -262,6 +334,16 @@ class EnvironmentResourceService extends Service {
// return auditWriterService.write(requestContext, auditEvent);
return auditWriterService.writeAndForget(requestContext, auditEvent);
}

async getKMS() {
const aws = await this.getAWS();
return new aws.sdk.KMS();
}

async getAWS() {
const aws = await this.service('aws');
return aws;
}
}

module.exports = EnvironmentResourceService;
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,36 @@ describe('envProvisioningPlugin', () => {
});

describe('preProvisioning', () => {
it('should do nothing if there is no study to be linked', async () => {
it('should invoke kms update for egress store if there is no study to be linked', async () => {
// BUILD
environmentScService.mustFind = jest.fn().mockResolvedValueOnce({ id: 'env-id' });
environmentScService.getStudies = jest.fn().mockResolvedValueOnce([]);
environmentScService.getMemberAccount = jest.fn().mockResolvedValueOnce({ accountId: '1234567' });
pluginRegistryService.visitPlugins = jest.fn();

// OPERATE
await plugin.onEnvPreProvisioning({ requestContext, container, envId: 'some-env-id' });
// TEST
expect(environmentScService.getMemberAccount).not.toHaveBeenCalled();
expect(pluginRegistryService.visitPlugins).not.toHaveBeenCalled();
expect(environmentScService.getMemberAccount).toHaveBeenCalledWith(requestContext, { id: 'env-id' });
expect(pluginRegistryService.visitPlugins).toHaveBeenCalledWith(
'study-access-strategy',
'updateKMSPolicyForEgress',
{
payload: expect.objectContaining({
environmentScEntity: {
id: 'env-id',
},
memberAccountId: '1234567',
requestContext: {
principal: {
isAdmin: true,
status: 'active',
},
},
studies: [],
}),
},
);
});

it('visit plugins with correct parameters if there is study to be linked', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,23 @@ async function getStudies({ requestContext, container, envId }) {
*/
async function preProvisioning({ requestContext, container, envId }) {
const { environmentScEntity, studies } = await getStudies({ requestContext, container, envId });

if (_.isEmpty(studies)) return { requestContext, container, envId };

const environmentScService = await container.find('environmentScService');
const memberAccount = await environmentScService.getMemberAccount(requestContext, environmentScEntity);
const pluginRegistryService = await container.find('pluginRegistryService');

if (_.isEmpty(studies)) {
await pluginRegistryService.visitPlugins('study-access-strategy', 'updateKMSPolicyForEgress', {
payload: {
requestContext,
container,
environmentScEntity,
studies,
memberAccountId: memberAccount.accountId,
},
});
return { requestContext, container, envId };
}

await pluginRegistryService.visitPlugins('study-access-strategy', 'allocateEnvStudyResources', {
payload: {
requestContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,15 @@ async function allocateEnvStudyResources(payload) {
return payload;
}

async function updateKMSPolicyForEgress(payload) {
const { requestContext, container, environmentScEntity, studies, memberAccountId } = payload;

const resourceService = await container.find('roles-only/environmentResourceService');
await resourceService.updateKMSPolicyForEgress(requestContext, { environmentScEntity, studies, memberAccountId });

return payload;
}

/**
* A plugin method to implement any specific logic for the 'roles-only' access logic when a environment
* is terminated or failed provisioning. This method simply delegates to the roles-only/EnvironmentResourceService
Expand Down Expand Up @@ -147,6 +156,7 @@ const plugin = {
onStudyRegistration,
provideAccountCfnTemplate,
allocateEnvStudyResources,
updateKMSPolicyForEgress,
deallocateEnvStudyResources,
provideEnvRolePolicy,
provideStudyMount,
Expand Down

0 comments on commit 3f0649c

Please sign in to comment.