| Message ID | 20221206121952.87145-1-ayoub.zaki@embetrix.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | archive handler: add clean-destination property | expand |
Ping On Tuesday, December 6, 2022 at 1:19:57 PM UTC+1 ayoub...@googlemail.com wrote: > this option if enabled will wipe out the destination folder before > unpacking the archive for security and size reasons > > Signed-off-by: Ayoub Zaki <ayoub...@embetrix.com> > --- > core/stream_interface.c | 4 ++-- > core/util.c | 24 +++++++++++++++++++++++- > doc/source/handlers.rst | 4 ++++ > doc/source/sw-description.rst | 4 ++++ > handlers/archive_handler.c | 13 +++++++++++++ > include/util.h | 3 ++- > 6 files changed, 48 insertions(+), 4 deletions(-) > > diff --git a/core/stream_interface.c b/core/stream_interface.c > index 81c26c3..2a2ec49 100644 > --- a/core/stream_interface.c > +++ b/core/stream_interface.c > @@ -659,8 +659,8 @@ void *network_initializer(void *data) > cleanup_files(software); > > #ifndef CONFIG_NOCLEANUP > - swupdate_remove_directory(SCRIPTS_DIR_SUFFIX); > - swupdate_remove_directory(DATADST_DIR_SUFFIX); > + swupdate_remove_tmp_directory(SCRIPTS_DIR_SUFFIX); > + swupdate_remove_tmp_directory(DATADST_DIR_SUFFIX); > #endif > > pthread_mutex_lock(&stream_mutex); > diff --git a/core/util.c b/core/util.c > index 20dec15..db49e48 100644 > --- a/core/util.c > +++ b/core/util.c > @@ -171,7 +171,29 @@ static int _remove_directory_cb(const char *fpath, > const struct stat *sb, > return remove(fpath); > } > > -int swupdate_remove_directory(const char* path) > +static int _remove_subdirectory_cb(const char *fpath, const struct stat > *sb, > + int typeflag, struct FTW *ftwbuf) > +{ > + (void)sb; > + (void)typeflag; > + (void)ftwbuf; > + /* Skip top level folder */ > + if (ftwbuf->level > 0) { > + return remove(fpath); > + } > + return 0; > +} > + > +int swupdate_remove_directory(const char* path, bool keep) > +{ > + if (keep) > + return nftw(path, _remove_subdirectory_cb, 64, FTW_DEPTH | FTW_PHYS); > + else > + return nftw(path, _remove_directory_cb, 64, FTW_DEPTH | FTW_PHYS); > + > +} > + > +int swupdate_remove_tmp_directory(const char* path) > { > char* dpath; > int ret; > diff --git a/doc/source/handlers.rst b/doc/source/handlers.rst > index 5f686fe..78ee00b 100644 > --- a/doc/source/handlers.rst > +++ b/doc/source/handlers.rst > @@ -878,6 +878,9 @@ permissions (except +x, always preserved) and extended > attributes. > The property `create-destination` can be set to the string `true` to have > swupdate create > the destination path before extraction. > > +The property `clean-destination` can be set to the string `true` to have > swupdate clean > +the destination path before extraction. > + > :: > > files: ( > @@ -889,6 +892,7 @@ the destination path before extraction. > installed-directly = true; > properties: { > create-destination = "true"; > + clean-destination = "true"; > } > } > ); > diff --git a/doc/source/sw-description.rst b/doc/source/sw-description.rst > index a5182d6..e2b3eae 100644 > --- a/doc/source/sw-description.rst > +++ b/doc/source/sw-description.rst > @@ -738,6 +738,10 @@ As a general rule, swupdate doesn't copy out a file > if the destination path > doesn't exists. This behavior could be changed using the special property > "create-destination". > > +As another rule, the archive handler doesn't clean the destination path > +before unpacking an archive. This behavior could be changed using the > special property > +"clean-destination". > + > As another general rule, the raw file handler installs the file directly > to the > specified path. If the target file already exists and the raw file handler > is interrupted, the existing file may be replaced by an empty or partially > diff --git a/handlers/archive_handler.c b/handlers/archive_handler.c > index 0ca22ca..ac14130 100644 > --- a/handlers/archive_handler.c > +++ b/handlers/archive_handler.c > @@ -298,6 +298,19 @@ static int install_archive_image(struct img_type *img, > } > } > > +#ifndef CONFIG_NOCLEANUP > + /* > + * Check if destination must be cleaned before > + */ > + if (strtobool(dict_get_value(&img->properties, "clean-destination"))) { > + ret = swupdate_remove_directory(path, true); > + if (ret < 0) { > + ERROR("I cannot clean path %s: %s", path, strerror(errno)); > + exitval = -EFAULT; > + goto out; > + } > + } > +#endif > /* > * Change to directory where tarball must be extracted > */ > diff --git a/include/util.h b/include/util.h > index 6a3295c..a6908e3 100644 > --- a/include/util.h > +++ b/include/util.h > @@ -238,7 +238,8 @@ const char* get_tmpdirscripts(void); > > void swupdate_create_directory(const char* path); > #ifndef CONFIG_NOCLEANUP > -int swupdate_remove_directory(const char* path); > +int swupdate_remove_directory(const char* path, bool keep); > +int swupdate_remove_tmp_directory(const char* path); > #endif > > int swupdate_mount(const char *device, const char *dir, const char > *fstype); > -- > 2.34.1 > >
Hi Ayoub, sorry, the original post disappeared form my mail client (surely I erroneously dropped it). Found it in patchwork: On 16.01.23 02:28, 'ayoub...@googlemail.com' via swupdate wrote: > Ping > > On Tuesday, December 6, 2022 at 1:19:57 PM UTC+1 ayoub...@googlemail.com > wrote: > > this option if enabled will wipe out the destination folder before > unpacking the archive for security and size reasons > > Signed-off-by: Ayoub Zaki <ayoub...@embetrix.com> > --- > core/stream_interface.c | 4 ++-- > core/util.c | 24 +++++++++++++++++++++++- > doc/source/handlers.rst | 4 ++++ > doc/source/sw-description.rst | 4 ++++ > handlers/archive_handler.c | 13 +++++++++++++ > include/util.h | 3 ++- > 6 files changed, 48 insertions(+), 4 deletions(-) > > diff --git a/core/stream_interface.c b/core/stream_interface.c > index 81c26c3..2a2ec49 100644 > --- a/core/stream_interface.c > +++ b/core/stream_interface.c > @@ -659,8 +659,8 @@ void *network_initializer(void *data) > cleanup_files(software); > > #ifndef CONFIG_NOCLEANUP > - swupdate_remove_directory(SCRIPTS_DIR_SUFFIX); > - swupdate_remove_directory(DATADST_DIR_SUFFIX); > + swupdate_remove_tmp_directory(SCRIPTS_DIR_SUFFIX); > + swupdate_remove_tmp_directory(DATADST_DIR_SUFFIX); > #endif > I do not like to introduce another wrapper just for this, and deleting from TMPDIR is just like calling swupdate_remove_directory() with other parameters. As far as I see, function is just called here in stream_interface.c. We can then easy update the signature like: swupdate_remove_directory(cpnst char *path, const char *dirname) And in stream_interface.c we can run: swupdate_remove_directory(get_tmpdir(), SCRIPTS_DIR_SUFFIX); swupdate_remove_directory(get_tmpdir(), DATADST_DIR_SUFFIX); Simply changing in swupdate_remove_directory like: int swupdate_remove_directory(const char *base, const char* path) { char* dpath; int ret; if (asprintf(&dpath, "%s%s", base, path) == Checking for null pointers , obviously. > pthread_mutex_lock(&stream_mutex); > diff --git a/core/util.c b/core/util.c > index 20dec15..db49e48 100644 > --- a/core/util.c > +++ b/core/util.c > @@ -171,7 +171,29 @@ static int _remove_directory_cb(const char > *fpath, const struct stat *sb, > return remove(fpath); > } > > -int swupdate_remove_directory(const char* path) > +static int _remove_subdirectory_cb(const char *fpath, const struct > stat *sb, > + int typeflag, struct FTW *ftwbuf) > +{ > + (void)sb; > + (void)typeflag; > + (void)ftwbuf; > + /* Skip top level folder */ > + if (ftwbuf->level > 0) { > + return remove(fpath); > + } > + return 0; > +} > + > +int swupdate_remove_directory(const char* path, bool keep) > +{ > + if (keep) > + return nftw(path, _remove_subdirectory_cb, 64, FTW_DEPTH | FTW_PHYS); > + else > + return nftw(path, _remove_directory_cb, 64, FTW_DEPTH | FTW_PHYS); > + I am blind, but which is the difference between the two branch ? > +} > + > +int swupdate_remove_tmp_directory(const char* path) > { > char* dpath; > int ret; > diff --git a/doc/source/handlers.rst b/doc/source/handlers.rst > index 5f686fe..78ee00b 100644 > --- a/doc/source/handlers.rst > +++ b/doc/source/handlers.rst > @@ -878,6 +878,9 @@ permissions (except +x, always preserved) and > extended attributes. > The property `create-destination` can be set to the string `true` to > have swupdate create > the destination path before extraction. > > +The property `clean-destination` can be set to the string `true` to > have swupdate clean > +the destination path before extraction. > + > :: > > files: ( > @@ -889,6 +892,7 @@ the destination path before extraction. > installed-directly = true; > properties: { > create-destination = "true"; > + clean-destination = "true"; > } > } > ); > diff --git a/doc/source/sw-description.rst > b/doc/source/sw-description.rst > index a5182d6..e2b3eae 100644 > --- a/doc/source/sw-description.rst > +++ b/doc/source/sw-description.rst > @@ -738,6 +738,10 @@ As a general rule, swupdate doesn't copy out a > file if the destination path > doesn't exists. This behavior could be changed using the special > property > "create-destination". > > +As another rule, the archive handler doesn't clean the destination > path > +before unpacking an archive. This behavior could be changed using > the special property > +"clean-destination". > + > As another general rule, the raw file handler installs the file > directly to the > specified path. If the target file already exists and the raw file > handler > is interrupted, the existing file may be replaced by an empty or > partially > diff --git a/handlers/archive_handler.c b/handlers/archive_handler.c > index 0ca22ca..ac14130 100644 > --- a/handlers/archive_handler.c > +++ b/handlers/archive_handler.c > @@ -298,6 +298,19 @@ static int install_archive_image(struct > img_type *img, > } > } > > +#ifndef CONFIG_NOCLEANUP > + /* This is anothe ruse case. CONFIG_CLEANUP is just for debugging, and it should leave extracted files to check it in case of corruption and so on. In case of handler, the property rule. This CONFIG_ does not apply it, and cleanup should happen if clean-destination is set. So the #ifndef is not needed in handler. > + * Check if destination must be cleaned before > + */ > + if (strtobool(dict_get_value(&img->properties, > "clean-destination"))) { > + ret = swupdate_remove_directory(path, true); > + if (ret < 0) { > + ERROR("I cannot clean path %s: %s", path, strerror(errno)); > + exitval = -EFAULT; > + goto out; > + } > + } > +#endif > /* > * Change to directory where tarball must be extracted > */ > diff --git a/include/util.h b/include/util.h > index 6a3295c..a6908e3 100644 > --- a/include/util.h > +++ b/include/util.h > @@ -238,7 +238,8 @@ const char* get_tmpdirscripts(void); > > void swupdate_create_directory(const char* path); > #ifndef CONFIG_NOCLEANUP > -int swupdate_remove_directory(const char* path); > +int swupdate_remove_directory(const char* path, bool keep); > +int swupdate_remove_tmp_directory(const char* path); > #endif > > int swupdate_mount(const char *device, const char *dir, const char > *fstype); > -- > 2.34.1 > Best regards, Stefano
diff --git a/core/stream_interface.c b/core/stream_interface.c index 81c26c3..2a2ec49 100644 --- a/core/stream_interface.c +++ b/core/stream_interface.c @@ -659,8 +659,8 @@ void *network_initializer(void *data) cleanup_files(software); #ifndef CONFIG_NOCLEANUP - swupdate_remove_directory(SCRIPTS_DIR_SUFFIX); - swupdate_remove_directory(DATADST_DIR_SUFFIX); + swupdate_remove_tmp_directory(SCRIPTS_DIR_SUFFIX); + swupdate_remove_tmp_directory(DATADST_DIR_SUFFIX); #endif pthread_mutex_lock(&stream_mutex); diff --git a/core/util.c b/core/util.c index 20dec15..db49e48 100644 --- a/core/util.c +++ b/core/util.c @@ -171,7 +171,29 @@ static int _remove_directory_cb(const char *fpath, const struct stat *sb, return remove(fpath); } -int swupdate_remove_directory(const char* path) +static int _remove_subdirectory_cb(const char *fpath, const struct stat *sb, + int typeflag, struct FTW *ftwbuf) +{ + (void)sb; + (void)typeflag; + (void)ftwbuf; + /* Skip top level folder */ + if (ftwbuf->level > 0) { + return remove(fpath); + } + return 0; +} + +int swupdate_remove_directory(const char* path, bool keep) +{ + if (keep) + return nftw(path, _remove_subdirectory_cb, 64, FTW_DEPTH | FTW_PHYS); + else + return nftw(path, _remove_directory_cb, 64, FTW_DEPTH | FTW_PHYS); + +} + +int swupdate_remove_tmp_directory(const char* path) { char* dpath; int ret; diff --git a/doc/source/handlers.rst b/doc/source/handlers.rst index 5f686fe..78ee00b 100644 --- a/doc/source/handlers.rst +++ b/doc/source/handlers.rst @@ -878,6 +878,9 @@ permissions (except +x, always preserved) and extended attributes. The property `create-destination` can be set to the string `true` to have swupdate create the destination path before extraction. +The property `clean-destination` can be set to the string `true` to have swupdate clean +the destination path before extraction. + :: files: ( @@ -889,6 +892,7 @@ the destination path before extraction. installed-directly = true; properties: { create-destination = "true"; + clean-destination = "true"; } } ); diff --git a/doc/source/sw-description.rst b/doc/source/sw-description.rst index a5182d6..e2b3eae 100644 --- a/doc/source/sw-description.rst +++ b/doc/source/sw-description.rst @@ -738,6 +738,10 @@ As a general rule, swupdate doesn't copy out a file if the destination path doesn't exists. This behavior could be changed using the special property "create-destination". +As another rule, the archive handler doesn't clean the destination path +before unpacking an archive. This behavior could be changed using the special property +"clean-destination". + As another general rule, the raw file handler installs the file directly to the specified path. If the target file already exists and the raw file handler is interrupted, the existing file may be replaced by an empty or partially diff --git a/handlers/archive_handler.c b/handlers/archive_handler.c index 0ca22ca..ac14130 100644 --- a/handlers/archive_handler.c +++ b/handlers/archive_handler.c @@ -298,6 +298,19 @@ static int install_archive_image(struct img_type *img, } } +#ifndef CONFIG_NOCLEANUP + /* + * Check if destination must be cleaned before + */ + if (strtobool(dict_get_value(&img->properties, "clean-destination"))) { + ret = swupdate_remove_directory(path, true); + if (ret < 0) { + ERROR("I cannot clean path %s: %s", path, strerror(errno)); + exitval = -EFAULT; + goto out; + } + } +#endif /* * Change to directory where tarball must be extracted */ diff --git a/include/util.h b/include/util.h index 6a3295c..a6908e3 100644 --- a/include/util.h +++ b/include/util.h @@ -238,7 +238,8 @@ const char* get_tmpdirscripts(void); void swupdate_create_directory(const char* path); #ifndef CONFIG_NOCLEANUP -int swupdate_remove_directory(const char* path); +int swupdate_remove_directory(const char* path, bool keep); +int swupdate_remove_tmp_directory(const char* path); #endif int swupdate_mount(const char *device, const char *dir, const char *fstype);
this option if enabled will wipe out the destination folder before unpacking the archive for security and size reasons Signed-off-by: Ayoub Zaki <ayoub.zaki@embetrix.com> --- core/stream_interface.c | 4 ++-- core/util.c | 24 +++++++++++++++++++++++- doc/source/handlers.rst | 4 ++++ doc/source/sw-description.rst | 4 ++++ handlers/archive_handler.c | 13 +++++++++++++ include/util.h | 3 ++- 6 files changed, 48 insertions(+), 4 deletions(-)