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
AWS - xpander 1852 #25339
AWS - xpander 1852 #25339
Conversation
Thank you for your contribution. Your generosity and caring are unrivaled! Make sure to register your contribution by filling the Contribution Registration form, so our content wizard @daryakoval will know the proposed changes are ready to bereviewed. |
@yucohen , i shared demo with you |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johnnywilkes Great work!!! Please see my comments :)
@efelmandar Do you think we need a demo for the playbook? Or the linked demo will do?
Automation to determine which interface on an EC2 instance has an over-permissive security group, determine which security groups have over-permissive rules, and replace them with a copy of the security group that has only the over-permissive portion removed. Over-permissive is defined as sensitive ports (SSH, RDP, etc) being exposed to the internet via IPv4. | ||
|
||
TODO ADD LINK AFTER PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a reminder so we won't forget to change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returns: | ||
Dict: Dict of the new SG to be used | ||
""" | ||
info = sg_info[0]['Contents']['AWS.EC2.SecurityGroups(val.GroupId === obj.GroupId)'][0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if sg_info
is a dict, why trying to get the first element?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! it is a list so I will change docstrings and types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
raise ValueError('failed to pull information on EC2 instance') | ||
|
||
|
||
''' COMMAND FUNCTION ''' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As scripts are running in the following structure: !<script-name> arg1=...
I think we can remove this comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a problem, will remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
''' COMMAND FUNCTION ''' | ||
|
||
|
||
def aws_recreate_sg_command(args: Dict[str, Any]) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and also can change the method name, as it is not an XSOAR command
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changing to aws_recreate_sg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for mapping in int_sg_mapping.keys(): | ||
for sg in int_sg_mapping[mapping]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for mapping in int_sg_mapping.keys(): | |
for sg in int_sg_mapping[mapping]: | |
for mapping in int_sg_mapping.values(): | |
for sg in mapping: |
This change is optional, but this way the code is easier to read :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made this change and unit tests were breaking. Will keep as-is
@ShirleyDenkberg , can you look this over as well, please? |
Packs/AWS-Enrichment-Remediation/Playbooks/AWS_-_Security_Group_Remediation_v2.yml
Outdated
Show resolved
Hide resolved
Packs/AWS-Enrichment-Remediation/Playbooks/AWS_-_Security_Group_Remediation_v2.yml
Outdated
Show resolved
Hide resolved
Packs/AWS-Enrichment-Remediation/Playbooks/AWS_-_Security_Group_Remediation_v2.yml
Outdated
Show resolved
Hide resolved
Packs/AWS-Enrichment-Remediation/Playbooks/AWS_-_Security_Group_Remediation_v2_README.md
Outdated
Show resolved
Hide resolved
Packs/AWS-Enrichment-Remediation/Scripts/AWSRecreateSG/AWSRecreateSG.yml
Outdated
Show resolved
Hide resolved
| **Argument Name** | **Description** | | ||
| --- | --- | | ||
| instance_id | EC2 Instance ID. | | ||
| port | TCP/UDP Port to be restricted. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| port | TCP/UDP Port to be restricted. | | |
| port | TCP/UDP port to be restricted. | |
@yucohen @efelmandar Doc review completed. |
Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com>
@yucohen the recorded demo is enough here, finished the review, and is ready for merge. |
8e95583
into
demisto:contrib/johnnywilkes_AWS-EXPANDR-1852
* add automation * init Unittest, docstrings, README, fix yml * more format, validate and lint * unit tests, lint and base playbook * playbook, README, RN * fix build issues * update tests * COVERAGE!!!!! * Yuval's feedback * simple mistake * Apply suggestions from code review * Update README.md --------- Co-authored-by: johnnywilkes <32227961+johnnywilkes@users.noreply.github.com> Co-authored-by: ShirleyDenkberg <62508050+ShirleyDenkberg@users.noreply.github.com> Co-authored-by: yucohen <86777474+yucohen@users.noreply.github.com>
Contributing to Cortex XSOAR Content
Make sure to register your contribution by filling the contribution registration form
The Pull Request will be reviewed only after the contribution registration form is filled.
Status
Related Issues
https://jira-hq.paloaltonetworks.local/browse/EXPANDR-1852
Description
"AWS - Security Group Remediation v2" playbook is an improvement on V1 because instead of just applying a limited security group, this determines which SG is over-permissive and creates a copy of it without the over-permissive parts.
Screenshots
N/A
Minimum version of Cortex XSOAR
Does it break backward compatibility?
Must have