[libvirt] [PATCH] test driver: Add authentication to test driver.
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). In the example above, two phony users are added: rich password: 123456 jane no password required The test driver will demand a username. If the password attribute is present (or if the username entered is wrong), then the password is also asked for and checked: $ virsh -c test://$(pwd)/testnode.xml list Enter username for localhost: rich Enter rich's password for localhost: *** Id Name State ---------------------------------------------------- 1 fv0 running 2 fc4 running Signed-off-by: Richard W.M. Jones <rjones@redhat.com> --- src/test/test_driver.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 109 insertions(+), 1 deletion(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index cde82a1..ecad351 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -58,6 +58,7 @@ #include "virrandom.h" #include "virstring.h" #include "cpu/cpu.h" +#include "virauth.h" #define VIR_FROM_THIS VIR_FROM_TEST @@ -82,6 +83,13 @@ typedef struct _testCell *testCellPtr; #define MAX_CELLS 128 +struct _testAuth { + char *username; + char *password; +}; +typedef struct _testAuth testAuth; +typedef struct _testAuth *testAuthPtr; + struct _testConn { virMutex lock; @@ -99,6 +107,8 @@ struct _testConn { virNodeDeviceObjList devs; int numCells; testCell cells[MAX_CELLS]; + int numAuths; + testAuthPtr auths; virObjectEventStatePtr eventState; }; @@ -1355,6 +1365,44 @@ error: return ret; } +static int +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; + } + /* 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; +} /* No shared state between simultaneous test connections initialized * from a file. */ @@ -1417,6 +1465,8 @@ testOpenFromFile(virConnectPtr conn, const char *file) goto error; if (testParseNodedevs(privconn, file, ctxt) < 0) goto error; + if (testParseAuthUsers(privconn, ctxt) < 0) + goto error; xmlXPathFreeContext(ctxt); xmlFreeDoc(doc); @@ -1439,9 +1489,63 @@ testOpenFromFile(virConnectPtr conn, const char *file) return VIR_DRV_OPEN_ERROR; } +static int +testConnectAuthenticate(virConnectPtr conn, + virConnectAuthPtr auth) +{ + testConnPtr privconn = conn->privateData; + int ret = -1; + int i; + char *username = NULL, *password = NULL; + + if (privconn->numAuths == 0) + return 0; + + /* 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"/*?*/); + if (!username) { + virReportError(VIR_ERR_AUTH_FAILED, "%s", + _("authentication failed when asking for username")); + goto cleanup; + } + + /* Does the username exist? */ + for (i = 0; i < privconn->numAuths; ++i) { + if (STREQ(privconn->auths[i].username, username)) + goto found_user; + } + i = -1; + +found_user: + /* Even if we didn't find the user, we still ask for a password. */ + if (i == -1 || privconn->auths[i].password != NULL) { + password = virAuthGetPassword(conn, auth, "test", + username, "localhost"); + if (password == NULL) { + virReportError(VIR_ERR_AUTH_FAILED, "%s", + _("authentication failed when asking for password")); + goto cleanup; + } + } + + if (i == -1 || + (password && STRNEQ(privconn->auths[i].password, password))) { + virReportError(VIR_ERR_AUTH_FAILED, "%s", + _("authentication failed, see test XML for the correct username/password")); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(username); + VIR_FREE(password); + return ret; +} static virDrvOpenStatus testConnectOpen(virConnectPtr conn, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConnectAuthPtr auth, unsigned int flags) { int ret; @@ -1479,6 +1583,10 @@ static virDrvOpenStatus testConnectOpen(virConnectPtr conn, if (ret != VIR_DRV_OPEN_SUCCESS) return ret; + /* Fake authentication. */ + if (testConnectAuthenticate(conn, auth) < 0) + return VIR_DRV_OPEN_ERROR; + return VIR_DRV_OPEN_SUCCESS; } -- 1.8.4.2
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
On Wed, Jan 08, 2014 at 12:33:20PM -0700, Eric Blake wrote:
+ username = virAuthGetUsername(conn, auth, "test", NULL, "localhost"/*?*/);
Is the /*?*/ intentional?
It's intentionally a question :-) The virAuthGetUsername is demanding that I pass a non-null hostname here. I don't have a non-null hostname. conn->uri->server is the closest thing, but in the test driver that would always be NULL (it might be non-NULL if visiting the test driver via the remote driver, but this code is on the wrong side of the connection). However this does not particularly matter. virAuthGetUsername just uses in a message for the user. v2 patch coming up shortly. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
On Wed, Jan 08, 2014 at 06:39:40PM +0000, 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).
The API is explicitly designed to avoid hardcoding a fixed notion of usernames + passwords, so I think the test driver should do the same. ie we'd want XML in terms of credential types. <auth> <subject> <credential type='username'>rich</credential> <credential type='password'>123456</credential> </subject> ...more subjects... </auth> Regards. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Fri, Jan 10, 2014 at 02:15:00PM +0000, Daniel P. Berrange wrote:
On Wed, Jan 08, 2014 at 06:39:40PM +0000, 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).
The API is explicitly designed to avoid hardcoding a fixed notion of usernames + passwords, so I think the test driver should do the same. ie we'd want XML in terms of credential types.
<auth> <subject> <credential type='username'>rich</credential> <credential type='password'>123456</credential> </subject> ...more subjects... </auth>
I'd agree too, but I tried a generic implementation along these lines and it was rather complex. Firstly an actual driver would always ask for some distinguishing user name so it knows who it's authenticating, making "username" special in some sense. (This is why <user>username</user> is not an XML property). Secondly a fully general authentication method is actually imperative. It could ask you to type your mum's maiden name first, then could come back with a second block of credential requests, and so on. (Even with loops!) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Richard W.M. Jones