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

Unresolvable hosts prevent remaining results #16

Closed
moehlone opened this issue Jun 18, 2019 · 10 comments
Closed

Unresolvable hosts prevent remaining results #16

moehlone opened this issue Jun 18, 2019 · 10 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@moehlone
Copy link

Hi@

when a scan finishes before timeout you firstly check if something went to stderr.

Lets assume the following:

nmap -p 80,443 asdasasdasdasasdaasdassad.com google.de

First host goes to stderr with message Failed to resolve "asdasasdasdasasdaasdassad.com". and the second one finishes just fine.

The early error exit (currently https://github.com/Ullaakut/nmap/blob/master/nmap.go#L87)
from your done channel prevents access to any other successful results if just one fails :/

@Ullaakut
Copy link
Owner

Hi @moehlone, thanks for opening this issue.

I see your point and I agree it could make sense in some cases to still want to see results while an error happened during the scan, especially in a multi-target configuration; but at the same time returning both positive and negative results simultaneously seems very counter-intuitive to me, from a user's perspective.

Do you have suggestions on how you would like this change to be implemented? I don't see a way that seems idiomatic, intuitive and responds to this issue 🤔

Thanks :)

@Ullaakut Ullaakut added enhancement New feature or request question Further information is requested labels Jun 18, 2019
@TerminalFi
Copy link
Collaborator

TerminalFi commented Aug 13, 2019

Hi @moehlone, thanks for opening this issue.

I see your point and I agree it could make sense in some cases to still want to see results while an error happened during the scan, especially in a multi-target configuration; but at the same time returning both positive and negative results simultaneously seems very counter-intuitive to me, from a user's perspective.

Do you have suggestions on how you would like this change to be implemented? I don't see a way that seems idiomatic, intuitive and responds to this issue 🤔

Thanks :)

Why not just use the same logic you are using for rawXML ? This would give it the ability to store the errors, print errors first or after then success results.

Probably could be implemented better but its a solution.

type Run struct {
	XMLName xml.Name `xml:"nmaprun"`

	Args             string         `xml:"args,attr" json:"args"`
	ProfileName      string         `xml:"profile_name,attr" json:"profile_name"`
	Scanner          string         `xml:"scanner,attr" json:"scanner"`
	StartStr         string         `xml:"startstr,attr" json:"start_str"`
	Version          string         `xml:"version,attr" json:"version"`
	XMLOutputVersion string         `xml:"xmloutputversion,attr" json:"xml_output_version"`
	Debugging        Debugging      `xml:"debugging" json:"debugging"`
	Stats            Stats          `xml:"runstats" json:"run_stats"`
	ScanInfo         ScanInfo       `xml:"scaninfo" json:"scan_info"`
	Start            Timestamp      `xml:"start,attr" json:"start"`
	Verbose          Verbose        `xml:"verbose" json:"verbose"`
	Hosts            []Host         `xml:"host" json:"hosts"`
	PostScripts      []Script       `xml:"postscript>script" json:"post_scripts"`
	PreScripts       []Script       `xml:"prescript>script" json:"pre_scripts"`
	Targets          []Target       `xml:"target" json:"targets"`
	TaskBegin        []Task         `xml:"taskbegin" json:"task_begin"`
	TaskProgress     []TaskProgress `xml:"taskprogress" json:"task_progress"`
	TaskEnd          []Task         `xml:"taskend" json:"task_end"`
	Errors           []Errors

	rawXML []byte
}

type Errors struct {
	ErrorMessage string
}



func Parse(content []byte, errors error) (*Run, error) {

	r := &Run{
		rawXML: content,
	}

	for _, e := range strings.Split(errors.Error(), "\n") {
		r.Errors = append(r.Errors, Errors{ErrorMessage: e})
	}

	err := xml.Unmarshal(content, r)

	return r, err
}


case <-done:
		// Scan finished before timeout.
		var returnedErrors error
		if stderr.Len() > 0 {
			returnedErrors = errors.New(strings.Trim(stderr.String(), "\n"))
		}

		result, err := Parse(stdout.Bytes(), returnedErrors)

