Bruno Haible
2018-05-06 11:33:10 UTC
Each time you see a call to the 'fileno' function, you should ask yourself
"What if some data has already been written to / read from the stream?
What about the buffers in the FILE structure that the kernel doesn't
know about?"
So I extended the test suite to test also the case of a FILE stream that
has some buffered data. It's no surprise that this test passes when
configuring with --without-linux-crypto, but fails when configuring
with the default (--with-linux-crypto):
FAIL: test-md5
FAIL: test-sha1
FAIL: test-sha256
FAIL: test-sha512
2018-05-06 Bruno Haible <***@clisp.org>
af_alg: Fix bug with streams that are not at position 0.
* lib/af_alg.c (afalg_stream): Before sendfile, invoke fflush. Don't
assume that the stream is positioned at position 0.
* lib/af_alg.h (afalg_stream): Mention restriction regarding the state
of the stream.
* lib/md5.h (md5_stream): Likewise.
* lib/sha1.h (sha1_stream): Likewise.
* lib/sha256.h (sha256_stream, sha224_stream): Likewise.
* lib/sha512.h (sha512_stream, sha384_stream): Likewise.
* modules/crypto/af_alg (Depends-on): Add fflush, lseek.
crypto/{md5,sha1,sha256,sha512} tests: Enhance test.
* tests/test-digest.h (test_digest_on_files): Add a test with a FILE
stream that is not positioned at the beginning.
diff --git a/lib/af_alg.c b/lib/af_alg.c
index 97bdff5..79e2195 100644
--- a/lib/af_alg.c
+++ b/lib/af_alg.c
@@ -25,6 +25,8 @@
#include <unistd.h>
#include <string.h>
+#include <stdio.h>
+#include <errno.h>
#include <linux/if_alg.h>
#include <sys/stat.h>
#include <sys/sendfile.h>
@@ -65,12 +67,35 @@ afalg_stream (FILE *stream, const char *alg, void *resblock, ssize_t hashlen)
}
/* if file is a regular file, attempt sendfile to pipe the data. */
+ int fd = fileno (stream);
struct stat st;
- if (fstat (fileno (stream), &st) == 0
+ if (fstat (fd, &st) == 0
&& (S_ISREG (st.st_mode) || S_TYPEISSHM (&st) || S_TYPEISTMO (&st))
&& 0 < st.st_size && st.st_size <= SYS_BUFSIZE_MAX)
{
- if (sendfile (ofd, fileno (stream), NULL, st.st_size) != st.st_size)
+ /* Make sure the offset of fileno (stream) reflects how many bytes
+ have been read from stream before this function got invoked.
+ Note: fflush on an input stream after ungetc does not work as expected
+ on some platforms. Therefore this situation is not supported here. */
+ if (fflush (stream))
+ {
+#if defined _WIN32 && ! defined __CYGWIN__
+ ret = -EIO;
+#else
+ ret = -errno;
+#endif
+ goto out_cfd;
+ }
+
+ off_t nbytes = st.st_size - lseek (fd, 0, SEEK_CUR);
+ /* On Linux < 4.9, the value for an empty stream is wrong (all zeroes).
+ See <https://patchwork.kernel.org/patch/9434741/>. */
+ if (nbytes <= 0)
+ {
+ ret = -EAFNOSUPPORT;
+ goto out_ofd;
+ }
+ if (sendfile (ofd, fd, NULL, nbytes) != nbytes)
{
ret = -EIO;
goto out_ofd;
diff --git a/lib/af_alg.h b/lib/af_alg.h
index a15a956..dfcc995 100644
--- a/lib/af_alg.h
+++ b/lib/af_alg.h
@@ -38,8 +38,13 @@ extern "C" {
# if USE_LINUX_CRYPTO_API
/* Compute a message digest of the contents of a file.
+
STREAM is an open file stream. Regular files are handled more efficiently.
+ The contents of STREAM from its current position to its end will be read.
+ The case that the last operation on STREAM was an 'ungetc' is not supported.
+
ALG is the message digest algorithm; see the file /proc/crypto.
+
RESBLOCK points to a block of HASHLEN bytes, for the result.
HASHLEN must be the length of the message digest, in bytes, in particular:
diff --git a/lib/md5.h b/lib/md5.h
index d5d3c01..47dc7d4 100644
--- a/lib/md5.h
+++ b/lib/md5.h
@@ -122,8 +122,11 @@ extern void *__md5_buffer (const char *buffer, size_t len,
void *resblock) __THROW;
# endif
-/* Compute MD5 message digest for bytes read from STREAM. The
- resulting message digest number will be written into the 16 bytes
+/* Compute MD5 message digest for bytes read from STREAM.
+ STREAM is an open file stream. Regular files are handled more efficiently.
+ The contents of STREAM from its current position to its end will be read.
+ The case that the last operation on STREAM was an 'ungetc' is not supported.
+ The resulting message digest number will be written into the 16 bytes
beginning at RESBLOCK. */
extern int __md5_stream (FILE *stream, void *resblock) __THROW;
diff --git a/lib/sha1.h b/lib/sha1.h
index e41ce4b..cca83fc 100644
--- a/lib/sha1.h
+++ b/lib/sha1.h
@@ -87,8 +87,11 @@ extern void *sha1_read_ctx (const struct sha1_ctx *ctx, void *resbuf);
extern void *sha1_buffer (const char *buffer, size_t len, void *resblock);
# endif
-/* Compute SHA1 message digest for bytes read from STREAM. The
- resulting message digest number will be written into the 20 bytes
+/* Compute SHA1 message digest for bytes read from STREAM.
+ STREAM is an open file stream. Regular files are handled more efficiently.
+ The contents of STREAM from its current position to its end will be read.
+ The case that the last operation on STREAM was an 'ungetc' is not supported.
+ The resulting message digest number will be written into the 20 bytes
beginning at RESBLOCK. */
extern int sha1_stream (FILE *stream, void *resblock);
diff --git a/lib/sha256.h b/lib/sha256.h
index e344986..19ed3cc 100644
--- a/lib/sha256.h
+++ b/lib/sha256.h
@@ -89,8 +89,11 @@ extern void *sha256_buffer (const char *buffer, size_t len, void *resblock);
extern void *sha224_buffer (const char *buffer, size_t len, void *resblock);
# endif
-/* Compute SHA256 (SHA224) message digest for bytes read from STREAM. The
- resulting message digest number will be written into the 32 (28) bytes
+/* Compute SHA256 (SHA224) message digest for bytes read from STREAM.
+ STREAM is an open file stream. Regular files are handled more efficiently.
+ The contents of STREAM from its current position to its end will be read.
+ The case that the last operation on STREAM was an 'ungetc' is not supported.
+ The resulting message digest number will be written into the 32 (28) bytes
beginning at RESBLOCK. */
extern int sha256_stream (FILE *stream, void *resblock);
extern int sha224_stream (FILE *stream, void *resblock);
diff --git a/lib/sha512.h b/lib/sha512.h
index 6a0aadb..2c39ab1 100644
--- a/lib/sha512.h
+++ b/lib/sha512.h
@@ -92,8 +92,11 @@ extern void *sha512_buffer (const char *buffer, size_t len, void *resblock);
extern void *sha384_buffer (const char *buffer, size_t len, void *resblock);
# endif
-/* Compute SHA512 (SHA384) message digest for bytes read from STREAM. The
- resulting message digest number will be written into the 64 (48) bytes
+/* Compute SHA512 (SHA384) message digest for bytes read from STREAM.
+ STREAM is an open file stream. Regular files are handled more efficiently.
+ The contents of STREAM from its current position to its end will be read.
+ The case that the last operation on STREAM was an 'ungetc' is not supported.
+ The resulting message digest number will be written into the 64 (48) bytes
beginning at RESBLOCK. */
extern int sha512_stream (FILE *stream, void *resblock);
extern int sha384_stream (FILE *stream, void *resblock);
diff --git a/modules/crypto/af_alg b/modules/crypto/af_alg
index c9602dc..0c498e0 100644
--- a/modules/crypto/af_alg
+++ b/modules/crypto/af_alg
@@ -8,6 +8,8 @@ lib/sys-limits.h
m4/af_alg.m4
Depends-on:
+fflush [test $USE_AF_ALG = 1]
+lseek [test $USE_AF_ALG = 1]
sys_socket
sys_stat
diff --git a/tests/test-digest.h b/tests/test-digest.h
index e80e2cf..e10f571 100644
--- a/tests/test-digest.h
+++ b/tests/test-digest.h
@@ -25,7 +25,7 @@ test_digest_on_files (int (*streamfunc) (FILE *, void *),
int pass;
unlink (TESTFILE);
- for (pass = 0; pass < 3; pass++)
+ for (pass = 0; pass < 4; pass++)
{
{
FILE *fp = fopen (TESTFILE, "wb");
@@ -44,6 +44,10 @@ test_digest_on_files (int (*streamfunc) (FILE *, void *),
fputs ("The quick brown fox jumps over the lazy dog.\n", fp);
break;
case 2:
+ /* Fill the small file, with some header that will be skipped. */
+ fputs ("ABCDThe quick brown fox jumps over the lazy dog.\n", fp);
+ break;
+ case 3:
/* Fill the large file (8 MiB). */
{
unsigned int i;
@@ -74,9 +78,9 @@ test_digest_on_files (int (*streamfunc) (FILE *, void *),
switch (pass)
{
- case 0: expected = expected_for_empty_file; break;
- case 1: expected = expected_for_small_file; break;
- case 2: expected = expected_for_large_file; break;
+ case 0: expected = expected_for_empty_file; break;
+ case 1: case 2: expected = expected_for_small_file; break;
+ case 3: expected = expected_for_large_file; break;
default: abort ();
}
@@ -86,6 +90,20 @@ test_digest_on_files (int (*streamfunc) (FILE *, void *),
fprintf (stderr, "Could not open file %s.\n", TESTFILE);
exit (1);
}
+ switch (pass)
+ {
+ case 2:
+ {
+ char header[4];
+ if (fread (header, 1, sizeof (header), fp) != sizeof (header))
+ {
+ fprintf (stderr, "Could not read the header of %s.\n",
+ TESTFILE);
+ exit (1);
+ }
+ }
+ break;
+ }
ret = streamfunc (fp, digest);
if (ret)
{
"What if some data has already been written to / read from the stream?
What about the buffers in the FILE structure that the kernel doesn't
know about?"
So I extended the test suite to test also the case of a FILE stream that
has some buffered data. It's no surprise that this test passes when
configuring with --without-linux-crypto, but fails when configuring
with the default (--with-linux-crypto):
FAIL: test-md5
FAIL: test-sha1
FAIL: test-sha256
FAIL: test-sha512
2018-05-06 Bruno Haible <***@clisp.org>
af_alg: Fix bug with streams that are not at position 0.
* lib/af_alg.c (afalg_stream): Before sendfile, invoke fflush. Don't
assume that the stream is positioned at position 0.
* lib/af_alg.h (afalg_stream): Mention restriction regarding the state
of the stream.
* lib/md5.h (md5_stream): Likewise.
* lib/sha1.h (sha1_stream): Likewise.
* lib/sha256.h (sha256_stream, sha224_stream): Likewise.
* lib/sha512.h (sha512_stream, sha384_stream): Likewise.
* modules/crypto/af_alg (Depends-on): Add fflush, lseek.
crypto/{md5,sha1,sha256,sha512} tests: Enhance test.
* tests/test-digest.h (test_digest_on_files): Add a test with a FILE
stream that is not positioned at the beginning.
diff --git a/lib/af_alg.c b/lib/af_alg.c
index 97bdff5..79e2195 100644
--- a/lib/af_alg.c
+++ b/lib/af_alg.c
@@ -25,6 +25,8 @@
#include <unistd.h>
#include <string.h>
+#include <stdio.h>
+#include <errno.h>
#include <linux/if_alg.h>
#include <sys/stat.h>
#include <sys/sendfile.h>
@@ -65,12 +67,35 @@ afalg_stream (FILE *stream, const char *alg, void *resblock, ssize_t hashlen)
}
/* if file is a regular file, attempt sendfile to pipe the data. */
+ int fd = fileno (stream);
struct stat st;
- if (fstat (fileno (stream), &st) == 0
+ if (fstat (fd, &st) == 0
&& (S_ISREG (st.st_mode) || S_TYPEISSHM (&st) || S_TYPEISTMO (&st))
&& 0 < st.st_size && st.st_size <= SYS_BUFSIZE_MAX)
{
- if (sendfile (ofd, fileno (stream), NULL, st.st_size) != st.st_size)
+ /* Make sure the offset of fileno (stream) reflects how many bytes
+ have been read from stream before this function got invoked.
+ Note: fflush on an input stream after ungetc does not work as expected
+ on some platforms. Therefore this situation is not supported here. */
+ if (fflush (stream))
+ {
+#if defined _WIN32 && ! defined __CYGWIN__
+ ret = -EIO;
+#else
+ ret = -errno;
+#endif
+ goto out_cfd;
+ }
+
+ off_t nbytes = st.st_size - lseek (fd, 0, SEEK_CUR);
+ /* On Linux < 4.9, the value for an empty stream is wrong (all zeroes).
+ See <https://patchwork.kernel.org/patch/9434741/>. */
+ if (nbytes <= 0)
+ {
+ ret = -EAFNOSUPPORT;
+ goto out_ofd;
+ }
+ if (sendfile (ofd, fd, NULL, nbytes) != nbytes)
{
ret = -EIO;
goto out_ofd;
diff --git a/lib/af_alg.h b/lib/af_alg.h
index a15a956..dfcc995 100644
--- a/lib/af_alg.h
+++ b/lib/af_alg.h
@@ -38,8 +38,13 @@ extern "C" {
# if USE_LINUX_CRYPTO_API
/* Compute a message digest of the contents of a file.
+
STREAM is an open file stream. Regular files are handled more efficiently.
+ The contents of STREAM from its current position to its end will be read.
+ The case that the last operation on STREAM was an 'ungetc' is not supported.
+
ALG is the message digest algorithm; see the file /proc/crypto.
+
RESBLOCK points to a block of HASHLEN bytes, for the result.
HASHLEN must be the length of the message digest, in bytes, in particular:
diff --git a/lib/md5.h b/lib/md5.h
index d5d3c01..47dc7d4 100644
--- a/lib/md5.h
+++ b/lib/md5.h
@@ -122,8 +122,11 @@ extern void *__md5_buffer (const char *buffer, size_t len,
void *resblock) __THROW;
# endif
-/* Compute MD5 message digest for bytes read from STREAM. The
- resulting message digest number will be written into the 16 bytes
+/* Compute MD5 message digest for bytes read from STREAM.
+ STREAM is an open file stream. Regular files are handled more efficiently.
+ The contents of STREAM from its current position to its end will be read.
+ The case that the last operation on STREAM was an 'ungetc' is not supported.
+ The resulting message digest number will be written into the 16 bytes
beginning at RESBLOCK. */
extern int __md5_stream (FILE *stream, void *resblock) __THROW;
diff --git a/lib/sha1.h b/lib/sha1.h
index e41ce4b..cca83fc 100644
--- a/lib/sha1.h
+++ b/lib/sha1.h
@@ -87,8 +87,11 @@ extern void *sha1_read_ctx (const struct sha1_ctx *ctx, void *resbuf);
extern void *sha1_buffer (const char *buffer, size_t len, void *resblock);
# endif
-/* Compute SHA1 message digest for bytes read from STREAM. The
- resulting message digest number will be written into the 20 bytes
+/* Compute SHA1 message digest for bytes read from STREAM.
+ STREAM is an open file stream. Regular files are handled more efficiently.
+ The contents of STREAM from its current position to its end will be read.
+ The case that the last operation on STREAM was an 'ungetc' is not supported.
+ The resulting message digest number will be written into the 20 bytes
beginning at RESBLOCK. */
extern int sha1_stream (FILE *stream, void *resblock);
diff --git a/lib/sha256.h b/lib/sha256.h
index e344986..19ed3cc 100644
--- a/lib/sha256.h
+++ b/lib/sha256.h
@@ -89,8 +89,11 @@ extern void *sha256_buffer (const char *buffer, size_t len, void *resblock);
extern void *sha224_buffer (const char *buffer, size_t len, void *resblock);
# endif
-/* Compute SHA256 (SHA224) message digest for bytes read from STREAM. The
- resulting message digest number will be written into the 32 (28) bytes
+/* Compute SHA256 (SHA224) message digest for bytes read from STREAM.
+ STREAM is an open file stream. Regular files are handled more efficiently.
+ The contents of STREAM from its current position to its end will be read.
+ The case that the last operation on STREAM was an 'ungetc' is not supported.
+ The resulting message digest number will be written into the 32 (28) bytes
beginning at RESBLOCK. */
extern int sha256_stream (FILE *stream, void *resblock);
extern int sha224_stream (FILE *stream, void *resblock);
diff --git a/lib/sha512.h b/lib/sha512.h
index 6a0aadb..2c39ab1 100644
--- a/lib/sha512.h
+++ b/lib/sha512.h
@@ -92,8 +92,11 @@ extern void *sha512_buffer (const char *buffer, size_t len, void *resblock);
extern void *sha384_buffer (const char *buffer, size_t len, void *resblock);
# endif
-/* Compute SHA512 (SHA384) message digest for bytes read from STREAM. The
- resulting message digest number will be written into the 64 (48) bytes
+/* Compute SHA512 (SHA384) message digest for bytes read from STREAM.
+ STREAM is an open file stream. Regular files are handled more efficiently.
+ The contents of STREAM from its current position to its end will be read.
+ The case that the last operation on STREAM was an 'ungetc' is not supported.
+ The resulting message digest number will be written into the 64 (48) bytes
beginning at RESBLOCK. */
extern int sha512_stream (FILE *stream, void *resblock);
extern int sha384_stream (FILE *stream, void *resblock);
diff --git a/modules/crypto/af_alg b/modules/crypto/af_alg
index c9602dc..0c498e0 100644
--- a/modules/crypto/af_alg
+++ b/modules/crypto/af_alg
@@ -8,6 +8,8 @@ lib/sys-limits.h
m4/af_alg.m4
Depends-on:
+fflush [test $USE_AF_ALG = 1]
+lseek [test $USE_AF_ALG = 1]
sys_socket
sys_stat
diff --git a/tests/test-digest.h b/tests/test-digest.h
index e80e2cf..e10f571 100644
--- a/tests/test-digest.h
+++ b/tests/test-digest.h
@@ -25,7 +25,7 @@ test_digest_on_files (int (*streamfunc) (FILE *, void *),
int pass;
unlink (TESTFILE);
- for (pass = 0; pass < 3; pass++)
+ for (pass = 0; pass < 4; pass++)
{
{
FILE *fp = fopen (TESTFILE, "wb");
@@ -44,6 +44,10 @@ test_digest_on_files (int (*streamfunc) (FILE *, void *),
fputs ("The quick brown fox jumps over the lazy dog.\n", fp);
break;
case 2:
+ /* Fill the small file, with some header that will be skipped. */
+ fputs ("ABCDThe quick brown fox jumps over the lazy dog.\n", fp);
+ break;
+ case 3:
/* Fill the large file (8 MiB). */
{
unsigned int i;
@@ -74,9 +78,9 @@ test_digest_on_files (int (*streamfunc) (FILE *, void *),
switch (pass)
{
- case 0: expected = expected_for_empty_file; break;
- case 1: expected = expected_for_small_file; break;
- case 2: expected = expected_for_large_file; break;
+ case 0: expected = expected_for_empty_file; break;
+ case 1: case 2: expected = expected_for_small_file; break;
+ case 3: expected = expected_for_large_file; break;
default: abort ();
}
@@ -86,6 +90,20 @@ test_digest_on_files (int (*streamfunc) (FILE *, void *),
fprintf (stderr, "Could not open file %s.\n", TESTFILE);
exit (1);
}
+ switch (pass)
+ {
+ case 2:
+ {
+ char header[4];
+ if (fread (header, 1, sizeof (header), fp) != sizeof (header))
+ {
+ fprintf (stderr, "Could not read the header of %s.\n",
+ TESTFILE);
+ exit (1);
+ }
+ }
+ break;
+ }
ret = streamfunc (fp, digest);
if (ret)
{