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

Features and recommendation suggestions #47

Closed
caribpa opened this issue Mar 5, 2021 · 39 comments
Closed

Features and recommendation suggestions #47

caribpa opened this issue Mar 5, 2021 · 39 comments

Comments

@caribpa
Copy link
Contributor

caribpa commented Mar 5, 2021

I've been checking AutoRecon, and there are some neat features (and other recommendations that occurred to me) that could be added to nmapAutomator:

  • When Support specifying more than one target at the same time #44 gets implemented, we could run a different nmap instance (simultaneously) for every target (using the shell's background task functionality &), though the logic of the code will need to change in order to prevent mixing the output of all the nmap instances
  • The same said above but for performing different scans over the same target simultaneously (unsure of the efficiency of this, though)
  • Add a -q/--quiet flag (and maybe another one only showing the progress-bar with the current scan, maybe -q shows the progress and -qq or -Q nothing), in case the user only wants to check the final file or folder with all the results
  • Add flags for saving the nmap output in different formats (-oG, greppable format is useful and may be parsed by different scripts)
  • Automatically detect if the terminal supports colors and disable them if it doesn't (this is really simple to implement), as well as some flags for forcing enabling or disabling them (--color, --no-color)
  • Allow custom scans with template files (see AutoRecon profiles), is way easier to implement for shell script as there's no need for intermediary template languages
  • Not a feature, just that it would be nice to indicate in the README that this script does not perform automatic exploitation, it is just an efficient wrapper of some enumeration tools, so that it can be used in the OSCP exam (and others with similar restrictions)
  • Also not a feature, just a recommendation that it would be wise to add a license (MIT, ISC, or BSD are great choices for the project and are basically the same), personally I like ISC better (is more compact 🙂)
@caribpa
Copy link
Contributor Author

caribpa commented Mar 5, 2021

The first two suggestions are optional flags inspired by AutoRecon -ct and -cs, btw

@21y4d
Copy link
Owner

21y4d commented Mar 5, 2021

I guess all are good ideas. To allow simultaneous scans we will need to make some big changes, so it's better to delay this until all other features are well implemented. Not sure how easy it would be to implement profiles, but also an interesting feature to consider once the script grows more.

@caribpa
Copy link
Contributor Author

caribpa commented Mar 5, 2021

Yes, of course. Until the current pull requests are merged and also the ones I plan to do afterwards (but slower) for refactoring the code a bit more, I won't start developing any of the other suggestions 🙂

@21y4d
Copy link
Owner

21y4d commented Mar 6, 2021

I added a license and contributing guidelines files.
Haven't considered a license before, but i guess with such tools it's always good to remove any liability.
Thanks for the tip.

@caribpa
Copy link
Contributor Author

caribpa commented Mar 6, 2021

One of the recommendations I also wanted to made (but forgot), is to ask the user at the beginning of the script if they want to run it as root (warning if some of the selected flags require it, and speed limitation of Connect Scan vs SYN Stealth):

  • If they answer "no", then don't ask in mid-script for the sudo password (like in places like this, blocking the script until the user inputs something) and add a warning with the skipped scan, because this current functionality prevents the script from being able to run 100% automatically
  • If the answer is "yes", re-launch the script with sudo: exec sudo $0 $@

@21y4d
Copy link
Owner

21y4d commented Mar 6, 2021

yeah i've implemented something like this in another script, but since in this case the only place where we need sudo is with UDP, i didn't want to run the entire script as root, as this is always a risk. I want to keep it limited to that UDP place, as in some scans users may not have root access 'i.e. lateral movement'.

perhaps the best option is to ask whether they need to run UDP as sudo, and skip if they don't, so that it doesn't hang the scan.

@caribpa
Copy link
Contributor Author

caribpa commented Mar 6, 2021

yeah i've implemented something like this in another script, but since in this case the only place where we need sudo is with UDP, i didn't want to run the entire script as root, as this is always a risk.

100% agree, that's why I suggested to ask the user at the beginning as I had in mind something more elaborate like dropping privileges for scans where definitively running as sudo didn't give any advantage (ffuf, and other third parties), as you can use sudo -u <user> to return to the non-root user. nmap definitely advantages from root permissions as it will be able to access RAW sockets (the reason it requires root for UDP scan), also needed for SYN scan (around 3 times faster than Connect scan).

@21y4d
Copy link
Owner

21y4d commented Mar 6, 2021

