Refactor HTTP response parsing

Remove need to nul-terminate the received buffers in HTTP fetch reply handling
and HTTP server request parsing.

Remove redundant copying of data.

More rigorous parsing code, probably less vulnerable to overrun exploits.

Better debug logging of requests and responses.

rhizome.h

@@ -163,6 +163,10 @@ int rhizome_str_is_file_hash(const char *text);
 
 #define alloca_tohex_bid(bid)           alloca_tohex((bid), RHIZOME_MANIFEST_ID_BYTES)
 
+int http_header_complete(const char *buf, size_t len, size_t tail);
+int str_startswith(char *str, const char *substring, char **afterp);
+int strcase_startswith(char *str, const char *substring, char **afterp);
+
 int rhizome_write_manifest_file(rhizome_manifest *m, const char *filename);
 int rhizome_manifest_selfsign(rhizome_manifest *m);
 int rhizome_drop_stored_file(const char *id,int maximum_priority);
@@ -248,4 +252,3 @@ int rhizome_ignore_manifest_check(rhizome_manifest *m,
 
 int rhizome_suggest_queue_manifest_import(rhizome_manifest *m,
 					  struct sockaddr_in *peerip);
-

rhizome_fetch.c

@@ -817,127 +817,130 @@ void rhizome_fetch_poll(struct sched_ent *alarm)
     return;
   }
   
