Re: [Rpm-maint] [rpm-software-management/rpm] verifySignature(): package signatures must be PGPSIGTYPE_BINARY (PR #1801)

2021-10-21 Thread Demi Marie Obenour
@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)

2021-10-21 Thread Demi Marie Obenour
@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)

2021-10-21 Thread Panu Matilainen
@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)

2021-10-21 Thread Panu Matilainen
@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)

2021-10-19 Thread Demi Marie Obenour
@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