[pacman-dev] [PATCH 1/3] Remove pre libarchive 3.0 code

newer
[pacman-dev] [PATCH v2 1/3] meson:...

older
[pacman-dev] [PATCH 1/3] meson:...

Emil Velikov

23 Dec 2020 23 Dec '20

10:43 p.m.

Pacman has required libarchive 3.0 or later for quite some time mow. Signed-off-by: Emil Velikov --- lib/libalpm/libarchive-compat.h | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/lib/libalpm/libarchive-compat.h b/lib/libalpm/libarchive-compat.h index 0ff005ae..596fd664 100644 --- a/lib/libalpm/libarchive-compat.h +++ b/lib/libalpm/libarchive-compat.h @@ -24,48 +24,28 @@ static inline int _alpm_archive_read_free(struct archive *archive) { -#if ARCHIVE_VERSION_NUMBER >= 3000000 return archive_read_free(archive); -#else - return archive_read_finish(archive); -#endif } static inline int64_t _alpm_archive_compressed_ftell(struct archive *archive) { -#if ARCHIVE_VERSION_NUMBER >= 3000000 return archive_filter_bytes(archive, -1); -#else - return archive_position_compressed(archive); -#endif } static inline int _alpm_archive_read_open_file(struct archive *archive, const char *filename, size_t block_size) { -#if ARCHIVE_VERSION_NUMBER >= 3000000 return archive_read_open_filename(archive, filename, block_size); -#else - return archive_read_open_file(archive, filename, block_size); -#endif } static inline int _alpm_archive_filter_code(struct archive *archive) { -#if ARCHIVE_VERSION_NUMBER >= 3000000 return archive_filter_code(archive, 0); -#else - return archive_compression(archive); -#endif } static inline int _alpm_archive_read_support_filter_all(struct archive *archive) { -#if ARCHIVE_VERSION_NUMBER >= 3000000 return archive_read_support_filter_all(archive); -#else - return archive_read_support_compression_all(archive); -#endif } #endif /* LIBARCHIVE_COMPAT_H */ -- 2.29.2

Show replies by date

Emil Velikov

23 Dec 23 Dec

10:43 p.m.

New subject: [pacman-dev] [PATCH 2/3] Move hex_representation() to src/common

We'll reuse the function in pacman with a later commit. Signed-off-by: Emil Velikov --- lib/libalpm/util.c | 24 ------------------------ src/common/util-common.c | 26 ++++++++++++++++++++++++++ src/common/util-common.h | 1 + 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index b70a8192..9ae08745 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -1014,30 +1014,6 @@ static int sha256_file(const char *path, unsigned char output[32]) } #endif /* HAVE_LIBSSL || HAVE_LIBNETTLE */ -/** Create a string representing bytes in hexadecimal. - * @param bytes the bytes to represent in hexadecimal - * @param size number of bytes to consider - * @return a NULL terminated string with the hexadecimal representation of - * bytes or NULL on error. This string must be freed. - */ -static char *hex_representation(const unsigned char *bytes, size_t size) -{ - static const char *hex_digits = "0123456789abcdef"; - char *str; - size_t i; - - MALLOC(str, 2 * size + 1, return NULL); - - for(i = 0; i < size; i++) { - str[2 * i] = hex_digits[bytes[i] >> 4]; - str[2 * i + 1] = hex_digits[bytes[i] & 0x0f]; - } - - str[2 * size] = '\0'; - - return str; -} - char SYMEXPORT *alpm_compute_md5sum(const char *filename) { unsigned char output[16]; diff --git a/src/common/util-common.c b/src/common/util-common.c index 7d43ac0d..fc892c11 100644 --- a/src/common/util-common.c +++ b/src/common/util-common.c @@ -25,6 +25,32 @@ #include "util-common.h" +/** Create a string representing bytes in hexadecimal. + * @param bytes the bytes to represent in hexadecimal + * @param size number of bytes to consider + * @return a NULL terminated string with the hexadecimal representation of + * bytes or NULL on error. This string must be freed. + */ +char *hex_representation(const unsigned char *bytes, size_t size) +{ + static const char *hex_digits = "0123456789abcdef"; + char *str = malloc(2 * size + 1); + size_t i; + + if(!str) { + return NULL; + } + + for(i = 0; i < size; i++) { + str[2 * i] = hex_digits[bytes[i] >> 4]; + str[2 * i + 1] = hex_digits[bytes[i] & 0x0f]; + } + + str[2 * size] = '\0'; + + return str; +} + /** Parse the basename of a program from a path. * @param path path to parse basename from * diff --git a/src/common/util-common.h b/src/common/util-common.h index 483d5da4..d9b6b4b7 100644 --- a/src/common/util-common.h +++ b/src/common/util-common.h @@ -23,6 +23,7 @@ #include #include /* struct stat */ +char *hex_representation(const unsigned char *bytes, size_t size); const char *mbasename(const char *path); char *mdirname(const char *path); -- 2.29.2

Emil Velikov

10:43 p.m.

New subject: [pacman-dev] [PATCH 3/3] pacman: add file checksum validation against mtree

With libarchive v3.5.0 we have API to fetch the digest from the mtree. Use that to validate if the installed files are modified or not. As always, a modified backup file will trigger a warning but will not result in an actual failure. TODO: localization... no idea how that is even done :-) NOTE: indentation is likely all over the place - first time I see ts=2 Signed-off-by: Emil Velikov --- src/pacman/check.c | 66 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 7 deletions(-) diff --git a/src/pacman/check.c b/src/pacman/check.c index 02217d0f..083c547d 100644 --- a/src/pacman/check.c +++ b/src/pacman/check.c @@ -176,19 +176,70 @@ static int check_file_size(const char *pkgname, const char *filepath, return 0; } -/* placeholders - libarchive currently does not read checksums from mtree files -static int check_file_md5sum(const char *pkgname, const char *filepath, - struct stat *st, struct archive_entry *entry, int backup) +#if ARCHIVE_VERSION_NUMBER >= 3005000 +static int check_file_cksum(const char *pkgname, const char *filepath, + int backup, const char *cksum_name, const char *cksum_calc, const char *cksum_mtree) { + if(!cksum_calc || !cksum_mtree) { + if(!config->quiet) { + pm_printf(ALPM_LOG_WARNING, _("%s: %s (failed to compute %s checksum)\n"), + pkgname, filepath, cksum_name); + } + return 1; + } + + if(strcmp(cksum_calc, cksum_mtree) != 0) { + if(backup) { + if(!config->quiet) { + printf("%s%s%s: ", config->colstr.title, _("backup file"), + config->colstr.nocolor); + printf(_("%s: %s (%s checksum mismatch)\n"), + pkgname, filepath, cksum_name); + } + return 0; + } + if(!config->quiet) { + pm_printf(ALPM_LOG_WARNING, _("%s: %s (%s checksum mismatch)\n"), + pkgname, filepath, cksum_name); + } + return 1; + } + return 0; } +#endif + +static int check_file_md5sum(const char *pkgname, const char *filepath, + struct archive_entry *entry, int backup) +{ + int errors = 0; +#if ARCHIVE_VERSION_NUMBER >= 3005000 + char *cksum_calc = alpm_compute_md5sum(filepath); + char *cksum_mtree = hex_representation(archive_entry_digest(entry, + ARCHIVE_ENTRY_DIGEST_MD5), 16); + errors = check_file_cksum(pkgname, filepath, backup, "MD5", cksum_calc, + cksum_mtree); + free(cksum_mtree); + free(cksum_calc); +#endif + return (errors != 0 ? 1 : 0); +} static int check_file_sha256sum(const char *pkgname, const char *filepath, - struct stat *st, struct archive_entry *entry, int backup) + struct archive_entry *entry, int backup) { - return 0; + int errors = 0; +#if ARCHIVE_VERSION_NUMBER >= 3005000 + char *cksum_calc = alpm_compute_sha256sum(filepath); + char *cksum_mtree = hex_representation(archive_entry_digest(entry, + ARCHIVE_ENTRY_DIGEST_SHA256), 32); + errors = check_file_cksum(pkgname, filepath, backup, "SHA256", cksum_calc, + cksum_mtree); + free(cksum_mtree); + free(cksum_calc); +#endif + return (errors != 0 ? 1 : 0); } -*/ /* Loop through the files of the package to check if they exist. */ int check_pkg_fast(alpm_pkg_t *pkg) @@ -369,7 +420,8 @@ int check_pkg_full(alpm_pkg_t *pkg) if(type == AE_IFREG) { file_errors += check_file_size(pkgname, filepath, &st, entry, backup); - /* file_errors += check_file_md5sum(pkgname, filepath, &st, entry, backup); */ + file_errors += check_file_md5sum(pkgname, filepath, entry, backup); + file_errors += check_file_sha256sum(pkgname, filepath, entry, backup); } if(config->quiet && file_errors) { -- 2.29.2

Allan McRae

29 Dec 29 Dec

3:02 a.m.

New subject: [pacman-dev] [PATCH 3/3] pacman: add file checksum validation against mtree

On 24/12/20 8:43 am, Emil Velikov wrote:

...

With libarchive v3.5.0 we have API to fetch the digest from the mtree. Use that to validate if the installed files are modified or not.

As always, a modified backup file will trigger a warning but will not result in an actual failure.

TODO: localization... no idea how that is even done :-)

