Skip to content

Commit bbae01a

Browse files
Merge master into feature/emr
2 parents 009f971 + 7b4257a commit bbae01a

File tree

8 files changed

+51
-41
lines changed

8 files changed

+51
-41
lines changed

packages/core/src/awsService/ec2/model.ts

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,12 @@ import { isCloud9 } from '../../shared/extensionUtilities'
1212
import { ToolkitError } from '../../shared/errors'
1313
import { SsmClient } from '../../shared/clients/ssmClient'
1414
import { Ec2Client } from '../../shared/clients/ec2Client'
15-
import { VscodeRemoteConnection, ensureDependencies, openRemoteTerminal } from '../../shared/remoteSession'
15+
import {
16+
VscodeRemoteConnection,
17+
ensureDependencies,
18+
getDeniedSsmActions,
19+
openRemoteTerminal,
20+
} from '../../shared/remoteSession'
1621
import { DefaultIamClient } from '../../shared/clients/iamClient'
1722
import { ErrorInformation } from '../../shared/errors'
1823
import { sshAgentSocketVariable, startSshAgent, startVscodeRemote } from '../../shared/extensions/ssh'
@@ -69,13 +74,10 @@ export class Ec2ConnectionManager {
6974
}
7075
}
7176

72-
public async hasProperPolicies(IamRoleArn: string): Promise<boolean> {
73-
const attachedPolicies = (await this.iamClient.listAttachedRolePolicies(IamRoleArn)).map(
74-
(policy) => policy.PolicyName!
75-
)
76-
const requiredPolicies = ['AmazonSSMManagedInstanceCore', 'AmazonSSMManagedEC2InstanceDefaultPolicy']
77+
public async hasProperPermissions(IamRoleArn: string): Promise<boolean> {
78+
const deniedActions = await getDeniedSsmActions(this.iamClient, IamRoleArn)
7779

78-
return requiredPolicies.length !== 0 && requiredPolicies.every((policy) => attachedPolicies.includes(policy))
80+
return deniedActions.length === 0
7981
}
8082

8183
public async isInstanceRunning(instanceId: string): Promise<boolean> {
@@ -102,12 +104,15 @@ export class Ec2ConnectionManager {
102104

103105
if (!IamRole) {
104106
const message = `No IAM role attached to instance: ${selection.instanceId}`
105-
this.throwConnectionError(message, selection, { code: 'EC2SSMPermission' })
107+
this.throwConnectionError(message, selection, {
108+
code: 'EC2SSMPermission',
109+
documentationUri: this.policyDocumentationUri,
110+
})
106111
}
107112

108-
const hasProperPolicies = await this.hasProperPolicies(IamRole!.Arn)
113+
const hasPermission = await this.hasProperPermissions(IamRole!.Arn)
109114

110-
if (!hasProperPolicies) {
115+
if (!hasPermission) {
111116
const message = `Ensure an IAM role with the required policies is attached to the instance. Found attached role: ${
112117
IamRole!.Arn
113118
}`

packages/core/src/shared/clients/ec2Client.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ export class Ec2Client {
6161

6262
return safeInstances
6363
.map(async (instance) => {
64-
return { ...instance, LastSeenStatus: await this.getInstanceStatus(instance.InstanceId!) }
64+
return { ...instance, LastSeenStatus: await this.getInstanceStatus(instance.InstanceId) }
6565
})
6666
.map((instance) => {
6767
return instanceHasName(instance!)

packages/core/src/shared/clients/iamClient.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,4 +104,12 @@ export class DefaultIamClient {
104104
}
105105
return response.InstanceProfile.Roles[0]
106106
}
107+
108+
public async putRolePolicy(roleArn: string, policyName: string, policyDocument: string): Promise<void> {
109+
const client = await this.createSdkClient()
110+
const roleName = this.getFriendlyName(roleArn)
111+
await client
112+
.putRolePolicy({ RoleName: roleName, PolicyName: policyName, PolicyDocument: policyDocument })
113+
.promise()
114+
}
107115
}

packages/core/src/shared/clients/ssmClient.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ export class SsmClient {
6464

6565
public async getTargetPlatformName(target: string): Promise<string> {
6666
const instanceInformation = await this.describeInstance(target)
67-
6867
return instanceInformation.PlatformName!
6968
}
7069

packages/core/src/shared/remoteSession.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,21 @@ import { getOrInstallCli } from './utilities/cliUtils'
1919
import { pushIf } from './utilities/collectionUtils'
2020
import { ChildProcess } from './utilities/childProcess'
2121
import { findSshPath, getVscodeCliPath } from './utilities/pathFind'
22+
import { IamClient } from './clients/iamClient'
23+
import { IAM } from 'aws-sdk'
2224

2325
export interface MissingTool {
2426
readonly name: 'code' | 'ssm' | 'ssh'
2527
readonly reason?: string
2628
}
2729

30+
const minimumSsmActions = [
31+
'ssmmessages:CreateControlChannel',
32+
'ssmmessages:CreateDataChannel',
33+
'ssmmessages:OpenControlChannel',
34+
'ssmmessages:OpenDataChannel',
35+
]
36+
2837
export async function openRemoteTerminal(options: vscode.TerminalOptions, onClose: () => void) {
2938
const timeout = new Timeout(60000)
3039

@@ -165,3 +174,12 @@ export async function handleMissingTool(tools: Err<MissingTool[]>) {
165174
})
166175
)
167176
}
177+
178+
export async function getDeniedSsmActions(client: IamClient, roleArn: string): Promise<IAM.EvaluationResult[]> {
179+
const deniedActions = await client.getDeniedActions({
180+
PolicySourceArn: roleArn,
181+
ActionNames: minimumSsmActions,
182+
})
183+
184+
return deniedActions
185+
}

packages/core/src/test/awsService/ec2/model.test.ts

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -42,36 +42,12 @@ describe('Ec2ConnectClient', function () {
4242
})
4343
})
4444

45-
describe('hasProperPolicies', async function () {
46-
it('correctly determines if proper policies are included', async function () {
47-
async function assertAcceptsPolicies(policies: IAM.Policy[], expectedResult: boolean) {
48-
sinon.stub(DefaultIamClient.prototype, 'listAttachedRolePolicies').resolves(policies)
49-
50-
const result = await client.hasProperPolicies('')
51-
assert.strictEqual(result, expectedResult)
52-
53-
sinon.restore()
54-
}
55-
await assertAcceptsPolicies(
56-
[{ PolicyName: 'name' }, { PolicyName: 'name2' }, { PolicyName: 'name3' }],
57-
false
58-
)
59-
await assertAcceptsPolicies(
60-
[
61-
{ PolicyName: 'AmazonSSMManagedInstanceCore' },
62-
{ PolicyName: 'AmazonSSMManagedEC2InstanceDefaultPolicy' },
63-
],
64-
true
65-
)
66-
await assertAcceptsPolicies([{ PolicyName: 'AmazonSSMManagedEC2InstanceDefaultPolicy' }], false)
67-
await assertAcceptsPolicies([{ PolicyName: 'AmazonSSMManagedEC2InstanceDefaultPolicy' }], false)
68-
})
69-
45+
describe('hasProperPermissions', async function () {
7046
it('throws error when sdk throws error', async function () {
7147
sinon.stub(DefaultIamClient.prototype, 'listAttachedRolePolicies').throws(new ToolkitError('error'))
7248

7349
try {
74-
await client.hasProperPolicies('')
50+
await client.hasProperPermissions('')
7551
assert.ok(false)
7652
} catch {
7753
assert.ok(true)
@@ -131,7 +107,7 @@ describe('Ec2ConnectClient', function () {
131107
it('throws EC2SSMAgent error if instance is running and has IAM Role, but agent is not running', async function () {
132108
sinon.stub(Ec2ConnectionManager.prototype, 'isInstanceRunning').resolves(true)
133109
sinon.stub(Ec2ConnectionManager.prototype, 'getAttachedIamRole').resolves({ Arn: 'testRole' } as IAM.Role)
134-
sinon.stub(Ec2ConnectionManager.prototype, 'hasProperPolicies').resolves(true)
110+
sinon.stub(Ec2ConnectionManager.prototype, 'hasProperPermissions').resolves(true)
135111
sinon.stub(SsmClient.prototype, 'getInstanceAgentPingStatus').resolves('offline')
136112

137113
try {
@@ -145,7 +121,7 @@ describe('Ec2ConnectClient', function () {
145121
it('does not throw an error if all checks pass', async function () {
146122
sinon.stub(Ec2ConnectionManager.prototype, 'isInstanceRunning').resolves(true)
147123
sinon.stub(Ec2ConnectionManager.prototype, 'getAttachedIamRole').resolves({ Arn: 'testRole' } as IAM.Role)
148-
sinon.stub(Ec2ConnectionManager.prototype, 'hasProperPolicies').resolves(true)
124+
sinon.stub(Ec2ConnectionManager.prototype, 'hasProperPermissions').resolves(true)
149125
sinon.stub(SsmClient.prototype, 'getInstanceAgentPingStatus').resolves('Online')
150126

151127
assert.doesNotThrow(async () => await client.checkForStartSessionError(instanceSelection))

packages/core/src/test/shared/extensions/ssh.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
33
* SPDX-License-Identifier: Apache-2.0
44
*/
5-
import assert from 'assert'
5+
import * as assert from 'assert'
66
import { ChildProcess } from '../../../shared/utilities/childProcess'
77
import { startSshAgent } from '../../../shared/extensions/ssh'
88

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Bug Fix",
3+
"description": "when connecting to ec2 instance, check for IAM role permitted actions, rather than full policies"
4+
}

0 commit comments

Comments
 (0)