From 5a40a392b3b551cf92d4be022b4efd900f339e39 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Tue, 26 May 2020 10:22:37 +0200 Subject: [PATCH] read-file: add flags to modify reading behavior * lib/read-file.h (RF_BINARY, RF_SENSITIVE): New define. (fread_file, read_file): Take FLAGS argument. (read_binary_file): Remove. * lib/read-file.c (internal_fread_file): Take into account of RF_SENSITIVE flag. (internal_read_file): Take into account of RF_BINARY and RF_SENSITIVE flags. * modules/read-file (Depends-on): Add explicit_bzero. This adds an alternative behavior of those functions to explicitly clear the internal memory block when it becomes unused. This is useful for reading sensitive information from a file. --- ChangeLog | 15 ++++++++ lib/read-file.c | 85 ++++++++++++++++++++++++++++------------- lib/read-file.h | 10 +++-- modules/read-file | 1 + modules/read-file-tests | 1 + tests/test-read-file.c | 17 +++++++-- 6 files changed, 97 insertions(+), 32 deletions(-) diff --git a/ChangeLog b/ChangeLog index 07d4d5124..d1ee01a54 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2020-05-26 Daiki Ueno + + read-file: add flags to modify reading behavior + * lib/read-file.h (RF_BINARY, RF_SENSITIVE): New define. + (fread_file, read_file): Take FLAGS argument. + (read_binary_file): Remove. + * lib/read-file.c (internal_fread_file): Take into account of + RF_SENSITIVE flag. + (internal_read_file): Take into account of RF_BINARY and + RF_SENSITIVE flags. + * modules/read-file (Depends-on): Add explicit_bzero. + This adds an alternative behavior of those functions to explicitly + clear the internal memory block when it becomes unused. This is + useful for reading sensitive information from a file. + 2020-05-26 Daiki Ueno read-file: make use of fopen-gnu diff --git a/lib/read-file.c b/lib/read-file.c index 293bc3e8a..d95aae168 100644 --- a/lib/read-file.c +++ b/lib/read-file.c @@ -31,16 +31,17 @@ /* Get malloc, realloc, free. */ #include +/* Get explicit_bzero, memcpy. */ +#include + /* Get errno. */ #include -/* Read a STREAM and return a newly allocated string with the content, - and set *LENGTH to the length of the string. The string is - zero-terminated, but the terminating zero byte is not counted in - *LENGTH. On errors, *LENGTH is undefined, errno preserves the - values set by system functions (if any), and NULL is returned. */ -char * -fread_file (FILE *stream, size_t *length) +/* Get assert. */ +#include + +static char * +internal_fread_file (FILE *stream, int flags, size_t *length) { char *buf = NULL; size_t alloc = BUFSIZ; @@ -94,7 +95,12 @@ fread_file (FILE *stream, size_t *length) /* Shrink the allocated memory if possible. */ if (size < alloc - 1) { - char *smaller_buf = realloc (buf, size + 1); + char *smaller_buf; + + if (flags & RF_SENSITIVE) + explicit_bzero (buf + size, alloc - size); + + smaller_buf = realloc (buf, size + 1); if (smaller_buf != NULL) buf = smaller_buf; } @@ -106,6 +112,7 @@ fread_file (FILE *stream, size_t *length) { char *new_buf; + size_t save_alloc = alloc; if (alloc == PTRDIFF_MAX) { @@ -118,7 +125,21 @@ fread_file (FILE *stream, size_t *length) else alloc = PTRDIFF_MAX; - if (!(new_buf = realloc (buf, alloc))) + if (flags & RF_SENSITIVE) + { + new_buf = malloc (alloc); + if (!new_buf) + { + /* BUF should be cleared below after the loop. */ + save_errno = errno; + break; + } + memcpy (new_buf, buf, save_alloc); + explicit_bzero (buf, save_alloc); + free (buf); + buf = new_buf; + } + else if (!(new_buf = realloc (buf, alloc))) { save_errno = errno; break; @@ -128,14 +149,32 @@ fread_file (FILE *stream, size_t *length) } } + if (flags & RF_SENSITIVE) + explicit_bzero (buf, alloc); + free (buf); errno = save_errno; return NULL; } } +/* Read a STREAM and return a newly allocated string with the content, + and set *LENGTH to the length of the string. The string is + zero-terminated, but the terminating zero byte is not counted in + *LENGTH. On errors, *LENGTH is undefined, errno preserves the + values set by system functions (if any), and NULL is returned. + + If the RF_SENSITIVE flag is set in FLAGS, the memory buffer + internally allocated will be cleared upon failure. */ +char * +fread_file (FILE *stream, int flags, size_t *length) +{ + return internal_fread_file (stream, flags, length); +} + static char * -internal_read_file (const char *filename, size_t *length, const char *mode) +internal_read_file (const char *filename, int flags, size_t *length, + const char *mode) { FILE *stream = fopen (filename, mode); char *out; @@ -144,7 +183,7 @@ internal_read_file (const char *filename, size_t *length, const char *mode) if (!stream) return NULL; - out = fread_file (stream, length); + out = fread_file (stream, flags, length); save_errno = errno; @@ -153,6 +192,8 @@ internal_read_file (const char *filename, size_t *length, const char *mode) if (out) { save_errno = errno; + if (flags & RF_SENSITIVE) + explicit_bzero (out, *length); free (out); } errno = save_errno; @@ -167,22 +208,14 @@ internal_read_file (const char *filename, size_t *length, const char *mode) the string. The string is zero-terminated, but the terminating zero byte is not counted in *LENGTH. On errors, *LENGTH is undefined, errno preserves the values set by system functions (if - any), and NULL is returned. */ -char * -read_file (const char *filename, size_t *length) -{ - return internal_read_file (filename, length, "re"); -} + any), and NULL is returned. -/* Open (on non-POSIX systems, in binary mode) and read the contents - of FILENAME, and return a newly allocated string with the content, - and set LENGTH to the length of the string. The string is - zero-terminated, but the terminating zero byte is not counted in - the LENGTH variable. On errors, *LENGTH is undefined, errno - preserves the values set by system functions (if any), and NULL is - returned. */ + If the RF_BINARY flag is set in FLAGS, the file is opened in binary + mode. If the RF_SENSITIVE flag is set in FLAGS, the memory buffer + internally allocated will be cleared upon failure. */ char * -read_binary_file (const char *filename, size_t *length) +read_file (const char *filename, int flags, size_t *length) { - return internal_read_file (filename, length, "rbe"); + const char *mode = (flags & RF_BINARY) ? "rbe" : "re"; + return internal_read_file (filename, flags, length, mode); } diff --git a/lib/read-file.h b/lib/read-file.h index bb28abd65..c2454ef68 100644 --- a/lib/read-file.h +++ b/lib/read-file.h @@ -24,10 +24,14 @@ /* Get FILE. */ #include -extern char *fread_file (FILE * stream, size_t * length); +/* Indicate that the file is treated as binary. */ +#define RF_BINARY 0x1 -extern char *read_file (const char *filename, size_t * length); +/* Indicate that the file content contains sensitive information. */ +#define RF_SENSITIVE 0x2 -extern char *read_binary_file (const char *filename, size_t * length); +extern char *fread_file (FILE * stream, int flags, size_t * length); + +extern char *read_file (const char *filename, int flags, size_t * length); #endif /* READ_FILE_H */ diff --git a/modules/read-file b/modules/read-file index a6e7faf0a..5d2be5bbf 100644 --- a/modules/read-file +++ b/modules/read-file @@ -7,6 +7,7 @@ lib/read-file.c m4/read-file.m4 Depends-on: +explicit_bzero fopen-gnu fstat ftello diff --git a/modules/read-file-tests b/modules/read-file-tests index 361bca806..299631644 100644 --- a/modules/read-file-tests +++ b/modules/read-file-tests @@ -1,5 +1,6 @@ Files: tests/test-read-file.c +tests/macros.h Depends-on: diff --git a/tests/test-read-file.c b/tests/test-read-file.c index 930cf4acb..154fb36b1 100644 --- a/tests/test-read-file.c +++ b/tests/test-read-file.c @@ -23,11 +23,13 @@ #include #include +#include "macros.h" + #define FILE1 "/etc/resolv.conf" #define FILE2 "/dev/null" int -main (void) +test_read_file (int flags) { struct stat statbuf; int err = 0; @@ -37,7 +39,7 @@ main (void) if (stat (FILE1, &statbuf) >= 0) { size_t len; - char *out = read_file (FILE1, &len); + char *out = read_file (FILE1, flags, &len); if (!out) { @@ -80,7 +82,7 @@ main (void) if (stat (FILE2, &statbuf) >= 0) { size_t len; - char *out = read_file (FILE2, &len); + char *out = read_file (FILE2, flags, &len); if (!out) { @@ -109,3 +111,12 @@ main (void) return err; } + +int +main (void) +{ + ASSERT (!test_read_file (0)); + ASSERT (!test_read_file (RF_BINARY)); + ASSERT (!test_read_file (RF_SENSITIVE)); + ASSERT (!test_read_file (RF_BINARY | RF_SENSITIVE)); +} -- 2.26.2