Re: [Patch] Automatically switch to privileged when Nmap has required capabilities

[Jay Bosamiya <jaybosamiya () gmail com>]

On Monday 18 August 2014 10:56 PM, Daniel Miller wrote:
> On Mon, Aug 18, 2014 at 11:18 AM, Patrick Donnelly
> <batrick () batbytes com <mailto:batrick () batbytes com>> wrote:
>
>     On Sun, Aug 17, 2014 at 9:31 AM, Jay Bosamiya
>     <jaybosamiya () gmail com <mailto:jaybosamiya () gmail com>> wrote:
>     > If capabilities exist for the nmap executable file, then use them
>     > If --unprivileged is used, then drop privileges (and also drop
>     capabilities)
>     > If run as root, then drop privileges without losing the
>     capabilities that it
>     > needs
>     > The user to drop to is currently fixed to "nobody" but the code
>     has been
>     > written in such a way that it will be trivial to add a CLI
>     option to choose
>     > the user to drop to (just have to do a `o.drop_user =
>     strdup(optarg);`). I
>     > have currently not added such a CLI option because of being
>     unsure of the
>     > security implications of this.
>     > Two new make targets
>     >
>     > "make setcap" grants capabilities to the nmap executable in the same
>     > directory
>     > "make setcap-install" grants capabilities to nmap executable in the
>     > installation path
>     >
>     > Feedback is welcome as always :)
>
>     Great work on this patch Jay! One last comment/question: how does this
>     change effect opening various files at startup, especially log files?
>
>     e.g. if as root I do:
>
>     $ pwd
>     /root
>     $ nmap -A localhost -oN nmap.txt
>
>     will that fail because nmap can't open nmap.txt after dropping
>     privileges? What about opening script files/libraries?
>
>     Do we/should we start opening these files ahead of time?
>
>
> Additionally, David brought up that some NSE scripts (whois-ip, I
> believe was the example) have the ability to save to output files,
> which would mean opening them later than privileges were dropped. I
> think at a minimum we need to make sure our error messages reflect
> that this may be the cause of a problem in opening a file.
>
> Another thought would be to explicitly limit capabilities to the
> required set, rather than just letting the setuid call do it. This
> way, we could support either of two workarounds:
>
> 1. Expose the uid to the CLI as you suggested, letting the user do
> --drop-user 0 or --drop-user root to avoid changing the UID at all.
> This means that a successful exploit against Nmap would be able to
> re-enable all capabilities, negating the security benefit, but it
> would at least be there for the majority of cases, or
>
> 2. All of 1, plus we default to not change the UID. This means
> complete backwards-compatibility for now, with the user able to choose
> to drop privileges if they want to. Coupled with appropriate warnings,
> this could give users some time to adjust to the idea before we would
> make dropping privileges the default in a future release.
>
> I am just brainstorming some possible solutions; I know that there are
> major downsides to each of these, but we need discussion to decide
> which way to go.
>
> Dan
I had completely missed out on the opening-of-files fact. If we use the
attached (modified) patch, then it should no longer be a problem for the
log files whatsoever.
Opening script files/libraries seems to work correctly as well.

However, if there are NSE scripts that open files, then we may have some
problems in this dropping-of-privileges thing. If we don't drop
privileges for scripts, then we are back to Nmap being vulnerable to
some malicious scripts, and if we drop privileges, then we will be
unable to use some features (such as writing files). This might be
something necessary to look into.

I'm not sure what you mean by "explicitly limit capabilities to the
required set, rather than just letting the setuid call do it". If you
look at the patch, it adds a function `drop_privileges_to(const char
*username, bool keep_capabilities)` that allows you to decide whether
you want to keep capabilities or not. If you decide to keep
cababilities, then the only ones that are kept are the ones which are in
CAPABILITY_LIST which is defined in nmap.h to be {CAP_NET_RAW,
CAP_NET_ADMIN, CAP_NET_BIND_SERVICE}.

As for point 1, I mentioned that the addition of the new CLI option will
be a very minor change the way I've written the code (just have to do a
`o.drop_user = strdup(optarg);`). However, this may have security
implications (though, no worse than what we have right now), and so I
did not implement it.

As for point 2, the security feature would then basically become an
opt-in and so would be benefiting very few users. I would prefer it
being an opt-out feature (i.e. enabled by default, and only users that
have a problem with it can disable it).

Hope to see more discussion on this before we come up on a final
solution since this really affects Nmap's security front a lot. :)

Cheers,
Jay

Attachment: capabilities_with_drop_privileges.patch
Description:

_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/