Re: [tor-bugs] #18322 [Core Tor/Tor]: Log unparseable votes so they can be analysed

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#18322: Log unparseable votes so they can be analysed
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:  andrea
     Type:  defect                               |         Status:
 Priority:  Medium                               |  needs_revision
Component:  Core Tor/Tor                         |      Milestone:  Tor:
 Severity:  Normal                               |  0.2.9.x-final
 Keywords:  tor-dos, TorCoreTeam201606, review-  |        Version:
  group-4                                        |     Resolution:
Parent ID:  #17293                               |  Actual Points:  3
 Reviewer:                                       |         Points:  2
                                                 |        Sponsor:
                                                 |  SponsorU-can
-------------------------------------------------+-------------------------

Comment (by andrea):

 > Don't hate me for this, but: we can't use size_t for the total size of a
 bunch of these files, I think. On 32-bit systems, size_t is 4 bytes, but
 file sizes can often be 8 bytes.

 Files we generate here can't be, since they had to fit in process memory
 to get written, but I guess you can make up some weird case where we also
 do the scanning for existing files thing, and the user switches between
 running a 32-bit and a 64-bit Tor on the same directory.  I'll switch it
 to uint64_t, though, just to be safe.

 > We don't have to say "if (foo) tor_free(foo);". Just say
 "tor_free(foo)."

 Okay.

 > It looks like the dumped descriptor limit is only applied over a number

 What does this mean?

 > Should these files go into a subdirectory of the datadir? That would
 give sysadmins the ability to stick it on another volume, give it
 different permissions, etc.

 Okay.

 > What do we do about NUL characters?

 If we decided something with a NUL in the middle of it constitutes a
 descriptor, we broke somewhere before we ever got to dump_desc(), since
 that function takes the bad descriptor as a NUL-terminated string.  I'd
 call that worth checking out what does happen, but orthogonal to this
 ticket.  If it can happen, we'd handle it by passing a length arg into
 dump_desc() and using write_mem_to_file() instead of write_str_to_file()
 for output, I suppose.

 > Shouldn't we scan the unparseable-desc files at startup, and count how
 big they are and when they were last modified? Otherwise we will still up
 the disk eventually if Tor stops and starts enough times.

 Okay.

 > This approach will break with the linux seccomp2 sandbox, since we can't
 predict the filenames in advance. But IMO it'll be fine to just say "if
 the sandbox is on, don't store these to disk."

 I'll add that check.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/18322#comment:22>
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