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
OS compatibility environment #507
Conversation
Can you add to all the OS compat machines a tag that is |
removed unused ubuntu machine
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.
Looks good, some minor comments.
envs/monkey_maker/README.md
Outdated
Also review `./terraform/config.tf` file. | ||
|
||
Launch the environment by going into `terraform` folder and running | ||
```angular2html |
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.
angular2html
? did you mean 'sh'?
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.
Pycharm added this by default for some reason
envs/monkey_maker/README.md
Outdated
|
||
Contents of `accessKeys` file should be as follows: | ||
|
||
``` |
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 ini
language specifier
envs/monkey_maker/README.md
Outdated
terraform apply | ||
``` | ||
|
||
### Usage |
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.
Level 2 header, not 3
envs/monkey_maker/terraform/infra.tf
Outdated
ingress { | ||
from_port = 0 | ||
to_port = 0 | ||
protocol = "-1" | ||
cidr_blocks = ["0.0.0.0/0"] | ||
} | ||
|
||
egress { | ||
from_port = 0 | ||
to_port = 0 | ||
protocol = "-1" | ||
cidr_blocks = ["0.0.0.0/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.
Seems overly permissive
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.
You mean ingress or egress?
Egress is ok.
Ingress, the alternative is locking it to the users current public IP, which requires the user to input it.
Instead of that, I'd rather user specify a SSH public key.
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.
Machines are not vulnerable and some of them require manual login for testing, so I think we're good
@@ -0,0 +1,25 @@ | |||
resource "aws_instance" "island_windows" { |
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.
Are these Islands, or just build machines? Name accordingly.
If they ARE Islands: Why do we need Islands here?
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.
We need islands to check results...
envs/os_compatibility/README.md
Outdated
## Usage | ||
|
||
1. Launch os_compat_ISLAND machine and upload your binaries/update island. Reset island environment. | ||
2. Launch/Reboot all other os_compat test machines (Can be filtered with tag "Puropose: os_compat_instance") |
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.
Puropose typo
|
||
For windows_2008_r2 Administrator:AGE(MP..txL | ||
|
||
The following machines does not download monkey automatically, so you'll have to manually check them: |
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.
Add instructions on how to manually verify these machines
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.
What do you mean verify?
@@ -0,0 +1,62 @@ | |||
import pytest | |||
|
|||
from envs.monkey_zoo.blackbox.island_client.monkey_island_client import MonkeyIslandClient |
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.
❤️
|
||
To login to windows use Administrator: %HwuzI!Uzsyfa=cB*XaQ6xxHqopfj)h) credentials | ||
|
||
You'll find docker files in `/home/ubuntu/docker_envs/linux/...` |
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.
Does this make sense? Why are we not pulling latest docker files from internet?
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.
We can skip the build step
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.
You want me to add that we also need to pull the latest changes before building? Isn't that kind of self explanatory, that if you want to build monkey of a certain branch you need to pull that branch? If we skip the build step, user will have to check image name manually?
ingress { | ||
from_port = 0 | ||
to_port = 0 | ||
protocol = "-1" | ||
cidr_blocks = ["0.0.0.0/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.
Doesn't need to be all ports, just 5000 and 22.
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 3389. And 5001. But I don't know if it's worth the hassle to allow only the ports that are the only ones open anyways. Or are there some unsecure open ports by default?
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.
Or we can just limit the source to LAN and add our remote machines manually
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.
Yes, or you can add it as a terraform parameter variable. Either works for me.
resource "aws_security_group" "os_compat_instance" { | ||
name = "os_compat_instance" | ||
description = "Disables remote access to vulnerable instances" | ||
vpc_id = "${aws_vpc.os_compat_vpc.id}" | ||
|
||
ingress { | ||
from_port = 0 | ||
to_port = 0 | ||
protocol = "-1" | ||
cidr_blocks = ["0.0.0.0/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.
Why do we want the target instances ot be world accessible? Does each one have a unique password?
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.
Yes
machine_list = { | ||
"10.0.0.36": "centos_6", | ||
"10.0.0.37": "centos_7", | ||
"10.0.0.38": "centos_8", | ||
"10.0.0.42": "suse_12", | ||
"10.0.0.41": "suse_11", | ||
"10.0.0.99": "kali_2019", | ||
"10.0.0.86": "rhel_6", | ||
"10.0.0.87": "rhel_7", | ||
"10.0.0.88": "rhel_8", | ||
"10.0.0.77": "debian_7", | ||
"10.0.0.78": "debian_8", | ||
"10.0.0.79": "debian_9", | ||
"10.0.0.66": "oracle_6", | ||
"10.0.0.67": "oracle_7", | ||
"10.0.0.22": "ubuntu_12", | ||
"10.0.0.24": "ubuntu_14", | ||
"10.0.0.29": "ubuntu_19", | ||
"10.0.0.4": "windows_2003_r2_32", | ||
"10.0.0.5": "windows_2003", | ||
"10.0.0.8": "windows_2008", | ||
"10.0.0.6": "windows_2008_32", | ||
"10.0.0.12": "windows_2012", | ||
"10.0.0.11": "windows_2012_r2", | ||
"10.0.0.116": "windows_2016", | ||
"10.0.0.119": "windows_2019", | ||
} |
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 for this PR, but is it possible to parse the terraform file and build the dict from that?
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.
Possible yes, trivial - I don't think so. And definitely not worth the trouble.
Codecov Report
@@ Coverage Diff @@
## develop #507 +/- ##
==========================================
Coverage ? 56.15%
==========================================
Files ? 105
Lines ? 3588
Branches ? 0
==========================================
Hits ? 2015
Misses ? 1573
Partials ? 0 Continue to review full report at Codecov.
|
OS compatibility environment
Addresses #506
Creates an env. on AWS that allows us to test OS compatibility
Workflow as follows:
terraform apply
Checklist