cgi/actions.c, lib/client*.[ch]: Don't use priv connection to check passwd.

If the CGI runs as the main `jukebox' user, then it can connect using
the special `.../private/socket', and the `find_server' function
arranges to do this if it can and no network address has been assigned
in the configuration.

The server doesn't bother to check passwords from clients on privileged
connections.  The result is that if the CGI program runs as the
`jukebox' user, its attempt to check the end-user's password through the
usual login machinery is stymied, since the library automatically sets
up a privileged connection and then the server ignores the password
entirely.  The end result is that, if you set things up in this way,
anyone can log into the CGI program with any known user and any password
at all, at which point they're given a cookie which can be used in
direct communication with the server.

Fix this as follows.

  * Introduce a new version (in the correct namespace for a change) of
    `find_server' which accepts some flags to guide the choice of server
    addresses.

  * Add a flag for the new `disorder_find_server' function to prevent
    trying the privileged socket.

  * Add a function to the client interface (and a bit of state to the
    client structure) to instruct the connection functions not to use a
    privileged connection even if they can.

  * Get the CGI program to use this new function when logging in with a
    password, so that it will always be checked.

Cookies are checked properly, even from privileged clients, so there's
nothing to fix there.

diff --git a/cgi/actions.c b/cgi/actions.c
index 6e180984cd6bb07650c34e6aac932cfb0f764aeb..6f094cb7780a98b928092fcfe45869d801c3e7c0 100644 (file)
--- a/cgi/actions.c
+++ b/cgi/actions.c
@@ -307,8 +307,12 @@ static int login_as(const char *username, const char *password) {
 
   if(dcgi_cookie && dcgi_client)
     disorder_revoke(dcgi_client);
-  /* We'll need a new connection as we are going to stop being guest */
+  /* We'll need a new connection as we are going to stop being guest.
+   * Make sure it's unprivileged, so that the server actually bothers checking
+   * the password we supply.
+   */
   c = disorder_new(0);
+  disorder_force_unpriv(c);
   if(disorder_connect_user(c, username, password)) {
     login_error("loginfailed");
     return -1;

diff --git a/lib/client-common.c b/lib/client-common.c
index a4a9851357f6ba703180584af76cfc11334981f1..ead7d2e3de88ad3f9b649f464e6e4ad687f273da 100644 (file)
--- a/lib/client-common.c
+++ b/lib/client-common.c
@@ -46,12 +46,13 @@
 
 /** @brief Figure out what address to connect to
  * @param c Configuration to honor
+ * @param flags Flags to guide the choice
  * @param sap Where to store pointer to sockaddr
  * @param namep Where to store socket name
  * @return Socket length, or (socklen_t)-1
  */
-socklen_t find_server(struct config *c,
-                      struct sockaddr **sap, char **namep) {
+socklen_t disorder_find_server(struct config *c, unsigned flags,
+                               struct sockaddr **sap, char **namep) {
   struct sockaddr *sa;
 #if !_WIN32
   struct sockaddr_un su;
@@ -71,10 +72,12 @@ socklen_t find_server(struct config *c,
     disorder_fatal(0, "local connections are not supported on Windows");
 #else
     /* use the private socket if possible (which it should be) */
-    name = config_get_file2(c, "private/socket");
-    if(access(name, R_OK) != 0) {
-      xfree(name);
-      name = NULL;
+    if (!(flags & DISORDER_FS_NOTPRIV)) {
+      name = config_get_file2(c, "private/socket");
+      if(access(name, R_OK) != 0) {
+        xfree(name);
+        name = NULL;
+      }
     }
     if(!name)
       name = config_get_file2(c, "socket");
@@ -99,6 +102,21 @@ socklen_t find_server(struct config *c,
   return len;
 }
 
+/** @brief Figure out what address to connect to
+ * @param c Configuration to honor
+ * @param sap Where to store pointer to sockaddr
+ * @param namep Where to store socket name
+ * @return Socket length, or (socklen_t)-1
+ *
+ * The function disorder_find_server() isn't a namespace violation, and has
+ * more functionality.  This function is equivalent, to disorder_find_server()
+ * with a zero @c flags argument.
+ */
+socklen_t find_server(struct config *c,
+                      struct sockaddr **sap, char **namep) {
+  return disorder_find_server(c, 0, sap, namep);
+}
+
 const char disorder__body[1];
 const char disorder__list[1];
 const char disorder__integer[1];

diff --git a/lib/client-common.h b/lib/client-common.h
index dd53a1d655e3d982f46b8bb70cac93c39dca5bff..4eca7e0d595f770c8e6d6a3752dd71d8f2d365c4 100644 (file)
--- a/lib/client-common.h
+++ b/lib/client-common.h
@@ -27,6 +27,10 @@
 # include <sys/socket.h>
 #endif
 
+#define DISORDER_FS_NOTPRIV 1u
+
+socklen_t disorder_find_server(struct config *c, unsigned flags,
+                               struct sockaddr **sap, char **namep);
 socklen_t find_server(struct config *c, struct sockaddr **sap, char **namep);
 
 /** @brief Marker for a command body */

diff --git a/lib/client.c b/lib/client.c
index 52c5b6851180005a804ee5cf520f0087918c8bd2..1967e0eea90a813225fd8570611c6088f242f771 100644 (file)
--- a/lib/client.c
+++ b/lib/client.c
@@ -85,6 +85,8 @@ struct disorder_client {
   int open;
   /** @brief Socket I/O context */
   struct socketio sio;
+  /** @brief Whether to try to open a privileged connection */
+  int trypriv;
 };
 
 /** @brief Create a new client
@@ -100,9 +102,21 @@ disorder_client *disorder_new(int verbose) {
 
   c->verbose = verbose;
   c->family = -1;
+  c->trypriv = 1;
   return c;
 }
 
+/** @brief Don't try to make a privileged connection
+ * @param c Client
+ *
+ * You must call this before any of the connection functions (e.g.,
+ * disorder_connect(), disorder_connect_user()), if at all.
+ */
+void disorder_force_unpriv(disorder_client *c) {
+  assert(!c->open);
+  c->trypriv = 0;
+}
+
 /** @brief Return the address family used by this client */
 int disorder_client_af(disorder_client *c) {
   return c->family;
@@ -432,7 +446,9 @@ int disorder_connect_generic(struct config *conf,
   socklen_t salen;
   char errbuf[1024];
 
-  if((salen = find_server(conf, &sa, &c->ident)) == (socklen_t)-1)
+  if((salen = disorder_find_server(conf,
+                                  (c->trypriv ? 0 : DISORDER_FS_NOTPRIV),
+                                  &sa, &c->ident)) == (socklen_t)-1)
     return -1;
   c->input = 0;
   c->output = 0;

diff --git a/lib/client.h b/lib/client.h
index c7964d6d75e74efcbd8363d3be92006cfcb70ac8..c443f7e8365523ccc01c448f470db9dbc84ac72b 100644 (file)
--- a/lib/client.h
+++ b/lib/client.h
@@ -39,6 +39,7 @@ struct kvp;
 struct sink;
 
 disorder_client *disorder_new(int verbose);
+void disorder_force_unpriv(disorder_client *c);
 int disorder_client_af(disorder_client *c);
 int disorder_connect(disorder_client *c);
 int disorder_connect_user(disorder_client *c,