From 0786909fdd181665c867e2347746db4514b9134e Mon Sep 17 00:00:00 2001
From: Ben Kibbey <bjk@luxsci.net>
Date: Sat, 6 Sep 2014 16:35:45 -0400
Subject: [PATCH] Fix STORE creating elements when not permitted.

Also fix incorrect return code when creating a new root element fails.
---
 src/commands.c |  6 ++--
 src/xml.c      | 96 +++++++++++++++++++++++++++++++---------------------------
 src/xml.h      |  2 +-
 3 files changed, 55 insertions(+), 49 deletions(-)

diff --git a/src/commands.c b/src/commands.c
index 2c2a1d5d..23a66f51 100644
--- a/src/commands.c
+++ b/src/commands.c
@@ -1213,11 +1213,11 @@ again:
   n = find_root_element (client, client->doc, &req, &rc, NULL, 0, 0);
   if (rc && rc == GPG_ERR_ELEMENT_NOT_FOUND)
     {
-      rc = new_root_element (client->doc, *req);
+      rc = new_root_element (client, client->doc, *req);
       if (rc)
 	{
 	  strv_free (req);
-	  return send_error (ctx, GPG_ERR_SYNTAX);
+	  return send_error (ctx, rc);
 	}
 
       goto again;
@@ -1873,7 +1873,7 @@ again:
       if (*rc != GPG_ERR_ELEMENT_NOT_FOUND)
 	goto fail;
 
-      *rc = new_root_element (client->doc, req[0]);
+      *rc = new_root_element (client, client->doc, req[0]);
       if (*rc)
 	goto fail;
 
diff --git a/src/xml.c b/src/xml.c
index 6e040ca4..c8e2c9f1 100644
--- a/src/xml.c
+++ b/src/xml.c
@@ -165,12 +165,57 @@ attr_ctime (xmlNodePtr n)
   return rc;
 }
 
+// FIXME add parameter to want RW or RO access to the XML node.
 static gpg_error_t
-create_new_element (xmlNodePtr parent, const char *name, xmlNodePtr * result)
+acl_check (struct client_s *client, xmlNodePtr n)
 {
-  xmlNodePtr n = xmlNewNode (NULL, (xmlChar *) "element");
   gpg_error_t rc;
+  xmlChar *acl = node_has_attribute (n, (xmlChar *) "_acl");
+  char **users = acl ? str_split((char *)acl, ",", 0) : NULL;
+  char **p;
+  int allowed = 0;
+
+  if (client->thd->peer->uid == invoking_uid)
+    return 0;
+
+  if (!acl || !users || !*users)
+    {
+      strv_free(users);
+
+      if (client->thd->peer->uid != invoking_uid)
+        return GPG_ERR_EPERM;
+
+      return 0;
+    }
+
+  for (p = users; *p; p++)
+    {
+      fprintf(stderr, "checking %s\n", *p);
+      rc = acl_check_common (client, *p, client->thd->peer->uid,
+                             client->thd->peer->gid, &allowed);
+      fprintf(stderr, "result: %i, %i\n", rc, allowed);
+    }
+
+  xmlFree(acl);
+  strv_free(users);
+  if (rc)
+    return rc;
+
+  return allowed ? 0 : GPG_ERR_EPERM;
+}
+
+static gpg_error_t
+create_new_element (struct client_s *client, xmlNodePtr parent,
+                    const char *name, xmlNodePtr * result)
+{
+  xmlNodePtr n;
+  gpg_error_t rc;
+
+  rc = acl_check(client, parent);
+  if (rc)
+    return rc;
 
+  n = xmlNewNode (NULL, (xmlChar *) "element");
   if (!n)
     return GPG_ERR_ENOMEM;
 
@@ -192,7 +237,7 @@ create_new_element (xmlNodePtr parent, const char *name, xmlNodePtr * result)
 }
 
 gpg_error_t
-new_root_element (xmlDocPtr doc, char *name)
+new_root_element (struct client_s *client, xmlDocPtr doc, char *name)
 {
   xmlNodePtr root = xmlDocGetRootElement (doc);
   char *p = name;
@@ -206,7 +251,7 @@ new_root_element (xmlDocPtr doc, char *name)
   if (!valid_xml_element ((xmlChar *) p))
     return GPG_ERR_INV_VALUE;
 
-  return create_new_element (root, p, NULL);
+  return create_new_element (client, root, p, NULL);
 }
 
 static xmlDocPtr
@@ -524,7 +569,7 @@ create_target_elements_cb (struct client_s *client,
 	{
 
           if (!*rc)
-            *rc = create_new_element (node, req[i], &node);
+            *rc = create_new_element (client, node, req[i], &node);
 
 	  if (*rc)
 	    return NULL;
@@ -587,7 +632,7 @@ create_elements_cb (struct client_s *client, xmlNodePtr node, char **elements,
 
       if (!n)
 	{
-	  *rc = create_new_element (node, req[i], &node);
+	  *rc = create_new_element (client, node, req[i], &node);
 	  if (*rc)
 	    return NULL;
 	}
@@ -598,45 +643,6 @@ create_elements_cb (struct client_s *client, xmlNodePtr node, char **elements,
   return node;
 }
 
-// FIXME add parameter to want RW or RO access to the XML node.
-static gpg_error_t
-acl_check (struct client_s *client, xmlNodePtr n)
-{
-  gpg_error_t rc;
-  xmlChar *acl = node_has_attribute (n, (xmlChar *) "_acl");
-  char **users = acl ? str_split((char *)acl, ",", 0) : NULL;
-  char **p;
-  int allowed = 0;
-
-  if (client->thd->peer->uid == invoking_uid)
-    return 0;
-
-  if (!acl || !users || !*users)
-    {
-      strv_free(users);
-
-      if (client->thd->peer->uid != invoking_uid)
-        return GPG_ERR_EPERM;
-
-      return 0;
-    }
-
-  for (p = users; *p; p++)
-    {
-      fprintf(stderr, "checking %s\n", *p);
-      rc = acl_check_common (client, *p, client->thd->peer->uid,
-                             client->thd->peer->gid, &allowed);
-      fprintf(stderr, "result: %i, %i\n", rc, allowed);
-    }
-
-  xmlFree(acl);
-  strv_free(users);
-  if (rc)
-    return rc;
-
-  return allowed ? 0 : GPG_ERR_EPERM;
-}
-
 /* The root element is really req[0]. It is need as a pointer in case there is
  * a target attribute so it can be updated. */
 xmlNodePtr
diff --git a/src/xml.h b/src/xml.h
index affb354b..68ffc244 100644
--- a/src/xml.h
+++ b/src/xml.h
@@ -40,7 +40,7 @@ struct element_list_s
   char **req;
 };
 
-gpg_error_t new_root_element (xmlDocPtr doc, char *name);
+gpg_error_t new_root_element (struct client_s *, xmlDocPtr doc, char *name);
 xmlDocPtr new_document (void);
 gpg_error_t list_root_elements (struct client_s *, xmlDocPtr doc,
                                 struct string_s **, int, int);
-- 
2.11.4.GIT