Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#12114 closed defect (fixed)

Don't open DirPortFrontPage when DirPort not set/enabled

Reported by: alphawolf Owned by:
Priority: Medium Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version: Tor: 0.2.5.4-alpha
Severity: Keywords: tor
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

It's possible to trigger #12028 after HUP with this torrc:

DirPortFrontPage /etc/tor/tor-exit-notice.html
Sandbox 1
May 23 17:10:40.000 [notice] Received reload signal (hup). Reloading config and resetting internal state.
May 23 17:10:40.000 [notice] Read configuration file "/usr/share/tor/tor-service-defaults-torrc".
May 23 17:10:40.000 [notice] Read configuration file "/etc/tor/torrc".
May 23 17:10:40.000 [notice] Tor 0.2.5.4-alpha-dev (git-cfd0ee514c279bc6) opening log file.
May 23 17:10:40.000 [warn] sandbox_intern_string(): Bug: No interned sandbox parameter found for /etc/tor/tor-exit-notice.html
May 23 17:10:40.000 [warn] Could not open "/etc/tor/tor-exit-notice.html": Permission denied
May 23 17:10:40.000 [warn] DirPortFrontPage file '/etc/tor/tor-exit-notice.html' not found. Continuing anyway.

In this case ClientOnly 1 is implied. I think this is separate from #12028, because I don't think we should try to open DirPortFrontPage when we're not a relay, or even if we're a relay but DirPort isn't set.

Child Tickets

Change History (5)

comment:1 Changed 4 years ago by arma

Is this what you had in mind?

diff --git a/src/or/config.c b/src/or/config.c
index 10df839..efd07de 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -1825,7 +1825,7 @@ options_act(const or_options_t *old_options)
   /* Load the webpage we're going to serve every time someone asks for '/' on
      our DirPort. */
   tor_free(global_dirfrontpagecontents);
-  if (options->DirPortFrontPage) {
+  if (options->DirPort_set && options->DirPortFrontPage) {
     global_dirfrontpagecontents =
       read_file_to_str(options->DirPortFrontPage, 0, NULL);
     if (!global_dirfrontpagecontents) {

Why should we not open the file if we're not a relay but we set DirPort? Seems to me that if somebody sets DirPort and also sets DirPortFrontPage, then they want their DirPort to serve that string. (There is a use case, though it's probably rare, for one Tor client to serve a DirPort to a bunch of other local Tor clients, so they don't all load down the directory mirrors.)

comment:2 in reply to:  1 Changed 4 years ago by alphawolf

Summary: Don't open DirPortFrontPage when not a relay or DirPort not setDon't open DirPortFrontPage when DirPort not set/enabled

Replying to arma:

Why should we not open the file if we're not a relay but we set DirPort? Seems to me that if somebody sets DirPort and also sets DirPortFrontPage, then they want their DirPort to serve that string.

Ahh, I mistakenly thought that tor required ORPort before it turned on any "server" functions (I first noticed this bug while ClientOnly 1 was accidentally set). I've revised the summary to be more accurate.

I tested your patch, and it works in both cases where DirPort is not set, or DirPort is set in addition to ClientOnly 1. Looks good!

comment:3 Changed 4 years ago by nickm

Resolution: fixed
Status: newclosed

14842de9a731b6bbb348583645a31a3f3ab68af8 should fix this.

comment:4 Changed 4 years ago by arma

My patch seems still useful, yes? Loading a string to keep it around in memory when you know you'll never use it, vs not doing that?

comment:5 Changed 4 years ago by nickm

Sure, though it isn't what this ticket is about. Do you want to write a changes file for it?

Note: See TracTickets for help on using tickets.