yeah that makes sense. we should give the option to run nmap as root if possible. for lateral movement users may not be able to use sudo at all, so sudo -u may not work.

probably if the user chooses to run as root, add sudo to $nmapType, which is used for all nmap commands.

@21y4d
Copy link
Owner

21y4d commented Mar 6, 2021

This should do it:

if [ $EUID -ne 0 ]; then
    echo -e "${RED}For faster nmap scans, we recommend running nmap commands with sudo..${NC}"
    echo -e "${Yellow}Run with sudo? yes/no${NC}"
    read -t 10 runSudo
    if [ "$runSudo" == "yes" ]; then
        nmapType="sudo ${nmapType}"
    fi
fi

@21y4d
Copy link
Owner

21y4d commented Mar 6, 2021

i'll make it POSIX and add this

@21y4d
Copy link
Owner

21y4d commented Mar 6, 2021

This is POSIX compatible:

        if [ "$(id -u)" -ne 0 ]; then
                printf "\n${Yellow}For faster nmap scans, we recommend running nmap commands with sudo..${NC}\n"
                printf "${Yellow}Run with sudo? yes/no${NC}\n"
                runSudo="$(sh -c '{ { sleep 1; kill -sINT $$; } & }; exec head -n 1')"
                if [ "${runSudo}" = "yes" ]; then
                        nmapType="sudo ${nmapType}"
                fi
        fi

It can be added to the end of the header() function.

But it is not timed, so it needs to be wrapped in a while loop.
But this introduces another issue. Since nmap output is written by root, it cannot be ready by normal users for other commands used in the progress bar..etc..
Perhaps the simplest solution to simply chmod +r the nmap output, though not sure how solid this is.

@caribpa
Copy link
Contributor Author

caribpa commented Mar 6, 2021

But this introduces another issue. Since nmap output is written by root, it cannot be ready by normal users for other commands used in the progress bar..etc..
Perhaps the simplest solution to simply chmod +r the nmap output, though not sure how solid this is.

If the file already exists (ie: it was touch by the non-root user), the permissions won't change even if root writes to it 🙂

@21y4d
Copy link
Owner

21y4d commented Mar 6, 2021

Yeah I guess that's a simpler solution. I'll add it and test that it doesn't cause any issues.

@caribpa
Copy link
Contributor Author

caribpa commented Mar 6, 2021

yeah that makes sense. we should give the option to run nmap as root if possible. for lateral movement users may not be able to use sudo at all, so sudo -u may not work.

I've been thinking about trying to substitute sudo with su, as su is an optional POSIX tool after all. Still, is not guaranteed that the system will have it, but way more likely than sudo (even a popular distro like Arch Linux doesn't come with sudo installed by default, but yes with su).

@21y4d
Copy link
Owner

21y4d commented Mar 6, 2021

Yeah you are right. Even a minimal Ubuntu image doesn't have sudo. Perhaps su is the way to go.

But we need to ensure it doesn't ask for passwords every time, or it will hang the script.

Perhaps switch user then exit before recon, but that doesn't seem very practical.

@caribpa
Copy link
Contributor Author

caribpa commented Mar 6, 2021

For lateral movement what I meant with the sudo/su is that the user gets asked whether they want the script to be run as root (after checking that the system has sudo/su):

  • They say 'no', then the script is run without root privileges (warning about the scans that cannot be performed)
  • They say 'yes', they are asked for the password (sudo -v, or su -), when it is introduced successfully, the current user is saved in a variable (nonRootUser=$USER), the script is relaunched with root privileges (exec sudo $0 $@), and, when going to run non-nmap tools, they get executed with sudo -u "${nonRootUser}" <tool>

To prevent making the logic of the code complex or repeating the same, a simple variable will do the trick:

...
[ -n "${nonRootUser}" ] && userRun="sudo -u ${nonRootUser}"
...
${userRun} ffuf ..  # Maybe this needs to wrapped in an `eval` to work
...

@caribpa
Copy link
Contributor Author

caribpa commented Mar 6, 2021

Yeah you are right. Even a minimal Ubuntu image doesn't have sudo. Perhaps su is the way to go.

But we need to ensure it doesn't ask for passwords every time, or it will hang the script.

Perhaps switch user then exit before recon, but that doesn't seem very practical.

I'll research about how to do it cleanly 🙂

@21y4d
Copy link
Owner

21y4d commented Mar 6, 2021