Adding the _() around the strings is all that needed done.

...

NOTE: indentation is likely all over the place - first time I see ts=2

For line wraps, we generally just use two indents, so really does not matter what the tab system is.

...

Signed-off-by: Emil Velikov --- src/pacman/check.c | 66 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 7 deletions(-)

diff --git a/src/pacman/check.c b/src/pacman/check.c index 02217d0f..083c547d 100644 --- a/src/pacman/check.c +++ b/src/pacman/check.c @@ -176,19 +176,70 @@ static int check_file_size(const char *pkgname, const char *filepath, return 0; }

-/* placeholders - libarchive currently does not read checksums from mtree files -static int check_file_md5sum(const char *pkgname, const char *filepath, - struct stat *st, struct archive_entry *entry, int backup) +#if ARCHIVE_VERSION_NUMBER >= 3005000

This does not need wrapped in #if. There is nothing libarchive specific in this function.

...

+static int check_file_cksum(const char *pkgname, const char *filepath, + int backup, const char *cksum_name, const char *cksum_calc, const char *cksum_mtree) { + if(!cksum_calc || !cksum_mtree) {

Only one of these failures matches the error message. Split into "checkusm information not available" and "failed to calculate checksum"

...

+ if(!config->quiet) { + pm_printf(ALPM_LOG_WARNING, _("%s: %s (failed to compute %s checksum)\n"), + pkgname, filepath, cksum_name); + } + return 1; + } + + if(strcmp(cksum_calc, cksum_mtree) != 0) { + if(backup) { + if(!config->quiet) { + printf("%s%s%s: ", config->colstr.title, _("backup file"), + config->colstr.nocolor); + printf(_("%s: %s (%s checksum mismatch)\n"), + pkgname, filepath, cksum_name); + } + return 0; + } + if(!config->quiet) { + pm_printf(ALPM_LOG_WARNING, _("%s: %s (%s checksum mismatch)\n"), + pkgname, filepath, cksum_name); + } + return 1;

OK.

...

+ } + return 0; } +#endif + +static int check_file_md5sum(const char *pkgname, const char *filepath, + struct archive_entry *entry, int backup) +{ + int errors = 0; +#if ARCHIVE_VERSION_NUMBER >= 3005000 + char *cksum_calc = alpm_compute_md5sum(filepath); + char *cksum_mtree = hex_representation(archive_entry_digest(entry, + ARCHIVE_ENTRY_DIGEST_MD5), 16); + errors = check_file_cksum(pkgname, filepath, backup, "MD5", cksum_calc, + cksum_mtree); + free(cksum_mtree); + free(cksum_calc); +#endif + return (errors != 0 ? 1 : 0); +}

static int check_file_sha256sum(const char *pkgname, const char *filepath, - struct stat *st, struct archive_entry *entry, int backup) + struct archive_entry *entry, int backup) { - return 0; + int errors = 0; +#if ARCHIVE_VERSION_NUMBER >= 3005000 + char *cksum_calc = alpm_compute_sha256sum(filepath); + char *cksum_mtree = hex_representation(archive_entry_digest(entry, + ARCHIVE_ENTRY_DIGEST_SHA256), 32); + errors = check_file_cksum(pkgname, filepath, backup, "SHA256", cksum_calc, + cksum_mtree); + free(cksum_mtree); + free(cksum_calc); +#endif + return (errors != 0 ? 1 : 0); } -*/

