Ullaakut / nmap

Idiomatic nmap library for go developers
MIT License
922 stars 102 forks source link

Fixing "Error handling difficult and not reliable #38" #39

Closed noneymous closed 4 years ago

noneymous commented 4 years ago

Here comes the implementation of my suggestion to fix https://github.com/Ullaakut/nmap/issues/38. Please have a look and see how this works for you. I tested it against some scan targets and am still running it against even more. You are probably using different Nmap arguments than me, so I'm curious about your experiences.

Result processing would be like: 1) Check if error is returned (includes package errors and nmap errors). If error is returned, something went most probably wrong and you can abort. Warning's might give you more details/flexibility. Most returned errors are defined as error constants in errors.go. Have a look there to see what issues might come back to handle them individually 2) OptionA: Check warnings for things that might be an issue for you and abort or proceed OptionB: Ignore warnings at all and just work with the returned XML. It already contains a lot of details

Here is sample usage code:

// Prepare Nmap scan
proc, err := nmap.NewScanner(options...)
if err != nil {
    fmt.Println("Could not initialize Nmap")
    return
}

// Execute Nmap scan
result, warnings, err := proc.Run()

// Check for nmap errors and optionally handle them individually
if err != nil {
    switch err {
        case nmap.ErrScanTimeout:
            fmt.Println("ERROR: Nmap scan ran into timeout")
        case nmap.ErrParseOutput:
            fmt.Printf("ERROR: Nmap output could not be parsed: %w\n", warnings[len(warnings)-1])
        case nmap.ErrExcludeList:
            fmt.Println("ERROR: Nmap could not resolve host(s) on exclude list")
        default:
            fmt.Printf("ERROR: Nmap scan failed with unexpected error: %w\n", err)
    }
    return
}

// Check for nmap warnings that might be relevant for you
for _, warning := range warnings {
    switch {
        case strings.Contains(warning, "Failed to resolve"):
            fmt.Println("A target could not be resolved.") 
        case strings.Contains(warning, "No targets were specified")
            fmt.Println("The only target is blacklisted.")
        case strings.Contains(warning, "TTVAR has grown")
            fmt.Println("Just some TTVAR change notification.")
        default:
            fmt.Println(fmt.Sprintf("Other warning: %s", warning))
    }
}

// Process result
fmt.Println(result)
noneymous commented 4 years ago

Good idea, I introduced named return values and it's already commited.

Just the warning unit test outstanding. Maybe tomorrow.

noneymous commented 4 years ago

Sorry, I'm in the wrong environment to get the tests running. I feel this would take me too much time but might be a simple thing for you...

TerminalFi commented 4 years ago

Awaiting Unit Tests

Ullaakut commented 4 years ago

I'll test it this weekend :) It's on my calendar now:

Screenshot 2019-11-22 at 10 20 42 AM
noneymous commented 4 years ago

Thank you guys

noneymous commented 4 years ago

What's the plan, when do you intend to merge?

TerminalFi commented 4 years ago

@noneymous we will merge once tests have been completed. We thank you for your patience. Both myself and @Ullaakut have busy schedules at times.

Ullaakut commented 4 years ago

I'll wait for @TheSecEng to test it as well and once it's fine for him, we can merge the PR :)

TerminalFi commented 4 years ago

Cleared.