[tor-bugs] #12088 [Pluggable transport]: goptlib should provide a method for querying the state location.

[Tor Bug Tracker & Wiki]

#12088: goptlib should provide a method for querying the state location.
-------------------------------------+------------------------------
     Reporter:  yawning              |      Owner:  dcf
         Type:  enhancement          |     Status:  needs_review
     Priority:  normal               |  Milestone:
    Component:  Pluggable transport  |    Version:
   Resolution:                       |   Keywords:  goptlib, library
Actual Points:                       |  Parent ID:
       Points:                       |
-------------------------------------+------------------------------

Comment (by dcf):

 Replying to [comment:1 dcf]:
 > Proposed patch:
 >  * https://gitweb.torproject.org/pluggable-
 transports/goptlib.git/commitdiff/15b960d55905877a840fe605a41a8139bffb5329

 Thanks for this patch. My thinking is that the API should work a bit
 differently. I don't think ClientSetup and ServerSetup should silently
 create a directory as a side effect, and I don't think
 TOR_PT_STATE_LOCATION should be required to be set for transports that
 don't use the state directory.

 Here's what I'm thinking for the API:
 {{{
 // Return the directory name in the TOR_PT_STATE_LOCATION environment
 variable,
 // creating it if it doesn't exist. Returns non-nil error if
 // TOR_PT_STATE_LOCATION is not set or if there is an error creating the
 // directory.
 func MakeStateDir() (string, error) {
         dir, err := getenvRequired("TOR_PT_STATE_LOCATION")
         if err != nil {
                 return "", err
         }
         err = os.MkdirAll(dir, 0700)
         return dir, err
 }
 }}}
 (With no changes to ClientInfo and ServerInfo, and nothing involving
 TOR_PT_STATE_LOCATION in ClientSetup and ServerSetup.)

 I don't know what's the best way to deal with errors from os.MkdirAll. The
 patch in comment:1 crafts a new ENV-ERROR, which is reasonable, even
 though failure to create a directory is not really an environment error.
 It is a bit awkward that the function above will print an error to stdout
 in one case, but not in the other. On the other hand, it doesn't matter so
 much, because any error is likely to be a fatal error; i.e.
 {{{
 stateDir, err := pt.MakeStateDir()
 if err != nil {
         log.Fatalf("failed to create state directory: %s", err)
 }
 }}}
 After all, there are many other errors that can happen during the
 execution of a transport (like the ORPort unexpectedly closing), and
 there's currently no way to report generic errors back up to tor other
 than simply terminating. Failure to create a directory could be considered
 one of these generic errors.

 What's your impression, how well would this approach work for obfs4?

 I think we need some tests for this new function:
  1. TOR_PT_STATE_LOCATION not set.
  1. Already existing directory.
  1. Nonexistent directory, parent exists.
  1. Nonexistent directory, parent doesn't exist.
  1. Name already exists, but is an ordinary file.
  1. Directory name that cannot be created (e.g., no write permission in
 parent).

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