bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

af_alg: Fix bug with streams that are not at position 0


From: Bruno Haible
Subject: af_alg: Fix bug with streams that are not at position 0
Date: Sun, 06 May 2018 13:33:10 +0200
User-agent: KMail/5.1.3 (Linux/4.4.0-119-generic; KDE/5.18.0; x86_64; ; )

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  <address@hidden>

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]