pry0cc / axiom

The dynamic infrastructure framework for everybody! Distribute the workload of many different scanning tools with ease, including nmap, ffuf, masscan, nuclei, meg and many more!
MIT License
4k stars 622 forks source link

feat(aws): stability and bugfixes #635

Closed nicoandmee closed 1 year ago

nicoandmee commented 1 year ago

Status: WIP, fixing outstanding issues

The AWS integration is far less reliable than DO- for that reason, I've gone through and fixed multiple bugs that were preventing me (and likely others...) from using the mother of all cloud providers.

providers/aws-functions.sh

interact/account-helpers/aws.sh

Coming soon...

@pry0cc Advice would be most welcome, as to if that makes sense given how the code in interact/axiom-fleet currently reads.

0xtavian commented 1 year ago

@nicoandmee first off, thanks for the impressive PR! Secondly, you are right about how axiom-fleet works and the limitations in our code wrt aws. LMK if I got this right, we store the AMI value in axiom.json, the current AMI value will change when the image is transferred and the newly created image in the new region also has a new AMI id. So we need a different way to reference the image, one that could be feed back into some function to get the correct AMI ID. im open to the idea of tagging, maybe there is another unique value returned by the api which we can add to the axiom.json, such as a timestamp.

I’m going to be implementing axiom-images features to aws-functions.sh just to get familiar with how images work.

do you have an example AWS cli commands of how to simply transfer an image (and I guess AMI) to a new region and spin up an axiom instance?

Feel free to join our discord as well https://discord.com/invite/c6BHVfn

thanks again for your support!

nicoandmee commented 1 year ago

@pry0cc Mostly right I think, except the original AMI id remains unchanged. Just the one in new region will have a new ID. When building the fleet in that region, we need to retrieve that ID and pass it to axiom-init.

Upon reviewing the docs a little closer, I think it might be possible to keep the same structure (a single .json file, I mean). Bash seems like a poor place for complexity haha. Using copy-image we can at least keep the name the same as the original AMI, si? Then, rather than using the ID as our key, we can look for an AMI ID with that name. We have the fx get_image_id who can do that.

Also, when we do the copy we can use image-available to make sure the copy is done before proceeding.

Added a commit includes the above:

If I'm thinking about this correctly, it should resolve https://github.com/pry0cc/axiom/issues/615 🙏 p.s. I guess I forgot - do we need to save the name in the .json config as well? Since that will identify the AMI regardless of region.

0xtavian commented 1 year ago

@nicoandmee lots of issues with the latest commit. The most important being that it breaks all other cloud providers, but that might be a simple fix by putting these lines into a conditional if statement and checking the provider. copy_image and get_image_id require three arguments (for aws-functions.sh) and only two are provided. aws ec2 copy-image needs a name, which can be taken from name=$(cat "$AXIOM_PATH"/axiom.json | jq -r .imageid) but its also possible to have multiple images with the same name, in the same region or in different regions, so we need another identifier stored in axiom.json.

p.s. I guess I forgot - do we need to save the name in the .json config as well? Since that will identify the AMI regardless of region.

By name you mean AMI ID? We already save the "name" in the axiom but not the AMI ID. Yea i'm thinking we'll have to add at least one more key:value pair for aws' axiom.json, which shouldnt be an issue.

nicoandmee commented 1 year ago

@0xtavian

@nicoandmee lots of issues with the latest commit. The most important being that it breaks all other cloud providers, but that might be a simple fix by putting these lines into a conditional if statement and checking the provider.

Ah, indeed that is not ideal. Will fix and test with linode/do to make sure its cross-compatible. Just to confirm- the issue is images aren't assoc. with regions on the other cloud providers in the same manner as AWS. So, I should only pass the region param in if we are dealing with AWS.

copy_image and get_image_id require three arguments (for aws-functions.sh) and only two are provided. aws ec2 copy-image needs a name, which can be taken from name=$(cat "$AXIOM_PATH"/axiom.json | jq -r .imageid) but its also possible to have multiple images with the same name, in the same region or in different regions, so we need another identifier stored in axiom.json.

Yeah, so this is the source of some confusion I think. imageid is the ID, not the name.

image

When the image is copied, we are able to name it, but the ID is autogenerated. Therefore- rather than storing the id as the name (current behavior), we can just store both the name and the id of the AMI in the default region.

When working on other region, we can retrieve the ID using that name. Yes - there can be AMIs with the same name, even in the same region. I was thinking to handle that we can sort the results by creation date, and use the latest. Just trying to avoid the need to store the additional unique AMI id for each region.

~Related to this, what is the purpose of "$query" here: https://github.com/pry0cc/axiom/blob/master/providers/aws-functions.sh#L79 ? If we filter down to the name --> pop from top (latest), it should be unnecessary I think?~ I understand now, query is simply the stuff we want to grab from the image (attributes).

By name you mean AMI ID? We already save the "name" in the axiom but not the AMI ID. Yea i'm thinking we'll have to add at least one more key:value pair for aws' axiom.json, which shouldn't be an issue.

Kind of addressed this above I think but tldr; we should store both the name (which will be shared across all regions used) and the initial AMI ID. Will hack around a bit more and do some testing 🧪 thank you for your help!

0xtavian commented 1 year ago

@nicoandmee Sorry for messing this PR up so bad. We should extract the useful changes and apply them to master. I'll be more careful in the future.