Yeah that logic makes sense. We should ensure that all output files are owned by the user and not root.

Excellent, thanks :)

@caribpa
Copy link
Contributor Author

caribpa commented Mar 6, 2021

Anyways, replacing sudo is not my priority for now, as there are some other things that I think are more important (in this order):

  1. Further refactor
  2. Add some more compatibility functionality (like check if the terminal can show colors and enable/disable them + --color and --no-color flags)
  3. Flip the positioning of the flags and values when running the script, so that the target is at the end, and make the --host flag optional
  4. Support multiple targets (but not simultaneously and without any major rework like the one proposed at the OP for the -ct and -cs AutoRecon flags)
  5. Implement the smart-parsing of flags (mentioned here)
  6. Split the --type in multiple flags that can be combined together and deprecate --type (though still recognize it as discussed in Allow combining individual scans using flags #45)
  7. Maybe add the -q and -qq (or -Q, still unsure) flags
  8. Maybe add the -oG flag (with a prettier name), and other output formats
  9. Replace sudo with su and the whole thing of asking the user to run as root, etc, as discussed right above
  10. Add an equivalent for the -ct and -cs flags, as well as correctly show the output without being mixed
  11. Recognize user-defined template files (shell files) for custom scans
  12. Everything else I am forgetting
  13. Maybe a vacation 🏝️

@21y4d
Copy link
Owner

21y4d commented Mar 6, 2021

Wow that's an amazing list of features :)
i've added the main ones to README.md.

i'll start working on adding a network scanning mode, enabling specifying nmapDirectory, and then replacing port/host scan with POSIX commands when nmap is not available. Perhaps after that even look to replace some recon commands with POSIX when not available.

No rush for anything though, you can work at your own pace :)

@caribpa
Copy link
Contributor Author

caribpa commented Mar 6, 2021

I'll try to do the first 2 tomorrow, as sometimes I have broken shells that don't like colors 😉

@caribpa
Copy link
Contributor Author

caribpa commented Mar 6, 2021

Btw, it would be nice to have some sort of small guide in the README (2, 3 lines) showing the commands to run for creating a static build of nmap (including some hints for cross-compiling) ready to deploy to compromised targets in order to use nmapAutomator from there 🙂

@21y4d
Copy link
Owner

21y4d commented Mar 6, 2021

yeah.. i think if the no color flag is set, changing the colors at the beginning to "" should stop nmapAutomator colors.
But you should also consider some special characters in the progress bar and reading user input, and perhaps some recon tools also output colors.

@21y4d
Copy link
Owner

21y4d commented Mar 6, 2021

oh yeah i'll definitely add that once the script accepts --nmap-dir..
users can simply use static-get tool to get a static version of many tools.
You can read mroe about it here: http://s.minos.io

@caribpa
Copy link
Contributor Author

caribpa commented Mar 7, 2021

yeah.. i think if the no color flag is set, changing the colors at the beginning to "" should stop nmapAutomator colors.

Yes, this is the idea.

But you should also consider some special characters in the progress bar and reading user input, and perhaps some recon tools also output colors.