-  switch(q->state) 
+  switch(q->state) {
-    {
     case RHIZOME_FETCH_CONNECTING:
     case RHIZOME_FETCH_SENDINGHTTPREQUEST:
       rhizome_fetch_write(q);
       break;
-    case RHIZOME_FETCH_RXFILE:
+    case RHIZOME_FETCH_RXFILE: {
-      /* Keep reading until we have the promised amount of data */
+	/* Keep reading until we have the promised amount of data */
-      
+	char buffer[8192];
-      sigPipeFlag=0;
+	sigPipeFlag = 0;
-      
+	int bytes = read_nonblock(q->alarm.poll.fd, buffer, sizeof buffer);
-      errno=0;
+	/* If we got some data, see if we have found the end of the HTTP request */
-      char buffer[8192];
+	if (bytes > 0) {
-
+	  rhizome_write_content(q, buffer, bytes);
-      int bytes=read(q->alarm.poll.fd,buffer,8192);
+	} else {
-      
+	  if (debug & DEBUG_RHIZOME_RX)
-      /* If we got some data, see if we have found the end of the HTTP request */
+	    DEBUG("Empty read, closing connection");
-      if (bytes>0)
+	  rhizome_fetch_close(q);
-	rhizome_write_content(q, buffer, bytes);
+	  return;
-	
+	}
-      break;
+	if (sigPipeFlag) {
-    case RHIZOME_FETCH_RXHTTPHEADERS:
+	  if (debug & DEBUG_RHIZOME_RX)
-      /* Keep reading until we have two CR/LFs in a row */
+	    DEBUG("Received SIGPIPE, closing connection");
-      sigPipeFlag=0;
+	  rhizome_fetch_close(q);
-      
+	  return;
-      errno=0;
+	}
-      bytes=read(q->alarm.poll.fd,&q->request[q->request_len],
+      }
-		 1024-q->request_len-1);
+      break;
-
+    case RHIZOME_FETCH_RXHTTPHEADERS: {
-      if (sigPipeFlag||((bytes==0)&&(errno==0))) {
+	/* Keep reading until we have two CR/LFs in a row */
-	/* broken pipe, so close connection */
+	sigPipeFlag = 0;
-	if (debug&DEBUG_RHIZOME) 
+	int bytes = read_nonblock(q->alarm.poll.fd, &q->request[q->request_len], 1024 - q->request_len - 1);
-	  DEBUG("Closing rhizome fetch connection due to sigpipe");
+	/* If we got some data, see if we have found the end of the HTTP reply */
-	rhizome_fetch_close(q);
+	if (bytes > 0) {
-	return;
+	  // reset timeout
-      }	 
+	  unschedule(&q->alarm);
-	
+	  q->alarm.alarm = overlay_gettime_ms() + RHIZOME_IDLE_TIMEOUT;
-      /* If we got some data, see if we have found the end of the HTTP request */
+	  schedule(&q->alarm);
-      if (bytes>0) {
+	  q->request_len += bytes;
-	int lfcount=0;
+	  if (http_header_complete(q->request, q->request_len, bytes + 4)) {
-	int i=q->request_len-160;
+	    if (debug & DEBUG_RHIZOME_RX)
-	
+	      DEBUGF("Got HTTP reply: %s", alloca_toprint(160, (unsigned char *)q->request, q->request_len));
-	// reset timeout
+	    /* We have all the reply headers, so parse them, taking care of any following bytes of
-	unschedule(&q->alarm);
+	      content. */
-	q->alarm.alarm=overlay_gettime_ms() + RHIZOME_IDLE_TIMEOUT;
+	    char *p = NULL;
-	schedule(&q->alarm);
+	    if (!str_startswith(q->request, "HTTP/1.0 ", &p)) {
-	
+	      if (debug&DEBUG_RHIZOME_RX)
-	if (i<0) i=0;
+		DEBUGF("Malformed HTTP reply: missing HTTP/1.0 preamble");
-	q->request_len+=bytes;
+	      rhizome_fetch_close(q);
-	if (q->request_len<1024)
+	      return;
-	  q->request[q->request_len]=0;
+	    }
-	
+	    int http_response_code = 0;
-	for(;i<(q->request_len+bytes);i++)
+	    char *nump;
-	  {
+	    for (nump = p; isdigit(*p); ++p)
-	    switch(q->request[i]) {
+	      http_response_code = http_response_code * 10 + *p - '0';
-	    case '\n': lfcount++; break;
+	    if (p == nump || *p != ' ') {
-	    case '\r': /* ignore CR */ break;
+	      if (debug&DEBUG_RHIZOME_RX)
-	    case 0: /* ignore NUL (telnet inserts them) */ break;
+		DEBUGF("Malformed HTTP reply: missing decimal status code");
-	    default: lfcount=0; break;
+	      rhizome_fetch_close(q);
-	    }
+	      return;
-	    if (lfcount==2) break;
+	    }
-	  }
+	    if (http_response_code != 200) {
-	
+	      if (debug & DEBUG_RHIZOME_RX)
-	if (debug&DEBUG_RHIZOME)
+		DEBUGF("Failed HTTP request: rhizome server returned %d != 200 OK", http_response_code);
-	  dump("http reply headers",(unsigned char *)q->request,lfcount==2?i:q->request_len);
+	      rhizome_fetch_close(q);
-	
+	      return;
-	if (lfcount==2) {
+	    }
-	  /* We have the response headers, so parse.
+	    // This loop will terminate, because http_header_complete() above found at least
-	     (we may also have some bytes of content, so we need to be a little
+	    // "\n\n" at the end of the header, and probably "\r\n\r\n".
-	     careful) */
+	    while (*p++ != '\n')
-
+	      ;
-	  /* Terminate string at end of headers */
+	    // Iterate over header lines until the last blank line.
-	  q->request[i]=0;
+	    long long content_length = -1;
-
+	    while (*p != '\r' && *p != '\n') {
-	  /* Get HTTP result code */
+	      if (strcase_startswith(p, "Content-Length:", &p)) {
-	  char *s=strstr(q->request,"HTTP/1.0 ");
+		while (*p == ' ')
-	  if (!s) { 
+		  ++p;
-	    if (debug&DEBUG_RHIZOME) DEBUGF("HTTP response lacked HTTP/1.0 response code.");
+		content_length = 0;
-	    rhizome_fetch_close(q);
+		for (nump = p; isdigit(*p); ++p)
-	    return;
+		  content_length = content_length * 10 + *p - '0';
-	  }
+		if (p == nump || (*p != '\r' && *p != '\n')) {
-	  int http_response_code=strtoll(&s[9],NULL,10);
+		  if (debug & DEBUG_RHIZOME_RX)  {
-	  if (http_response_code!=200) {
+		    DEBUGF("Invalid HTTP reply: malformed Content-Length header");
-	    if (debug&DEBUG_RHIZOME) DEBUGF("Rhizome web server returned %d != 200 OK",http_response_code);
+		    rhizome_fetch_close(q);
-	    rhizome_fetch_close(q);
+		    return;
-	    return;
+		  }
-	  }
+		}
-	  /* Get content length */
+	      }
-	  s=strstr(q->request,"Content-length: ");
+	      while (*p++ != '\n')
-	  if (!s) {
+		;
-	    if (debug&DEBUG_RHIZOME) 
+	    }
-	      DEBUGF("Missing Content-Length: header.");
+	    if (*p == '\r')
-	    rhizome_fetch_close(q);
+	      ++p;
-	    return;
+	    ++p; // skip '\n' at end of blank line
-	  }
+	    if (content_length == -1) {
-	  q->file_len=strtoll(&s[16],NULL,10);
+	      if (debug & DEBUG_RHIZOME_RX)
-	  
+		DEBUGF("Invalid HTTP reply: missing Content-Length header");
-	  if (q->file_len<0) {
+	      rhizome_fetch_close(q);
-	    if (debug&DEBUG_RHIZOME) 
+	      return;
-	      DEBUGF("Illegal file size (%d).",q->file_len);
+	    }
-	    rhizome_fetch_close(q);
+	    q->file_len = content_length;
-	    return;
+	    /* We have all we need.  The file is already open, so just write out any initial bytes of
-	  }
+	      the body we read.
-
+	    */
-	  /* Okay, we have both, and are all set.
+	    q->state = RHIZOME_FETCH_RXFILE;
-	     File is already open, so just write out any initial bytes of the
+	    int content_bytes = q->request + q->request_len - p;
-	     file we read, and update state flag.
+	    if (content_bytes > 0)
-	  */
+	      rhizome_write_content(q, p, content_bytes);
-	  
+	  }
-	  q->state=RHIZOME_FETCH_RXFILE;
+	} else {
-	  int fileRxBytes=q->request_len-(i+1);
+	  if (debug & DEBUG_RHIZOME_RX)
-	  
+	    DEBUG("Empty read, closing connection");
-	  if (fileRxBytes>0)
+	  rhizome_fetch_close(q);
-	    rhizome_write_content(q, &q->request[i+1], fileRxBytes);
+	  return;
-
+	}
+	if (sigPipeFlag) {
+	  if (debug & DEBUG_RHIZOME_RX)
+	    DEBUG("Received SIGPIPE, closing connection");
+	  rhizome_fetch_close(q);
+	  return;
 	}
       }
-      
       break;
     default:
-      if (debug&DEBUG_RHIZOME) 
+      if (debug & DEBUG_RHIZOME_RX) 
 	DEBUG("Closing rhizome fetch connection due to illegal/unimplemented state.");
       rhizome_fetch_close(q);
       return;

rhizome_http.c

@@ -225,30 +225,15 @@ void rhizome_client_poll(struct sched_ent *alarm)
       /* Keep reading until we have two CR/LFs in a row */
       r->request[r->request_length] = '\0';
       sigPipeFlag=0;
-      int bytes = read_nonblock(r->alarm.poll.fd, &r->request[r->request_length], RHIZOME_HTTP_REQUEST_MAXLEN - r->request_length - 1);
+      int bytes = read_nonblock(r->alarm.poll.fd, &r->request[r->request_length], RHIZOME_HTTP_REQUEST_MAXLEN - r->request_length);
       /* If we got some data, see if we have found the end of the HTTP request */
       if (bytes > 0) {
 	// reset inactivity timer
 	r->alarm.alarm = overlay_gettime_ms() + RHIZOME_IDLE_TIMEOUT;
 	unschedule(&r->alarm);
 	schedule(&r->alarm);
-	int i = r->request_length - 160;
+	r->request_length += bytes;
-	if (i<0) i=0;
+	if (http_header_complete(r->request, r->request_length, bytes + 4)) {
-	r->request_length+=bytes;
-	if (r->request_length<RHIZOME_HTTP_REQUEST_MAXLEN)
-	  r->request[r->request_length]=0;
-	if (0)
-	  dump("request", (unsigned char *)r->request,r->request_length);
-	int lfcount;
-	for(lfcount = 0; lfcount < 2 && i < r->request_length + bytes; ++i) {
-	  switch (r->request[i]) {
-	  case '\n': ++lfcount; break;
-	  case '\r': break;
-	  case '\0': break; // ignore NUL (telnet inserts them)
-	  default: lfcount = 0; break;
-	  }
-	}
-	if (lfcount == 2) {
 	  /* We have the request. Now parse it to see if we can respond to it */
 	  rhizome_server_parse_http_request(r);
 	}
@@ -273,7 +258,6 @@ void rhizome_client_poll(struct sched_ent *alarm)
   return;
 }
 
-
 void rhizome_server_poll(struct sched_ent *alarm)
 {
   struct sockaddr addr;
@@ -498,15 +482,56 @@ static int rhizome_server_sql_query_fill_buffer(rhizome_http_request *r, char *t
   return 0;  
 }
 
-static int strcmp_prefix(char *str, const char *prefix, char **afterp)
+int http_header_complete(const char *buf, size_t len, size_t tail)
 {
-  while (*prefix && *str && *prefix == *str)
+  const char *bufend = buf + len;
-    ++prefix, ++str;
+  if (tail < len)
-  if (*prefix)
+    buf = bufend - tail;
-    return (unsigned char)*str - (unsigned char)*prefix;
+  int count = 0;
+  for (; count < 2 && buf != bufend; ++buf) {
+    switch (*buf) {
+      case '\n': ++count; break;
+      case '\r': break;
+      case '\0': break; // ignore NUL (telnet inserts them)
+      default: count = 0; break;
+    }
+  }
+  return count == 2;
+}
+
+/* Check if a given string starts with a given sub-string.  If so, return 1 and, if afterp is not
+   NULL, set *afterp to point to the character immediately following the substring.  Otherwise
+   return 0.
+   This function is used to parse HTTP headers and responses, which are typically not
+   nul-terminated, but are held in a buffer which has an associated length.  To avoid this function
+   running past the end of the buffer, the caller must ensure that the buffer contains a sub-string
+   that is not part of the sub-string being sought, eg, "\r\n\r\n" as detected by
+   http_header_complete().  This guarantees that this function will return nonzero before running
+   past the end of the buffer.
+   @author Andrew Bettison <andrew@servalproject.com>
+ */
+int str_startswith(char *str, const char *substring, char **afterp)
+{
+  while (*substring && *substring == *str)
+    ++substring, ++str;
+  if (*substring)
+    return 0;
   if (afterp)
     *afterp = str;
-  return 0;
+  return 1;
+}
+
+/* Case-insensitive form of str_startswith().
+ */
+int strcase_startswith(char *str, const char *substring, char **afterp)
+{
+  while (*substring && *str && toupper(*substring) == toupper(*str))
+    ++substring, ++str;
+  if (*substring)
+    return 0;
+  if (afterp)
+    *afterp = str;
+  return 1;
 }
 
 static int rhizome_server_parse_http_request(rhizome_http_request *r)
@@ -514,18 +539,30 @@ static int rhizome_server_parse_http_request(rhizome_http_request *r)
   /* Switching to writing, so update the call-back */
   r->alarm.poll.events=POLLOUT;
   watch(&r->alarm);
-  
+  // Start building up a response.
-  /* Clear request type flags */
+  r->request_type = 0;
-  r->request_type=0;
+  // Parse the HTTP "GET" line.
-  char path[1024];
+  char *path = NULL;
-  if (sscanf(r->request, "GET %1024s HTTP/1.%*1[01]%*[\r\n]", path) != 1) {
+  size_t pathlen = 0;
-    if (debug & DEBUG_RHIZOME_TX)
+  if (str_startswith(r->request, "GET ", &path)) {
-      DEBUGF("Received malformed HTTP request: %s", alloca_toprint(120, (unsigned char *)r->request, r->request_length));
+    char *p;
-    rhizome_server_simple_http_response(r, 400, "<html><h1>Malformed request</h1></html>\r\n");
+    // This loop is guaranteed to terminate before the end of the buffer, because we know that the
-  } else {
+    // buffer contains at least "\n\n" and maybe "\r\n\r\n" at the end of the header block.
+    for (p = path; !isspace(*p); ++p)
+      ;
+    pathlen = p - path;
+    if ( str_startswith(p, " HTTP/1.", &p)
+      && (str_startswith(p, "0", &p) || str_startswith(p, "1", &p))
+      && (str_startswith(p, "\r\n", &p) || str_startswith(p, "\n", &p))
+    )
+      path[pathlen] = '\0';
+    else
+      path = NULL;
+  }
+  if (path) {
     char *id = NULL;
     if (debug & DEBUG_RHIZOME_TX)
-      DEBUGF("GET %s", path);
+      DEBUGF("GET %s", alloca_toprint(1024, (unsigned char *)path, pathlen));
     if (strcmp(path, "/favicon.ico") == 0) {
       r->request_type = RHIZOME_HTTP_REQUEST_FAVICON;
       rhizome_server_http_response_header(r, 200, "image/vnd.microsoft.icon", favicon_len);
@@ -538,7 +575,7 @@ static int rhizome_server_parse_http_request(rhizome_http_request *r)
     } else if (strcmp(path, "/rhizome/bars") == 0) {
       /* Return the list of known BARs */
       rhizome_server_sql_query_http_response(r, "bar", "manifests", "from manifests", 32, 0);
-    } else if (strcmp_prefix(path, "/rhizome/file/", &id) == 0) {
+    } else if (str_startswith(path, "/rhizome/file/", &id)) {
       /* Stream the specified payload */
       if (!rhizome_str_is_file_hash(id)) {
 	rhizome_server_simple_http_response(r, 400, "<html><h1>Invalid payload ID</h1></html>\r\n");
@@ -558,12 +595,16 @@ static int rhizome_server_parse_http_request(rhizome_http_request *r)
 	  r->request_type |= RHIZOME_HTTP_REQUEST_BLOB;
 	}
       }
-    } else if (strcmp_prefix(path, "/rhizome/manifest/", &id) == 0) {
+    } else if (str_startswith(path, "/rhizome/manifest/", &id)) {
-      /* TODO: Stream the specified manifest */
+      // TODO: Stream the specified manifest
       rhizome_server_simple_http_response(r, 500, "<html><h1>Not implemented</h1></html>\r\n");
     } else {
       rhizome_server_simple_http_response(r, 404, "<html><h1>Not found</h1></html>\r\n");
     }
+  } else {
+    if (debug & DEBUG_RHIZOME_TX)
+      DEBUGF("Received malformed HTTP request: %s", alloca_toprint(120, (unsigned char *)r->request, r->request_length));
+    rhizome_server_simple_http_response(r, 400, "<html><h1>Malformed request</h1></html>\r\n");
   }
   
   /* Try sending data immediately. */