feat(aws-cdk): add security impact prompt to 'cdk deploy' (#1240)

Add awareness of potentially risky permissions changes to the
cfnspec and diff tool, and surface those changes in an easily
digestible format.

Control feature using `--require-approval` command-line flag or `requireApproval`
field in `cdk.json`. Also see http://bit.ly/cdk-2EhF7Np

This is an initial version, and there will be future changes to this feature.

Fixes #978, fixes #1299.

docs/src/tools.rst

@@ -46,7 +46,7 @@ Here are the actions you can take on your CDK app
 .. code-block:: sh
 
     Usage: cdk -a <cdk-app> COMMAND
-    
+
     Commands:
       list                        Lists all stacks in the app          [aliases: ls]
       synthesize [STACKS..]       Synthesizes and prints the CloudFormation template
@@ -64,7 +64,7 @@ Here are the actions you can take on your CDK app
                                   used.
       docs                        Opens the documentation in a browser[aliases: doc]
       doctor                      Check your set-up for potential problems
-    
+
     Options:
       --app, -a            REQUIRED: Command-line for executing your CDK app (e.g.
                            "node bin/my-app.js")                            [string]
@@ -90,12 +90,43 @@ Here are the actions you can take on your CDK app
       --role-arn, -r       ARN of Role to use when invoking CloudFormation  [string]
       --version            Show version number                             [boolean]
       --help               Show help                                       [boolean]
-    
+
     If your app has a single stack, there is no need to specify the stack name
-    
+
     If one of cdk.json or ~/.cdk.json exists, options specified there will be used
     as defaults. Settings in cdk.json take precedence.
 
+.. _security-changes:
+
+Security-related changes
+========================
+
+In order to protect you against unintended changes that affect your security posture,
+the CDK toolkit will prompt you to approve security-related changes before deploying
+them.
+
+You change the level of changes that requires approval by specifying:
+
+.. code-block::
+
+   cdk deploy --require-approval LEVEL
+
+Where ``LEVEL`` can be one of:
+
+* ``never`` - approval is never required.
+* ``any-change`` - require approval on any IAM or security-group related change.
+* ``broadening`` (default) - require approval when IAM statements or traffic rules are added. Removals
+  do not require approval.
+
+The setting also be configured in **cdk.json**:
+
+.. code-block:: js
+
+    {
+        "app": "...",
+        "requireApproval": "never"
+    }
+
 .. _version-reporting:
 
 Version Reporting

packages/@aws-cdk/cfnspec/build-tools/build.ts

@@ -10,6 +10,7 @@ import fs = require('fs-extra');
 import md5 = require('md5');
 import path = require('path');
 import { schema } from '../lib';
+import { detectScrutinyTypes } from './scrutiny';
 
 async function main() {
   const inputDir = path.join(process.cwd(), 'spec-source');
@@ -25,6 +26,8 @@ async function main() {
     }
   }
 
+  detectScrutinyTypes(spec);
+
   spec.Fingerprint = md5(JSON.stringify(normalize(spec)));
 
   const outDir = path.join(process.cwd(), 'spec');

packages/@aws-cdk/cfnspec/build-tools/scrutiny.ts

@@ -0,0 +1,89 @@
+import { schema } from '../lib';
+import { PropertyScrutinyType, ResourceScrutinyType } from '../lib/schema';
+
+/**
+ * Auto-detect common properties to apply scrutiny to by using heuristics
+ *
+ * Manually enhancing scrutiny attributes for each property does not scale
+ * well. Fortunately, the most important ones follow a common naming scheme and
+ * we tag all of them at once in this way.
+ *
+ * If the heuristic scheme gets it wrong in some individual cases, those can be
+ * fixed using schema patches.
+ */
+export function detectScrutinyTypes(spec: schema.Specification) {
+  for (const [typeName, typeSpec] of Object.entries(spec.ResourceTypes)) {
+    if (typeSpec.ScrutinyType !== undefined) { continue; } // Already assigned
+
+    detectResourceScrutiny(typeName, typeSpec);
+
+    // If a resource scrutiny is set by now, we don't need to look at the properties anymore
+    if (typeSpec.ScrutinyType !== undefined) { continue; }
+
+    for (const [propertyName, propertySpec] of Object.entries(typeSpec.Properties || {})) {
+      if (propertySpec.ScrutinyType !== undefined) { continue; } // Already assigned
+
+      detectPropertyScrutiny(typeName, propertyName, propertySpec);
+
+    }
+  }
+}
+
+/**
+ * Detect and assign a scrutiny type for the resource
+ */
+function detectResourceScrutiny(typeName: string, typeSpec: schema.ResourceType) {
+  const properties = Object.entries(typeSpec.Properties || {});
+
+  // If this resource is named like *Policy and has a PolicyDocument property
+  if (typeName.endsWith('Policy') && properties.some(apply2(isPolicyDocumentProperty))) {
+    typeSpec.ScrutinyType = isIamType(typeName) ? ResourceScrutinyType.IdentityPolicyResource : ResourceScrutinyType.ResourcePolicyResource;
+    return;
+  }
+}
+
+/**
+ * Detect and assign a scrutiny type for the property
+ */
+function detectPropertyScrutiny(_typeName: string, propertyName: string, propertySpec: schema.Property) {
+  // Detect fields named like ManagedPolicyArns
+  if (propertyName === 'ManagedPolicyArns') {
+    propertySpec.ScrutinyType = PropertyScrutinyType.ManagedPolicies;
+    return;
+  }
+
+  if (propertyName === "Policies" && schema.isComplexListProperty(propertySpec) && propertySpec.ItemType === 'Policy') {
+    propertySpec.ScrutinyType = PropertyScrutinyType.InlineIdentityPolicies;
+    return;
+  }
+
+  if (isPolicyDocumentProperty(propertyName, propertySpec)) {
+    propertySpec.ScrutinyType = PropertyScrutinyType.InlineResourcePolicy;
+    return;
+  }
+}
+
+function isIamType(typeName: string) {
+  return typeName.indexOf('::IAM::') > 1;
+}
+
+function isPolicyDocumentProperty(propertyName: string, propertySpec: schema.Property) {
+  const nameContainsPolicy = propertyName.indexOf('Policy') > -1;
+  const primitiveType = schema.isPrimitiveProperty(propertySpec) && propertySpec.PrimitiveType;
+
+  if (nameContainsPolicy && primitiveType === 'Json') {
+    return true;
+  }
+  return false;
+}
+
+/**
+ * Make a function that takes 2 arguments take an array of 2 elements instead
+ *
+ * Makes it possible to map it over an array of arrays. TypeScript won't allow
+ * me to overload this type declaration so we need a different function for
+ * every # of arguments.
+ */
+function apply2<T1, T2, R>(fn: (a1: T1, a2: T2) => R): (as: [T1, T2]) => R {
+  return (as) => fn.apply(fn, as);
+}

packages/@aws-cdk/cfnspec/lib/index.ts

@@ -5,18 +5,48 @@ export { schema };
 /**
  * The complete AWS CloudFormation Resource specification, having any CDK patches and enhancements included in it.
  */
-// tslint:disable-next-line:no-var-requires
-export const specification: schema.Specification = require('../spec/specification.json');
+export function specification(): schema.Specification {
+  return require('../spec/specification.json');
+}
+
+/**
+ * Return the resource specification for the given typename
+ *
+ * Validates that the resource exists. If you don't want this validating behavior, read from
+ * specification() directly.
+ */
+export function resourceSpecification(typeName: string): schema.ResourceType {
+  const ret = specification().ResourceTypes[typeName];
+  if (!ret) {
+    throw new Error(`No such resource type: ${typeName}`);
+  }
+  return ret;
+}
+
+/**
+ * Return the property specification for the given resource's property
+ */
+export function propertySpecification(typeName: string, propertyName: string): schema.Property {
+  const ret = resourceSpecification(typeName).Properties![propertyName];
+  if (!ret) {
+    throw new Error(`Resource ${typeName} has no property: ${propertyName}`);
+  }
+  return ret;
+}
 
 /**
  * The list of resource type names defined in the ``specification``.
  */
-export const resourceTypes = Object.keys(specification.ResourceTypes);
+export function resourceTypes() {
+  return Object.keys(specification().ResourceTypes);
+}
 
 /**
  * The list of namespaces defined in the ``specification``, that is resource name prefixes down to the second ``::``.
  */
-export const namespaces = Array.from(new Set(resourceTypes.map(n => n.split('::', 2).join('::'))));
+export function namespaces() {
+  return Array.from(new Set(resourceTypes().map(n => n.split('::', 2).join('::'))));
+}
 
 /**
  * Obtain a filtered version of the AWS CloudFormation specification.
@@ -30,14 +60,16 @@ export const namespaces = Array.from(new Set(resourceTypes.map(n => n.split('::'
  *     to the selected resource types.
  */
 export function filteredSpecification(filter: string | RegExp | Filter): schema.Specification {
-  const result: schema.Specification = { ResourceTypes: {}, PropertyTypes: {}, Fingerprint: specification.Fingerprint };
+  const spec = specification();
+
+  const result: schema.Specification = { ResourceTypes: {}, PropertyTypes: {}, Fingerprint: spec.Fingerprint };
   const predicate: Filter = makePredicate(filter);
-  for (const type of resourceTypes) {
+  for (const type of resourceTypes()) {
     if (!predicate(type)) { continue; }
-    result.ResourceTypes[type] = specification.ResourceTypes[type];
+    result.ResourceTypes[type] = spec.ResourceTypes[type];
     const prefix = `${type}.`;
-    for (const propType of Object.keys(specification.PropertyTypes!).filter(n => n.startsWith(prefix))) {
-      result.PropertyTypes[propType] = specification.PropertyTypes![propType];
+    for (const propType of Object.keys(spec.PropertyTypes!).filter(n => n.startsWith(prefix))) {
+      result.PropertyTypes[propType] = spec.PropertyTypes![propType];
     }
   }
   result.Fingerprint = crypto.createHash('sha256').update(JSON.stringify(result)).digest('base64');
@@ -64,3 +96,35 @@ function makePredicate(filter: string | RegExp | Filter): Filter {
     return s => s.match(filter) != null;
   }
 }
+
+/**
+ * Return the properties of the given type that require the given scrutiny type
+ */
+export function scrutinizablePropertyNames(resourceType: string, scrutinyTypes: schema.PropertyScrutinyType[]): string[] {
+  const impl = specification().ResourceTypes[resourceType];
+  if (!impl) { return []; }
+
+  const ret = new Array<string>();
+
+  for (const [propertyName, propertySpec] of Object.entries(impl.Properties || {})) {
+    if (scrutinyTypes.includes(propertySpec.ScrutinyType || schema.PropertyScrutinyType.None)) {
+      ret.push(propertyName);
+    }
+  }
+
+  return ret;
+}
+
+/**
+ * Return the names of the resource types that need to be subjected to additional scrutiny
+ */
+export function scrutinizableResourceTypes(scrutinyTypes: schema.ResourceScrutinyType[]): string[] {
+  const ret = new Array<string>();
+  for (const [resourceType, resourceSpec] of Object.entries(specification().ResourceTypes)) {
+    if (scrutinyTypes.includes(resourceSpec.ScrutinyType || schema.ResourceScrutinyType.None)) {
+      ret.push(resourceType);
+    }
+  }
+
+  return ret;
+}

packages/@aws-cdk/cfnspec/lib/schema/property.ts

@@ -16,6 +16,13 @@ export interface PropertyBase extends Documented {
    * example, which other properties you updated.
    */
   UpdateType: UpdateType;
+
+  /**
+   * During a stack update, what kind of additional scrutiny changes to this property should be subjected to
+   *
+   * @default None
+   */
+  ScrutinyType?: PropertyScrutinyType;
 }
 
 export interface PrimitiveProperty extends PropertyBase {
@@ -154,3 +161,39 @@ export function isUnionProperty(prop: Property): prop is UnionProperty {
   const castProp = prop as UnionProperty;
   return !!(castProp.ItemTypes || castProp.PrimitiveTypes || castProp.Types);
 }
+
+export enum PropertyScrutinyType {
+  /**
+   * No additional scrutiny
+   */
+  None = 'None',
+
+  /**
+   * This is an IAM policy directly on a resource
+   */
+  InlineResourcePolicy = 'InlineResourcePolicy',
+
+  /**
+   * Either an AssumeRolePolicyDocument or a dictionary of policy documents
+   */
+  InlineIdentityPolicies = 'InlineIdentityPolicies',
+
+  /**
+   * A list of managed policies (on an identity resource)
+   */
+  ManagedPolicies = 'ManagedPolicies',
+
+  /**
+   * A set of ingress rules (on a security group)
+   */
+  IngressRules = 'IngressRules',
+
+  /**
+   * A set of egress rules (on a security group)
+   */
+  EgressRules = 'EgressRules',
+}
+
+export function isPropertyScrutinyType(str: string): str is PropertyScrutinyType {
+  return (PropertyScrutinyType as any)[str] !== undefined;
+}

packages/@aws-cdk/cfnspec/lib/schema/resource-type.ts

@@ -19,6 +19,13 @@ export interface ResourceType extends Documented {
    * What kind of value the 'Ref' operator refers to, if any.
    */
   RefKind?: string;
+
+  /**
+   * During a stack update, what kind of additional scrutiny changes to this resource should be subjected to
+   *
+   * @default None
+   */
+  ScrutinyType?: ResourceScrutinyType;
 }
 
 export type Attribute = PrimitiveAttribute | ListAttribute;
@@ -71,3 +78,43 @@ export enum SpecialRefKind {
    */
   Arn = 'Arn'
 }
+
+export enum ResourceScrutinyType {
+  /**
+   * No additional scrutiny
+   */
+  None = 'None',
+
+  /**
+   * An externally attached policy document to a resource
+   *
+   * (Common for SQS, SNS, S3, ...)
+   */
+  ResourcePolicyResource = 'ResourcePolicyResource',
+
+  /**
+   * This is an IAM policy on an identity resource
+   *
+   * (Basically saying: this is AWS::IAM::Policy)
+   */
+  IdentityPolicyResource = 'IdentityPolicyResource',
+
+  /**
+   * This is a Lambda Permission policy
+   */
+  LambdaPermission = 'LambdaPermission',
+
+  /**
+   * An ingress rule object
+   */
+  IngressRuleResource = 'IngressRuleResource',
+
+  /**
+   * A set of egress rules
+   */
+  EgressRuleResource = 'EgressRuleResource',
+}
+
+export function isResourceScrutinyType(str: string): str is ResourceScrutinyType {
+  return (ResourceScrutinyType as any)[str] !== undefined;
+}