Skip to content

Commit 96b8f35

Browse files
naveenkoppulanaveenkoppula
authored
fix(core): Fixing SSM-Document-Share (#666)
* Fixing SSM Document share and other issues - Fixing number of accountIds passing to ssm modify-document-permission - Fixing number of accounts passing to GuardDuty createMembers, updateMembers and deleteMembers - Adding security-hub-excl-regions and not enabling security hub in those regions * Enable SecurityHub based on sucurity-hub flag * Fixing pageSize for guardDuty and SSM Document share
1 parent fe4b5df commit 96b8f35

File tree

7 files changed

+164
-74
lines changed

7 files changed

+164
-74
lines changed

‎src/deployments/cdk/src/deployments/security-hub/step-1.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,20 @@ export interface SecurityHubStep1Props {
1212
outputs: StackOutput[];
1313
}
1414

15+
/**
16+
*
17+
* @param props
18+
* @returns
19+
*
20+
* Enables SecurityHub in Audit Account also send invites
21+
* to sub accounts in all regions excluding security-hub-excl-regions
22+
*/
1523
export async function step1(props: SecurityHubStep1Props) {
1624
const { accounts, accountStacks, config, outputs } = props;
1725
const globalOptions = config['global-options'];
26+
if (!globalOptions['central-security-services']['security-hub']) {
27+
return;
28+
}
1829
const regions = globalOptions['supported-regions'];
1930
const securityAccountKey = config.getMandatoryAccountKey('central-security');
2031
const securityMasterAccount = accounts.find(a => a.key === securityAccountKey);
@@ -36,7 +47,12 @@ export async function step1(props: SecurityHubStep1Props) {
3647
return;
3748
}
3849

50+
const securityHubExclRegions = globalOptions['central-security-services']['security-hub-excl-regions'] || [];
3951
for (const region of regions) {
52+
if (securityHubExclRegions.includes(region)) {
53+
console.info(`Security Hub is disabled in region "${region}" based on global-options/security-hub-excl-regions'`);
54+
continue;
55+
}
4056
const securityMasterAccountStack = accountStacks.tryGetOrCreateAccountStack(securityAccountKey, region);
4157
if (!securityMasterAccountStack) {
4258
console.warn(`Cannot find security stack in region ${region}`);

‎src/deployments/cdk/src/deployments/security-hub/step-2.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,17 @@ export interface SecurityHubStep2Props {
1111
accountStacks: AccountStacks;
1212
outputs: StackOutput[];
1313
}
14-
14+
/**
15+
*
16+
* @param props
17+
* Accept invites in sub accounts in all regions excluding security-hub-excl-regions
18+
*/
1519
export async function step2(props: SecurityHubStep2Props) {
1620
const { accounts, accountStacks, config, outputs } = props;
1721
const globalOptions = config['global-options'];
22+
if (!globalOptions['central-security-services']['security-hub']) {
23+
return;
24+
}
1825
const regions = globalOptions['supported-regions'];
1926
const securityAccountKey = config.getMandatoryAccountKey('central-security');
2027
const securityMasterAccount = accounts.find(a => a.key === securityAccountKey);
@@ -33,7 +40,14 @@ export async function step2(props: SecurityHubStep2Props) {
3340
continue;
3441
}
3542

43+
const securityHubExclRegions = globalOptions['central-security-services']['security-hub-excl-regions'] || [];
3644
for (const region of regions) {
45+
if (securityHubExclRegions.includes(region)) {
46+
console.info(
47+
`Security Hub is disabled in region "${region}" based on global-options/security-hub-excl-regions'`,
48+
);
49+
continue;
50+
}
3751
const memberAccountStack = accountStacks.tryGetOrCreateAccountStack(account.key, region);
3852
if (!memberAccountStack) {
3953
console.warn(`Cannot find account stack ${account.key} in region ${region}`);

‎src/deployments/cdk/src/deployments/security-hub/step-3.ts‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,19 @@ export interface SecurityHubStep3Props {
1111
accountStacks: AccountStacks;
1212
outputs: StackOutput[];
1313
}
14-
14+
/**
15+
*
16+
* @param props
17+
* Disable SecurityHub Controls in all accounts all regions excluding security-hub-excl-regions
18+
* Disable based on "global-options/security-hub-frameworks/standards/[*]/controls-to-disable"
19+
*/
1520
export async function step3(props: SecurityHubStep3Props) {
1621
const { accounts, accountStacks, config, outputs } = props;
1722
const globalOptions = config['global-options'];
23+
if (!globalOptions['central-security-services']['security-hub']) {
24+
return;
25+
}
1826
const regions = globalOptions['supported-regions'];
19-
2027
for (const account of accounts) {
2128
const securityHubRoleOutput = IamRoleOutputFinder.tryFindOneByName({
2229
outputs,
@@ -27,7 +34,14 @@ export async function step3(props: SecurityHubStep3Props) {
2734
continue;
2835
}
2936

37+
const securityHubExclRegions = globalOptions['central-security-services']['security-hub-excl-regions'] || [];
3038
for (const region of regions) {
39+
if (securityHubExclRegions.includes(region)) {
40+
console.info(
41+
`Security Hub is disabled in region "${region}" based on global-options/security-hub-excl-regions'`,
42+
);
43+
continue;
44+
}
3145
const accountStack = accountStacks.tryGetOrCreateAccountStack(account.key, region);
3246
if (!accountStack) {
3347
console.warn(`Cannot find account stack ${account.key} in region ${region}`);

‎src/lib/common-config/src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -679,6 +679,7 @@ export const CentralServicesConfigType = t.interface({
679679
account: NonEmptyString,
680680
region: NonEmptyString,
681681
'security-hub': fromNullable(t.boolean, false),
682+
'security-hub-excl-regions': optional(t.array(t.string)),
682683
guardduty: fromNullable(t.boolean, false),
683684
'guardduty-excl-regions': optional(t.array(t.string)),
684685
'guardduty-s3': fromNullable(t.boolean, false),

‎src/lib/custom-resources/cdk-cfn-utils/cdk/index.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,7 @@ export const isThrottlingError = (e: any) =>
2727
export async function delay(ms: number) {
2828
return new Promise((resolve, reject) => setTimeout(resolve, ms));
2929
}
30+
31+
export function paginate<T>(input: T[], pageNumber: number, pageSize: number): T[] {
32+
return input.slice((pageNumber - 1) * pageSize, pageNumber * pageSize);
33+
}

‎src/lib/custom-resources/cdk-guardduty-admin-setup/runtime/src/index.ts‎

Lines changed: 52 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,14 @@ import {
77
CloudFormationCustomResourceDeleteEvent,
88
} from 'aws-lambda';
99
import { errorHandler } from '@aws-accelerator/custom-resource-runtime-cfn-response';
10-
import { throttlingBackOff } from '@aws-accelerator/custom-resource-cfn-utils';
10+
import { throttlingBackOff, paginate } from '@aws-accelerator/custom-resource-cfn-utils';
1111

1212
const physicalResourceId = 'GaurdDutyDeligatedAdminAccountSetup';
1313
const guardduty = new AWS.GuardDuty();
1414

15+
// Guardduty CreateMembers, UpdateMembers and DeleteMembers apis only supports max 50 accounts per request
16+
const pageSize = 50;
17+
1518
export interface AccountDetail {
1619
AccountId: string;
1720
Email: string;
@@ -95,15 +98,20 @@ async function getDetectorId(): Promise<string | undefined> {
9598
// Step 2 of https://docs.aws.amazon.com/guardduty/latest/ug/guardduty_organizations.html
9699
async function createMembers(memberAccounts: AccountDetail[], detectorId: string) {
97100
try {
98-
console.log(`Calling api "guardduty.createMembers()", ${memberAccounts}, ${detectorId}`);
99-
await throttlingBackOff(() =>
100-
guardduty
101-
.createMembers({
102-
AccountDetails: memberAccounts,
103-
DetectorId: detectorId,
104-
})
105-
.promise(),
106-
);
101+
let pageNumber = 1;
102+
let currentAccounts: AccountDetail[] = paginate(memberAccounts, pageNumber, pageSize);
103+
while (currentAccounts.length > 0) {
104+
console.log(`Calling api "guardduty.createMembers()", ${currentAccounts}, ${detectorId}`);
105+
await throttlingBackOff(() =>
106+
guardduty
107+
.createMembers({
108+
AccountDetails: currentAccounts,
109+
DetectorId: detectorId,
110+
})
111+
.promise(),
112+
);
113+
currentAccounts = paginate(memberAccounts, ++pageNumber, pageSize);
114+
}
107115
} catch (error) {
108116
console.error(
109117
`Error Occurred while creating members in Delegator Account of GuardDuty ${error.code}: ${error.message}`,
@@ -160,22 +168,27 @@ async function updateMemberDataSource(memberAccounts: AccountDetail[], detectorI
160168
return;
161169
}
162170
try {
163-
console.log(
164-
`Calling api "guardduty.updateMemberDetectors()", ${memberAccounts}, ${detectorId} to disable S3Protection`,
165-
);
166-
await throttlingBackOff(() =>
167-
guardduty
168-
.updateMemberDetectors({
169-
AccountIds: memberAccounts.map(acc => acc.AccountId),
170-
DetectorId: detectorId,
171-
DataSources: {
172-
S3Logs: {
173-
Enable: false,
171+
let pageNumber = 1;
172+
let currentAccounts: AccountDetail[] = paginate(memberAccounts, pageNumber, pageSize);
173+
while (currentAccounts.length > 0) {
174+
console.log(
175+
`Calling api "guardduty.updateMemberDetectors()", ${currentAccounts}, ${detectorId} to disable S3Protection`,
176+
);
177+
await throttlingBackOff(() =>
178+
guardduty
179+
.updateMemberDetectors({
180+
AccountIds: currentAccounts.map(acc => acc.AccountId),
181+
DetectorId: detectorId,
182+
DataSources: {
183+
S3Logs: {
184+
Enable: false,
185+
},
174186
},
175-
},
176-
})
177-
.promise(),
178-
);
187+
})
188+
.promise(),
189+
);
190+
currentAccounts = paginate(memberAccounts, ++pageNumber, pageSize);
191+
}
179192
} catch (error) {
180193
console.error(`Error Occurred while updateMemberDetectors of GuardDuty ${error.code}: ${error.message}`);
181194
throw error;
@@ -204,15 +217,20 @@ async function updateS3Protection(detectorId: string, s3Protection: boolean) {
204217

205218
async function deleteMembers(memberAccounts: AccountDetail[], detectorId: string) {
206219
try {
207-
console.log(`Calling api "guardduty.createMembers()", ${memberAccounts}, ${detectorId}`);
208-
await throttlingBackOff(() =>
209-
guardduty
210-
.deleteMembers({
211-
DetectorId: detectorId,
212-
AccountIds: memberAccounts.map(acc => acc.AccountId),
213-
})
214-
.promise(),
215-
);
220+
let pageNumber = 1;
221+
let currentAccounts: AccountDetail[] = paginate(memberAccounts, pageNumber, pageSize);
222+
while (currentAccounts.length > 0) {
223+
console.log(`Calling api "guardduty.createMembers()", ${currentAccounts}, ${detectorId}`);
224+
await throttlingBackOff(() =>
225+
guardduty
226+
.deleteMembers({
227+
DetectorId: detectorId,
228+
AccountIds: currentAccounts.map(acc => acc.AccountId),
229+
})
230+
.promise(),
231+
);
232+
currentAccounts = paginate(memberAccounts, ++pageNumber, pageSize);
233+
}
216234
} catch (error) {
217235
console.error(
218236
`Error Occurred while creating members in Delegator Account of GuardDuty ${error.code}: ${error.message}`,

‎src/lib/custom-resources/cdk-ssm-document-share/runtime/src/index.ts‎

Lines changed: 60 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,15 @@ import {
77
CloudFormationCustomResourceUpdateEvent,
88
} from 'aws-lambda';
99
import { errorHandler } from '@aws-accelerator/custom-resource-runtime-cfn-response';
10-
import { throttlingBackOff } from '@aws-accelerator/custom-resource-cfn-utils';
10+
import { throttlingBackOff, paginate } from '@aws-accelerator/custom-resource-cfn-utils';
1111

1212
export interface HandlerProperties {
1313
name: string;
1414
accountIds: string[];
1515
}
1616

17+
// SSM modifyDocumentPermission api only supports max 20 accounts per request
18+
const pageSize = 20;
1719
const ssm = new AWS.SSM();
1820

1921
export const handler = errorHandler(onEvent);
@@ -35,15 +37,21 @@ async function onEvent(event: CloudFormationCustomResourceEvent) {
3537

3638
async function onCreate(event: CloudFormationCustomResourceCreateEvent) {
3739
const { accountIds, name } = (event.ResourceProperties as unknown) as HandlerProperties;
38-
await throttlingBackOff(() =>
39-
ssm
40-
.modifyDocumentPermission({
41-
Name: name,
42-
PermissionType: 'Share',
43-
AccountIdsToAdd: accountIds,
44-
})
45-
.promise(),
46-
);
40+
let pageNumber = 1;
41+
let currentAccountIds: string[] = paginate(accountIds, pageNumber, pageSize);
42+
while (currentAccountIds.length > 0) {
43+
await throttlingBackOff(() =>
44+
ssm
45+
.modifyDocumentPermission({
46+
Name: name,
47+
PermissionType: 'Share',
48+
AccountIdsToAdd: currentAccountIds,
49+
})
50+
.promise(),
51+
);
52+
currentAccountIds = paginate(accountIds, ++pageNumber, pageSize);
53+
}
54+
4755
return {
4856
physicalResourceId: `SSMDocumentShare-${name}`,
4957
};
@@ -58,27 +66,37 @@ async function onUpdate(event: CloudFormationCustomResourceUpdateEvent) {
5866
const unShareAccounts = oldProperties.accountIds.filter(accountId => !accountIds.includes(accountId));
5967

6068
if (shareAccounts.length > 0) {
61-
await throttlingBackOff(() =>
62-
ssm
63-
.modifyDocumentPermission({
64-
Name: name,
65-
PermissionType: 'Share',
66-
AccountIdsToAdd: shareAccounts,
67-
})
68-
.promise(),
69-
);
69+
let pageNumber = 1;
70+
let currentAccountIds: string[] = paginate(shareAccounts, pageNumber, pageSize);
71+
while (currentAccountIds.length > 0) {
72+
await throttlingBackOff(() =>
73+
ssm
74+
.modifyDocumentPermission({
75+
Name: name,
76+
PermissionType: 'Share',
77+
AccountIdsToAdd: currentAccountIds,
78+
})
79+
.promise(),
80+
);
81+
currentAccountIds = paginate(shareAccounts, ++pageNumber, pageSize);
82+
}
7083
}
7184

7285
if (unShareAccounts.length > 0) {
73-
await throttlingBackOff(() =>
74-
ssm
75-
.modifyDocumentPermission({
76-
Name: name,
77-
PermissionType: 'Share',
78-
AccountIdsToRemove: unShareAccounts,
79-
})
80-
.promise(),
81-
);
86+
let pageNumber = 1;
87+
let currentAccountIds: string[] = paginate(unShareAccounts, pageNumber, pageSize);
88+
while (currentAccountIds.length > 0) {
89+
await throttlingBackOff(() =>
90+
ssm
91+
.modifyDocumentPermission({
92+
Name: name,
93+
PermissionType: 'Share',
94+
AccountIdsToRemove: currentAccountIds,
95+
})
96+
.promise(),
97+
);
98+
currentAccountIds = paginate(unShareAccounts, ++pageNumber, pageSize);
99+
}
82100
}
83101

84102
return {
@@ -96,15 +114,20 @@ async function onDelete(event: CloudFormationCustomResourceDeleteEvent) {
96114
};
97115
}
98116
try {
99-
await throttlingBackOff(() =>
100-
ssm
101-
.modifyDocumentPermission({
102-
Name: name,
103-
PermissionType: 'Share',
104-
AccountIdsToRemove: accountIds,
105-
})
106-
.promise(),
107-
);
117+
let pageNumber = 1;
118+
let currentAccountIds: string[] = paginate(accountIds, pageNumber, pageSize);
119+
while (currentAccountIds.length > 0) {
120+
await throttlingBackOff(() =>
121+
ssm
122+
.modifyDocumentPermission({
123+
Name: name,
124+
PermissionType: 'Share',
125+
AccountIdsToRemove: currentAccountIds,
126+
})
127+
.promise(),
128+
);
129+
currentAccountIds = paginate(accountIds, ++pageNumber, pageSize);
130+
}
108131
} catch (error) {
109132
console.warn(error);
110133
}

0 commit comments

Comments
 (0)