[tor-bugs] #13111 [Tor]: Tor fails to start if onion keys are zero length

[Tor Bug Tracker & Wiki]

#13111: Tor fails to start if onion keys are zero length
-------------------------+--------------------------------
     Reporter:  ioerror  |      Owner:  teor
         Type:  defect   |     Status:  needs_review
     Priority:  normal   |  Milestone:  Tor: 0.2.6.x-final
    Component:  Tor      |    Version:
   Resolution:           |   Keywords:  tor-relay lorax
Actual Points:           |  Parent ID:
       Points:           |
-------------------------+--------------------------------
Changes (by teor):

 * keywords:  tor-relay lorax easy => tor-relay lorax


Comment:

 Nick, I've created a git branch using enum FN_EMPTY in file_status() as
 you suggested, rather than my original strategy of creating a new
 file_size() function:
 Branch: bug13111-empty-key-files-fn-empty
 Repository: ​​​https://github.com/teor2345/tor.git

 This avoids cluttering the API, and eliminates a bug in the previous
 branch, where a FIFO with no data to read would be treated like an empty
 file (when, really, only regular empty files should be treated as empty).

 And despite my expectations, the implementation is cleaner and much less
 error-prone. Developers will have to explicitly handle the empty file case
 from now on, making the code clearer, too.

 '''Bug Fixes:'''
 * empty RSA & curve25519 key files - overwrite empty key files rather than
 failing to start tor
 * empty RSA & curve25519 key files - don't overwrite a .old key file with
 an empty file
 * missing RSA .old key file - stop generating a fresh .old RSA key file
 * stop crashing when a NULL filename is passed to file_status() - return
 FN_ERROR instead

 '''Improved Performance: (slightly?)'''
 * empty stats file while reading in extrainfo for router descriptor - skip
 reading file
 * empty router / extra info store files - skip reload
 * empty state file - skip load
 * empty key files - skip load
 * zero-length filename passed to file_status() - return FN_ERROR
 immediately

 '''Unit Tests:'''
 All unit tests pass.

 '''Coverage:'''
 After running make check, make test, benchmarks, and chutney --flavour
 bridges+ipv6, the modified file_status() function has been run 471 times.

 '''Testing:'''

 Check desired behaviour: zero-length key file -> regenerate
 {{{
 src/or/tor --ShutdownWaitLength 0 --DataDirectory /tmp/tor --ORPort 12345
 ^C # when the keys have been generated
 ls -lh  /tmp/tor/keys/
 rm /tmp/tor/keys/secret_id_key
 touch /tmp/tor/keys/secret_id_key
 rm /tmp/tor/keys/secret_onion_key
 touch /tmp/tor/keys/secret_onion_key
 rm /tmp/tor/keys/secret_onion_key_ntor
 touch /tmp/tor/keys/secret_onion_key_ntor
 src/or/tor --ShutdownWaitLength 0 --DataDirectory /tmp/tor --ORPort 12345
 ^C # when the keys have been regenerated
 ls -lh /tmp/tor/keys/
 }}}

 Ensure previous behaviour: no key file -> regenerate
 {{{
 src/or/tor --ShutdownWaitLength 0 --DataDirectory /tmp/tor --ORPort 12345
 ^C # when the keys have been generated
 ls -lh /tmp/tor/keys/
 rm /tmp/tor/keys/secret_id_key
 rm /tmp/tor/keys/secret_onion_key
 rm /tmp/tor/keys/secret_onion_key_ntor
 src/or/tor --ShutdownWaitLength 0 --DataDirectory /tmp/tor --ORPort 12345
 ls -lh /tmp/tor/keys/
 }}}

 Nick, should I incorporate these tests into the unit tests? Or maybe the
 extra (python-based) tests?
 If so, how can I do that, and which file should I edit?

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/13111#comment:8>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online