Opened 7 years ago

Last modified 3 months ago

#2362 new defect

"GETINFO config-text" adds spurious DataDirectory, Log entries

Reported by: atagar Owned by:
Priority: Low Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-relay tor-control torrc easy
Cc: Actual Points:
Parent ID: Points: 2
Reviewer: Sponsor:

Description

Repro steps:

  • use a blank torrc (just setting the ControlPort)
  • run tor
  • issue a GETINFO get-text

result: it provides a 'Log notice stdout' entry
expected: we should only get back entries that differ from their defaults (in this case only the ControlPort)

from irc:
15:03 < armadev> atagar: it is a tor bug, i believe. i just reproduced.
15:03 < atagar> ok, I'll file a ticket - thanks :)
15:04 < armadev> if (smartlist_len(elts) == 0)
15:04 < armadev> smartlist_add(elts, tor_strdup("stdout"));
15:04 < armadev> in options_init_logs()
15:04 < armadev> that's where it comes from. we add it if there are no other logs.
15:04 < armadev> hmmmm. no, i take that back.
15:05 < armadev> /* Special case on first boot if no Log options are given. */
15:05 < armadev> if (!options->Logs && !options->RunAsDaemon && !from_setconf)
15:05 < armadev> config_line_append(&options->Logs, "Log", "notice stdout");
15:05 < armadev> there we go.

Child Tickets

Change History (9)

comment:1 Changed 7 years ago by arma

Milestone: Tor: 0.2.3.x-final

The trouble here is that logs are a linelist, so a controller that appends a new log entry to the linelist will leave the old one around, and our original behavior goal was to have the stdout log entry only when there aren't any others.

Option one is to ignore it until a controller that uses Tor where stdout is relevant actually wants to do this behavior of modifying log lines. Is arm there yet, atagar?

Option two is to tag torrc options with "not for save" or something. Ugh.

Option three is to special-case a stdout log and not write it if it's the only line and it wasn't explicitly asked for. Semi-ugh.

Option four is ...?

comment:2 Changed 6 years ago by atagar

Just discovered that it also autogenerates a nickname field if configured to be a relay. For instance with the torrc of:
ORPort 9001
ExitPolicy reject *:*
ControlPort 9051
CookieAuthentication 1

"GETINFO config-text" provides:
ORPort 9001
ControlPort 9051
Nickname fenrir
ExitPolicy reject *:*
CookieAuthentication 1

This isn't really a problem, though we should be better documenting SAVECONF (which by extension defines 'GETINFO config-text' behavior). Currently it doesn't really say what options it decides to save or omit - in practice this seems to be "Writes config options that differ from their defaults, with the exception of Log and Nickname which might be saved anyway".

comment:3 Changed 5 years ago by nickm

Milestone: Tor: 0.2.3.x-finalTor: unspecified

comment:4 Changed 5 years ago by nickm

Keywords: tor-relay added

comment:5 Changed 5 years ago by nickm

Component: Tor RelayTor

comment:6 in reply to:  2 Changed 3 years ago by arma

Replying to atagar:

Just discovered that it also autogenerates a nickname field if configured to be a relay.

For posterity: this behavior was removed in 0.2.2.25-alpha as part of #2979.

comment:7 Changed 3 years ago by arma

Imo the fix here should be to separate the list of logs that the torrc requests from the list of logs that are active right now. That way we can add a new temporary stdout log to the 'active' list without modifying the 'requested' list.

comment:8 Changed 3 months ago by nickm

Keywords: tor-control torrc added
Points: 2
Severity: Normal
Summary: get-text always reports Log entry"GETINFO config-text" adds spurious DataDirectory, Log entries

The current output, given an empty file with only a ControlPort, is:

ControlPort 9999
DataDirectory /home/nickm/.tor
Log notice stdout

comment:9 Changed 3 months ago by nickm

Keywords: easy added

The correct fix here is to apply the same pattern we apply elsewhere when we're thinking about changing the get_options() content depending on anything besides a new torrc or whatever: we have one option that represents the value-as-set-by-the-user, and one that represents the value-as-used.

Note: See TracTickets for help on using tickets.