Bruno Haible <br...@clisp.org> writes:
> It would be useful to first concentrate on the first part, the refactoring
> that introduces flags and RF_BINARY. This would provide a patch that is easier
> to review and does not have the following problems:
> - internal_fread_file still exists, although fread_file is a no-op wrapper
> around it.
> - In tests/test-read-file.c, please terminate the main() function with a
> return statement. We assume C99 only in modules that explicitly list 'c99'
> as a dependency. If it's trivial to avoid this dependency, let's do it.
> - The NEWS file needs an entry.
Indeed, I missed the last paragraph of your suggestion in the previous
mail; sorry about that. Here are the two separate commits for this.
>From 60608590e2b106708dd74fd31331567af5166d2e Mon Sep 17 00:00:00 2001
From: Daiki Ueno <u...@gnu.org>
Date: Wed, 27 May 2020 08:14:44 +0200
Subject: [PATCH 1/2] read-file: add flags to modify reading behavior
* lib/read-file.h (RF_BINARY): New define.
(fread_file, read_file): Take FLAGS argument.
(read_binary_file): Remove.
* lib/read-file.c (internal_read_file): Merge into ...
(read_file): ... here.
* modules/read-file-tests (Files): Add "tests/macros.h".
* tests/test-read-file.c (main): Refactor using ASSERT macro.
* NEWS: Mention this change.
---
ChangeLog | 12 ++++++++++++
NEWS | 5 +++++
lib/read-file.c | 43 ++++++++++++++---------------------------
lib/read-file.h | 7 ++++---
modules/read-file-tests | 1 +
tests/test-read-file.c | 17 +++++++++++++---
6 files changed, 50 insertions(+), 35 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 07d4d5124..94faf6984 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2020-05-27 Daiki Ueno <u...@gnu.org>
+
+ read-file: add flags to modify reading behavior
+ * lib/read-file.h (RF_BINARY): New define.
+ (fread_file, read_file): Take FLAGS argument.
+ (read_binary_file): Remove.
+ * lib/read-file.c (internal_read_file): Merge into ...
+ (read_file): ... here.
+ * modules/read-file-tests (Files): Add "tests/macros.h".
+ * tests/test-read-file.c (main): Refactor using ASSERT macro.
+ * NEWS: Mention this change.
+
2020-05-26 Daiki Ueno <u...@gnu.org>
read-file: make use of fopen-gnu
diff --git a/NEWS b/NEWS
index 99973c5c3..c559a65e9 100644
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,11 @@ Important general notes
Date Modules Changes
+2020-05-27 read-file The functions provided by this module now take an
+ 'int flags' argument to modify the file reading
+ behavior. The read_binary_file function has been
+ removed as it is no longer necessary.
+
2019-12-11 Support for These modules are now supported in C++ mode as well.
ISO C or POSIX This means, while the autoconfiguration uses the C
functions compiler, the resulting header files and function
diff --git a/lib/read-file.c b/lib/read-file.c
index 293bc3e8a..904f1c901 100644
--- a/lib/read-file.c
+++ b/lib/read-file.c
@@ -40,7 +40,7 @@
*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)
+fread_file (FILE *stream, int flags _GL_UNUSED, size_t *length)
{
char *buf = NULL;
size_t alloc = BUFSIZ;
@@ -134,9 +134,19 @@ fread_file (FILE *stream, size_t *length)
}
}
-static char *
-internal_read_file (const char *filename, size_t *length, const char *mode)
+/* Open 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 *LENGTH. 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. */
+char *
+read_file (const char *filename, int flags, size_t *length)
{
+ const char *mode = (flags & RF_BINARY) ? "rbe" : "re";
FILE *stream = fopen (filename, mode);
char *out;
int save_errno;
@@ -144,7 +154,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;
@@ -161,28 +171,3 @@ internal_read_file (const char *filename, size_t *length, const char *mode)
return out;
}
-
-/* Open 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 *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");
-}
-
-/* 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. */
-char *
-read_binary_file (const char *filename, size_t *length)
-{
- return internal_read_file (filename, length, "rbe");
-}
diff --git a/lib/read-file.h b/lib/read-file.h
index bb28abd65..7ff82ca77 100644
--- a/lib/read-file.h
+++ b/lib/read-file.h
@@ -24,10 +24,11 @@
/* Get FILE. */
#include <stdio.h>
-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);
+extern char *fread_file (FILE * stream, int flags, size_t * length);
-extern char *read_binary_file (const char *filename, 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-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..84b904994 100644
--- a/tests/test-read-file.c
+++ b/tests/test-read-file.c
@@ -23,11 +23,13 @@
#include <stdlib.h>
#include <sys/stat.h>
+#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));
+
+ return 0;
+}
--
2.26.2
>From 874faad5aa189203d659b345427ff80cfab9301b Mon Sep 17 00:00:00 2001
From: Daiki Ueno <u...@gnu.org>
Date: Tue, 26 May 2020 10:22:37 +0200
Subject: [PATCH 2/2] read-file: add RF_SENSITIVE flag
* lib/read-file.h (RF_SENSITIVE): New define.
* lib/read-file.c (fread_file, read_file): Take into account of
RF_SENSITIVE flag.
* 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 | 11 +++++++++++
lib/read-file.c | 42 +++++++++++++++++++++++++++++++++++++-----
lib/read-file.h | 3 +++
modules/read-file | 1 +
tests/test-read-file.c | 2 ++
5 files changed, 54 insertions(+), 5 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 94faf6984..4a160faa6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2020-05-27 Daiki Ueno <u...@gnu.org>
+
+ read-file: add RF_SENSITIVE flag
+ * lib/read-file.h (RF_SENSITIVE): New define.
+ * lib/read-file.c (fread_file, read_file): Take into account of
+ RF_SENSITIVE flag.
+ * 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-27 Daiki Ueno <u...@gnu.org>
read-file: add flags to modify reading behavior
diff --git a/lib/read-file.c b/lib/read-file.c
index 904f1c901..8bf3fdbe4 100644
--- a/lib/read-file.c
+++ b/lib/read-file.c
@@ -31,6 +31,9 @@
/* Get malloc, realloc, free. */
#include <stdlib.h>
+/* Get explicit_bzero, memcpy. */
+#include <string.h>
+
/* Get errno. */
#include <errno.h>
@@ -38,9 +41,12 @@
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. */
+ 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 _GL_UNUSED, size_t *length)
+fread_file (FILE *stream, int flags, size_t *length)
{
char *buf = NULL;
size_t alloc = BUFSIZ;
@@ -94,7 +100,12 @@ fread_file (FILE *stream, int flags _GL_UNUSED, 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 +117,7 @@ fread_file (FILE *stream, int flags _GL_UNUSED, size_t *length)
{
char *new_buf;
+ size_t save_alloc = alloc;
if (alloc == PTRDIFF_MAX)
{
@@ -118,7 +130,21 @@ fread_file (FILE *stream, int flags _GL_UNUSED, 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,6 +154,9 @@ fread_file (FILE *stream, int flags _GL_UNUSED, size_t *length)
}
}
+ if (flags & RF_SENSITIVE)
+ explicit_bzero (buf, alloc);
+
free (buf);
errno = save_errno;
return NULL;
@@ -142,7 +171,8 @@ fread_file (FILE *stream, int flags _GL_UNUSED, size_t *length)
any), and NULL is returned.
If the RF_BINARY flag is set in FLAGS, the file is opened in binary
- mode. */
+ mode. If the RF_SENSITIVE flag is set in FLAGS, the memory buffer
+ internally allocated will be cleared upon failure. */
char *
read_file (const char *filename, int flags, size_t *length)
{
@@ -163,6 +193,8 @@ read_file (const char *filename, int flags, size_t *length)
if (out)
{
save_errno = errno;
+ if (flags & RF_SENSITIVE)
+ explicit_bzero (out, *length);
free (out);
}
errno = save_errno;
diff --git a/lib/read-file.h b/lib/read-file.h
index 7ff82ca77..c2454ef68 100644
--- a/lib/read-file.h
+++ b/lib/read-file.h
@@ -27,6 +27,9 @@
/* Indicate that the file is treated as binary. */
#define RF_BINARY 0x1
+/* Indicate that the file content contains sensitive information. */
+#define RF_SENSITIVE 0x2
+
extern char *fread_file (FILE * stream, int flags, size_t * length);
extern char *read_file (const char *filename, int flags, size_t * length);
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/tests/test-read-file.c b/tests/test-read-file.c
index 84b904994..b37f875b2 100644
--- a/tests/test-read-file.c
+++ b/tests/test-read-file.c
@@ -117,6 +117,8 @@ 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));
return 0;
}
--
2.26.2
Regards,
--
Daiki Ueno
