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
Scripting Engine Implementation #242
Conversation
Right now it works exactly the same, nmap runs as before just with the new structure. So this is actually a working version, just for the record. :) |
#81 the issue |
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.
Ideally, scripts should also be able to access the config file. I think that'd open up a significant amount of customisation.
Here's my idea:
/// Finds the directory of where the scripts are stored
/// and returns a vector of fully qualified paths to each script
fn find_scripts{
}
/// Given a vector of scripts, store the tags of each script
/// in a pre-defined struct so we can easily search / sort them later
/// Some tags include:
/// * Language
/// * Developer
/// * Core Approved
/// * How to call the script, so `python3 {locatinon} --ip {ip} --port {port}
/// Our script will fill in the {} for them
fn get_tags{
}
/// Given structs and the tag system
/// Calculate what the tag does
/// I.E the tag(s) might be "(Popular and Core) not Port 80"
/// This returns a vector of paths (or the structs) that meet the tag requirement
/// We will also need to check compatiability with script.
/// If script is Python, do we have Python on the system?
/// If script requires port 80, do we have port 80 open?
/// If the script needs a host name instead of IP, do we have that?
/// You get the idea -- we calculate them instead of the script because
/// multiple scripts may want Python, so we will only need to calculate it once
/// also reduces complexity for script creators
fn calculate_tags_get_scripts{
}
/// Runs the scripts according to how the user says to run them
fn run_scripts{
}
Can you resolve comments after they're done? and re-request code reviews when you need them too :D |
1211395
to
653e1f7
Compare
After some discussions on discord, we are getting close to the final idea, (or maybe already there). Please check out the new implementation with examples and let me know what you think. @bee-san @bernardoamc It lacks some testing, hopefully we can make a Hacktoberfest issue from them. |
575dff7
to
28618b1
Compare
I made the adjustments we discussed previously, changed I know somebody was talking about the placement of the script_config file, and also renaming it. But I can't seem to find that comment. |
@bergabman conflicts! :( But other than conflicts, how's this coming? Are we ready to release? Do you need any help on anything? ❤️ |
.spawn() | ||
.expect("failed to execute nmap process"); | ||
// This part allows us to add commandline arguments to the Script call_format, appending them to the end of the command. | ||
if opts.command.len() > 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.
I wonder if it's make sense for us to keep the command
flag. I can't see how a command would be compatible with multiple scripts.
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 think it makes sense, in case the user want to simply append an extra argument to the execution of the scripts. For example in case of an nmap script, we give the possibility to just append extra arguments to RustScan that gets passed to the script. An extra -sC
switch for nmap will be just a quick append to the RustScan commandline arguments. If we remove it, the users will have to change the script file every time they want to append a new argument to it. This will also improve development time and experience for writing RustScan script files, as the writer of the script can test arguments in terminal and append them to the script execution format if they fit.
There are a few details/performance improvements that we can tackle, but if we are happy with the functionality we can always merge this and keep iterating on it. Great job on this @bergabman <3 The things that we can tackle later on:
|
I improved the PR based on the reviews. I think this version of the ScriptingEngine will be a good start to let people write custom scripts for RustScan. It certainly can be improved and as @bernardoamc said we should tackel efficiency points like using references and lifetimes. Luckily we don't have to parse a lot of script files as of now, and I can imagine people will most likely run 1 script with the results of the portscan, so memory allocations are not that big of a deal. But to have a more elegant and correct code we will have to tackle those points. The module lacks testing, but the example files in the |
Considering this issue is for an initial RSE, I see no reason why this shouldn't be merged :) The most important thing now should be writing docs on how to use the thing 😅 |
I started to implement the scripting engine how I thought it could work smooth. We can make this structure dynamic enugh to fill in all the necessary info we talked about before, like
core_approved
and description of the script. My idea would be to make the interpreted language files in a self contained way, which means the first 4-5 lines of the file could contain the tags in comment space for the interpreter, and the rest of the file can be the script itself.I'm not sure how we should handle binary files, if the user want's to run a binary we have to store the tags on a different location.
For a main rustscan_scripting config file we could use toml format, where we store some basic variables in case we have any. To read the scripting config, we could add a new arg like
--scripting
Please check it out how it looks and give comments how to imporve, or if it's even a good idea to do it this way. @bee-san @bernardoamc @CMNatic