Example usage

func main() {
	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
	defer cancel()

	// Equivalent to `/usr/local/bin/nmap -p 80,443,843 google.com facebook.com youtube.com`,
	// with a 5 minute timeout.
	scanner, err := nmap.NewScanner(
		nmap.WithTargets("facebook.baddomain", "google.baddomain", "google.com.doesnasdasdasda", "facebook.com"),
		nmap.WithPorts("80,443,843"),
		nmap.WithContext(ctx),
	)
	if err != nil {
		log.Fatalf("unable to create nmap scanner: %v", err)
	}

	result, err := scanner.Run()
	if err != nil {
		fmt.Println("Errors encountered")
	}

	for _, e := range result.Errors {
		fmt.Printf("[ ERROR ] - %v\n", e.ErrorMessage)
	}

	// Use the results to print an example output
	for _, host := range result.Hosts {
		if len(host.Ports) == 0 || len(host.Addresses) == 0 {
			continue
		}

		fmt.Printf("[ SUCCESS ] - Host %q:\n", host.Addresses[0])

		for _, port := range host.Ports {
			fmt.Printf("\t\tPort %d/%s %s %s\n", port.ID, port.Protocol, port.State, port.Service.Name)
		}
	}

	fmt.Printf("Nmap done: %d hosts up scanned in %.2f seconds\n", len(result.Hosts), result.Stats.Finished.Elapsed)
}

@Ullaakut
Copy link
Owner

Sounds like something doable indeed! I'll look into that, thanks for the suggestion @TheSecEng

@TerminalFi
Copy link
Collaborator

#21 PR Resolves this

@TerminalFi
Copy link
Collaborator

Closed with #21

@Ullaakut can be closed now.

@noneymous
Copy link
Contributor

Awesome, I discovered the same bug and developed a patch, just to find out that you guys already had it fixed in a newer version ;-)

Anyways, I wanted to add something to this issue. The issue is/was not limited to errors. Nmap also writes warnings to stderr. Where else should it, stdout is reserved for valid xml data.

In our case this lead to the situation that the majority of our network scans failed without results, although there were no errors but warnings.

A common notification printed by Nmap (on stderr) is:

RTTVAR has grown to over 2.3 seconds, decreasing to 2.0

I think NSE scripts might also print some information.

The people from the Pyhon community once ran into the same issue:

    # If there was something on stderr, there was a problem so abort...  in
    # fact not always. As stated by AlenLPeacock :
    # This actually makes python-nmap mostly unusable on most real-life
    # networks -- a particular subnet might have dozens of scannable hosts,
    # but if a single one is unreachable or unroutable during the scan,
    # nmap.scan() returns nothing. This behavior also diverges significantly
    # from commandline nmap, which simply stderrs individual problems but
    # keeps on trucking.

(https://bitbucket.org/xael/python-nmap/src/default/nmap/nmap.py)

They basically had the same bug once and fixed it the same way we did. But they went one step further and tried splitting strerr into errors and warnings.

With our current code it is still impossible to tell whether relevant errors ocurred or just warnings. It is not useful to check results.errors if it may contain other stuff...

@noneymous
Copy link
Contributor

noneymous commented Nov 14, 2019

Furthermore, I just noted that we have three positions where errors need to be checked:

  • returned by Run()
  • results.NmapErrors
  • result.Stats.Finished.ErrorMsg

🤔

@Ullaakut
Copy link
Owner

@noneymous Thanks for your message! This is definitely an issue you're right 🤔 We should probably have some logic to abstract nmap's error handling and basically return an error only in Run(), which could contain the error(s) from any source, and exempted of warnings of course.

Could you open a new issue @noneymous with your initial comment, which describes the issue very well?

I'll look into implementing a fix as soon as I have some time, or if you feel like it feel free to give it a try :)

@TerminalFi
Copy link
Collaborator

TerminalFi commented Nov 14, 2019 via email

@noneymous
Copy link
Contributor

I took quite some time to properly prepare this issue. There it is: #38

I'm busy right now, but if no one is faster, I'm probably able to deliver something within a very few weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants