| Message ID | 20200731165442.34336-1-anatol.pomozov@gmail.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [pacman-dev] Fix error during keyring checking | expand |
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 <anatol.pomozov@gmail.com> > --- > 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); >
Hello On Tue, Aug 4, 2020 at 4:45 AM Allan McRae <allan@archlinux.org> 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 <anatol.pomozov@gmail.com> > > --- > > 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); > >
On 5/8/20 3:16 am, Anatol Pomozov wrote: > Hello > > On Tue, Aug 4, 2020 at 4:45 AM Allan McRae <allan@archlinux.org> 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 <anatol.pomozov@gmail.com> >>> --- >>> 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. > Good point... Patch is good A
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) { - 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);
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 <anatol.pomozov@gmail.com> --- lib/libalpm/package.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)