Skip to content
This repository was archived by the owner on Sep 17, 2021. It is now read-only.

Conversation

@zollman
Copy link
Contributor

@zollman zollman commented Sep 2, 2016

To help "roll up" issues into larger groupings (e.g. an IAM user
is out-of-compliance if the managed policy attached to that user
is noncompliant), this patch allows auditors to link issues together,
rolling them up into higher technology types.

This commit builds on the dynamic watchers & auditors framework submitted in #405 , as well as the changes built into #406 .

Anne Ebie and others added 3 commits September 2, 2016 12:28
Change summary:
  * Fix Error in exception handling
  * Adding audit class to distinguish items created by different
    auditors of the same type
  * Adding custom directories
  * Adding development guidelines documentation
  * Fixing links in documentation
  * Removing duplicate auditors
  * Adding init test for scheduler
  * Fixing DB error in scheduler test
  * Removing unneeded DB insert from test case
  * Race condition with watcher_registry
  * Fixing DB migration conflict
  * Picking up pubspec.lock changes
  * Code style cleanup
  * Update file headers for contribution prep

Change-Id: Id72322f6dbccedc701e9c17dc9a5b8dc26bf30c1
List of added watchers:
    * CloudTrail
    * AWSConfig
    * AWSConfigRecorder
    * DirectConnect::Connection
    * DirectConnect::VirtualGateway
    * EC2::EbsSnapshot
    * EC2::EbsVolume
    * EC2::Image
    * EC2::Instance
    * ENI
    * KMS::Grant
    * KMS::Key
    * Lambda
    * RDS::ClusterSnapshot
    * RDS::DBCluster
    * RDS::DBInstace
    * RDS::Snapshot
    * RDS::SubnetGroup
    * Route53
    * Route53Domains
    * TrustedAdvisor
    * VPC::DHCP
    * VPC::Endpoint
    * VPC::FlowLog
    * VPC::NatGateway
    * VPC::NetworkACL
    * VPC::Peering

Additional changes:
  * Move rds[security_group] to rds/ directory.
  * Update vpc/route_table to use boto3 lib.
  * Add tests for tech types supported by moto
  * Initialize name to avoid UnboundLocalError
  * Update RDS watcher ephemeral values
  * Use boto3.session.get_available_regions in select watchers
  * Convert routetable watcher to decorator pattern
  * Convert route53 domains to decorator pattern
  * Handle the case where the aws principle is a string
  * Catching assume role exception in decorator
@zollman zollman changed the title Auditor dependencies Link together issues by enabling auditor dependcies Sep 2, 2016
@zollman zollman changed the title Link together issues by enabling auditor dependcies Link together issues by enabling auditor dependencies Sep 2, 2016
@scriptsrc scriptsrc added this to the 0.7.0 milestone Sep 6, 2016
@scriptsrc scriptsrc modified the milestones: 0.7.5, 0.7.0 Sep 20, 2016
@aebie
Copy link

aebie commented Sep 22, 2016

@MonkeySecurity I was pulling these PR branches plus the 0.6.0 release back into out local repo and notices some tricky merge issues with the custom accounts branch. It looks like you will be pulling in this branch first? If so, I can go ahead and resolve the merge conflicts in custom accounts.

@scriptsrc
Copy link
Contributor

Hey @aebie,

Yeah, I was going to merge this one #407 before #408.

These two will be in a separate release (0.7.5). I've been spending this week looking for any bugs in the 0.7.0 release before I push that out.

@aebie
Copy link

aebie commented Sep 26, 2016

@MonkeySecurity I will work on an update to merge 407 into 408 then. We do have one outstanding bug fix for 407 that is in the review process, but has to do with auditors that have more than one support watcher so it will not manifest with the auditors currently submitted so probably is not critical to get in right away.

Curtis Stewart and others added 2 commits October 18, 2016 11:43
Type: Bugfix

Why is this change necessary:
When find_changes is run, it has the ability to update specific tech
types outside of the normal reporter run. In these cases the monitor
must rerun auditors for the updated tech types, plus any other auditors
for other tech types that are dependent on the updated types. It
determines this by checking the support_watcher_indexes and
support_auditor_indexs for each auditor associated with the tech types
updated. The check for the support_watcher_indexes were only being run
for the last auditor because it was indented incorrectly.

This change addresses the need by:
Changing the indent so that it is run for all auditors
@scriptsrc
Copy link
Contributor

Applying "Needs Work" label as I believe Curtis is working on updating this PR. (See gitter.im chat from Oct 13 15:55)

@cstewart87
Copy link

UDPATE: still working on these updates. Hope to post w/in 1 week

@cstewart87
Copy link

Build will probably fail until #438 is merged

@scriptsrc
Copy link
Contributor

I merged 438 and instructed travis to rebuild. Build is now working.

I think I'm still waiting for new commits. Is that correct?

@cstewart87
Copy link

@MonkeySecurity - we just pushed some updates out today. Should have all of the commits needed for review 👍

@cstewart87
Copy link

@MonkeySecurity - not sure why the dates are funky, prob something to do with cherry-picking, but the 2 latest commits listed under "Oct 18" section (https://github.com/Netflix/security_monkey/pull/407/commits) are what was pushed today

# TODO: Replace hardcoded region with `get_available_regions` after next boto3 release
# Specific PR here: https://github.com/boto/boto3/pull/531/files
# AWS Config currently only supports the following regions
regions = ["us-east-1", "us-west-1", "us-west-2", "eu-west-1", "eu-central-1",
Copy link
Contributor

Choose a reason for hiding this comment

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

That boto3 pr was merged on Mar3. Is the hardcoded region list still necessary?

Copy link

Choose a reason for hiding this comment

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

@MonkeySecurity They are not. Let me look into why they are still there.

Copy link

Choose a reason for hiding this comment

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

@MonkeySecurity we have the correct code internally and it looks like it is correct in you master. We will push a fix for this branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

I went ahead and made the change for config_recorder and another very minor change to kms.py.

@scriptsrc scriptsrc merged commit 8aa5e93 into Netflix:develop Nov 8, 2016
@zollman zollman deleted the auditor_dependencies branch November 30, 2016 20:48
@scriptsrc scriptsrc mentioned this pull request Dec 2, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants