From: rjk@greenend.org.uk <>
Date: Fri, 28 Dec 2007 16:58:50 +0000 (+0000)
Subject: keep cookie more private to disorder.cgi
X-Git-Tag: 3.0~155
X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~mdw/git/disorder/commitdiff_plain/36bde473fdfeb75de65fb7b1920a6fb137f32f3c?hp=62ef2216d2c7c1c563ea163e2a0fdacccb54e31e

keep cookie more private to disorder.cgi
---

diff --git a/lib/Makefile.am b/lib/Makefile.am
index 7b4ce0a..9b84937 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -66,6 +66,7 @@ libdisorder_a_SOURCES=charset.c charset.h		\
 	timeval.h					\
 	trackdb.h trackdb.c trackdb-int.h		\
 	trackname.c trackname.h				\
+	url.h url.c					\
 	user.h user.c					\
 	unicode.h unicode.c				\
 	unidata.h unidata.c				\
diff --git a/lib/kvp.c b/lib/kvp.c
index 471de78..f9081ef 100644
--- a/lib/kvp.c
+++ b/lib/kvp.c
@@ -128,7 +128,7 @@ int urlencode(struct sink *sink, const char *s, size_t n) {
  * @param s String to encode
  * @return Encoded string
  */
-const char *urlencodestring(const char *s) {
+char *urlencodestring(const char *s) {
   struct dynstr d;
 
   dynstr_init(&d);
@@ -140,13 +140,14 @@ const char *urlencodestring(const char *s) {
 /** @brief URL-decode @p s
  * @param s String to decode
  * @param ns Length of string
- * @return Decoded string
+ * @return Decoded string or NULL
  */
-const char *urldecodestring(const char *s, size_t ns) {
+char *urldecodestring(const char *s, size_t ns) {
   struct dynstr d;
 
   dynstr_init(&d);
-  urldecode(sink_dynstr(&d), s, ns);
+  if(urldecode(sink_dynstr(&d), s, ns))
+    return NULL;
   dynstr_terminate(&d);
   return d.vec;
 }
diff --git a/lib/kvp.h b/lib/kvp.h
index 98c62cd..5240526 100644
--- a/lib/kvp.h
+++ b/lib/kvp.h
@@ -52,10 +52,10 @@ int urlencode(struct sink *sink, const char *s, size_t n);
 /* url-encode the @n@ bytes at @s@, writing to @sink@.  Return 0 on
  * success, -1 on error.  */
 
-const char *urlencodestring(const char *s);
+char *urlencodestring(const char *s);
 /* return the url-encoded form of @s@ */
 
-const char *urldecodestring(const char *s, size_t ns);
+char *urldecodestring(const char *s, size_t ns);
 
 #endif /* KVP_H */
 
diff --git a/lib/mime.c b/lib/mime.c
index 7ba4254..9883a3c 100644
--- a/lib/mime.c
+++ b/lib/mime.c
@@ -457,6 +457,8 @@ char *mime_qp(const char *s) {
  * @param s Header field value
  * @param cd Where to store result
  * @return 0 on success, non-0 on error
+ *
+ * See <a href="http://tools.ietf.org/html/rfc2109">RFC 2109</a>.
  */
 int parse_cookie(const char *s,
 		 struct cookiedata *cd) {
@@ -529,6 +531,41 @@ const struct cookie *find_cookie(const struct cookiedata *cd,
   return 0;
 }
 
+/** @brief RFC822 quoting
+ * @param s String to quote
+ * @param force If non-0, always quote
+ * @return Possibly quoted string
+ */
+char *quote822(const char *s, int force) {
+  const char *t;
+  struct dynstr d[1];
+  int c;
+
+  if(!force) {
+    /* See if we need to quote */
+    for(t = s; (c = (unsigned char)*t); ++t) {
+      if(tspecial(c) || http_separator(c) || whitespace(c))
+	break;
+    }
+    if(*t)
+      force = 1;
+  }
+
+  if(!force)
+    return xstrdup(s);
+
+  dynstr_init(d);
+  dynstr_append(d, '"');
+  for(t = s; (c = (unsigned char)*t); ++t) {
+    if(c == '"' || c == '\\')
+      dynstr_append(d, '\\');
+    dynstr_append(d, c);
+  }
+  dynstr_append(d, '"');
+  dynstr_terminate(d);
+  return d->vec;
+}
+
 /*
 Local Variables:
 c-basic-offset:2
diff --git a/lib/mime.h b/lib/mime.h
index b2286be..0fa8aea 100644
--- a/lib/mime.h
+++ b/lib/mime.h
@@ -55,7 +55,10 @@ int mime_rfc2388_content_disposition(const char *s,
 
 char *mime_qp(const char *s);
 
-/** @brief Parsed form of an HTTP Cookie: header field */
+/** @brief Parsed form of an HTTP Cookie: header field
+ *
+ * See <a href="http://tools.ietf.org/html/rfc2109">RFC 2109</a>.
+ */
 struct cookiedata {
   /** @brief @c $Version or NULL if not set */
   char *version;
@@ -67,7 +70,11 @@ struct cookiedata {
   int ncookies;
 };
 
-/** @brief A parsed cookie */
+/** @brief A parsed cookie
+ *
+ * See <a href="http://tools.ietf.org/html/rfc2109">RFC 2109</a> and @ref
+ * cookiedata.
+ */
 struct cookie {
   /** @brief Cookie name */
   char *name;
@@ -87,7 +94,7 @@ int parse_cookie(const char *s,
 		 struct cookiedata *cd);
 const struct cookie *find_cookie(const struct cookiedata *cd,
 				 const char *name);
-
+char *quote822(const char *s, int force);
 
 #endif /* MIME_H */
 
diff --git a/lib/test.c b/lib/test.c
index 0f38fc3..61b63fd 100644
--- a/lib/test.c
+++ b/lib/test.c
@@ -62,6 +62,7 @@
 #include "configuration.h"
 #include "addr.h"
 #include "base64.h"
+#include "url.h"
 
 static int tests, errors;
 static int fail_first;
@@ -1371,6 +1372,8 @@ static void test_addr(void) {
     0
   };
 
+  printf("test_addr\n");
+
   a.n = 1;
   a.s = (char **)s;
   s[0] = "smtp";
@@ -1402,6 +1405,26 @@ static void test_addr(void) {
   check_string(name, "host localhost service nntp");
 }
 
+static void test_url(void) {
+  struct url p;
+  
+  printf("test_url\n");
+
+  insist(parse_url("http://www.example.com/example/path", &p) == 0);
+  check_string(p.scheme, "http");
+  check_string(p.host, "www.example.com");
+  insist(p.port == -1);
+  check_string(p.path, "/example/path");
+  insist(p.query == 0);
+
+  insist(parse_url("https://www.example.com:82/example%2fpath?+query+", &p) == 0);
+  check_string(p.scheme, "https");
+  check_string(p.host, "www.example.com");
+  insist(p.port == 82);
+  check_string(p.path, "/example/path");
+  check_string(p.query, "+query+");
+}
+
 int main(void) {
   mem_init();
   fail_first = !!getenv("FAIL_FIRST");
@@ -1468,6 +1491,7 @@ int main(void) {
   /* selection.c */
   test_selection();
   test_hash();
+  test_url();
   fprintf(stderr,  "%d errors out of %d tests\n", errors, tests);
   return !!errors;
 }
diff --git a/lib/url.c b/lib/url.c
new file mode 100644
index 0000000..784ee9f
--- /dev/null
+++ b/lib/url.c
@@ -0,0 +1,143 @@
+/*
+ * This file is part of DisOrder
+ * Copyright (C) 2007 Richard Kettlewell
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
+ * USA
+ */
+/** @file lib/url.c
+ * @brief URL support functions
+ */
+
+#include <config.h>
+#include "types.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+
+#include "mem.h"
+#include "log.h"
+#include "printf.h"
+#include "url.h"
+#include "kvp.h"
+
+/** @brief Infer the for the web interface
+ * @return Inferred URL
+ *
+ * See <a href="http://tools.ietf.org/html/rfc3875">RFC 3875</a>.
+ */
+char *infer_url(void) {
+  const char *scheme = "http", *server, *script, *e;
+  char *url;
+  int port;
+  
+  /* Figure out the server.  'MUST' be set and we don't cope if it
+   * is not. */
+  if(!(server = getenv("SERVER_NAME")))
+    fatal(0, "SERVER_NAME is not set");
+  server = xstrdup(server);
+  
+  /* Figure out the port.  'MUST' be set but we cope if it is not. */
+  if((e = getenv("SERVER_PORT")))
+    port = atoi(e);
+  else
+    port = 80;
+  
+  /* Figure out path to ourselves */
+  if(!(script = getenv("SCRIPT_NAME")))
+    fatal(0, "SCRIPT_NAME is not set");
+  if(script[0] != '/')
+    fatal(0, "SCRIPT_NAME does not start with a '/'");
+  script = xstrdup(script);
+  
+  if(port == 80)
+    byte_xasprintf(&url, "%s://%s%s",
+		   scheme, server, script);
+  else
+    byte_xasprintf(&url, "%s://%s:%d%s",
+		   scheme, server, port, script);
+  return url;
+}
+
+/** @brief Parse a URL
+ * @param url URL to parsed
+ * @param parsed Where to store parsed URL data
+ * @return 0 on success, non-0 on error
+ *
+ * NB that URLs with usernames and passwords are NOT currently supported.
+ */
+int parse_url(const char *url, struct url *parsed) {
+  const char *s;
+  long n;
+
+  /* The scheme */
+  for(s = url; *s && *s != '/' && *s != ':'; ++s)
+    ;
+  if(*s == ':') {
+    parsed->scheme = xstrndup(url, s - url);
+    url = s + 1;
+  } else
+    parsed->scheme = 0;
+
+  /* The host and port */
+  if(*url == '/' && url[1] == '/') {
+    /* //user:password@host:port, but we don't support the
+     * user:password@ part. */
+    url += 2;
+    for(s = url; *s && *s != '/' && *s != ':'; ++s)
+      ;
+    parsed->host = xstrndup(url, s - url);
+    if(*s == ':') {
+      /* We have host:port[/...] */
+      ++s;
+      errno = 0;
+      n = strtol(s, (char **)&url, 10);
+      if(errno)
+	return -1;
+      if(n < 0 || n > 65535)
+	return -1;
+      parsed->port = n;
+    } else {
+      /* We just have host[/...] */
+      url = s;
+      parsed->port = -1;
+    }
+  }
+
+  /* The path */
+  for(s = url; *s && *s != '?'; ++s)
+    ;
+  if(!(parsed->path = urldecodestring(url, s - url)))
+    return -1;
+  url = s;
+
+  /* The query */
+  if(*url == '?')
+    parsed->query = xstrdup(url + 1);
+  else
+    parsed->query = 0;
+
+  return 0;
+}
+
+/*
+Local Variables:
+c-basic-offset:2
+comment-column:40
+fill-column:79
+indent-tabs-mode:nil
+End:
+*/
diff --git a/lib/url.h b/lib/url.h
new file mode 100644
index 0000000..3331456
--- /dev/null
+++ b/lib/url.h
@@ -0,0 +1,83 @@
+/*
+ * This file is part of DisOrder
+ * Copyright (C) 2007 Richard Kettlewell
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
+ * USA
+ */
+/** @file lib/url.h
+ * @brief URL support functions
+ */
+
+#ifndef URL_H
+#define URL_H
+
+/** @brief A parsed HTTP URL */
+struct url {
+  /** @brief URL scheme
+   *
+   * Typically "http" or "https".  Might be NULL for a relative URL.
+   */
+  char *scheme;
+
+  /** @brief Username
+   *
+   * Might well be NULL.  NB not supported currently.
+   */
+  char *user;
+
+  /** @brief Password
+   *
+   * Migth well be NULL.  NB not supported currently.
+   */
+  char *password;
+  
+  /** @brief Hostname
+   *
+   * Might be NULL for a relative URL.
+   */
+  char *host;
+
+  /** @brief Port number or -1 if none specified */
+  long port;
+
+  /** @brief Path
+   *
+   * May be the empty string.  Never NULL.  Will be decoded from the
+   * original URL.
+   */
+  char *path;
+
+  /** @brief Query
+   *
+   * NULL if there was no query part.  Will NOT be decoded from the
+   * original URL.
+   */
+  char *query;
+};
+
+char *infer_url(void);
+int parse_url(const char *url, struct url *parsed);
+
+#endif /* URL_H */
+
+/*
+Local Variables:
+c-basic-offset:2
+comment-column:40
+fill-column:79
+indent-tabs-mode:nil
+End:
+*/
diff --git a/server/cgimain.c b/server/cgimain.c
index 53f7a9b..b377c8e 100644
--- a/server/cgimain.c
+++ b/server/cgimain.c
@@ -41,43 +41,7 @@
 #include "mime.h"
 #include "printf.h"
 #include "dcgi.h"
-
-/** @brief Infer the base URL for the web interface if it's not set
- *
- * See <a href="http://tools.ietf.org/html/rfc3875">RFC 3875</a>.
- */
-static void infer_url(void) {
-  if(!config->url) {
-    const char *scheme = "http", *server, *script, *e;
-    int port;
-
-    /* Figure out the server.  'MUST' be set and we don't cope if it
-     * is not. */
-    if(!(server = getenv("SERVER_NAME")))
-      fatal(0, "SERVER_NAME is not set");
-    server = xstrdup(server);
-
-    /* Figure out the port.  'MUST' be set but we cope if it is not. */
-    if((e = getenv("SERVER_PORT")))
-      port = atoi(e);
-    else
-      port = 80;
-
-    /* Figure out path to ourselves */
-    if(!(script = getenv("SCRIPT_NAME")))
-      fatal(0, "SCRIPT_NAME is not set");
-    if(script[0] != '/')
-      fatal(0, "SCRIPT_NAME does not start with a '/'");
-    script = xstrdup(script);
-
-    if(port == 80)
-      byte_xasprintf(&config->url, "%s://%s%s",
-		     scheme, server, script);
-    else
-      byte_xasprintf(&config->url, "%s://%s:%d%s",
-		     scheme, server, port, script);
-  }
-}
+#include "url.h"
 
 int main(int argc, char **argv) {
   const char *cookie_env, *conf;
@@ -92,7 +56,8 @@ int main(int argc, char **argv) {
   if((conf = getenv("DISORDER_CONFIG"))) configfile = xstrdup(conf);
   if(getenv("DISORDER_DEBUG")) debugging = 1;
   if(config_read(0)) exit(EXIT_FAILURE);
-  infer_url();
+  if(!config->url)
+    config->url = infer_url();
   memset(&g, 0, sizeof g);
   memset(&s, 0, sizeof s);
   s.g = &g;
diff --git a/server/dcgi.c b/server/dcgi.c
index 08203f8..a4f774a 100644
--- a/server/dcgi.c
+++ b/server/dcgi.c
@@ -54,6 +54,8 @@
 #include "trackname.h"
 #include "charset.h"
 #include "dcgi.h"
+#include "url.h"
+#include "mime.h"
 
 char *login_cookie;
 
@@ -103,21 +105,28 @@ static const char *front_url(void) {
 
 static void header_cookie(struct sink *output) {
   struct dynstr d[1];
-  char *s;
+  struct url u;
 
+  memset(&u, 0, sizeof u);
+  dynstr_init(d);
+  parse_url(config->url, &u);
   if(login_cookie) {
-    dynstr_init(d);
-    for(s = login_cookie; *s; ++s) {
-      if(*s == '"')
-	dynstr_append(d, '\\');
-      dynstr_append(d, *s);
-    }
-    dynstr_terminate(d);
-    byte_xasprintf(&s, "disorder=\"%s\"", d->vec); /* TODO domain, path, expiry */
-    cgi_header(output, "Set-Cookie", s);
-  } else
+    dynstr_append_string(d, "disorder=");
+    dynstr_append_string(d, quote822(login_cookie, 0));
+  } else {
     /* Force browser to discard cookie */
-    cgi_header(output, "Set-Cookie", "disorder=none;Max-Age=0");
+    dynstr_append_string(d, "disorder=none;Max-Age=0");
+  }
+  if(u.path) {
+    /* The default domain matches the request host, so we need not override
+     * that.  But the default path only goes up to the rightmost /, which would
+     * cause the browser to expose the cookie to other CGI programs on the same
+     * web server. */
+    dynstr_append_string(d, ";Path=");
+    dynstr_append_string(d, quote822(u.path, 0));
+  }
+  dynstr_terminate(d);
+  cgi_header(output, "Set-Cookie", d->vec);
 }
 
 static void redirect(struct sink *output) {