Re: [libvirt] [PATCH] test driver: Add authentication to test driver.

8 Jan 2014


      On 01/08/2014 11:39 AM, Richard W.M. Jones wrote:
...
There is no easy way to test authentication against libvirt.  This
commit modifies the test driver to allow simple username/password
authentication.
You modify the test XML by adding:
<node>
   ...
   <auth>
     <user password="123456">rich</user>
     <user>jane</user>
   </auth>
 </node>
If there are any /node/auth/user elements, then authentication is
required by the test driver (if none are present, then the test driver
will work as before and not require authentication).
Cool - just the sort of thing the test:/// URI is intended for :)
...
@@ -99,6 +107,8 @@ struct _testConn {
     virNodeDeviceObjList devs;
     int numCells;
     testCell cells[MAX_CELLS];
+    int numAuths;
size_t
...
+    testAuthPtr auths;

...
+testParseAuthUsers(testConnPtr privconn,
+                   xmlXPathContextPtr ctxt)
+{
+    int num, ret = -1;
+    size_t i;
+    xmlNodePtr *nodes = NULL;
+
+    num = virXPathNodeSet("/node/auth/user", ctxt, &nodes);
+    if (num < 0)
+        goto error;
+
+    privconn->numAuths = num;
+    if (num && VIR_ALLOC_N(privconn->auths, num) < 0)
+        goto error;
+
+    for (i = 0; i < num; i++) {
+        char *username, *password;
+
+        ctxt->node = nodes[i];
+        username = virXPathString("string(.)", ctxt);
+        if (!username || STREQ(username, "")) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("missing username in /node/auth/user field"));
+            goto error;
+        }
If username is "",...
...
+        /* This field is optional. */
+        password = virXMLPropString(nodes[i], "password");
+
+        privconn->auths[i].username = username;
+        privconn->auths[i].password = password;
+    }
+
+    ret = 0;
+error:
+    VIR_FREE(nodes);
+    return ret;
...then you just leaked malloc'd memory.
...
+    /* Authentication is required because the test XML contains a
+     * non-empty <auth/> section.  First we must ask for a username.
+     */
+    username = virAuthGetUsername(conn, auth, "test", NULL, "localhost"/*?*/);
Is the /*?*/ intentional?
...
+
+found_user:
+    /* Even if we didn't find the user, we still ask for a password. */
+    if (i == -1 || privconn->auths[i].password != NULL) {
Nice - matches good security practice of not hinting to the user which
usernames are valid.  (Not that any user/pw pair in the text XML can be
considered secure so much as a way to test the code base... Anyone
sticking a password they value in the test XML deserves what they get)

This is probably worth having in 1.2.1, if you clean up the problems in
time.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

    