Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accuracy of exposed_internet flag on EC2 Instances #31

Closed
khourybrazil opened this issue Mar 29, 2019 · 6 comments
Closed

Accuracy of exposed_internet flag on EC2 Instances #31

khourybrazil opened this issue Mar 29, 2019 · 6 comments
Labels
schema Graph data schema issues

Comments

@khourybrazil
Copy link

If an instance doesn't have an EIP assigned to it, has an ENI attached to a private subnet in a VPC and has 0.0.0.0/0 permitted by a security group would it still be flagged with exposed_internet? It probably is worth flagging but maybe with something that conveyed the excessive permissiveness instead?

@achantavy
Copy link
Contributor

achantavy commented Mar 29, 2019

If an instance doesn't have an EIP assigned to it, has an ENI attached to a private subnet in a VPC and has 0.0.0.0/0 permitted by a security group would it still be flagged with exposed_internet?

Yes, see here. The security group allows the 0.0.0.0/0 subnet via an IP Permission.

It probably is worth flagging but maybe with something that conveyed the excessive permissiveness instead?

Let me see if I understand your suggestion correctly: there are cases with EC2 instances that have no public IP addresses and no public DNS names, but Cartography still marks them as exposed_internet = True because they are part of an EC2 security group that allows traffic from 0.0.0.0/0. The instance itself would be exposed to the internet if it had a public DNS name and a public IP but since it doesn't, we might want a more nuanced way to label this scenario than just marking it with exposed_internet = True.

Is that about right?

We'll think on this, thanks for pointing it out.

@achantavy achantavy added the schema Graph data schema issues label Mar 29, 2019
@achantavy
Copy link
Contributor

Hi @khourybrazil, talked with @sachafaust on this and we agree this is a bug.

It looks like there are 2 cases that we need to fix.

  1. The EC2 instance is marked as exposed_internet=True but there is no network interface connected to it:
    match(e:EC2Instance{exposed_internet:True})-[r:RESOURCE]-(a:AWSAccount) where not (e)-[:NETWORK_INTERFACE]-(:NetworkInterface) return *

  2. The EC2 instance is marked as exposed_internet=True but it does not have a publicdnsname:
    match(ni:NetworkInterface)-[r:NETWORK_INTERFACE]-(e:EC2Instance{exposed_internet:True})-[r2:RESOURCE]-(a:AWSAccount) where e.publicdnsname='' or not exists(e.publicdnsname) return *

A side note for case 2 above: e.publicdnsname gets set to the empty string by default and we might want to change that to None.

sachafaust added a commit that referenced this issue Apr 4, 2019
achantavy pushed a commit that referenced this issue Apr 4, 2019
@achantavy
Copy link
Contributor

@sachafaust Do we need to also take into account this case?

match(e:EC2Instance{exposed_internet:True})-[r:RESOURCE]-(a:AWSAccount) where not (e)-[:NETWORK_INTERFACE]-(:NetworkInterface) return *

@achantavy
Copy link
Contributor

Scratch that, yeah the fix covers both cases (at least on our own data 😛)

@achantavy
Copy link
Contributor

@sachafaust
Copy link
Contributor

@khourybrazil hope we fixed the bug you found. Tks a lot !

sachafaust added a commit that referenced this issue Apr 22, 2019
* Fix for issue #31

* test multi region peering

* Update __init__.py

* region peering and vpc

* added comment on region attribute
sachafaust added a commit that referenced this issue Oct 27, 2019
* Fix for issue #31

* internal_commit

* adding factors

* trusted origin, full sync ok

* api doc

* run clean - added transform layer

* Update oktaintel.py

working full sync

* Update oktaintel.py

* updated doc

* unit test foundation

* Update __init__.py

* lint

* Update oktaintel.py

* Update oktaintel.py

* Update oktaintel.py

* Update oktaintel.py

* unit tests

* cred setup

* sync integration

* Update okta_import_cleanup.json

* lint bs

* lint bs

* Group member bug fix

* PR Feedback

* Update oktaintel.py

* Update README.md

* removing cli parameter

* evan review part 1

* evan feedback part 2 - refactoring into smaller chunks

* evan feedback - part 2 - CLI parameters

* lint

* utils

* testing

* bug fix

* Update cli.py

* splitting get  and transform

* Update users.py

* Update groups.py

* Update roles.py

* fix doc

* change unit test

* Update test_syntax.py

* fix unit test

* fix index

* Address Alex feedback - store data in memory vs graph call

* fix

* lint

* Update okta_import_cleanup.json
achantavy pushed a commit that referenced this issue Nov 6, 2019
* Fix for issue #31

* internal_commit

* adding factors

* trusted origin, full sync ok

* api doc

* run clean - added transform layer

* Update oktaintel.py

working full sync

* Update oktaintel.py

* updated doc

* unit test foundation

* Update __init__.py

* lint

* Update oktaintel.py

* Update oktaintel.py

* Update oktaintel.py

* Update oktaintel.py

* unit tests

* cred setup

* sync integration

* Update okta_import_cleanup.json

* lint bs

* lint bs

* Group member bug fix

* PR Feedback

* Update oktaintel.py

* Update README.md

* removing cli parameter

* evan review part 1

* evan feedback part 2 - refactoring into smaller chunks

* evan feedback - part 2 - CLI parameters

* lint

* utils

* testing

* bug fix

* Update cli.py

* splitting get  and transform

* Update users.py

* Update groups.py

* Update roles.py

* fix doc

* change unit test

* Update test_syntax.py

* fix unit test

* fix index

* Added in reply uris for okta applications

* Add in dns querying for reply urls

* added awssaml module

* Adding in awssaml module into okta

* Uncomment other syncs

* Made CLI updates to get regex and fixed the applications unit test

* added awssaml unittest

* Add in CAN_ASSUME_ROLE mapping between AWSRole and Humans. Added in cleanup jobs

* Updated readme's

* Fix numbering in the readme

* Fixed lint

* Add an index for ReplyUri's

* get reply urls in alphabetical order

* Added CLI parameter for replyuri dns resolution

* Reorder okta cleanup

* Fix unit test to make sure its all working

* Revert "Added CLI parameter for replyuri dns resolution"

This reverts commit 9b65a68.

* Revert "Fix unit test to make sure its all working"

This reverts commit 6cb885f.

* fix unit test

* remove unneded list traversal

* Fix docstring

* Fix a comment in the cli help

* Fixing PR feedback

* removing extra line

* reformat cleanup
nishils pushed a commit to nishils/cartography that referenced this issue Apr 5, 2020
nishils pushed a commit to nishils/cartography that referenced this issue Apr 5, 2020
* Fix for issue lyft#31

* test multi region peering

* Update __init__.py

* region peering and vpc

* added comment on region attribute
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
schema Graph data schema issues
Projects
None yet
Development

No branches or pull requests

3 participants