From 94e49e3f0e82b8b059d71b131f5e6be2a1d6de2d Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Mon, 7 Jan 2008 15:21:33 +0000 Subject: [PATCH] Fix config file reading to not truncate large files --- ChangeLog | 12 +- src/conf.c | 27 +++-- src/qemu_conf.c | 6 +- src/util.c | 22 ++-- src/util.h | 4 +- tests/confdata/libvirtd.conf | 222 +++++++++++++++++++++++++++++++++++ tests/confdata/libvirtd.out | 180 ++++++++++++++++++++++++++++ tests/test_conf.sh | 3 + 8 files changed, 452 insertions(+), 24 deletions(-) create mode 100644 tests/confdata/libvirtd.conf create mode 100644 tests/confdata/libvirtd.out diff --git a/ChangeLog b/ChangeLog index bf0f4fdc0b..4ee5a22dd5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,14 @@ -Wed Jan 5 16:02:00 UTC 2008 Richard W.M. Jones +Mon Jan 7 10:19:00 EST 2008 Daniel P. Berrange + + * src/util.c, src/util.h: virFileReadAll() now allocates its + own buffer + * src/conf.c: Use virFileReadAll() to avoid truncating config + files + * src/qemu_conf.c: Update for new virFileReadAll() contract + * tests/test_conf.sh, tests/confdata/libvirtd.conf, + tests/libvirtd.out: New test case for large config file + +Sat Jan 5 16:02:00 UTC 2008 Richard W.M. Jones Miscellaneous fixes for building on Windows (MinGW). * configure.in: xdr functions may require -lxdr. diff --git a/src/conf.c b/src/conf.c index 8190de1dea..62c42ff11e 100644 --- a/src/conf.c +++ b/src/conf.c @@ -21,6 +21,7 @@ #include "internal.h" #include "buf.h" #include "conf.h" +#include "util.h" /************************************************************************ * * @@ -693,6 +694,9 @@ error: * * ************************************************************************/ +/* 10 MB limit on config file size as a sanity check */ +#define MAX_CONFIG_FILE_SIZE (1024*1024*10) + /** * __virConfReadFile: * @filename: the path to the configuration file. @@ -705,26 +709,25 @@ error: virConfPtr __virConfReadFile(const char *filename) { - char content[4096]; - int fd; + char *content; int len; + virConfPtr conf; if (filename == NULL) { virConfError(NULL, VIR_ERR_INVALID_ARG, __FUNCTION__, 0); return(NULL); } - fd = open(filename, O_RDONLY); - if (fd < 0) { + + if ((len = virFileReadAll(filename, MAX_CONFIG_FILE_SIZE, &content)) < 0) { virConfError(NULL, VIR_ERR_OPEN_FAILED, filename, 0); - return(NULL); + return NULL; } - len = read(fd, content, sizeof(content)); - close(fd); - if (len <= 0) { - virConfError(NULL, VIR_ERR_READ_FAILED, filename, 0); - return(NULL); - } - return(virConfParse(filename, content, len)); + + conf = virConfParse(filename, content, len); + + free(content); + + return conf; } /** diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 2b87c884a4..15779b7ab0 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -2604,7 +2604,7 @@ int qemudScanConfigDir(struct qemud_driver *driver, } while ((entry = readdir(dir))) { - char xml[QEMUD_MAX_XML_LEN]; + char *xml; char path[PATH_MAX]; char autostartLink[PATH_MAX]; @@ -2626,13 +2626,15 @@ int qemudScanConfigDir(struct qemud_driver *driver, continue; } - if (virFileReadAll(path, xml, QEMUD_MAX_XML_LEN) < 0) + if (virFileReadAll(path, QEMUD_MAX_XML_LEN, &xml) < 0) continue; if (isGuest) qemudLoadConfig(driver, entry->d_name, path, xml, autostartLink); else qemudLoadNetworkConfig(driver, entry->d_name, path, xml, autostartLink); + + free(xml); } closedir(dir); diff --git a/src/util.c b/src/util.c index dff34400f2..0fa361177d 100644 --- a/src/util.c +++ b/src/util.c @@ -272,8 +272,8 @@ ssize_t safewrite(int fd, const void *buf, size_t count) int virFileReadAll(const char *path, - char *buf, - unsigned int buflen) + int maxlen, + char **buf) { FILE *fh; struct stat st; @@ -296,20 +296,28 @@ int virFileReadAll(const char *path, goto error; } - if (st.st_size >= (buflen-1)) { - virLog("File '%s' is too large", path); + if (st.st_size > maxlen) { + virLog("File '%s' is too large %d, max %d", path, st.st_size, maxlen); goto error; } - if ((ret = fread(buf, st.st_size, 1, fh)) != 1) { + *buf = malloc(st.st_size + 1); + if (*buf == NULL) { + virLog("Failed to allocate data"); + goto error; + } + + if ((ret = fread(*buf, st.st_size, 1, fh)) != 1) { + free(buf); + *buf = NULL; virLog("Failed to read config file '%s': %s", path, strerror(errno)); goto error; } - buf[st.st_size] = '\0'; + (*buf)[st.st_size] = '\0'; - ret = 0; + ret = st.st_size; error: if (fh) diff --git a/src/util.h b/src/util.h index a7c68d5b7a..393f71f319 100644 --- a/src/util.h +++ b/src/util.h @@ -33,8 +33,8 @@ int saferead(int fd, void *buf, size_t count); ssize_t safewrite(int fd, const void *buf, size_t count); int virFileReadAll(const char *path, - char *buf, - unsigned int buflen); + int maxlen, + char **buf); int virFileMatchesNameSuffix(const char *file, const char *name, diff --git a/tests/confdata/libvirtd.conf b/tests/confdata/libvirtd.conf new file mode 100644 index 0000000000..985a80f0d5 --- /dev/null +++ b/tests/confdata/libvirtd.conf @@ -0,0 +1,222 @@ +# Master libvirt daemon configuration file +# +# For further information consult http://libvirt.org/format.html + + +################################################################# +# +# Network connectivitiy controls +# + +# Flag listening for secure TLS connections on the public TCP/IP port. +# NB, must pass the --listen flag to the libvirtd process for this to +# have any effect. +# +# It is neccessary to setup a CA and issue server certificates before +# using this capability. +# +# This is enabled by default, uncomment this to disable it +listen_tls = 0 + +# Listen for unencrypted TCP connections on the public TCP/IP port. +# NB, must pass the --listen flag to the libvirtd process for this to +# have any effect. +# +# Using the TCP socket requires SASL authentication by default. Only +# SASL mechanisms which support data encryption are allowed. This is +# DIGEST_MD5 and GSSAPI (Kerberos5) +# +# This is disabled by default, uncomment this to enable it. +listen_tcp = 1 + + + +# Override the port for accepting secure TLS connections +# This can be a port number, or service name +# +tls_port = "16514" + +# Override the port for accepting insecure TCP connections +# This can be a port number, or service name +# +tcp_port = "16509" + + + +# Flag toggling mDNS advertizement of the libvirt service. +# +# Alternatively can disable for all services on a host by +# stopping the Avahi daemon +# +# This is enabled by default, uncomment this to disable it +mdns_adv = 0 + +# Override the default mDNS advertizement name. This must be +# unique on the immediate broadcast network. +# +# The default is "Virtualization Host HOSTNAME", where HOSTNAME +# is subsituted for the short hostname of the machine (without domain) +# +mdns_name = "Virtualization Host Joe Demo" + + +################################################################# +# +# UNIX socket access controls +# + +# Set the UNIX domain socket group ownership. This can be used to +# allow a 'trusted' set of users access to management capabilities +# without becoming root. +# +# This is restricted to 'root' by default. +unix_sock_group = "libvirt" + +# Set the UNIX socket permissions for the R/O socket. This is used +# for monitoring VM status only +# +# Default allows any user. If setting group ownership may want to +# restrict this to: +unix_sock_ro_perms = "0777" + +# Set the UNIX socket permissions for the R/W socket. This is used +# for full management of VMs +# +# Default allows only root. If PolicyKit is enabled on the socket, +# the default will change to allow everyone (eg, 0777) +# +# If not using PolicyKit and setting group ownership for access +# control then you may want to relax this to: +unix_sock_rw_perms = "0770" + + + +################################################################# +# +# Authentication. +# +# - none: do not perform auth checks. If you can connect to the +# socket you are allowed. This is suitable if there are +# restrictions on connecting to the socket (eg, UNIX +# socket permissions), or if there is a lower layer in +# the network providing auth (eg, TLS/x509 certificates) +# +# - sasl: use SASL infrastructure. The actual auth scheme is then +# controlled from /etc/sasl2/libvirt.conf. For the TCP +# socket only GSSAPI & DIGEST-MD5 mechanisms will be used. +# For non-TCP or TLS sockets, any scheme is allowed. +# +# - polkit: use PolicyKit to authenticate. This is only suitable +# for use on the UNIX sockets. The default policy will +# require a user to supply their own password to gain +# full read/write access (aka sudo like), while anyone +# is allowed read/only access. +# +# Set an authentication scheme for UNIX read-only sockets +# By default socket permissions allow anyone to connect +# +# To restrict monitoring of domains you may wish to enable +# an authentication mechanism here +auth_unix_ro = "none" + +# Set an authentication scheme for UNIX read-write sockets +# By default socket permissions only allow root. If PolicyKit +# support was compiled into libvirt, the default will be to +# use 'polkit' auth. +# +# If the unix_sock_rw_perms are changed you may wish to enable +# an authentication mechanism here +auth_unix_rw = "none" + +# Change the authentication scheme for TCP sockets. +# +# If you don't enable SASL, then all TCP traffic is cleartext. +# Don't do this outside of a dev/test scenario. For real world +# use, always enable SASL and use the GSSAPI or DIGEST-MD5 +# mechanism in /etc/sasl2/libvirt.conf +auth_tcp = "sasl" + +# Change the authentication scheme for TLS sockets. +# +# TLS sockets already have encryption provided by the TLS +# layer, and limited authentication is done by certificates +# +# It is possible to make use of any SASL authentication +# mechanism as well, by using 'sasl' for this option +auth_tls = "none" + + + +################################################################# +# +# TLS x509 certificate configuration +# + + +# Override the default server key file path +# +key_file = "/etc/pki/libvirt/private/serverkey.pem" + +# Override the default server certificate file path +# +cert_file = "/etc/pki/libvirt/servercert.pem" + +# Override the default CA certificate path +# +ca_file = "/etc/pki/CA/cacert.pem" + +# Specify a certificate revocation list. +# +# Defaults to not using a CRL, uncomment to enable it +crl_file = "/etc/pki/CA/crl.pem" + + + +################################################################# +# +# Authorization controls +# + + +# Flag to disable verification of client certificates +# +# Client certificate verification is the primary authentication mechanism. +# Any client which does not present a certificate signed by the CA +# will be rejected. +# +# Default is to always verify. Uncommenting this will disable +# verification - make sure an IP whitelist is set +tls_no_verify_certificate = 1 + + +# A whitelist of allowed x509 Distinguished Names +# This list may contain wildcards such as +# +# "C=GB,ST=London,L=London,O=Red Hat,CN=*" +# +# See the POSIX fnmatch function for the format of the wildcards. +# +# NB If this is an empty list, no client can connect, so comment out +# entirely rather than using empty list to disable these checks +# +# By default, no DN's are checked +tls_allowed_dn_list = ["DN1", "DN2"] + + +# A whitelist of allowed SASL usernames. The format for usernames +# depends on the SASL authentication mechanism. Kerberos usernames +# look like username@REALM +# +# This list may contain wildcards such as +# +# "*@EXAMPLE.COM" +# +# See the POSIX fnmatch function for the format of the wildcards. +# +# NB If this is an empty list, no client can connect, so comment out +# entirely rather than using empty list to disable these checks +# +# By default, no Username's are checked +sasl_allowed_username_list = ["joe@EXAMPLE.COM", "fred@EXAMPLE.COM" ] + + diff --git a/tests/confdata/libvirtd.out b/tests/confdata/libvirtd.out new file mode 100644 index 0000000000..2fb9cc6866 --- /dev/null +++ b/tests/confdata/libvirtd.out @@ -0,0 +1,180 @@ +# Master libvirt daemon configuration file +# +# For further information consult http://libvirt.org/format.html +################################################################# +# +# Network connectivitiy controls +# +# Flag listening for secure TLS connections on the public TCP/IP port. +# NB, must pass the --listen flag to the libvirtd process for this to +# have any effect. +# +# It is neccessary to setup a CA and issue server certificates before +# using this capability. +# +# This is enabled by default, uncomment this to disable it +listen_tls = 0 +# Listen for unencrypted TCP connections on the public TCP/IP port. +# NB, must pass the --listen flag to the libvirtd process for this to +# have any effect. +# +# Using the TCP socket requires SASL authentication by default. Only +# SASL mechanisms which support data encryption are allowed. This is +# DIGEST_MD5 and GSSAPI (Kerberos5) +# +# This is disabled by default, uncomment this to enable it. +listen_tcp = 1 +# Override the port for accepting secure TLS connections +# This can be a port number, or service name +# +tls_port = "16514" +# Override the port for accepting insecure TCP connections +# This can be a port number, or service name +# +tcp_port = "16509" +# Flag toggling mDNS advertizement of the libvirt service. +# +# Alternatively can disable for all services on a host by +# stopping the Avahi daemon +# +# This is enabled by default, uncomment this to disable it +mdns_adv = 0 +# Override the default mDNS advertizement name. This must be +# unique on the immediate broadcast network. +# +# The default is "Virtualization Host HOSTNAME", where HOSTNAME +# is subsituted for the short hostname of the machine (without domain) +# +mdns_name = "Virtualization Host Joe Demo" +################################################################# +# +# UNIX socket access controls +# +# Set the UNIX domain socket group ownership. This can be used to +# allow a 'trusted' set of users access to management capabilities +# without becoming root. +# +# This is restricted to 'root' by default. +unix_sock_group = "libvirt" +# Set the UNIX socket permissions for the R/O socket. This is used +# for monitoring VM status only +# +# Default allows any user. If setting group ownership may want to +# restrict this to: +unix_sock_ro_perms = "0777" +# Set the UNIX socket permissions for the R/W socket. This is used +# for full management of VMs +# +# Default allows only root. If PolicyKit is enabled on the socket, +# the default will change to allow everyone (eg, 0777) +# +# If not using PolicyKit and setting group ownership for access +# control then you may want to relax this to: +unix_sock_rw_perms = "0770" +################################################################# +# +# Authentication. +# +# - none: do not perform auth checks. If you can connect to the +# socket you are allowed. This is suitable if there are +# restrictions on connecting to the socket (eg, UNIX +# socket permissions), or if there is a lower layer in +# the network providing auth (eg, TLS/x509 certificates) +# +# - sasl: use SASL infrastructure. The actual auth scheme is then +# controlled from /etc/sasl2/libvirt.conf. For the TCP +# socket only GSSAPI & DIGEST-MD5 mechanisms will be used. +# For non-TCP or TLS sockets, any scheme is allowed. +# +# - polkit: use PolicyKit to authenticate. This is only suitable +# for use on the UNIX sockets. The default policy will +# require a user to supply their own password to gain +# full read/write access (aka sudo like), while anyone +# is allowed read/only access. +# +# Set an authentication scheme for UNIX read-only sockets +# By default socket permissions allow anyone to connect +# +# To restrict monitoring of domains you may wish to enable +# an authentication mechanism here +auth_unix_ro = "none" +# Set an authentication scheme for UNIX read-write sockets +# By default socket permissions only allow root. If PolicyKit +# support was compiled into libvirt, the default will be to +# use 'polkit' auth. +# +# If the unix_sock_rw_perms are changed you may wish to enable +# an authentication mechanism here +auth_unix_rw = "none" +# Change the authentication scheme for TCP sockets. +# +# If you don't enable SASL, then all TCP traffic is cleartext. +# Don't do this outside of a dev/test scenario. For real world +# use, always enable SASL and use the GSSAPI or DIGEST-MD5 +# mechanism in /etc/sasl2/libvirt.conf +auth_tcp = "sasl" +# Change the authentication scheme for TLS sockets. +# +# TLS sockets already have encryption provided by the TLS +# layer, and limited authentication is done by certificates +# +# It is possible to make use of any SASL authentication +# mechanism as well, by using 'sasl' for this option +auth_tls = "none" +################################################################# +# +# TLS x509 certificate configuration +# +# Override the default server key file path +# +key_file = "/etc/pki/libvirt/private/serverkey.pem" +# Override the default server certificate file path +# +cert_file = "/etc/pki/libvirt/servercert.pem" +# Override the default CA certificate path +# +ca_file = "/etc/pki/CA/cacert.pem" +# Specify a certificate revocation list. +# +# Defaults to not using a CRL, uncomment to enable it +crl_file = "/etc/pki/CA/crl.pem" +################################################################# +# +# Authorization controls +# +# Flag to disable verification of client certificates +# +# Client certificate verification is the primary authentication mechanism. +# Any client which does not present a certificate signed by the CA +# will be rejected. +# +# Default is to always verify. Uncommenting this will disable +# verification - make sure an IP whitelist is set +tls_no_verify_certificate = 1 +# A whitelist of allowed x509 Distinguished Names +# This list may contain wildcards such as +# +# "C=GB,ST=London,L=London,O=Red Hat,CN=*" +# +# See the POSIX fnmatch function for the format of the wildcards. +# +# NB If this is an empty list, no client can connect, so comment out +# entirely rather than using empty list to disable these checks +# +# By default, no DN's are checked +tls_allowed_dn_list = [ "DN1", "DN2" ] +# A whitelist of allowed SASL usernames. The format for usernames +# depends on the SASL authentication mechanism. Kerberos usernames +# look like username@REALM +# +# This list may contain wildcards such as +# +# "*@EXAMPLE.COM" +# +# See the POSIX fnmatch function for the format of the wildcards. +# +# NB If this is an empty list, no client can connect, so comment out +# entirely rather than using empty list to disable these checks +# +# By default, no Username's are checked +sasl_allowed_username_list = [ "joe@EXAMPLE.COM", "fred@EXAMPLE.COM" ] diff --git a/tests/test_conf.sh b/tests/test_conf.sh index 7735a666a6..ac0bca6e28 100755 --- a/tests/test_conf.sh +++ b/tests/test_conf.sh @@ -8,6 +8,9 @@ do diff $outfile conftest.$$ > /dev/null if [ $? != 0 ] then + if [ -n "$DEBUG_TESTS" ]; then + diff -u $outfile conftest.$$ + fi echo "$f FAILED" NOK=1 else