[pacman-dev] [GIT] The official pacman repository branch, master, updated. v5.2.1-87-g5f6ef895
This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "The official pacman repository". The branch, master has been updated via 5f6ef895b1dac04c7fb1b63acab2d42c19f91922 (commit) via 23b50d60e34e324cf6f420c05293f7fa8a909623 (commit) via 3674144a74cfe897ec3ff46c18681df293290caa (commit) via 454ea024383eab60295e4c4fdf2c329475887b2c (commit) from 8ce142a2552418f64a74e773f659d92b065d6209 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log - commit 5f6ef895b1dac04c7fb1b63acab2d42c19f91922 Author: Allan McRae Date: Wed May 20 14:17:11 2020 +1000 libalpm/signing.c: Fix calculation of packet size in parse_subpacket Given RFC 4880 provides the code to do this calculation, I am not sure how I managed to stuff that up! This bug was only exposed when a signature made with "include-key-block" was added to the Arch repos, which provided a subpacket with the required size to hit this issue. Signed-off-by: Allan McRae commit 23b50d60e34e324cf6f420c05293f7fa8a909623 Author: Dave Reisner Date: Wed May 13 14:43:53 2020 -0400 Avoid depending on side effects in assert(...) expressions When building with -DNDEBUG, assert statements are compiled out to no-ops. Thus, we can't depend on assignments or other computations occurring inside the assert(). Signed-off-by: Allan McRae commit 3674144a74cfe897ec3ff46c18681df293290caa Author: Eli Schwartz Date: Mon May 11 00:16:30 2020 -0400 libmakepkg/strip: don't re-add the same debug source multiple times It's either a waste of work, or triggers edge cases in some packages (like coreutils-8.31) where the source file is readonly and cp gets a permission denied error trying to overwrite it with an identical copy of itself. Also while we are at it, make the variable names be something readable, because I could barely tell what this was doing while editing it. Signed-off-by: Eli Schwartz Signed-off-by: Allan McRae commit 454ea024383eab60295e4c4fdf2c329475887b2c Author: Allan McRae Date: Mon May 11 12:59:51 2020 +1000 Remove autotools support This removes support for autotools in favour of meson. --- Summary of changes: .gitignore | 21 -- .gitlab-ci.yml | 17 -- HACKING | 13 - INSTALL | 370 -- Makefile.am | 78 - RELEASE | 9 +- configure.ac| 594 doc/Makefile.am | 162 -- doc/index.asciidoc | 8 +- etc/Makefile.am | 27 -- lib/libalpm/Makefile.am | 78 - lib/libalpm/signing.c | 2 +- scripts/Makefile.am | 287 - scripts/libmakepkg/tidy/strip.sh.in | 11 +- src/common/Makefile.am | 3 - src/pacman/Makefile.am | 69 - src/pacman/callback.c | 8 +- src/util/Makefile.am| 27 -- test/pacman/Makefile.am | 22 -- test/pacman/tests/Makefile.am | 5 - test/scripts/Makefile.am| 10 - test/util/Makefile.am | 8 - 22 files changed, 22 insertions(+), 1807 deletions(-) delete mode 100644 INSTALL delete mode 100644 Makefile.am delete mode 100644 configure.ac delete mode 100644 doc/Makefile.am delete mode 100644 etc/Makefile.am delete mode 100644 lib/libalpm/Makefile.am delete mode 100644 scripts/Makefile.am delete mode 100644 src/common/Makefile.am delete mode 100644 src/pacman/Makefile.am delete mode 100644 src/util/Makefile.am delete mode 100644 test/pacman/Makefile.am delete mode 100644 test/pacman/tests/Makefile.am delete mode 100644 test/scripts/Makefile.am delete mode 100644 test/util/Makefile.am hooks/post-receive -- The official pacman repository
Re: [pacman-dev] [PATCH] libalpm/signing.c: Fix calculation of packet size in parse_subpacket
On 1/6/20 5:51 am, Andrew Gregory wrote: > On 05/20/20 at 02:22pm, Allan McRae wrote: >> Given RFC 4880 provides the code to do this calculation, I am not sure >> how I managed to stuff that up! This bug was only exposed when a >> signature made with "include-key-block" was added to the Arch repos, >> which provided a subpacket with the required size to hit this issue. > > LGTM. Though, it might be worth it to use + instead of | just so we > match 4880 and extract_keyid exactly. > Done.
Re: [pacman-dev] [PATCH] libalpm/signing.c: Fix calculation of packet size in parse_subpacket
On 05/20/20 at 02:22pm, Allan McRae wrote:
> Given RFC 4880 provides the code to do this calculation, I am not sure
> how I managed to stuff that up! This bug was only exposed when a
> signature made with "include-key-block" was added to the Arch repos,
> which provided a subpacket with the required size to hit this issue.
LGTM. Though, it might be worth it to use + instead of | just so we
match 4880 and extract_keyid exactly.
> Signed-off-by: Allan McRae
> ---
>
> Also appropriate for 5.2.2
>
> lib/libalpm/signing.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c
> index c8eaaca2..422523b6 100644
> --- a/lib/libalpm/signing.c
> +++ b/lib/libalpm/signing.c
> @@ -1058,7 +1058,7 @@ static int parse_subpacket(alpm_handle_t *handle, const
> char *identifier,
> if(length_check(len, spos, 2, handle,
> identifier) != 0){
> return -1;
> }
> - slen = (sig[spos] << 8) | sig[spos + 1];
> + slen = (((sig[spos] - 192) << 8) | sig[spos +
> 1]) + 192;
> spos = spos + 2;
> } else {
> if(length_check(len, spos, 5, handle,
> identifier) != 0) {
> --
> 2.26.2
Re: [pacman-dev] [PATCH v2] Fallback to detached signatures during keyring check
On 05/31/20 at 01:37am, Anatol Pomozov wrote:
> Hi Andrew, thank you for the quick response
>
> On Sat, May 30, 2020 at 9:31 PM Andrew Gregory
> wrote:
> >
> > On 05/30/20 at 07:51pm, Anatol Pomozov wrote:
> > > Pacman has a 'key in keyring' verification step that makes sure the
> > > signatures
> > > have a valid keyid. Currently pacman parses embedded package signatures
> > > only.
> > >
> > > Add a fallback to detached signatures. If embedded signature is missing
> > > then it
> > > tries to read corresponding *.sig file and get keyid from there.
> > >
> > > Verification:
> > > debug: found cached pkg:
> > > /var/cache/pacman/pkg/glib-networking-2.64.3-1-x86_64.pkg.tar.zst
> > > debug: found detached signature
> > > /var/cache/pacman/pkg/glib-networking-2.64.3-1-x86_64.pkg.tar.zst.sig
> > > with size 310
> > > debug: found signature key: A5E9288C4FA415FA
> > > debug: looking up key A5E9288C4FA415FA locally
> > > debug: key lookup success, key exists
> > >
> > > Signed-off-by: Anatol Pomozov
> > > ---
> > > lib/libalpm/alpm.h| 10 ++
> > > lib/libalpm/package.c | 31 +++
> > > lib/libalpm/sync.c| 17 -
> > > lib/libalpm/util.c| 35 +++
> > > lib/libalpm/util.h| 1 +
> > > 5 files changed, 85 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> > > index c6a13273..9c5fff73 100644
> > > --- a/lib/libalpm/alpm.h
> > > +++ b/lib/libalpm/alpm.h
> > > @@ -1403,6 +1403,16 @@ alpm_db_t *alpm_pkg_get_db(alpm_pkg_t *pkg);
> > > */
> > > const char *alpm_pkg_get_base64_sig(alpm_pkg_t *pkg);
> > >
> > > +/** Extracts package signature either from embedded package signature
> > > + * or if it is absent then reads data from detached signature file.
> > > + * @param pkg a pointer to package
> > > + * @param sig output parameter for signature data. Callee function
> > > allocates
> > > + * buffer needed for the signature data. Caller is responsible for
> > > + * freeing this buffer.
> > > + * @return size of a signature or negative number if error.
> > > + */
> > > +int alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig);
> > > +
> > > /** Returns the method used to validate a package during install.
> > > * @param pkg a pointer to package
> > > * @return an enum member giving the validation method
> > > diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
> > > index 5c5fa073..e0e4d987 100644
> > > --- a/lib/libalpm/package.c
> > > +++ b/lib/libalpm/package.c
> > > @@ -268,6 +268,37 @@ const char SYMEXPORT
> > > *alpm_pkg_get_base64_sig(alpm_pkg_t *pkg)
> > > return pkg->base64_sig;
> > > }
> > >
> > > +int SYMEXPORT alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig)
> >
> > This API is weird, why are you returning an int when neither of the
> > successful values are int and can potentially overflow an int? You're
> > returning the length of an allocated string, size_t is the appropriate
> > type.
>
> size_t is unsigned int. But this function returns positive length in
> case of successful signature read or negative value in case of error.
> Thus return value cannot be size_t.
I get that, but you chose that interface. Why? Surely it makes more
sense to either return 0 on error, since you treat a missing sig as an
error anyway, or take a size_t* arg like decode_signature.
> >
> > > +{
> > > + ASSERT(pkg != NULL, return -1);
> > > + pkg->handle->pm_errno = ALPM_ERR_OK;
> >
> > I don't like clearing pm_errno without a good reason. It generally
> > doesn't serve any purpose and can get us into trouble if a function
> > gets called during cleanup somewhere.
>
> I personally do not feel that "pm_errno = ALPM_ERR_OK" is useful here
> and I am more than happy to remove it.
>
> But I also I see this pattern is used a lot. Only this file
> (lib/libalpm/package.c) has like 20 use-cases of it. e.g.
>
> off_t SYMEXPORT alpm_pkg_get_isize(alpm_pkg_t *pkg)
> {
> ASSERT(pkg != NULL, return -1);
> pkg->handle->pm_errno = ALPM_ERR_OK;
> return pkg->ops->get_isize(pkg);
> }
>
> So I was trying to follow a standard practice here. Or are you trying
> to say that lib/libalpm/package.c examples I was looking at are
> incorrect?
I consider this an unfortunate anti-pattern. It has bitten us in the
past by clearing pm_errno during cleanup just like I said. I haven't
been motivated enough to go through and remove it so far, but I don't
want to add any more instances of it.
> > > + if(pkg->base64_sig) {
> > > + size_t sig_len;
> > > + int ret = alpm_decode_signature(pkg->base64_sig, sig,
> > > &sig_len);
> > > + return ret == 0 ? (int)sig_len : -1;
> > > + } else {
> > > + char *pkgpath, *sigpath;
> > > + int len;
> > > +
> > > + pkgpath = _alpm_filecache_find(pkg->handle, pkg->filename);
> > > + if(!pkgpath) {
> > >
Re: [pacman-dev] [PATCH v2] Fallback to detached signatures during keyring check
Hi Andrew, thank you for the quick response
On Sat, May 30, 2020 at 9:31 PM Andrew Gregory
wrote:
>
> On 05/30/20 at 07:51pm, Anatol Pomozov wrote:
> > Pacman has a 'key in keyring' verification step that makes sure the
> > signatures
> > have a valid keyid. Currently pacman parses embedded package signatures
> > only.
> >
> > Add a fallback to detached signatures. If embedded signature is missing
> > then it
> > tries to read corresponding *.sig file and get keyid from there.
> >
> > Verification:
> > debug: found cached pkg:
> > /var/cache/pacman/pkg/glib-networking-2.64.3-1-x86_64.pkg.tar.zst
> > debug: found detached signature
> > /var/cache/pacman/pkg/glib-networking-2.64.3-1-x86_64.pkg.tar.zst.sig with
> > size 310
> > debug: found signature key: A5E9288C4FA415FA
> > debug: looking up key A5E9288C4FA415FA locally
> > debug: key lookup success, key exists
> >
> > Signed-off-by: Anatol Pomozov
> > ---
> > lib/libalpm/alpm.h| 10 ++
> > lib/libalpm/package.c | 31 +++
> > lib/libalpm/sync.c| 17 -
> > lib/libalpm/util.c| 35 +++
> > lib/libalpm/util.h| 1 +
> > 5 files changed, 85 insertions(+), 9 deletions(-)
> >
> > diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> > index c6a13273..9c5fff73 100644
> > --- a/lib/libalpm/alpm.h
> > +++ b/lib/libalpm/alpm.h
> > @@ -1403,6 +1403,16 @@ alpm_db_t *alpm_pkg_get_db(alpm_pkg_t *pkg);
> > */
> > const char *alpm_pkg_get_base64_sig(alpm_pkg_t *pkg);
> >
> > +/** Extracts package signature either from embedded package signature
> > + * or if it is absent then reads data from detached signature file.
> > + * @param pkg a pointer to package
> > + * @param sig output parameter for signature data. Callee function
> > allocates
> > + * buffer needed for the signature data. Caller is responsible for
> > + * freeing this buffer.
> > + * @return size of a signature or negative number if error.
> > + */
> > +int alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig);
> > +
> > /** Returns the method used to validate a package during install.
> > * @param pkg a pointer to package
> > * @return an enum member giving the validation method
> > diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
> > index 5c5fa073..e0e4d987 100644
> > --- a/lib/libalpm/package.c
> > +++ b/lib/libalpm/package.c
> > @@ -268,6 +268,37 @@ const char SYMEXPORT
> > *alpm_pkg_get_base64_sig(alpm_pkg_t *pkg)
> > return pkg->base64_sig;
> > }
> >
> > +int SYMEXPORT alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig)
>
> This API is weird, why are you returning an int when neither of the
> successful values are int and can potentially overflow an int? You're
> returning the length of an allocated string, size_t is the appropriate
> type.
size_t is unsigned int. But this function returns positive length in
case of successful signature read or negative value in case of error.
Thus return value cannot be size_t.
>
> > +{
> > + ASSERT(pkg != NULL, return -1);
> > + pkg->handle->pm_errno = ALPM_ERR_OK;
>
> I don't like clearing pm_errno without a good reason. It generally
> doesn't serve any purpose and can get us into trouble if a function
> gets called during cleanup somewhere.
I personally do not feel that "pm_errno = ALPM_ERR_OK" is useful here
and I am more than happy to remove it.
But I also I see this pattern is used a lot. Only this file
(lib/libalpm/package.c) has like 20 use-cases of it. e.g.
off_t SYMEXPORT alpm_pkg_get_isize(alpm_pkg_t *pkg)
{
ASSERT(pkg != NULL, return -1);
pkg->handle->pm_errno = ALPM_ERR_OK;
return pkg->ops->get_isize(pkg);
}
So I was trying to follow a standard practice here. Or are you trying
to say that lib/libalpm/package.c examples I was looking at are
incorrect?
>
> > + if(pkg->base64_sig) {
> > + size_t sig_len;
> > + int ret = alpm_decode_signature(pkg->base64_sig, sig,
> > &sig_len);
> > + return ret == 0 ? (int)sig_len : -1;
> > + } else {
> > + char *pkgpath, *sigpath;
> > + int len;
> > +
> > + pkgpath = _alpm_filecache_find(pkg->handle, pkg->filename);
> > + if(!pkgpath) {
> > + RET_ERR(pkg->handle, ALPM_ERR_PKG_NOT_FOUND, -1);
> > + }
> > + sigpath = _alpm_sigpath(pkg->handle, pkgpath);
> > + if(!sigpath || _alpm_access(pkg->handle, NULL, sigpath,
> > R_OK)) {
> > + FREE(pkgpath);
> > + FREE(sigpath);
> > + RET_ERR(pkg->handle, ALPM_ERR_SIG_MISSING, -1);
> > + }
> > + len = _alpm_read_file(sigpath, sig);
>
> You need to check for and handle failure.
Do you mean to handle a file read failure? But both successful and
erroneous codepaths are the same - free() the resources and return
"len".
> > + _alpm_log(pkg->handle, ALPM_LOG_DEB