/* Loop through the files of the package to check if they exist. */ int check_pkg_fast(alpm_pkg_t *pkg) @@ -369,7 +420,8 @@ int check_pkg_full(alpm_pkg_t *pkg)

if(type == AE_IFREG) { file_errors += check_file_size(pkgname, filepath, &st, entry, backup); - /* file_errors += check_file_md5sum(pkgname, filepath, &st, entry, backup); */ + file_errors += check_file_md5sum(pkgname, filepath, entry, backup); + file_errors += check_file_sha256sum(pkgname, filepath, entry, backup); }

if(config->quiet && file_errors) {

Emil Velikov

4 Jan 4 Jan

2:04 p.m.

New subject: [pacman-dev] [PATCH 3/3] pacman: add file checksum validation against mtree

On Tue, 29 Dec 2020 at 03:02, Allan McRae wrote:

...

On 24/12/20 8:43 am, Emil Velikov wrote:

...
With libarchive v3.5.0 we have API to fetch the digest from the mtree. Use that to validate if the installed files are modified or not.

As always, a modified backup file will trigger a warning but will not result in an actual failure.

TODO: localization... no idea how that is even done :-)

Adding the _() around the strings is all that needed done.

Perfect thanks.

...

...
NOTE: indentation is likely all over the place - first time I see ts=2

