Re: [pacman-dev] [PATCH] Fix error during keyring checking

2020-08-04 Thread Anatol Pomozov
Hello

On Tue, Aug 4, 2020 at 4:45 AM Allan McRae  wrote:
>
> On 1/8/20 2:54 am, Anatol Pomozov wrote:
> > With current master version the 'keyring checking' step produces an error:
> >   debug: returning error 6 from alpm_pkg_get_sig (../lib/libalpm/package.c: 
> > 274) : wrong or NULL argument passed
> >
> > The package signature is still checked later at the integrity verification 
> > step though.
> >
> > This commit fixes keyring checking and now the debug log looks like this:
> >   debug: found cached pkg: 
> > /var/cache/pacman/pkg/ruby-2.7.1-2-x86_64.pkg.tar.zst
> >   debug: found detached signature 
> > /var/cache/pacman/pkg/ruby-2.7.1-2-x86_64.pkg.tar.zst.sig with size 566
> >   debug: found signature key: 786C63F330D7CB92
> >   debug: looking up key 786C63F330D7CB92 locally
> >   debug: key lookup success, key exists
> >
> > Signed-off-by: Anatol Pomozov 
> > ---
> >  lib/libalpm/package.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
> > index 0885b27b..a4356518 100644
> > --- a/lib/libalpm/package.c
> > +++ b/lib/libalpm/package.c
> > @@ -270,9 +270,7 @@ const char SYMEXPORT 
> > *alpm_pkg_get_base64_sig(alpm_pkg_t *pkg)
> >
> >  int SYMEXPORT alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig, 
> > size_t *sig_len)
> >  {
> > - if(pkg != NULL) {
>
> Surely the fix is to change != to == there.  That way we still get some
> form of error handling and not an abort.

If "pkg == NULL" then the statement "pkg->handle" below is invalid and
we cannot set the error code to the handle. The best thing we can do
in case of an absent package is to return "-1" the same way as done in
the functions above.

>
> > - RET_ERR(pkg->handle, ALPM_ERR_WRONG_ARGS, -1);
> > - }
> > + ASSERT(pkg != NULL, return -1);
> >
> >   if(pkg->base64_sig) {
> >   int ret = alpm_decode_signature(pkg->base64_sig, sig, 
> > sig_len);
> >


Re: [pacman-dev] [PATCH] Fix error during keyring checking

2020-08-04 Thread Allan McRae
On 1/8/20 2:54 am, Anatol Pomozov wrote:
> With current master version the 'keyring checking' step produces an error:
>   debug: returning error 6 from alpm_pkg_get_sig (../lib/libalpm/package.c: 
> 274) : wrong or NULL argument passed
> 
> The package signature is still checked later at the integrity verification 
> step though.
> 
> This commit fixes keyring checking and now the debug log looks like this:
>   debug: found cached pkg: 
> /var/cache/pacman/pkg/ruby-2.7.1-2-x86_64.pkg.tar.zst
>   debug: found detached signature 
> /var/cache/pacman/pkg/ruby-2.7.1-2-x86_64.pkg.tar.zst.sig with size 566
>   debug: found signature key: 786C63F330D7CB92
>   debug: looking up key 786C63F330D7CB92 locally
>   debug: key lookup success, key exists
> 
> Signed-off-by: Anatol Pomozov 
> ---
>  lib/libalpm/package.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
> index 0885b27b..a4356518 100644
> --- a/lib/libalpm/package.c
> +++ b/lib/libalpm/package.c
> @@ -270,9 +270,7 @@ const char SYMEXPORT *alpm_pkg_get_base64_sig(alpm_pkg_t 
> *pkg)
>  
>  int SYMEXPORT alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig, size_t 
> *sig_len)
>  {
> - if(pkg != NULL) {

Surely the fix is to change != to == there.  That way we still get some
form of error handling and not an abort.

> - RET_ERR(pkg->handle, ALPM_ERR_WRONG_ARGS, -1);
> - }
> + ASSERT(pkg != NULL, return -1);
>  
>   if(pkg->base64_sig) {
>   int ret = alpm_decode_signature(pkg->base64_sig, sig, sig_len);
>