POSIX awk accepts the \b escape, which basically moves the cursor back one character, allowing overriding something previously printed without using ANSI terminal escaping. I tried to use it in the script for trimming characters but I found some complication (because I didn't dig deep enough) and decided to replace it with a pipe to sed. This time I'll properly check if \b can be used (smartly/hackishly) for cleaning the lines in the countdown (using a broken shell to test it).

Regarding the output of other tools, I don't think we can do anything about it, if the user doesn't like it (and they don't have any flags for changing the color output) they can always use the (future) -q flag to simply print the progress bar 🤷

@caribpa
Copy link
Contributor Author

caribpa commented Mar 7, 2021

oh yeah i'll definitely add that once the script accepts --nmap-dir..
users can simply use static-get tool to get a static version of many tools.
You can read mroe about it here: http://s.minos.io

Didn't know about it! Thanks for the tip 🎉

@21y4d
Copy link
Owner

21y4d commented Mar 7, 2021

Excellent! I agree regarding the colors.

I just pushed the network hosts scan, and the static nmap flag.
I tested using a static nmap binary and it works fine, even on remote machine. The only thing that will not work is the Vulns scans, but this is expected, since it depends on the lua scripts, which do not come with a static binary.

@caribpa
Copy link
Contributor Author

caribpa commented Mar 7, 2021

The only thing that will not work is the Vulns scans, but this is expected, since it depends on the lua scripts, which do not come with a static binary.

Is there any way for providing the scripts separately and use a flag to tell nmap where to find them (like if they were installed in a non-standard path)?

@21y4d
Copy link
Owner

21y4d commented Mar 7, 2021

If it is possible to use custom scripts it should be possible. Perhaps later I'll look into it and check which scripts are being used, so nmapAutomator can put them in an archive to be transferred to the remote machine on the same nmappath, and if it finds the archive it'll extract the scripts and use them.

Currently the vulbs scan when used with a static binary simply outputs nothing, so the fix is not urgent.

@caribpa
Copy link
Contributor Author

caribpa commented Mar 7, 2021

Perhaps later I'll look into it and check which scripts are being used, so nmapAutomator can put them in an archive to be transferred to the remote machine on the same nmappath, and if it finds the archive it'll extract the scripts and use them.

I don't think you can expect to find the scripts in the same path in every system (and I'm not sure there's a portable way of finding them without using find in the root folder), so the easiest thing to do is to maybe leave that to the user (the packaging and sending of the script folder), and make nmapAutomator accept the --datadir (or the $NMAPDIR env variable, check the NSE manual).

@21y4d
Copy link
Owner

21y4d commented Mar 7, 2021

Yeah I guess that's probably better. I was mainly thinking to utilize the dirs nmap looks in.
Anyway, I'll look for the easiest and most convenient way to do it, and will apply it after testing.

@caribpa
Copy link
Contributor Author

caribpa commented Mar 7, 2021

Well, if nmapAutomator accepts the --datadir flag or $NMAPDIR variable to run the NSE scripts from the specified path, then it could also use the --datadir flag or $NMAPDIR variable to locate and archive the scripts in the given path like you said here 🙂

@21y4d
Copy link
Owner

21y4d commented Mar 8, 2021

initial implementation of Remote Mode pushed as well..

@caribpa
Copy link
Contributor Author

caribpa commented Mar 8, 2021

Perfect, I'll start refactoring later in the day 🙂

@21y4d
Copy link
Owner

21y4d commented Mar 14, 2021

I remembered why I didn't use --open initially.. it was because I was grepping for 'open' in the nmap output file, and as this is in the command it would be grpped as well.

Anyway, now it only affects one place 'recon', and it doesn't cause any issues.

@caribpa
Copy link
Contributor Author

caribpa commented Mar 14, 2021

Glad to know I didn't break anything 😅

Btw, I'm going to be quite busy with a personal matter for around a month and I don't think I'll be able to contribute until then, unfortunately 🙁

The most urgent things to fix (that are breaking the POSIX compatibility) are:

  • head -c is not POSIX (used here), replace it with sed
  • stty size is not POSIX (here), use to extract the columns either:
    • stty -a | sed -n 's/.*columns \([^;]*\).*/\1/p'
    • $COLUMNS variable (set by most modern shells)
    • or fallback to 80, as stty and $COLUMNS may not be available in some systems (for example my router 🙂)

Other compatibility issues come from:

  • ping with the timeout flag, currently nmapAutomator doesn't work in OpenBSD because of it, so I recommend removing the timeout flag and using this POSIX timeout alternative
  • Replace basename (used here) in favor of variable substring substitution: ${NMAPPATH##*/} as it may not be implemented in some systems

And I think that's it to regain the POSIX and compatibility for all systems 🏆

Note that the changes I mentioned above shall be done in more than one place (other than the linked ones) 🙂

@21y4d
Copy link
Owner

21y4d commented Mar 14, 2021

Thanks for the notes and for the contributions.

I'll work on fixing these and adding other features.

Wish you all the best

@caribpa
Copy link
Contributor Author

caribpa commented Mar 14, 2021

Btw, this is not goodbye, I seriously want to finish what I promised above when I am truly able to return, after all I use this tool in my assessments (and have a list of things to refactor, improve, and add), just wait a bit (as the people's-problems I am currently facing take longer to solve than software's) and keep me updated (here or in other issue) if you face some difficulties/dilemmas when adding more features 🙂

Keep up the good work! 🥇

@21y4d
Copy link
Owner

21y4d commented Mar 14, 2021

Yeah sure.. thanks for all the efforts, and I'll try to keep pushing features and fixes 😄

@21y4d 21y4d closed this as completed Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants