[gnutls-dev] [PATCH] error handling large CA files

[Ian Peters]

--=-BrvIWqqp9XCcFMLzkM68
Content-Type: text/plain
Content-Transfer-Encoding: 7bit

In lib/gnutls_x509.c, the functions read_cert_file, read_ca_file, and
read_key_file all read into a stack-allocated buffer of MAX_FILE_SIZE,
which is defined as 1024*100.  The default CA file that comes with most
web browsers exceeds this (it is closer to 245k).  This means that the
file is truncated, and parse_pem_cert_mem only sees part of the file,
resulting in only some of the trusted certificates being loaded. 
Because of the pem file format, this is not a fatal error, and program
execution continues, but some certificates that should be trusted are
not.

The attached patch fixes these three functions to read passed file into
a heap-allocated buffer, parse the memory, and then free the buffer. 
You'll probably want to tweak things to fit into gnutls better
stylistically.

Ian

--=-BrvIWqqp9XCcFMLzkM68
Content-Disposition: attachment; filename=gnutls-0.8.4-large-ca-file.patch
Content-Type: text/x-patch; name=gnutls-0.8.4-large-ca-file.patch; charset=UTF-8
Content-Transfer-Encoding: 7bit

diff -r -u gnutls-0.8.4/lib/gnutls_x509.c gnutls-0.8.4.new/lib/gnutls_x509.c
--- gnutls-0.8.4/lib/gnutls_x509.c	2003-02-04 07:52:24.000000000 -0500
+++ gnutls-0.8.4.new/lib/gnutls_x509.c	2003-03-11 15:01:29.000000000 -0500
@@ -1602,26 +1602,66 @@
 }
 
 
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+extern int errno;
+static int read_file(const char *filename, char **data, int *len)
+{
+	int fd;
+	struct stat s;
+	char *buf;
+	unsigned int bytes_read = 0;
+
+	if ((fd = open( filename, O_RDONLY)) < 0) {
+		gnutls_assert();
+		return GNUTLS_E_FILE_ERROR;
+	}
+
+	if ((fstat( fd, &s)) < 0) {
+		gnutls_assert();
+		close(fd);
+		return GNUTLS_E_FILE_ERROR;
+	}
+
+	buf = gnutls_malloc(s.st_size + 1);
+	buf[s.st_size] = 0;
+
+	while (bytes_read < s.st_size) {
+		int ret;
+		ret = read( fd, buf + bytes_read, s.st_size - bytes_read);
+		if (ret < 0 && errno != EINTR && errno != EAGAIN) {
+			gnutls_assert();
+			gnutls_free(buf);
+			close(fd);
+			return GNUTLS_E_FILE_ERROR;
+		}
+		bytes_read += ret;
+	}
+
+	*data = buf;
+	*len = s.st_size + 1;
+	return 0;
+}
+
 /* Reads a certificate file
  */
 static int read_cert_file(gnutls_certificate_credentials res, const char *certfile,
 	gnutls_x509_certificate_format type)
 {
 	int siz;
-	char x[MAX_FILE_SIZE];
-	FILE *fd1;
-
-	fd1 = fopen(certfile, "rb");
-	if (fd1 == NULL)
-		return GNUTLS_E_FILE_ERROR;
-
-	siz = fread(x, 1, sizeof(x)-1, fd1);
-	fclose(fd1);
-
-	x[siz] = 0;
+	char *x;
+	int ret;
 
-	return read_cert_mem( res, x, siz, type);
+	if ((ret = read_file( certfile, &x, &siz)) < 0) {
+		gnutls_assert();
+		return ret;
+	}
 
+	ret = read_cert_mem( res, x, siz, type);
+	gnutls_free(x);
+	return ret;
 }
 
 /* Reads a base64 encoded CA file (file contains multiple certificate
@@ -1631,21 +1671,17 @@
 	gnutls_x509_certificate_format type)
 {
 	int siz;
-	char x[MAX_FILE_SIZE];
-	FILE *fd1;
+	char *x;
+	int ret;
 
-	fd1 = fopen(cafile, "rb");
-	if (fd1 == NULL) {
+	if ((ret = read_file( cafile, &x, &siz)) < 0) {
 		gnutls_assert();
-		return GNUTLS_E_FILE_ERROR;
+		return ret;
 	}
 
-	siz = fread(x, 1, sizeof(x)-1, fd1);
-	fclose(fd1);
-
-	x[siz] = 0;
-
-	return read_ca_mem( res, x, siz, type);
+	ret = read_ca_mem( res, x, siz, type);
+	gnutls_free(x);
+	return ret;
 }
 
 
@@ -1656,19 +1692,17 @@
 	gnutls_x509_certificate_format type)
 {
 	int siz;
-	char x[MAX_FILE_SIZE];
-	FILE *fd2;
-
-	fd2 = fopen(keyfile, "rb");
-	if (fd2 == NULL)
-		return GNUTLS_E_FILE_ERROR;
-
-	siz = fread(x, 1, sizeof(x)-1, fd2);
-	fclose(fd2);
+	char *x;
+	int ret;
 
-	x[siz] = 0;
+	if ((ret = read_file( keyfile, &x, &siz)) < 0) {
+		gnutls_assert();
+		return ret;
+	}
 
-	return read_key_mem( res, x, siz, type);
+	ret = read_key_mem( res, x, siz, type);
+	gnutls_free(x);
+	return ret;
 }
 
 /**

--=-BrvIWqqp9XCcFMLzkM68--