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

Documentation does not match the actual config format - 'Application Options' does not get parsed at all #337

Open
Nirusu opened this issue Dec 21, 2021 · 5 comments

Comments

@Nirusu
Copy link
Contributor

Nirusu commented Dec 21, 2021

Basically the same as #236, so it's a duplicate... But since @mzpqnxow closed the issue with this still being quite a major issue, I am reposting this.

The README.md says you can define a config file like this:

[Application Options]
output-file="output.txt"
input-file="input.txt"
[http]
name="http80"
port=80
endpoint="/"
[http]
name="http8080"
port=8080
endpoint="/"
[ssh]
port=22

And pass it as: ./zgrab2 multiple -c multiple.ini.

It also says:

Application Options must be the initial section name.

Now, this is in fact not true. zgrab2 does not parse this section AT ALL. You can put any other global setting in there, it is getting ignored.

I am not sure how this even happened... But as a workaround, everything you want to pass in the config file as Application Options, you need to pass to argv.

So, if you want to pass input and output, you need to do it like this: ./zgrab2 -f input.csv -o output.csv multiple -c multiple.ini.
Same goes for other global options. For example, if you want to increase the size limit per scan (here, to 1 GiB), you need to do it as followed:
./zgrab2 -f input.csv -o output.csv multiple -c --read-limit-per-host 1048576 multiple.ini.

Quite a major issue, in my eyes...

@mzpqnxow
Copy link
Contributor

mzpqnxow commented Jan 4, 2022

Basically the same as #236, so it's a duplicate... But since @mzpqnxow closed the issue with this still being quite a major issue, I am reposting this.

I would love to see it fixed- it's a big annoyance for me but it doesn't prevent me from doing what I need. That's why I (personally) wouldn't call it major

I am not sure how this even happened

Me neither, from what I can tell there is no reference to Application Options anywhere in the source- so either it's a relic of a very, very, very old revision that didn't get merged along with the docs for it. It may even be from zgrab, which predates zgrab2.

Because there's not even a basic stub that looks at the Application Options section, the only fix options as I see them are:

  1. Update the documentation and remove references to Application Options
  2. Implement the Application Options "from scratch"

Normally I would send a PR myself, but because the functionality is missing entirely as opposed to misbehaving or broken, I didn't want to spend the time on it- plus I figured I was the only one bothered by it ;)

I'll follow this issue and hope you or someone else send's a PR, but I'm not holding my breath

FWIW- I'm not a part of the zmap team but based on my experience contributing small bug-fixes and features, I'm sure they would accept a PR for this as long as it doesn't impact anything else

@Nirusu
Copy link
Contributor Author

Nirusu commented Jan 4, 2022

I'd call it major because the docs are pointing to Application Options as the main way to use the multiple mode, even though it does literally nothing and in some cases it starts zgrab while waiting from data from STDIN, which is not obvious at all except that you do not get any output. But yeah, I guess you can argue on the priority, because if you know the workaround, it's more a nuisance rather than a major issue ;)

I also digged a bit around since I was thinking of creating a PR for this, too... From my understanding of the code, it turns out the Application Options parsing is part of zflags, and is coded specifically to ignore this section when parsing an INI. Instead, you are supposed to pass a struct holding this section.

Now, when creating the parser, you can pass a struct holding the Application Options and, the unit tests from zflags also do it thisway..

I guess you can make zgrab creates the INI parser, this way without patching zflags , but when I gave it a quick shot I ran into getting errors that some options have been defined multiple times and zgrab exited, at which point I gave up since I am not sure if I was doing this the right way or not for this to get this through a PR, since I found the way this is handled a bit confusing. Creating a parser which wants data ahead of time from the file it is supposed to parse seems like a weird design choice to me, but then I could be confused as to how this exactly works or is supposed to work.

@mzpqnxow
Copy link
Contributor

mzpqnxow commented Jan 9, 2022

it turns out the Application Options parsing is part of zflags, and is coded specifically to ignore this section

Ah! That explains why I was unable to find a reference to the "Application Options" string literal in the zgrab2 source

Now, when creating the parser, you can pass a struct holding the Application Options and, the unit tests from zflags also do it thisway..

So maybe a PR wouldn't be as much effort as I thought since there is in fact an existing interface for it

I guess you can make zgrab creates the INI parser, this way without patching zflags , but when I gave it a quick shot I ran into getting errors that some options have been defined multiple times and zgrab exited, at which point I gave up since I am not sure if I was doing this the right way or not for this to get this through a PR, since I found the way this is handled a bit confusing

I see, I'll take a look if I have some time. Now that you've tracked down both the interface itself and uses in the test-suite it seems like less of a time investment, even if using it in zgrab2 as it is now is not intuitive. It could be that the changes required to zgrab2 are very invasive in which case I'll probably also give up

Creating a parser which wants data ahead of time from the file it is supposed to parse seems like a weird design choice to me, but then I could be confused as to how this exactly works or is supposed to work.

Maybe this helps answer the question as to why it wasn't actually used in the end ;)

@LloydLabs
Copy link

+1 for this, I was wondering why some of my captures when using the multiple module were being truncated. The work-around works fine for now.

@mzpqnxow
Copy link
Contributor

@Nirusu any time/interest in providing a PR for this? I'm sifting through issues to set aside time to work on. I've identified a few but given the limited time I have to work on them, I won't get to all or even most of them 😞

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

3 participants