[libvirt] [PATCH 0/3] Experimental support for DTrace probes

This patchset provides the infrastructure for supporting dynamic probing of libvirtd, using static DTrace markers. This can be used by SystemTAP on Linux, or DTrace on Solaris/OS-X/BSD for low overhead tracing. The proof of concept provides a handful of markers wrt to network client connections, security & auth. Obviously it can be expanded to cover a huge area of our codebase for different tasks. The hard bit is deciding what should be exposed as a probe point. Ideally probes should not be changed/removed once added, since this would break any user tracing scripts. So a little care needs to be taken in placing probes to be robust against future code re-factoring. Daniel

Refactor some daemon code to facilitate the introductioin of static probes, sanitizing function exit paths in many places * daemon/libvirtd.c: Pass the dname string into remoteCheckDN to let caller deal with failure paths. Add separate exit paths to remoteCheckCertificate for auth failure vs denial. Merge all exit paths in qemudDispatchServer to one cleanup block * daemon/remote.c: Add separate exit paths to SASL & PolicyKit functions for auth failure vs denial --- daemon/libvirtd.c | 160 +++++++++++++++++++++++++++-------------------------- daemon/remote.c | 83 ++++++++++++++++++++++------ 2 files changed, 147 insertions(+), 96 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 711360b..5e18077 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1116,19 +1116,9 @@ remoteInitializeTLSSession (void) /* Check DN is on tls_allowed_dn_list. */ static int -remoteCheckDN (gnutls_x509_crt_t cert) +remoteCheckDN (const char *dname) { - char name[256]; - size_t namesize = sizeof name; char **wildcards; - int err; - - err = gnutls_x509_crt_get_dn (cert, name, &namesize); - if (err != 0) { - VIR_ERROR(_("remoteCheckDN: gnutls_x509_cert_get_dn: %s"), - gnutls_strerror (err)); - return 0; - } /* If the list is not set, allow any DN. */ wildcards = tls_allowed_dn_list; @@ -1136,62 +1126,62 @@ remoteCheckDN (gnutls_x509_crt_t cert) return 1; while (*wildcards) { - if (fnmatch (*wildcards, name, 0) == 0) + if (fnmatch (*wildcards, dname, 0) == 0) return 1; wildcards++; } /* Print the client's DN. */ - DEBUG(_("remoteCheckDN: failed: client DN is %s"), name); + DEBUG(_("remoteCheckDN: failed: client DN is %s"), dname); return 0; // Not found. } static int -remoteCheckCertificate (gnutls_session_t session) +remoteCheckCertificate(struct qemud_client *client) { int ret; unsigned int status; const gnutls_datum_t *certs; unsigned int nCerts, i; time_t now; + char name[256]; + size_t namesize = sizeof name; + + memset(name, 0, namesize); - if ((ret = gnutls_certificate_verify_peers2 (session, &status)) < 0){ - VIR_ERROR(_("remoteCheckCertificate: verify failed: %s"), + if ((ret = gnutls_certificate_verify_peers2 (client->tlssession, &status)) < 0){ + VIR_ERROR(_("Failed to verify certificate peers: %s"), gnutls_strerror (ret)); - return -1; + goto authdeny; } if (status != 0) { if (status & GNUTLS_CERT_INVALID) - VIR_ERROR0(_("remoteCheckCertificate: " - "the client certificate is not trusted.")); + VIR_ERROR0(_("The client certificate is not trusted.")); if (status & GNUTLS_CERT_SIGNER_NOT_FOUND) - VIR_ERROR0(_("remoteCheckCertificate: the client " - "certificate has unknown issuer.")); + VIR_ERROR0(_("The client certificate has unknown issuer.")); if (status & GNUTLS_CERT_REVOKED) - VIR_ERROR0(_("remoteCheckCertificate: " - "the client certificate has been revoked.")); + VIR_ERROR0(_("The client certificate has been revoked.")); #ifndef GNUTLS_1_0_COMPAT if (status & GNUTLS_CERT_INSECURE_ALGORITHM) - VIR_ERROR0(_("remoteCheckCertificate: the client certificate" - " uses an insecure algorithm.")); + VIR_ERROR0(_("The client certificate uses an insecure algorithm.")); #endif - return -1; + goto authdeny; } - if (gnutls_certificate_type_get (session) != GNUTLS_CRT_X509) { - VIR_ERROR0(_("remoteCheckCertificate: certificate is not X.509")); - return -1; + if (gnutls_certificate_type_get(client->tlssession) != GNUTLS_CRT_X509) { + VIR_ERROR0(_("Only x509 certificates are supported")); + goto authdeny; } - if (!(certs = gnutls_certificate_get_peers(session, &nCerts))) { - VIR_ERROR0(_("remoteCheckCertificate: no peers")); - return -1; + if (!(certs = gnutls_certificate_get_peers(client->tlssession, &nCerts))) { + VIR_ERROR0(_("The certificate has no peers")); + goto authdeny; } now = time (NULL); @@ -1200,40 +1190,57 @@ remoteCheckCertificate (gnutls_session_t session) gnutls_x509_crt_t cert; if (gnutls_x509_crt_init (&cert) < 0) { - VIR_ERROR0(_("remoteCheckCertificate: gnutls_x509_crt_init failed")); - return -1; + VIR_ERROR0(_("Unable to initialize certificate")); + goto authfail; } if (gnutls_x509_crt_import(cert, &certs[i], GNUTLS_X509_FMT_DER) < 0) { + VIR_ERROR0(_("Unable to load certificate")); gnutls_x509_crt_deinit (cert); - return -1; + goto authfail; + } + + if (i == 0) { + ret = gnutls_x509_crt_get_dn (cert, name, &namesize); + if (ret != 0) { + VIR_ERROR(_("Failed to get certificate distinguished name: %s"), + gnutls_strerror(ret)); + gnutls_x509_crt_deinit (cert); + goto authfail; + } + + if (!remoteCheckDN (name)) { + /* This is the most common error: make it informative. */ + VIR_ERROR0(_("Client's Distinguished Name is not on the list " + "of allowed clients (tls_allowed_dn_list). Use " + "'certtool -i -infile clientcert.pem' to view the" + "Distinguished Name field in the client certificate," + "or run this daemon with --verbose option.")); + gnutls_x509_crt_deinit (cert); + goto authdeny; + } } if (gnutls_x509_crt_get_expiration_time (cert) < now) { - VIR_ERROR0(_("remoteCheckCertificate: " - "the client certificate has expired")); + VIR_ERROR0(_("The client certificate has expired")); gnutls_x509_crt_deinit (cert); - return -1; + goto authdeny; } if (gnutls_x509_crt_get_activation_time (cert) > now) { - VIR_ERROR0(_("remoteCheckCertificate: the client " - "certificate is not yet activated")); + VIR_ERROR0(_("The client certificate is not yet active")); gnutls_x509_crt_deinit (cert); - return -1; - } - - if (i == 0) { - if (!remoteCheckDN (cert)) { - /* This is the most common error: make it informative. */ - VIR_ERROR0(_("remoteCheckCertificate: client's Distinguished Name is not on the list of allowed clients (tls_allowed_dn_list). Use 'openssl x509 -in clientcert.pem -text' to view the Distinguished Name field in the client certificate, or run this daemon with --verbose option.")); - gnutls_x509_crt_deinit (cert); - return -1; - } + goto authdeny; } } return 0; + +authdeny: + return -1; + +authfail: + return -1; } /* Check the client's access. */ @@ -1243,7 +1250,7 @@ remoteCheckAccess (struct qemud_client *client) struct qemud_client_message *confirm; /* Verify client certificate. */ - if (remoteCheckCertificate (client->tlssession) == -1) { + if (remoteCheckCertificate (client) == -1) { VIR_ERROR0(_("remoteCheckCertificate: " "failed to verify client's certificate")); if (!tls_no_verify_certificate) return -1; @@ -1301,7 +1308,7 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket int fd; struct sockaddr_storage addr; socklen_t addrlen = (socklen_t) (sizeof addr); - struct qemud_client *client; + struct qemud_client *client = NULL; int no_slow_start = 1; int i; @@ -1316,14 +1323,12 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket if (server->nclients >= max_clients) { VIR_ERROR(_("Too many active clients (%d), dropping connection"), max_clients); - close(fd); - return -1; + goto error; } if (VIR_REALLOC_N(server->clients, server->nclients+1) < 0) { VIR_ERROR0(_("Out of memory allocating clients")); - close(fd); - return -1; + goto error; } #ifdef __sun @@ -1335,14 +1340,12 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket (privs = ucred_getprivset (ucred, PRIV_EFFECTIVE)) == NULL) { if (ucred != NULL) ucred_free (ucred); - close (fd); - return -1; + goto error; } if (!priv_ismember (privs, PRIV_VIRT_MANAGE)) { ucred_free (ucred); - close (fd); - return -1; + goto error; } ucred_free (ucred); @@ -1355,16 +1358,14 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket if (virSetCloseExec(fd) < 0 || virSetNonBlock(fd) < 0) { - close(fd); - return -1; + goto error; } if (VIR_ALLOC(client) < 0) - goto cleanup; + goto error; if (virMutexInit(&client->lock) < 0) { VIR_ERROR0(_("cannot initialize mutex")); - VIR_FREE(client); - goto cleanup; + goto error; } client->magic = QEMUD_CLIENT_MAGIC; @@ -1381,7 +1382,7 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket /* Prepare one for packet receive */ if (VIR_ALLOC(client->rx) < 0) - goto cleanup; + goto error; client->rx->bufferLength = REMOTE_MESSAGE_HEADER_XDR_LEN; @@ -1395,7 +1396,7 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket pid_t pid; if (qemudGetSocketIdentity(client->fd, &uid, &pid) < 0) - goto cleanup; + goto error; /* Client is running as root, so disable auth */ if (uid == 0) { @@ -1408,13 +1409,13 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket if (client->type != QEMUD_SOCK_TYPE_TLS) { /* Plain socket, so prepare to read first message */ if (qemudRegisterClientEvent (server, client) < 0) - goto cleanup; + goto error; } else { int ret; client->tlssession = remoteInitializeTLSSession (); if (client->tlssession == NULL) - goto cleanup; + goto error; gnutls_transport_set_ptr (client->tlssession, (gnutls_transport_ptr_t) (long) fd); @@ -1426,21 +1427,21 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket /* Unlikely, but ... Next step is to check the certificate. */ if (remoteCheckAccess (client) == -1) - goto cleanup; + goto error; /* Handshake & cert check OK, so prepare to read first message */ if (qemudRegisterClientEvent(server, client) < 0) - goto cleanup; + goto error; } else if (ret == GNUTLS_E_INTERRUPTED || ret == GNUTLS_E_AGAIN) { /* Most likely, need to do more handshake data */ client->handshake = 1; if (qemudRegisterClientEvent (server, client) < 0) - goto cleanup; + goto error; } else { VIR_ERROR(_("TLS handshake failed: %s"), gnutls_strerror (ret)); - goto cleanup; + goto error; } } @@ -1461,13 +1462,14 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket return 0; - cleanup: - if (client && - client->tlssession) gnutls_deinit (client->tlssession); +error: + if (client) { + if (client->tlssession) gnutls_deinit (client->tlssession); + if (client) + VIR_FREE(client->rx); + VIR_FREE(client); + } close (fd); - if (client) - VIR_FREE(client->rx); - VIR_FREE(client); return -1; } diff --git a/daemon/remote.c b/daemon/remote.c index 118654c..3db9790 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -3460,7 +3460,9 @@ error: /* We asked for an SSF layer, so sanity check that we actually - * got what we asked for */ + * got what we asked for + * Returns 0 if ok, -1 on error, -2 if rejected + */ static int remoteSASLCheckSSF (struct qemud_client *client, remote_error *rerr) { @@ -3487,7 +3489,7 @@ remoteSASLCheckSSF (struct qemud_client *client, remoteDispatchAuthError(rerr); sasl_dispose(&client->saslconn); client->saslconn = NULL; - return -1; + return -2; } /* Only setup for read initially, because we're about to send an RPC @@ -3502,6 +3504,9 @@ remoteSASLCheckSSF (struct qemud_client *client, return 0; } +/* + * Returns 0 if ok, -1 on error, -2 if rejected + */ static int remoteSASLCheckAccess (struct qemud_server *server, struct qemud_client *client, @@ -3553,7 +3558,7 @@ remoteSASLCheckAccess (struct qemud_server *server, remoteDispatchAuthError(rerr); sasl_dispose(&client->saslconn); client->saslconn = NULL; - return -1; + return -2; } @@ -3625,12 +3630,14 @@ remoteDispatchAuthSaslStart (struct qemud_server *server, if (err == SASL_CONTINUE) { ret->complete = 0; } else { - if (remoteSASLCheckSSF(client, rerr) < 0) - goto error; - /* Check username whitelist ACL */ - if (remoteSASLCheckAccess(server, client, rerr) < 0) - goto error; + if ((err = remoteSASLCheckAccess(server, client, rerr)) < 0 || + (err = remoteSASLCheckSSF(client, rerr)) < 0) { + if (err == -2) + goto authdeny; + else + goto authfail; + } REMOTE_DEBUG("Authentication successful %d", client->fd); ret->complete = 1; @@ -3642,6 +3649,11 @@ remoteDispatchAuthSaslStart (struct qemud_server *server, authfail: remoteDispatchAuthError(rerr); + goto error; + +authdeny: + goto error; + error: virMutexUnlock(&client->lock); return -1; @@ -3714,12 +3726,14 @@ remoteDispatchAuthSaslStep (struct qemud_server *server, if (err == SASL_CONTINUE) { ret->complete = 0; } else { - if (remoteSASLCheckSSF(client, rerr) < 0) - goto error; - /* Check username whitelist ACL */ - if (remoteSASLCheckAccess(server, client, rerr) < 0) - goto error; + if ((err = remoteSASLCheckAccess(server, client, rerr)) < 0 || + (err = remoteSASLCheckSSF(client, rerr)) < 0) { + if (err == -2) + goto authdeny; + else + goto authfail; + } REMOTE_DEBUG("Authentication successful %d", client->fd); ret->complete = 1; @@ -3731,6 +3745,11 @@ remoteDispatchAuthSaslStep (struct qemud_server *server, authfail: remoteDispatchAuthError(rerr); + goto error; + +authdeny: + goto error; + error: virMutexUnlock(&client->lock); return -1; @@ -3792,13 +3811,16 @@ remoteDispatchAuthPolkit (struct qemud_server *server, void *args ATTRIBUTE_UNUSED, remote_auth_polkit_ret *ret) { - pid_t callerPid; - uid_t callerUid; + pid_t callerPid = -1; + uid_t callerUid = -1; const char *action; int status = -1; char pidbuf[50]; + char ident[100]; int rv; + memset(ident, 0, sizeof ident); + virMutexLock(&server->lock); virMutexLock(&client->lock); virMutexUnlock(&server->lock); @@ -3834,6 +3856,12 @@ remoteDispatchAuthPolkit (struct qemud_server *server, goto authfail; } + rv = snprintf(ident, sizeof ident, "pid:%d,uid:%d", callerPid, callerUid); + if (rv < 0 || rv >= sizeof ident) { + VIR_ERROR(_("Caller identity was too large %d:%d"), callerPid, callerUid); + goto authfail; + } + if (virRun(pkcheck, &status) < 0) { VIR_ERROR(_("Cannot invoke %s"), PKCHECK_PATH); goto authfail; @@ -3841,7 +3869,7 @@ remoteDispatchAuthPolkit (struct qemud_server *server, if (status != 0) { VIR_ERROR(_("Policy kit denied action %s from pid %d, uid %d, result: %d"), action, callerPid, callerUid, status); - goto authfail; + goto authdeny; } VIR_INFO(_("Policy allowed action %s from pid %d, uid %d"), action, callerPid, callerUid); @@ -3852,6 +3880,12 @@ remoteDispatchAuthPolkit (struct qemud_server *server, return 0; authfail: + goto error; + +authdeny: + goto error; + +error: remoteDispatchAuthError(rerr); virMutexUnlock(&client->lock); return -1; @@ -3875,6 +3909,9 @@ remoteDispatchAuthPolkit (struct qemud_server *server, PolKitResult pkresult; DBusError err; const char *action; + char ident[100]; + + memset(ident, 0, sizeof ident); virMutexLock(&server->lock); virMutexLock(&client->lock); @@ -3895,6 +3932,12 @@ remoteDispatchAuthPolkit (struct qemud_server *server, goto authfail; } + rv = snprintf(ident, sizeof ident, "pid:%d,uid:%d", callerPid, callerUid); + if (rv < 0 || rv >= sizeof ident) { + VIR_ERROR(_("Caller identity was too large %d:%d"), callerPid, callerUid); + goto authfail; + } + VIR_INFO(_("Checking PID %d running as %d"), callerPid, callerUid); dbus_error_init(&err); if (!(pkcaller = polkit_caller_new_from_pid(server->sysbus, @@ -3951,7 +3994,7 @@ remoteDispatchAuthPolkit (struct qemud_server *server, VIR_ERROR(_("Policy kit denied action %s from pid %d, uid %d, result: %s"), action, callerPid, callerUid, polkit_result_to_string_representation(pkresult)); - goto authfail; + goto authdeny; } VIR_INFO(_("Policy allowed action %s from pid %d, uid %d, result %s"), action, callerPid, callerUid, @@ -3963,6 +4006,12 @@ remoteDispatchAuthPolkit (struct qemud_server *server, return 0; authfail: + goto error; + +authdeny: + goto error; + +error: remoteDispatchAuthError(rerr); virMutexUnlock(&client->lock); return -1; -- 1.7.2.2

The addrToString methods were not coping with UNIX domain sockets which have no normal host+port address. Hardcode special handling for these so that SASL routines can work over UNIX sockets. Also fix up SSF logic in remote client so that it presumes that a UNIX socket is secure * daemon/remote.c: Fix addrToString for UNIX sockets. * src/remote/remote_driver.c: Fix addrToString for UNIX sockets and fix SSF logic to work for TLS + UNIX sockets in the same manner --- daemon/remote.c | 8 ++++++++ src/remote/remote_driver.c | 22 +++++++++++++++------- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 3db9790..6b67678 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -3263,6 +3263,14 @@ static char *addrToString(remote_error *rerr, int err; struct sockaddr *sa = (struct sockaddr *)ss; + if (sa->sa_family == AF_UNIX) { + if (!(addr = strdup("127.0.0.1;0"))) { + virReportOOMError(); + return NULL; + } + return addr; + } + if ((err = getnameinfo(sa, salen, host, sizeof(host), port, sizeof(port), diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a945710..acba01e 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6676,6 +6676,14 @@ static char *addrToString(struct sockaddr_storage *ss, socklen_t salen) int err; struct sockaddr *sa = (struct sockaddr *)ss; + if (sa->sa_family == AF_UNIX) { + if (!(addr = strdup("127.0.0.1;0"))) { + virReportOOMError(); + return NULL; + } + return addr; + } + if ((err = getnameinfo(sa, salen, host, sizeof(host), port, sizeof(port), @@ -6977,12 +6985,12 @@ remoteAuthSASL (virConnectPtr conn, struct private_data *priv, int in_open, } memset (&secprops, 0, sizeof secprops); - /* If we've got TLS, we don't care about SSF */ - secprops.min_ssf = priv->uses_tls ? 0 : 56; /* Equiv to DES supported by all Kerberos */ - secprops.max_ssf = priv->uses_tls ? 0 : 100000; /* Very strong ! AES == 256 */ + /* If we've got a secure channel (TLS or UNIX sock), we don't care about SSF */ + secprops.min_ssf = priv->is_secure ? 0 : 56; /* Equiv to DES supported by all Kerberos */ + secprops.max_ssf = priv->is_secure ? 0 : 100000; /* Very strong ! AES == 256 */ secprops.maxbufsize = 100000; - /* If we're not TLS, then forbid any anonymous or trivially crackable auth */ - secprops.security_flags = priv->uses_tls ? 0 : + /* If we're not secure, then forbid any anonymous or trivially crackable auth */ + secprops.security_flags = priv->is_secure ? 0 : SASL_SEC_NOANONYMOUS | SASL_SEC_NOPLAINTEXT; err = sasl_setprop(saslconn, SASL_SEC_PROPS, &secprops); @@ -7164,8 +7172,8 @@ remoteAuthSASL (virConnectPtr conn, struct private_data *priv, int in_open, } } - /* Check for suitable SSF if non-TLS */ - if (!priv->uses_tls) { + /* Check for suitable SSF if not already secure (TLS or UNIX sock) */ + if (!priv->is_secure) { err = sasl_getprop(saslconn, SASL_SSF, &val); if (err != SASL_OK) { remoteError(VIR_ERR_AUTH_FAILED, -- 1.7.2.2

Adds initial support for dtrace static probes in libvirtd daemon, assuming use of systemtap dtrace compat shim on Linux. The probes are inserted for network client connect, disconnect, TLS handshake states and authentication protocol states. This can be tested by running the xample program and then attempting to connect with any libvirt client (virsh, virt-manager, etc). # stap examples/systemtap/client.stp Client fd=44 connected readonly=0 Client fd=44 auth polkit deny pid:24997,uid:500 Client fd=44 disconnected Client fd=46 connected readonly=1 Client fd=46 auth sasl allow test Client fd=46 disconnected For unknown reasons, libvirtd must be restarted after the stap script is launched, otherwise the probes are not enabled. This bug needs to be fixed, probably in systemtap itself, to allow probing an existing running daemon. The connect/disconnect events need to be augmented to include the socket address data. The libvirtd.stp file should also really not be required, since it is duplicated info that is already available in the main probes.d definition file. A script to autogenerate the .stp file is needed, either in libvirtd tree, or better as part of systemtap itself. * Makefile.am: Add examples/systemtap subdir * autobuild.sh: Disable dtrace for mingw32 * configure.ac: Add check for dtrace * daemon/.gitignore: Ignore generated dtrace probe file * daemon/Makefile.am: Build dtrace probe header & object files * daemon/libvirtd.stp: SystemTAP convenience probeset * daemon/libvirtd.c: Add connect/disconnect & TLS probes * daemon/remote.c: Add SASL and PolicyKit auth probes * daemon/probes.d: Master probe definition * daemon/libvirtd.h: Add convenience macro for probes so that compilation is a no-op when dtrace is not available * examples/systemtap/Makefile.am, examples/systemtap/client.stp Example systemtap script using dtrace probe markers * libvirt.spec.in: Enable dtrace on F13/RHEL6 * mingw32-libvirt.spec.in: Force disable dtrace --- Makefile.am | 2 +- autobuild.sh | 1 + configure.ac | 25 ++++++++++++++++ daemon/.gitignore | 1 + daemon/Makefile.am | 24 ++++++++++++++- daemon/libvirtd.c | 9 ++++++ daemon/libvirtd.h | 13 ++++++++ daemon/libvirtd.stp | 63 ++++++++++++++++++++++++++++++++++++++++ daemon/probes.d | 12 +++++++ daemon/remote.c | 16 ++++++++++ examples/systemtap/Makefile.am | 2 + examples/systemtap/client.stp | 28 +++++++++++++++++ libvirt.spec.in | 15 +++++++++- mingw32-libvirt.spec.in | 1 + 14 files changed, 209 insertions(+), 3 deletions(-) create mode 100644 daemon/libvirtd.stp create mode 100644 daemon/probes.d create mode 100644 examples/systemtap/Makefile.am create mode 100644 examples/systemtap/client.stp diff --git a/Makefile.am b/Makefile.am index c5d278b..b51bd2b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -6,7 +6,7 @@ GENHTML = genhtml SUBDIRS = gnulib/lib include src daemon tools proxy docs gnulib/tests \ python tests po examples/domain-events/events-c examples/hellolibvirt \ examples/dominfo examples/domsuspend examples/python examples/apparmor \ - examples/xml/nwfilter examples/openauth + examples/xml/nwfilter examples/openauth examples/systemtap ACLOCAL_AMFLAGS = -I m4 -I gnulib/m4 diff --git a/autobuild.sh b/autobuild.sh index c527479..02a5037 100755 --- a/autobuild.sh +++ b/autobuild.sh @@ -85,6 +85,7 @@ if [ -x /usr/bin/i686-pc-mingw32-gcc ]; then --without-one \ --without-phyp \ --without-netcf \ + --without-dtrace \ --without-libvirtd make diff --git a/configure.ac b/configure.ac index ecaf9cb..9076fce 100644 --- a/configure.ac +++ b/configure.ac @@ -1057,6 +1057,29 @@ fi AM_CONDITIONAL([WITH_SECDRIVER_APPARMOR], [test "$with_secdriver_apparmor" != "no"]) +dnl DTrace static probes +AC_ARG_WITH([dtrace], + AC_HELP_STRING([--with-dtrace], [use dtrace for static probing @<:@default=check@:>@]), + [], + [with_dtrace=check]) + +if test "$with_dtrace" != "no" ; then + AC_PATH_PROG([DTRACE], [dtrace], [], [/bin:/usr/bin]) + if test -z "$DTRACE" ; then + if test "$with_dtrace" = "check"; then + with_dtrace=no + else + AC_MSG_ERROR([You must install the 'dtrace' binary to enable libvirt static probes]) + fi + else + with_dtrace=yes + fi + if test "$with_dtrace" = "yes"; then + AC_DEFINE_UNQUOTED([WITH_DTRACE], 1, [whether DTrace static probes are available]) + fi +fi +AM_CONDITIONAL([WITH_DTRACE], [test "$with_dtrace" != "no"]) + dnl NUMA lib AC_ARG_WITH([numactl], @@ -2147,6 +2170,7 @@ AC_OUTPUT(Makefile src/Makefile include/Makefile docs/Makefile \ examples/openauth/Makefile \ examples/python/Makefile \ examples/hellolibvirt/Makefile \ + examples/systemtap/Makefile \ examples/xml/nwfilter/Makefile) AC_MSG_NOTICE([]) @@ -2308,6 +2332,7 @@ AC_MSG_NOTICE([ Debug: $enable_debug]) AC_MSG_NOTICE([ Warnings: $enable_compile_warnings]) AC_MSG_NOTICE([ Readline: $lv_use_readline]) AC_MSG_NOTICE([ Python: $with_python]) +AC_MSG_NOTICE([ DTrace: $with_dtrace]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Privileges]) AC_MSG_NOTICE([]) diff --git a/daemon/.gitignore b/daemon/.gitignore index 8ed7784..840ddd8 100644 --- a/daemon/.gitignore +++ b/daemon/.gitignore @@ -10,3 +10,4 @@ libvirtd.init libvirtd*.logrotate libvirtd.pod libvirtd.8 +probes.h diff --git a/daemon/Makefile.am b/daemon/Makefile.am index b020b77..c51b4a9 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -1,5 +1,7 @@ ## Process this file with automake to produce Makefile.in +CLEANFILES = + DAEMON_SOURCES = \ event.c event.h \ libvirtd.c libvirtd.h \ @@ -36,6 +38,7 @@ EXTRA_DIST = \ test_libvirtd.aug \ THREADING.txt \ libvirtd.pod.in \ + libvirtd.stp \ $(AVAHI_SOURCES) \ $(DAEMON_SOURCES) @@ -163,6 +166,25 @@ libvirtd_CFLAGS += $(AVAHI_CFLAGS) libvirtd_LDADD += $(AVAHI_LIBS) endif +EXTRA_DIST += probes.d libvirtd.stp + +if WITH_DTRACE +libvirtd_LDADD += probes.o +libvirtd_SOURCES += probes.h + +BUILT_SOURCES += probes.h + +tapsetdir = $(datadir)/systemtap/tapsets +tapset_DATA = libvirtd.stp + +probes.h: probes.d + $(AM_V_GEN)$(DTRACE) -o $@ -h -s $< + +probes.o: probes.d + $(AM_V_GEN)$(DTRACE) -o $@ -G -s $< + +CLEANFILES += probes.h probes.o +endif install-data-local: install-init install-data-sasl install-data-polkit \ install-logrotate @@ -319,5 +341,5 @@ uninstall-data-sasl: endif -CLEANFILES = $(BUILT_SOURCES) $(man_MANS) libvirtd.pod +CLEANFILES += $(BUILT_SOURCES) $(man_MANS) libvirtd.pod CLEANFILES += *.cov *.gcov .libs/*.gcda .libs/*.gcno *.gcno *.gcda diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 5e18077..884aaff 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1234,12 +1234,15 @@ remoteCheckCertificate(struct qemud_client *client) } } + PROBE(CLIENT_TLS_ALLOW, client->fd, name); return 0; authdeny: + PROBE(CLIENT_TLS_DENY, client->fd, name); return -1; authfail: + PROBE(CLIENT_TLS_FAIL, client->fd); return -1; } @@ -1321,6 +1324,8 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket return -1; } + PROBE(CLIENT_CONNECT, fd, sock->readonly); + if (server->nclients >= max_clients) { VIR_ERROR(_("Too many active clients (%d), dropping connection"), max_clients); goto error; @@ -1439,6 +1444,7 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket if (qemudRegisterClientEvent (server, client) < 0) goto error; } else { + PROBE(CLIENT_TLS_FAIL, client->fd); VIR_ERROR(_("TLS handshake failed: %s"), gnutls_strerror (ret)); goto error; @@ -1470,6 +1476,7 @@ error: VIR_FREE(client); } close (fd); + PROBE(CLIENT_DISCONNECT, fd); return -1; } @@ -1513,6 +1520,7 @@ void qemudDispatchClientFailure(struct qemud_client *client) { client->tlssession = NULL; } if (client->fd != -1) { + PROBE(CLIENT_DISCONNECT, client->fd); close(client->fd); client->fd = -1; } @@ -2073,6 +2081,7 @@ qemudDispatchClientHandshake(struct qemud_client *client) { direction has changed */ qemudUpdateClientEvent (client); } else { + PROBE(CLIENT_TLS_FAIL, client->fd); /* Fatal error in handshake */ VIR_ERROR(_("TLS handshake failed: %s"), gnutls_strerror (ret)); diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index 3f13fb1..1d06886 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -49,6 +49,19 @@ # include "logging.h" # include "threads.h" +# if WITH_DTRACE +# ifndef LIBVIRTD_PROBES_H +# define LIBVIRTD_PROBES_H +# include "probes.h" +# endif /* LIBVIRTD_PROBES_H */ +# define PROBE(NAME, ...) \ + if (LIBVIRTD_ ## NAME ## _ENABLED()) { \ + LIBVIRTD_ ## NAME(__VA_ARGS__); \ + } +# else +# define PROBE(NAME, ...) +# endif + # ifdef __GNUC__ # ifdef HAVE_ANSIDECL_H # include <ansidecl.h> diff --git a/daemon/libvirtd.stp b/daemon/libvirtd.stp new file mode 100644 index 0000000..6e8b929 --- /dev/null +++ b/daemon/libvirtd.stp @@ -0,0 +1,63 @@ +probe libvirt.daemon.client.connect = process("libvirtd").mark("client_connect") +{ + fd = $arg1; + readonly = $arg2; +} + +probe libvirt.daemon.client.disconnect = process("libvirtd").mark("client_disconnect") +{ + fd = $arg1; +} + + +probe libvirt.daemon.client.tls_allow = process("libvirtd").mark("client_tls_allow") +{ + fd = $arg1; + x509dname = user_string($arg2); +} + +probe libvirt.daemon.client.tls_deny = process("libvirtd").mark("client_tls_deny") +{ + fd = $arg1; + x509dname = user_string($arg2); +} + +probe libvirt.daemon.client.tls_fail = process("libvirtd").mark("client_tls_fail") +{ + fd = $arg1; +} + + +function authtype_to_string(authtype) { + if (authtype == 0) + return "none" + if (authtype == 1) + return "sasl" + if (authtype == 2) + return "polkit" + return "unknown" +} + + +probe libvirt.daemon.client.auth_allow = process("libvirtd").mark("client_auth_allow") +{ + fd = $arg1; + authtype = $arg2; + authname = authtype_to_string($arg2); + identity = user_string($arg3); +} + +probe libvirt.daemon.client.auth_deny = process("libvirtd").mark("client_auth_deny") +{ + fd = $arg1; + authtype = $arg2; + authname = authtype_to_string($arg2); + identity = user_string($arg3); +} + +probe libvirt.daemon.client.auth_fail = process("libvirtd").mark("client_auth_fail") +{ + fd = $arg1; + authtype = $arg2; + authname = authtype_to_string($arg2); +} diff --git a/daemon/probes.d b/daemon/probes.d new file mode 100644 index 0000000..d8cc77c --- /dev/null +++ b/daemon/probes.d @@ -0,0 +1,12 @@ +provider libvirtd { + probe client_connect(int fd, int readonly); + probe client_disconnect(int fd); + + probe client_auth_allow(int fd, int authtype, const char *identity); + probe client_auth_deny(int fd, int authtype, const char *identity); + probe client_auth_fail(int fd, int authtype); + + probe client_tls_allow(int fd, const char *x509dname); + probe client_tls_deny(int fd, const char *x509dname); + probe client_tls_fail(int fd); +}; diff --git a/daemon/remote.c b/daemon/remote.c index 6b67678..26f9ed4 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -3462,6 +3462,7 @@ remoteDispatchAuthSaslInit (struct qemud_server *server, authfail: remoteDispatchAuthError(rerr); error: + PROBE(CLIENT_AUTH_FAIL, client->fd, REMOTE_AUTH_SASL); virMutexUnlock(&client->lock); return -1; } @@ -3648,6 +3649,7 @@ remoteDispatchAuthSaslStart (struct qemud_server *server, } REMOTE_DEBUG("Authentication successful %d", client->fd); + PROBE(CLIENT_AUTH_ALLOW, client->fd, REMOTE_AUTH_SASL, client->saslUsername); ret->complete = 1; client->auth = REMOTE_AUTH_NONE; } @@ -3656,10 +3658,12 @@ remoteDispatchAuthSaslStart (struct qemud_server *server, return 0; authfail: + PROBE(CLIENT_AUTH_FAIL, client->fd, REMOTE_AUTH_SASL); remoteDispatchAuthError(rerr); goto error; authdeny: + PROBE(CLIENT_AUTH_DENY, client->fd, REMOTE_AUTH_SASL, client->saslUsername); goto error; error: @@ -3744,6 +3748,7 @@ remoteDispatchAuthSaslStep (struct qemud_server *server, } REMOTE_DEBUG("Authentication successful %d", client->fd); + PROBE(CLIENT_AUTH_ALLOW, client->fd, REMOTE_AUTH_SASL, client->saslUsername); ret->complete = 1; client->auth = REMOTE_AUTH_NONE; } @@ -3752,10 +3757,12 @@ remoteDispatchAuthSaslStep (struct qemud_server *server, return 0; authfail: + PROBE(CLIENT_AUTH_FAIL, client->fd, REMOTE_AUTH_SASL); remoteDispatchAuthError(rerr); goto error; authdeny: + PROBE(CLIENT_AUTH_DENY, client->fd, REMOTE_AUTH_SASL, client->saslUsername); goto error; error: @@ -3775,6 +3782,7 @@ remoteDispatchAuthSaslInit (struct qemud_server *server ATTRIBUTE_UNUSED, remote_auth_sasl_init_ret *ret ATTRIBUTE_UNUSED) { VIR_ERROR0(_("client tried unsupported SASL init request")); + PROBE(CLIENT_AUTH_FAIL, client->fd, REMOTE_AUTH_SASL); remoteDispatchAuthError(rerr); return -1; } @@ -3789,6 +3797,7 @@ remoteDispatchAuthSaslStart (struct qemud_server *server ATTRIBUTE_UNUSED, remote_auth_sasl_start_ret *ret ATTRIBUTE_UNUSED) { VIR_ERROR0(_("client tried unsupported SASL start request")); + PROBE(CLIENT_AUTH_FAIL, client->fd, REMOTE_AUTH_SASL); remoteDispatchAuthError(rerr); return -1; } @@ -3803,6 +3812,7 @@ remoteDispatchAuthSaslStep (struct qemud_server *server ATTRIBUTE_UNUSED, remote_auth_sasl_step_ret *ret ATTRIBUTE_UNUSED) { VIR_ERROR0(_("client tried unsupported SASL step request")); + PROBE(CLIENT_AUTH_FAIL, client->fd, REMOTE_AUTH_SASL); remoteDispatchAuthError(rerr); return -1; } @@ -3879,6 +3889,7 @@ remoteDispatchAuthPolkit (struct qemud_server *server, action, callerPid, callerUid, status); goto authdeny; } + PROBE(CLIENT_AUTH_ALLOW, client->fd, REMOTE_AUTH_POLKIT, ident); VIR_INFO(_("Policy allowed action %s from pid %d, uid %d"), action, callerPid, callerUid); ret->complete = 1; @@ -3888,9 +3899,11 @@ remoteDispatchAuthPolkit (struct qemud_server *server, return 0; authfail: + PROBE(CLIENT_AUTH_FAIL, client->fd, REMOTE_AUTH_POLKIT); goto error; authdeny: + PROBE(CLIENT_AUTH_DENY, client->fd, REMOTE_AUTH_POLKIT, ident); goto error; error: @@ -4004,6 +4017,7 @@ remoteDispatchAuthPolkit (struct qemud_server *server, polkit_result_to_string_representation(pkresult)); goto authdeny; } + PROBE(CLIENT_AUTH_ALLOW, client->fd, REMOTE_AUTH_POLKIT, ident); VIR_INFO(_("Policy allowed action %s from pid %d, uid %d, result %s"), action, callerPid, callerUid, polkit_result_to_string_representation(pkresult)); @@ -4014,9 +4028,11 @@ remoteDispatchAuthPolkit (struct qemud_server *server, return 0; authfail: + PROBE(CLIENT_AUTH_FAIL, client->fd, REMOTE_AUTH_POLKIT); goto error; authdeny: + PROBE(CLIENT_AUTH_DENY, client->fd, REMOTE_AUTH_POLKIT, ident); goto error; error: diff --git a/examples/systemtap/Makefile.am b/examples/systemtap/Makefile.am new file mode 100644 index 0000000..084081e --- /dev/null +++ b/examples/systemtap/Makefile.am @@ -0,0 +1,2 @@ + +EXTRA_DIST = client.stp diff --git a/examples/systemtap/client.stp b/examples/systemtap/client.stp new file mode 100644 index 0000000..00df15a --- /dev/null +++ b/examples/systemtap/client.stp @@ -0,0 +1,28 @@ +#!/usr/bin/stap + +probe libvirt.daemon.client.connect { + printf("Client fd=%d connected readonly=%d\n", fd, readonly); +} +probe libvirt.daemon.client.disconnect { + printf("Client fd=%d disconnected\n", fd); +} + +probe libvirt.daemon.client.tls_allow { + printf("Client fd=%d tls allow %s\n", fd, x509dname); +} +probe libvirt.daemon.client.tls_deny { + printf("Client fd=%d tls deny %s\n", fd, x509dname); +} +probe libvirt.daemon.client.tls_fail { + printf("Client fd=%d tls fail\n", fd); +} + +probe libvirt.daemon.client.auth_allow { + printf("Client fd=%d auth %s allow %s\n", fd, authname, identity); +} +probe libvirt.daemon.client.auth_deny { + printf("Client fd=%d auth %s deny %s\n", fd, authname, identity); +} +probe libvirt.daemon.client.auth_fail { + printf("Client fd=%d auth %s fail\n", fd, authname); +} diff --git a/libvirt.spec.in b/libvirt.spec.in index a58be54..f03a474 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -65,6 +65,7 @@ %define with_libpcap 0%{!?_without_libpcap:0} %define with_macvtap 0%{!?_without_macvtap:0} %define with_libnl 0%{!?_without_libnl:0} +%define with_dtrace 0%{!?_without_dtrace:0} # Non-server/HV driver defaults which are always enabled %define with_python 0%{!?_without_python:1} @@ -162,6 +163,10 @@ %define with_libnl 1 %endif +%if 0%{?fedora} >= 13 || 0%{?rhel} >= 6 +%define with_dtrace 1 +%endif + # Force QEMU to run as non-root %if 0%{?fedora} >= 12 || 0%{?rhel} >= 6 %define qemu_user qemu @@ -545,6 +550,10 @@ of recent versions of Linux (and other OSes). %define _without_macvtap --without-macvtap %endif +%if ! %{with_dtrace} +%define _without_dtrace --without-dtrace +%endif + %configure %{?_without_xen} \ %{?_without_qemu} \ %{?_without_openvz} \ @@ -586,7 +595,7 @@ gzip -9 ChangeLog rm -fr %{buildroot} %makeinstall -for i in domain-events/events-c dominfo domsuspend hellolibvirt openauth python xml/nwfilter +for i in domain-events/events-c dominfo domsuspend hellolibvirt openauth python xml/nwfilter systemtap do (cd examples/$i ; make clean ; rm -rf .deps .libs Makefile Makefile.in) done @@ -742,6 +751,9 @@ fi %{_sysconfdir}/rc.d/init.d/libvirtd %config(noreplace) %{_sysconfdir}/sysconfig/libvirtd %config(noreplace) %{_sysconfdir}/libvirt/libvirtd.conf +%if %{with_dtrace} +%{_datadir}/systemtap/tapsets/libvirtd.stp +%endif %dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/qemu/ %dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/lxc/ %dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/uml/ @@ -888,6 +900,7 @@ fi %doc examples/domsuspend %doc examples/openauth %doc examples/xml +%doc examples/systemtap %if %{with_python} %files python diff --git a/mingw32-libvirt.spec.in b/mingw32-libvirt.spec.in index 4bbbc3b..0e0bef6 100644 --- a/mingw32-libvirt.spec.in +++ b/mingw32-libvirt.spec.in @@ -57,6 +57,7 @@ MinGW Windows libvirt virtualization library. --without-one \ --without-phyp \ --without-netcf \ + --without-dtrace \ --without-libvirtd make -- 1.7.2.2

On Tue, Sep 14, 2010 at 07:30:20PM +0100, Daniel P. Berrange wrote:
This patchset provides the infrastructure for supporting dynamic probing of libvirtd, using static DTrace markers. This can be used by SystemTAP on Linux, or DTrace on Solaris/OS-X/BSD for low overhead tracing.
The proof of concept provides a handful of markers wrt to network client connections, security & auth. Obviously it can be expanded to cover a huge area of our codebase for different tasks. The hard bit is deciding what should be exposed as a probe point. Ideally probes should not be changed/removed once added, since this would break any user tracing scripts. So a little care needs to be taken in placing probes to be robust against future code re-factoring.
Very interesting, I'm just a bit surprized by the patch set. patch 1 and 2 are really unrelated, I think they should go in independantly. Patch 1 is pure refactoring, I would rather apply it early in the cycle for 0.8.5 (or whatever our next release might be named), and glancing at it it looks finr to me ACK (will jsut need a bit of rebase due to Justin patch) Patch 2 looks simple enough, ACK too For patch 3 I'm a bit surprized, I think I would have started by adding probes for all the public API places first on entry and second on exit with the return value provided. It can be helpful for debugging connection problems but well I would use SystemTap more for debugging and tuning of a running system (i.e. it runs and you don't want to disturb it or minimally) We should understand the problem of the restart being needed to before pushing patch3 I think, maybe we need to talk to one of the SystemTap developpers so he can explain what is going on and how to fix this :-) I really like this, I'm actually eager to start playing with it ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Thu, Sep 16, 2010 at 05:04:42PM +0200, Daniel Veillard wrote:
On Tue, Sep 14, 2010 at 07:30:20PM +0100, Daniel P. Berrange wrote:
This patchset provides the infrastructure for supporting dynamic probing of libvirtd, using static DTrace markers. This can be used by SystemTAP on Linux, or DTrace on Solaris/OS-X/BSD for low overhead tracing.
The proof of concept provides a handful of markers wrt to network client connections, security & auth. Obviously it can be expanded to cover a huge area of our codebase for different tasks. The hard bit is deciding what should be exposed as a probe point. Ideally probes should not be changed/removed once added, since this would break any user tracing scripts. So a little care needs to be taken in placing probes to be robust against future code re-factoring.
Very interesting, I'm just a bit surprized by the patch set. patch 1 and 2 are really unrelated, I think they should go in independantly. Patch 1 is pure refactoring, I would rather apply it early in the cycle for 0.8.5 (or whatever our next release might be named), and glancing at it it looks finr to me ACK (will jsut need a bit of rebase due to Justin patch) Patch 2 looks simple enough, ACK too
For patch 3 I'm a bit surprized, I think I would have started by adding probes for all the public API places first on entry and second on exit with the return value provided. It can be helpful for debugging connection problems but well I would use SystemTap more for debugging and tuning of a running system (i.e. it runs and you don't want to disturb it or minimally)
Adding probes for public API places would require me to add 266 probes! This patch let me demo it adding < 10 probes. I'm not 100% sure whether we need to add explicit probes for public API places, because I think dtrace/systemtap may provide generic support for probes at function enty and return points. If adding explicit probes lets it work without needing -debuginfo installed then it could still be worthwhile to mark public API places.
We should understand the problem of the restart being needed to before pushing patch3 I think, maybe we need to talk to one of the SystemTap developpers so he can explain what is going on and how to fix this :-)
Yep, there's already someone looking at it. One of the problems was caused by LXC containers - uprobes kernel module has some bad assumptions that LXC invalidates. The other problem looks like a data caching / race issue in systemtap/uprobes to me. So I don't think this needs to hold us up. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (2)
-
Daniel P. Berrange
-
Daniel Veillard