Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#5070 closed defect (fixed)

tor complains there's no ClientTransportPlugin line when actually it just can't launch the exe

Reported by: arma Owned by: asn
Priority: Medium Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: pt tor-bridge
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

When I set my torrc to say

ClientTransportPlugin obfs2 exec /path/to/nonexistent/file --managed

then the way Tor tells me it couldn't launch or find the file is by logging

Feb 10 03:23:28.000 [warn] Could not launch managed proxy executable!
Feb 10 03:23:28.000 [warn] You have a Bridge line using the obfs2 pluggable transport, but there doesn't seem to be a corresponding ClientTransportPlugin line.

That second line is confusing people. It's not true.

If I then change the line to

ClientTransportPlugin obfs2 exec /bin/false --managed

I get

Feb 10 03:29:57.000 [notice] Managed proxy stream closed. Most probably application stopped running
Feb 10 03:29:57.000 [notice] Failed to terminate process with PID '13672'
Feb 10 03:29:57.000 [warn] You have a Bridge line using the obfs2 pluggable transport, but there doesn't seem to be a corresponding ClientTransportPlugin line.
Feb 10 03:29:58.000 [notice] Bootstrapped 5%: Connecting to directory server.
Feb 10 03:29:58.000 [warn] Tried to connect through proxy, but proxy address could not be found.

Again the warn line is what people look to. Also, should the "it closed" be a warn in this case? Also also, what's with the "tried to connect through proxy" line -- why does it tell me the proxy address could not be found?

Child Tickets

TicketStatusOwnerSummaryComponent
#5602closedWhen managed proxy dies, we get "Tried to connect through proxy, but proxy address could not be found."Core Tor/Tor

Change History (13)

comment:1 in reply to:  description Changed 8 years ago by asn

Keywords: pt added

I splitted this ticket into two tickets -- #5601 and #5602.

I also prepared a small tor.git branch which promotes the Managed proxy stream closed. ... log message to warn severity. This log message happens in configure_proxy() (that is, when the managed proxy protocol takes place), so a user should be interested in it.'
Please see branch bug5070 in https://git.gitorious.org/mytor/mytor.git.

BTW, the false Failed to terminate process with PID '13672' messages are probably because of #4952.

comment:2 Changed 8 years ago by nickm

Status: newneeds_revision

Hm. Promoting those warnings is a good idea, but it won't do much good without clarifying them substantially. A warning message should be clear enough that it can tell the user what happened, whether they should care, and what action is required. So the "unknown stream status" message should probably be a LD_BUG, right? And both messages should probably say which proxy stopped running or couldn't be configured.

(btw, please remember to put a ticket into needs_review when it has a patch on it.)

comment:3 in reply to:  2 Changed 8 years ago by asn

Status: needs_revisionneeds_review

Replying to nickm:

Hm. Promoting those warnings is a good idea, but it won't do much good without clarifying them substantially. A warning message should be clear enough that it can tell the user what happened, whether they should care, and what action is required. So the "unknown stream status" message should probably be a LD_BUG, right? And both messages should probably say which proxy stopped running or couldn't be configured.

Makes sense.
Please see branch bug5070_take2 in https://git.gitorious.org/mytor/mytor.git.

It also improves logging around src/or/transports.c a bit.

comment:4 Changed 8 years ago by nickm

Looks good; have a look at the stuff I added in "branch5070_take2" in my public repository.

comment:5 in reply to:  4 Changed 8 years ago by asn

Replying to nickm:

Looks good; have a look at the stuff I added in "branch5070_take2" in my public repository.

I agree with your commit, and stating those assumptions seems like a good idea!

Maybe we should also do s/NUL\ pointer/NULL/g in parse_{server,client}_transport_line(), while we are at it, or should I open a new ticket?

    *tmp = NULL; /*terminated with NUL pointer, just like execve() likes it*/

comment:6 Changed 8 years ago by nickm

Worth fixing; will do when I merge this.

comment:7 Changed 8 years ago by nickm

Merged. (I also fixed a long line, and stared at the standard a little before deciding that I'm not sure whether printfing an enum with %d is allowed or not.)

I can't close this, because it has child tickets. Please close it when/if appropriate?

comment:8 in reply to:  7 ; Changed 8 years ago by asn

Replying to nickm:

Merged. (I also fixed a long line, and stared at the standard a little before deciding that I'm not sure whether printfing an enum with %d is allowed or not.)

I can't close this, because it has child tickets. Please close it when/if appropriate?

Will do.

(I didn't know that Using %d to printf an enum may not be by-the-standard okay.. Thanks.)

comment:9 in reply to:  8 Changed 8 years ago by nickm

Replying to asn:

(I didn't know that Using %d to printf an enum may not be by-the-standard okay.. Thanks.)

No problem. As I understand it, while C guarantees what the values of an enumeration's various enumerators are, it does _not_ guarantee the size of the enumerated type itself. The standard says that an enumerated type "shall be compatible with char, a signed integer type, or an unsigned integer type. The choice of type is implementation-defined..." So under the covers, an enum is _usually_ an int, but you aren't technically allowed to depend on that. (Again, this is assuming that I understand and am reading correctly.)

comment:10 Changed 7 years ago by ln5

Milestone: Tor: 0.2.3.x-finalTor: 0.2.4.x-final

This ticket no longer depends on anything still open for 0.2.3. Moving it to 0.2.4.

comment:11 Changed 7 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Closing this one; this was merged, then the child was merged and turned into a placeholder for something else.

comment:12 Changed 7 years ago by nickm

Keywords: tor-bridge added

comment:13 Changed 7 years ago by nickm

Component: Tor BridgeTor
Note: See TracTickets for help on using tickets.