torrc : ClientOnionAuthDir after include directives breaks client to v2 services

If I append these two statements to torrc, in this order :

ClientOnionAuthDir /etc/tor/auth/ %include /etc/tor/torrc.d/

and restart tor.service, I can connect to 1 × v3 and 4 × v2 services, within seconds.

But if I reverse their order ( %include first ), I can only connect to the v3 service -- all other connections will eventually time out.

In man, I missed to find any recommandation about ordering these statements.

( this is on Debian Stretch with torproject's stretch repo )

Thanks!

Trac:
Username: xaho

changed milestone to %Tor: 0.3.5.x-final

added 035-backport 040-backport 041-backport 042-backport 042-must BugSmashFund actualpoints::0.4 component::core tor/tor consider-backport-after-0423 milestone::Tor: 0.3.5.x-final owner::asn points::0.3 priority::very high regression reporter::xaho resolution::fixed severity::normal status::closed tor-hs type::defect version::tor 0.3.1.1-alpha labels

n.b. if it matters, all of these 5 services are set with an auth/stealth key

Trac:
Username: xaho

Trac:
Keywords: torrc include ClientOnionAuthDir order deleted, tor-hs regression added
Milestone: N/A to Tor: 0.4.2.x-final

Trac:
Keywords: tor-hs regression deleted, tor-hs regression 042-must added

Make all 042-must objects "Very High" priority.

Trac:
Priority: Medium to Very High

What's in the rest of your torrc file? How many files are in torrc.d, and what's in them?

(We have a new option testing framework in #31637 (moved), we could try using it to replicate this issue. And to ensure it doesn't happen again.)

In torrc, the above two lines are the only non-commented lines, appended to the maintainer's version.

In torrc.d, there are twelve files containing one or more commented or empty lines, and only one non-commented HidServAuth statement each, all in the format :

HidServAuth name.onion stealthkey # client : hostname

Yet another file is named 'wip' and contains only commented lines. :)

Trac:
Username: xaho

Trac:
Owner: N/A to asn
Status: new to assigned

I think it might be useful to have both torrc and the actual files within /etc/tor/torrc.d/ with any sensitive information replaced by 'x'. I am thinking this could be a parsing bug and things like whitespace and new line differences and empty files vs files with only comments/whitespace could matter here.

I chased the culprit and it is... wip ! :) As I wrote, it has only comments and empty lines.

Now to break the config/option parser, I just need any file in torrc.d, with a single # char in it and still, order ClientOnionAuthDir after %include in torrc, as reported.

An empty file or line, seems not enough to break the option parser : it has to contain at least one, or more comment(s), but only comments and ( optional ) empty lines, so I suspect, the file as a whole end up « nullified ». Inserting any valid option in that file, does fix the issue.

Trac:
Username: xaho

The cause of this problem is a bug in the processing of included folders containing files with only comments or whitespace. I was able to reproduce this problem with the following configuration:

torrc:

%include /etc/tor/torrc.d/
Log notice stderr # can be any valid option

/etc/tor/torrc.d/01_invalid:

InvalidOption

/etc/tor/torrc.d/02_comment:

# comment

Running tor --verify-config -f torrc will incorrectly identify this as a valid configuration despite the invalid option on /etc/tor/torrc.d/01_invalid.

The bug is that any files on a %included folder where the last file contains only comments or whitespace are being ignored. Also the bug is only triggered if there is a valid option after the %include.

The cause of this bug is that, when creating the list of configurations options from the included files, a pointer to the last element of the list is incorrectly set to NULL when processing a file with only comments or whitespace. This can cause the options list to be built incorrectly, causing this behaviour.

I've made a pull request with a fix for this problem and a test case: https://github.com/torproject/tor/pull/1347

Trac:
Status: assigned to needs_review

Trac:
Cc: N/A to danielpinto52@gmail.com

Thanks, and sorry for the confusion ! This statement of mine was bogus:

any file in torrc.d, with a single # char

Indeed the filename also must be last, I just verified it.

Trac:
Username: xaho

This looks like a fix we should backport to 0.3.5 and later.

I'll make branches for 0.3.5 and later, using git cherry-pick, and our git-merge-forward.sh test branch mode.

Trac:
Keywords: tor-hs regression 042-must deleted, 041-backport, tor-hs regression 042-must 035-backport, 040-backport, 042-backport added

Here is a backported pull request, with some changes file fixes:

• 0.3.5: https://github.com/torproject/tor/pull/1348

I added a conf regression test on master using the test framework from #31637 (moved):

• master: https://github.com/torproject/tor/pull/1349

The merge forward is clean, the test branches are here:

• https://github.com/teor2345/tor/branches/all?query=31408

Trac:
Actualpoints: N/A to 0.3
Status: needs_review to merge_ready
Points: N/A to 0.3
Version: Tor: 0.4.0.5 to Tor: 0.3.1.1-alpha
Keywords: tor-hs regression 042-must 035-backport deleted, BugSmashFund tor-hs regression 042-must consider-backport-after-0423, 035-backport added

I've commented on your pull request: your test uses an empty (0 byte) file but the issue is only reproduced with a non-empty file containing no options.

Trac:
Status: merge_ready to needs_revision

Thanks, I originally had a non-empty file, but I must have made a mistake while minimising the test case.

I fixed the changes file, test, and test commit message, and force-pushed.

Trac:
Status: needs_revision to needs_review

Trac:
Actualpoints: 0.3 to 0.4