[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]