[pacman-dev] [GIT] The official pacman repository branch, master, updated. v5.2.1-87-g5f6ef895

2020-05-31 Thread Allan McRae
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

2020-05-31 Thread Allan McRae
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

2020-05-31 Thread Andrew Gregory
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

2020-05-31 Thread Andrew Gregory
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

2020-05-31 Thread Anatol Pomozov
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