Fix config file reading to not truncate large files

ChangeLog

@@ -1,4 +1,14 @@
-Wed Jan  5 16:02:00 UTC 2008 Richard W.M. Jones <rjones@redhat.com>
+Mon Jan  7 10:19:00 EST 2008 Daniel P. Berrange <berrange@redhat.com>
+
+	* 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 <rjones@redhat.com>
 
 	Miscellaneous fixes for building on Windows (MinGW).
 	* configure.in: xdr functions may require -lxdr.

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];
+    char *content;
-    int fd;
     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);
+    conf = virConfParse(filename, content, len);
-    if (len <= 0) {
+
-        virConfError(NULL, VIR_ERR_READ_FAILED, filename, 0);
+    free(content);
-	return(NULL);
+
-    }
+    return conf;
-    return(virConfParse(filename, content, len));
 }
 
 /**

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);

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,
+                   int maxlen,
-                   unsigned int buflen)
+                   char **buf)
 {
     FILE *fh;
     struct stat st;
@@ -296,20 +296,28 @@ int virFileReadAll(const char *path,
         goto error;
     }
 
-    if (st.st_size >= (buflen-1)) {
+    if (st.st_size > maxlen) {
-        virLog("File '%s' is too large", path);
+        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)

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,
+		   int maxlen,
-                   unsigned int buflen);
+                   char **buf);
 
 int virFileMatchesNameSuffix(const char *file,
                              const char *name,

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" ]
+
+

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" ]

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