[PATCH] debuginfod-client: restrict cleanup to client-pattern files

[PATCH] debuginfod-client: restrict cleanup to client-pattern files

[Aaron Merey]

[-- Attachment #1: Type: text/plain, Size: 2 bytes --]



[-- Attachment #2: 0001-debuginfod-client-restrict-cleanup-to-client-pattern.patch --]
[-- Type: text/x-patch, Size: 1942 bytes --]

Avoid deleting contents of a general directory in case a user
mis-sets $DEBUGINFOD_CACHE_PATH.

Signed-off-by: Aaron Merey <amerey@redhat.com>
---
 debuginfod/ChangeLog           |  5 +++++
 debuginfod/debuginfod-client.c | 13 ++++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index d812e6d7..b5ff2525 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,8 @@
+2020-02-11  Aaron Merey  <amerey@redhat.com>
+
+	* debuginfod-client.c (debuginfod_clean_cache): Restrict
+	cleanup to client-pattern files.
+
 2020-02-05  Frank Ch. Eigler  <fche@redhat.com>
 
 	* debuginfod.cxx (argp options): Add -Z option.
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index e5a2e824..186aa90a 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -50,6 +50,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <fts.h>
+#include <regex.h>
 #include <string.h>
 #include <stdbool.h>
 #include <linux/limits.h>
@@ -241,10 +242,19 @@ debuginfod_clean_cache(debuginfod_client *c,
   if (fts == NULL)
     return -errno;
 
+  regex_t re;
+  const char * pattern = ".*/[a-f0-9]+/(debuginfo|executable|source.*)$";
+  if (regcomp (&re, pattern, REG_EXTENDED | REG_NOSUB) != 0)
+    return -ENOMEM;
+
   FTSENT *f;
   long files = 0;
   while ((f = fts_read(fts)) != NULL)
     {
+      /* ignore any files that do not match the pattern.  */
+      if (regexec (&re, f->fts_path, 0, NULL, 0) != 0)
+        continue;
+
       files++;
       if (c->progressfn) /* inform/check progress callback */
         if ((c->progressfn) (c, files, 0))
@@ -268,7 +278,8 @@ debuginfod_clean_cache(debuginfod_client *c,
           ;
         }
     }
-  fts_close(fts);
+  fts_close (fts);
+  regfree (&re);
 
   /* Update timestamp representing when the cache was last cleaned.  */
   utime (interval_path, NULL);
-- 
2.24.1

Re: [PATCH] debuginfod-client: restrict cleanup to client-pattern files

[Konrad Kleine]

[-- Attachment #1.1.1: Type: text/plain, Size: 30 bytes --]

Can we have a test for this?

[-- Attachment #1.1.2: 0xC0A02C32BCB73099.asc --]
[-- Type: application/pgp-keys, Size: 6531 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

Re: [PATCH] debuginfod-client: restrict cleanup to client-pattern files

[Aaron Merey]

[-- Attachment #1: Type: text/plain, Size: 210 bytes --]

Hi Konrad,

You're right, it's better to include a test. I've added one to the patch.

Aaron

On Tue, Feb 11, 2020 at 2:01 PM Konrad Kleine <kkleine@redhat.com> wrote:
>
> Can we have a test for this?

[-- Attachment #2: 0001-debuginfod-client-restrict-cleanup-to-client-pattern.patch --]
[-- Type: text/x-patch, Size: 3983 bytes --]

From de46a13aa67d65d61ae206d1ecf8b20f5d3623cf Mon Sep 17 00:00:00 2001
From: Aaron Merey <amerey@redhat.com>
Date: Tue, 11 Feb 2020 15:48:46 -0500
Subject: [PATCH] debuginfod-client: restrict cleanup to client-pattern files

Avoid deleting general files and directories in case a user
mis-sets $DEBUGINFOD_CACHE_PATH or accidentally moves a file
into the cache.

Signed-off-by: Aaron Merey <amerey@redhat.com>
---
 debuginfod/ChangeLog           |  5 +++++
 debuginfod/debuginfod-client.c | 13 ++++++++++++-
 tests/ChangeLog                |  5 +++++
 tests/run-debuginfod-find.sh   | 12 ++++++++++++
 4 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index d812e6d7..b5ff2525 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,8 @@
+2020-02-11  Aaron Merey  <amerey@redhat.com>
+
+	* debuginfod-client.c (debuginfod_clean_cache): Restrict
+	cleanup to client-pattern files.
+
 2020-02-05  Frank Ch. Eigler  <fche@redhat.com>
 
 	* debuginfod.cxx (argp options): Add -Z option.
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index e5a2e824..186aa90a 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -50,6 +50,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <fts.h>
+#include <regex.h>
 #include <string.h>
 #include <stdbool.h>
 #include <linux/limits.h>
@@ -241,10 +242,19 @@ debuginfod_clean_cache(debuginfod_client *c,
   if (fts == NULL)
     return -errno;
 
+  regex_t re;
+  const char * pattern = ".*/[a-f0-9]+/(debuginfo|executable|source.*)$";
+  if (regcomp (&re, pattern, REG_EXTENDED | REG_NOSUB) != 0)
+    return -ENOMEM;
+
   FTSENT *f;
   long files = 0;
   while ((f = fts_read(fts)) != NULL)
     {
+      /* ignore any files that do not match the pattern.  */
+      if (regexec (&re, f->fts_path, 0, NULL, 0) != 0)
+        continue;
+
       files++;
       if (c->progressfn) /* inform/check progress callback */
         if ((c->progressfn) (c, files, 0))
@@ -268,7 +278,8 @@ debuginfod_clean_cache(debuginfod_client *c,
           ;
         }
     }
-  fts_close(fts);
+  fts_close (fts);
+  regfree (&re);
 
   /* Update timestamp representing when the cache was last cleaned.  */
   utime (interval_path, NULL);
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 84953adb..49b93352 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,8 @@
+2020-02-11  Aaron Merey  <amerey@redhat.com>
+
+	* run-debuginfod-find.sh: Test that files unrelated to debuginfod
+	survive cache cleaning.
+
 2020-02-05  Frank Ch. Eigler  <fche@redhat.com>
 
 	* debuginfo-tars/*: New test files from Eli Schwartz of ArchLinux.
diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
index 1cc8f406..2964e7c0 100755
--- a/tests/run-debuginfod-find.sh
+++ b/tests/run-debuginfod-find.sh
@@ -365,6 +365,12 @@ testrun ${abs_builddir}/debuginfod_build_id_find -e F/prog2 1
 
 ########################################################################
 
+# Add some files to the cache that do not fit its naming format.
+# They should survive cache cleaning.
+mkdir $DEBUGINFOD_CACHE_PATH/malformed
+touch $DEBUGINFOD_CACHE_PATH/malformed0
+touch $DEBUGINFOD_CACHE_PATH/malformed/malformed1
+
 # Trigger a cache clean and run the tests again. The clients should be unable to
 # find the target.
 echo 0 > $DEBUGINFOD_CACHE_PATH/cache_clean_interval_s
@@ -374,6 +380,12 @@ testrun ${abs_builddir}/debuginfod_build_id_find -e F/prog 1
 
 testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID2 && false || true
 
+if [ ! -f $DEBUGINFOD_CACHE_PATH/malformed0 ] \
+    || [ ! -f $DEBUGINFOD_CACHE_PATH/malformed/malformed1 ]; then
+  echo "unrelated files did not survive cache cleaning"
+  exit 1
+fi
+
 # Test debuginfod without a path list; reuse $PORT1
 env LD_LIBRARY_PATH=$ldpath ${abs_builddir}/../debuginfod/debuginfod $VERBOSE -F -U -d :memory: -p $PORT1 -L -F &
 PID3=$!
-- 
2.24.1

Re: [PATCH] debuginfod-client: restrict cleanup to client-pattern files

[Konrad Kleine]

[-- Attachment #1.1.1: Type: text/plain, Size: 360 bytes --]

Hi Aaron,

this looks good to me. Thank you for taking care of the tests.

- Konrad

On 11/02/2020 21:54, Aaron Merey wrote:
> Hi Konrad,
> 
> You're right, it's better to include a test. I've added one to the patch.
> 
> Aaron
> 
> On Tue, Feb 11, 2020 at 2:01 PM Konrad Kleine <kkleine@redhat.com> wrote:
>>
>> Can we have a test for this?


[-- Attachment #1.1.2: 0xC0A02C32BCB73099.asc --]
[-- Type: application/pgp-keys, Size: 6531 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]