For line wraps, we generally just use two indents, so really does not matter what the tab system is.

Was mostly curious about wrapping of multiline function calls. It seems that "use an extra tab" is common practise - worth adding an example in the Coding style section in HACKING?

...

...
Signed-off-by: Emil Velikov --- src/pacman/check.c | 66 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 7 deletions(-)

diff --git a/src/pacman/check.c b/src/pacman/check.c index 02217d0f..083c547d 100644 --- a/src/pacman/check.c +++ b/src/pacman/check.c @@ -176,19 +176,70 @@ static int check_file_size(const char *pkgname, const char *filepath, return 0; }

-/* placeholders - libarchive currently does not read checksums from mtree files -static int check_file_md5sum(const char *pkgname, const char *filepath, - struct stat *st, struct archive_entry *entry, int backup) +#if ARCHIVE_VERSION_NUMBER >= 3005000

This does not need wrapped in #if. There is nothing libarchive specific in this function.

Without this the compiler will flag it as -Wunused-function. I could silence that by annotating the function as __attribute__((used)). Would you prefer that or you have something else in mind?

...

...
+static int check_file_cksum(const char *pkgname, const char *filepath, + int backup, const char *cksum_name, const char *cksum_calc, const char *cksum_mtree) { + if(!cksum_calc || !cksum_mtree) {

Only one of these failures matches the error message. Split into "checkusm information not available" and "failed to calculate checksum"

Ack will do. Thanks Emil

Allan McRae

11:45 p.m.

New subject: [pacman-dev] [PATCH 3/3] pacman: add file checksum validation against mtree

On 5/1/21 12:04 am, Emil Velikov via pacman-dev wrote:

...

On Tue, 29 Dec 2020 at 03:02, Allan McRae wrote:

...
On 24/12/20 8:43 am, Emil Velikov wrote:

...
With libarchive v3.5.0 we have API to fetch the digest from the mtree. Use that to validate if the installed files are modified or not.

As always, a modified backup file will trigger a warning but will not result in an actual failure.

TODO: localization... no idea how that is even done :-)

Adding the _() around the strings is all that needed done.

Perfect thanks.

...
...
NOTE: indentation is likely all over the place - first time I see ts=2

For line wraps, we generally just use two indents, so really does not matter what the tab system is.

Was mostly curious about wrapping of multiline function calls. It seems that "use an extra tab" is common practise - worth adding an example in the Coding style section in HACKING?

...
...
Signed-off-by: Emil Velikov --- src/pacman/check.c | 66 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 7 deletions(-)

diff --git a/src/pacman/check.c b/src/pacman/check.c index 02217d0f..083c547d 100644 --- a/src/pacman/check.c +++ b/src/pacman/check.c @@ -176,19 +176,70 @@ static int check_file_size(const char *pkgname, const char *filepath, return 0; }

-/* placeholders - libarchive currently does not read checksums from mtree files -static int check_file_md5sum(const char *pkgname, const char *filepath, - struct stat *st, struct archive_entry *entry, int backup) +#if ARCHIVE_VERSION_NUMBER >= 3005000

This does not need wrapped in #if. There is nothing libarchive specific in this function.

Without this the compiler will flag it as -Wunused-function. I could silence that by annotating the function as __attribute__((used)). Would you prefer that or you have something else in mind?

Ah - missed that. Keep the #if then A

...

...
...
+static int check_file_cksum(const char *pkgname, const char *filepath, + int backup, const char *cksum_name, const char *cksum_calc, const char *cksum_mtree) { + if(!cksum_calc || !cksum_mtree) {

Only one of these failures matches the error message. Split into "checkusm information not available" and "failed to calculate checksum"

Ack will do.

Thanks Emil .

Allan McRae

29 Dec 29 Dec

2:44 a.m.

On 24/12/20 8:43 am, Emil Velikov wrote:

...

Pacman has required libarchive 3.0 or later for quite some time mow.

Signed-off-by: Emil Velikov --- lib/libalpm/libarchive-compat.h | 20 -------------------- 1 file changed, 20 deletions(-)

Looks good.

1217

Age (days ago)

1229

Last active (days ago)

List overview

Download

6 comments

2 participants

participants (2)

Allan McRae
Emil Velikov