Opened 2 years ago

Closed 2 years ago

#23908 closed defect (fixed)

write_http_status_line(): Bug: status line too long. (on Tor 0.3.1.7 )

Reported by: micah Owned by:
Priority: High Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

This line seems to happen quite frequently on directory authorities who are running 0.3.1.7. Unsure if it happens to others too.

Child Tickets

Change History (8)

comment:1 Changed 2 years ago by arma

The line should be resolved in 0.3.2, because of the patches that I made for #23499 (git commit 771fb7e7 in particular).

But I am curious which line is too long in 0.3.1 -- because each time this log line happens, there is a user who gets no response to a directory request.

Is there somebody who is hitting this bug who would be able to run a patched Tor? I could whip up a patch and then we could see what the line is trying to be.

comment:2 Changed 2 years ago by nickm

Component: Core TorCore Tor/Tor
Milestone: Tor: 0.3.1.x-final
Priority: MediumHigh

comment:3 Changed 2 years ago by arma

diff --git a/src/or/directory.c b/src/or/directory.c
index 45fbd1d..7c67e08 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -3285,7 +3285,8 @@ write_http_status_line(dir_connection_t *conn, int status,
   char buf[256];
   if (tor_snprintf(buf, sizeof(buf), "HTTP/1.0 %d %s\r\n\r\n",
       status, reason_phrase ? reason_phrase : "OK") < 0) {
-    log_warn(LD_BUG,"status line too long.");
+    log_warn(LD_BUG,"status line 'HTTP/1.0 %d %s' too long.",
+             status, reason_phrase ? reason_phrase : "<empty>");
     return;
   }
   log_debug(LD_DIRSERV,"Wrote status 'HTTP/1.0 %d %s'", status, reason_phrase);

comment:4 Changed 2 years ago by Sebastian

HTTP/1.0 400 Looks like your keypair has changed? This authority previously recorded a different RSA identity for this Ed25519 identity (or vice versa.) Did you replace or copy some of your key files, but not the others? You should either restore the expected keypair, or delete your keys and restart Tor to start your relay with a new identity.

comment:5 Changed 2 years ago by Sebastian

that's quite a bit too long, 345 chars.

comment:6 Changed 2 years ago by arma

Ok. So the code path is likely in directory_handle_command_post() where it does

    was_router_added_t r = dirserv_add_multiple_descriptors(body, purpose,
                                             conn->base_.address, &msg);
[...]
    } else {
      log_info(LD_DIRSERV,
               "Rejected router descriptor or extra-info from %s "
               "(\"%s\").",
               conn->base_.address, msg);
      write_http_status_line(conn, 400, msg);
    }

comment:7 Changed 2 years ago by arma

Here is a manual backport of commit 771fb7e7:

diff --git a/src/or/directory.c b/src/or/directory.c
index 45fbd1d..bef65d3 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -3282,14 +3282,12 @@ static void
 write_http_status_line(dir_connection_t *conn, int status,
                        const char *reason_phrase)
 {
-  char buf[256];
-  if (tor_snprintf(buf, sizeof(buf), "HTTP/1.0 %d %s\r\n\r\n",
-      status, reason_phrase ? reason_phrase : "OK") < 0) {
-    log_warn(LD_BUG,"status line too long.");
-    return;
-  }
+  char *buf = NULL;
+  tor_asprintf(&buf, "HTTP/1.0 %d %s\r\n\r\n",
+               status, reason_phrase ? reason_phrase : "OK");
   log_debug(LD_DIRSERV,"Wrote status 'HTTP/1.0 %d %s'", status, reason_phrase);
   connection_write_to_buf(buf, strlen(buf), TO_CONN(conn));
+  tor_free(buf);
 }
 
 /** Write the header for an HTTP/1.0 response onto <b>conn</b>-\>outbuf,

catalyst also suggests "Make the msg shorter, like make it a link or something" as a fine fix.

comment:8 Changed 2 years ago by nickm

Resolution: fixed
Status: newclosed

okay, I've turned that into af33fdd7c1860399fe8d6861c163e5d64b0292b9 and merged it to 0.3.1.

Please consider making commits and changes messages when you can?

Note: See TracTickets for help on using tickets.