Re: [Rpm-maint] [rpm-software-management/rpm] verifySignature(): package signatures must be PGPSIGTYPE_BINARY (PR #1801)
@DemiMarie commented on this pull request. Thanks for the feedback @pmatilai. > @@ -426,6 +426,11 @@ static int pgpVersion(const uint8_t *h, size_t hlen, > uint8_t *version) return 0; } +int pgpSignatureType(pgpDigParams _digp) { +assert(_digp->tag == PGPTAG_SIGNATURE); That is a good point I had not considered, thanks. If it is okay with you I can make a PR to add this to `CONTRIBUTING.md` > @@ -566,7 +566,9 @@ static rpmRC verifyDigest(struct rpmsinfo_s *sinfo) static rpmRC verifySignature(rpmKeyring keyring, struct rpmsinfo_s *sinfo) { -rpmRC res = rpmKeyringVerifySig(keyring, sinfo->sig, sinfo->ctx); +rpmRC res = RPMRC_FAIL; +if (sinfo->sig && pgpSignatureType(sinfo->sig) == PGPSIGTYPE_BINARY) Done, thanks! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1801#pullrequestreview-785916855___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] verifySignature(): package signatures must be PGPSIGTYPE_BINARY (PR #1801)
@DemiMarie pushed 1 commit. 17c93c31eceed4cd9a3e78385756f4804c047f50 verifySignature(): package signatures must be PGPSIGTYPE_BINARY -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1801/files/f4521694942624460c7238eb7e61a52f290e35cc..17c93c31eceed4cd9a3e78385756f4804c047f50 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] verifySignature(): package signatures must be PGPSIGTYPE_BINARY (PR #1801)
@pmatilai commented on this pull request. > @@ -566,7 +566,9 @@ static rpmRC verifyDigest(struct rpmsinfo_s *sinfo) static rpmRC verifySignature(rpmKeyring keyring, struct rpmsinfo_s *sinfo) { -rpmRC res = rpmKeyringVerifySig(keyring, sinfo->sig, sinfo->ctx); +rpmRC res = RPMRC_FAIL; +if (sinfo->sig && pgpSignatureType(sinfo->sig) == PGPSIGTYPE_BINARY) See below, handle NULL inside pgpSignatureType() instead which makes it nicer for the user. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1801#pullrequestreview-785335226___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] verifySignature(): package signatures must be PGPSIGTYPE_BINARY (PR #1801)
@pmatilai commented on this pull request. > @@ -426,6 +426,11 @@ static int pgpVersion(const uint8_t *h, size_t hlen, > uint8_t *version) return 0; } +int pgpSignatureType(pgpDigParams _digp) { +assert(_digp->tag == PGPTAG_SIGNATURE); Imagine for a moment what life would be like if the kernel and libc did this: blew up your program each time they get input that isn't just so. Never, ever assert when you can just handle the error. If you want to keep contributing to rpm, please take some time to digest the overall style of how we do things. If you look around the codebase, you'll find that assert() is not used like this here. There are reasons for that. Instead you'll find that for public APIs we also generally check for NULL pointers as a type of EINVAL rather than blow up - in fact this would blow up on NULL pointer dereference *in* the assert. That's not a way to do an interface where you can just trivially handle the error instead, and return `-1` for it. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1801#pullrequestreview-785333297___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] verifySignature(): package signatures must be PGPSIGTYPE_BINARY (PR #1801)
@DemiMarie pushed 1 commit. f4521694942624460c7238eb7e61a52f290e35cc verifySignature(): package signatures must be PGPSIGTYPE_BINARY -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1801/files/9069a4cd36597badd292bf5c6aac3f3ed89e0c73..f4521694942624460c7238eb7e61a52f290e35cc ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint