[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #8195 [Tor]: tor and capabilities

#8195: tor and capabilities
-----------------------------------------------+---------------------------
 Reporter:  weasel                             |          Owner:
     Type:  enhancement                        |         Status:
 Priority:  Medium                             |  needs_revision
Component:  Tor                                |      Milestone:  Tor:
 Severity:  Normal                             |  0.2.8.x-final
 Keywords:  tor-relay, security, pre028-patch  |        Version:
Parent ID:                                     |     Resolution:
  Sponsor:                                     |  Actual Points:
                                               |         Points:  small
-----------------------------------------------+---------------------------
Changes (by asn):

 * status:  needs_review => needs_revision


Comment:

 Hello,

 nice code!

 here is an initial review. I didn't know anything about Linux caps before
 reviewing this, but I read a few things on the way.

 Some comments:

 - Should there be a log message for when a user has asked to use
 `KeepCapabilities` but their system does not have capability support
 (`HAVE_LINUX_CAPABILITIES` is not set)? As it is now, `switch_id()` will
 fail to set capabilitities silently, I think.

 - Also, in `switch_id()` the variable `keep_bindlow` will be unused (gcc
 error) if no `HAVE_LINUX_CAPABILITIES`.

 - If you run `make tests`, it will fail with an assertion error in
 `options_act_reversible()` because `have_low_ports` is still `-1`. That
 assert looks a bit hyperaggressive, can we relax it a bit?

 And some nitpicking:

 - Should we call the option `KeepCapabilities` if it only concerns the
 capability of binding to low ports? Are we going to introduce more caps in
 the future?

 - Might be nice to document `SWITCH_ID_KEEP_BINDLOW`. It confused me a bit
 before I realized it's just a cheap argument enum.

 - The comment of `check_server_ports` mispells the `n_low_ports_out`
 argument.

 I ran the tests in my system (with capabilities enabled) and they work.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/8195#comment:39>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs