Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#5589 closed defect (fixed)

validate_pluggable_transports_config() is not called after SIGHUP

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

Description

  /** 11c. validate pluggable transports configuration if we need to */
  if (!has_validated_pt &&
      (options->Bridges || options->ClientTransportPlugin)) {
    if (validate_pluggable_transports_config() == 0) {
      has_validated_pt = 1;
    }
  }

where has_validated_pt is a static variable. This means that after getting set to 1, validate_pluggable_transports_config() will never be called again (even after a SIGHUP).

Child Tickets

Change History (9)

comment:1 Changed 7 years ago by nickm

Seems like we ought to clear it when the relevant configuration is changed. Would that have side-effects?

comment:2 Changed 7 years ago by asn

The fix for this is not super-trivial since the relevant configuration in this case are Bridges: the flag should be cleared if a new bridge was added or removed.

I'm currently considering if completely removing validate_pluggable_transports_config() is a good idea, since #5602 implemented the exact same message in a different code path (see comment:3:ticket:5602).

comment:3 Changed 7 years ago by asn

Status: newneeds_review

Please see bug5589 in https://git.gitorious.org/mytor/mytor.git.

I've heard that fixing bugs by removing code is a good thing, and this is such a fix.

I've removed validate_pluggable_transports_config() and now rely on connection_or_connect() to issue the warning. Since tor will call connection_or_connect() when it tries to get the descriptor of a bridge, or when it tries to build a circuit through it, I think that the warning will be issued in all relevant cases.

comment:4 Changed 7 years ago by nickm

Status: needs_reviewneeds_revision

Seems plausible! Needs a changes file.

comment:5 Changed 7 years ago by nickm

Also, it creates a warning when I build with --enable-gcc-warnings:

main.c:1144:14: error: unused variable ‘has_validated_pt’ [-Werror=unused-variable]

comment:6 Changed 7 years ago by asn

Status: needs_revisionneeds_review

Please see bug5589_take2 in https://git.gitorious.org/mytor/mytor.git.

comment:7 Changed 7 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged; thanks!

comment:8 Changed 7 years ago by nickm

Keywords: tor-bridge added

comment:9 Changed 7 years ago by nickm

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