[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #12088 [Pluggable transport]: goptlib should provide a method for querying the state location.
#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
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs