Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix sinfoCmp to order signatures correctly #3194

Closed
wants to merge 2 commits into from
Closed

Fix sinfoCmp to order signatures correctly #3194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
23a0bea
Fix sinfoCmp to order signatures correctly
ffesti Jul 4, 2024
a242ec7
Report most severe signature issue
ffesti Jul 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/rpmvs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,9 +440,9 @@ static int sinfoCmp(const void *a, const void *b)
rc = sb->type - sa->type;
/* strongest (in the "newer is better" sense) algos first */
if (rc == 0)
rc = sb->sigalgo - sb->sigalgo;
rc = sb->sigalgo - sa->sigalgo;
if (rc == 0)
rc = sb->hashalgo - sb->hashalgo;
rc = sb->hashalgo - sa->hashalgo;
/* last resort, these only makes sense from consistency POV */
if (rc == 0)
rc = sb->id - sa->id;
Expand Down
65 changes: 51 additions & 14 deletions lib/transaction.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1177,38 +1177,75 @@ struct vfydata_s {
int vfylevel;
};

/* order rpmRC codes by severity */
static int rpmRCseverity(rpmRC rc)
{
switch (rc) {
case RPMRC_OK:
return 0;
case RPMRC_NOTFOUND:
return 1;
case RPMRC_NOKEY:
return 2;
case RPMRC_NOTTRUSTED:
return 3;
case RPMRC_FAIL:
return 4;
}
return rc;
}

static int vfyCb(struct rpmsinfo_s *sinfo, void *cbdata)
{
struct vfydata_s *vd = (struct vfydata_s *)cbdata;
int newerror = 0;


int severity = rpmRCseverity(sinfo->rc);

if (sinfo->type & RPMSIG_VERIFIABLE_TYPE && sinfo->rc != RPMRC_NOTFOUND) {
int res = (sinfo->rc != RPMRC_OK);
/* Take care not to override a previous failure with success */
if (res > vd->type[sinfo->type])
vd->type[sinfo->type] = res;
if (severity > vd->type[sinfo->type]) {
vd->type[sinfo->type] = severity;
Copy link
Member

@pmatilai pmatilai Jul 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't. Don't abuse a completely unrelated field when there's no reason to do so, just because you think you might get away with it. It only achieves making fairly complicated logic even more confusing and that's not what you want in security sensitive code. This is an internal struct, if you need a severity field then add it. And since it can then be used to aid sorting these items, maybe it'll all fall into place more naturally that way. For one, using a nicer data structure (now that we have plenty available) might make it all much saner.

This verification callback fubar was written under RHEL deadline pressure six years ago and isn't exactly one of my pridest moments...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ehm, okay so vd->type is something entirely different (a field badly named by me), I first thought this was abusing the data contained in sinfo->type. This is more harmless but also there's nothing saved by avoiding an extra field for the purpose, this code is bad enough as it is 😆

newerror = 1;
}
}

/*
* Legacy compat: if signatures are not required, install must
* succeed despite missing key.
*/
if (sinfo->rc == RPMRC_NOKEY && !(vd->vfylevel & RPMSIG_SIGNATURE_TYPE)) {
sinfo->rc = RPMRC_OK;
severity = rpmRCseverity(sinfo->rc);
newerror = 0;
}

/* Nothing new */
if (!newerror && !(sinfo->rc == RPMRC_NOTFOUND))
return 1;

/* Don't overwrite more important errors */
for (int type=0; type < (sizeof(vd->type)/sizeof(vd->type[0])); type++) {
if ((type != sinfo->type || sinfo->rc == RPMRC_NOTFOUND) && vd->type[type] >= severity) {
return 1;
}
}

switch (sinfo->rc) {
case RPMRC_OK:
break;
case RPMRC_NOTFOUND:
vd->msg = _free(vd->msg);
vd->msg = xstrdup((sinfo->type == RPMSIG_SIGNATURE_TYPE) ?
_("no signature") : _("no digest"));
break;
case RPMRC_NOKEY:
/*
* Legacy compat: if signatures are not required, install must
* succeed despite missing key.
*/
if (!(vd->vfylevel & RPMSIG_SIGNATURE_TYPE))
sinfo->rc = RPMRC_OK;
/* fallthrough */
default:
if (sinfo->rc)
vd->msg = rpmsinfoMsg(sinfo);
vd->msg = _free(vd->msg);
vd->msg = rpmsinfoMsg(sinfo);
break;
}
return (sinfo->rc == 0);
return 1;
}

static int verifyPackageFiles(rpmts ts, rpm_loff_t total)
Expand Down
4 changes: 2 additions & 2 deletions tests/rpmi.at
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,9 @@ error: unpacking of archive failed: cpio: Bad magic
error: hello-2.0-1.x86_64: install failed
INSTALL 3
warning: /tmp/hello-2.0-1.x86_64-signed.rpm: Header V4 RSA/SHA256 Signature, key ID 1964c5fc: NOKEY
package hello-2.0-1.x86_64 does not verify: Header V4 RSA/SHA256 Signature, key ID 1964c5fc: NOKEY
package hello-2.0-1.x86_64 does not verify: Payload SHA256 digest: BAD (Expected 84a7338287bf19715c4eed0243f5cdb447eeb0ade37b2af718d4060aefca2f7c != 3129d507d00b1dc60745d9637010b5d82059ebeff2318b2db75b26272b823586)
INSTALL 4
package hello-2.0-1.x86_64 does not verify: no signature
package hello-2.0-1.x86_64 does not verify: Payload SHA256 digest: BAD (Expected 84a7338287bf19715c4eed0243f5cdb447eeb0ade37b2af718d4060aefca2f7c != 3129d507d00b1dc60745d9637010b5d82059ebeff2318b2db75b26272b823586)
INSTALL 5
warning: /tmp/hello-2.0-1.x86_64-signed.rpm: Header V4 RSA/SHA256 Signature, key ID 1964c5fc: NOKEY
error: unpacking of archive failed: cpio: Bad magic
Expand Down
34 changes: 17 additions & 17 deletions tests/rpmsigdig.at
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,13 +308,13 @@ runroot rpmkeys --define '_pkgverify_level all' -Kv --nosignature /data/RPMS/hel
[0],
[[Checking package before importing key:
/data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm:
Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOKEY
Header DSA signature: NOTFOUND
Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOKEY
Header SHA256 digest: OK
Header SHA1 digest: OK
Payload SHA256 digest: OK
RSA signature: NOTFOUND
DSA signature: NOTFOUND
RSA signature: NOTFOUND
MD5 digest: OK
1
Importing key:
Expand All @@ -334,8 +334,8 @@ Checking package after importing key, no digest:
Header V4 RSA/SHA512 Signature, key ID 15217ee0: OK
Payload SHA256 digest: NOTFOUND
Payload SHA256 ALT digest: NOTFOUND
RSA signature: NOTFOUND
DSA signature: NOTFOUND
RSA signature: NOTFOUND
1
Checking package after importing key, no signature:
/data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm:
Expand Down Expand Up @@ -372,13 +372,13 @@ runroot rpmkeys --define '_pkgverify_level all' -Kv --nosignature /data/RPMS/hel
[0],
[Checking package before importing key:
/data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm:
Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOKEY
Header DSA signature: NOTFOUND
Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOKEY
Header SHA256 digest: OK
Header SHA1 digest: OK
Payload SHA256 digest: OK
RSA signature: NOTFOUND
DSA signature: NOTFOUND
RSA signature: NOTFOUND
MD5 digest: OK
1
Importing key:
Expand All @@ -392,13 +392,13 @@ RPMOUTPUT_SEQUOIA([error: Verifying a signature using certificate B6542F92F30650
RPMOUTPUT_SEQUOIA([ Key 1F71177215217EE0 invalid: key is not alive])dnl
RPMOUTPUT_SEQUOIA([ because: The subkey is not live])dnl
RPMOUTPUT_SEQUOIA([ because: Expired on 2022-04-12T00:00:15Z])dnl
Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOTTRUSTED
Header DSA signature: NOTFOUND
Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOTTRUSTED
Header SHA256 digest: OK
Header SHA1 digest: OK
Payload SHA256 digest: OK
RSA signature: NOTFOUND
DSA signature: NOTFOUND
RSA signature: NOTFOUND
MD5 digest: OK
1
Checking package after importing key, no digest:
Expand All @@ -408,10 +408,10 @@ RPMOUTPUT_SEQUOIA([error: Verifying a signature using certificate B6542F92F30650
RPMOUTPUT_SEQUOIA([ Key 1F71177215217EE0 invalid: key is not alive])dnl
RPMOUTPUT_SEQUOIA([ because: The subkey is not live])dnl
RPMOUTPUT_SEQUOIA([ because: Expired on 2022-04-12T00:00:15Z])dnl
Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOTTRUSTED
Header DSA signature: NOTFOUND
RSA signature: NOTFOUND
Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOTTRUSTED
DSA signature: NOTFOUND
RSA signature: NOTFOUND
1
Checking package after importing key, no signature:
/data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm:
Expand Down Expand Up @@ -448,13 +448,13 @@ runroot rpmkeys --define '_pkgverify_level all' -Kv --nosignature /data/RPMS/hel
[0],
[Checking package before importing key:
/data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm:
Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOKEY
Header DSA signature: NOTFOUND
Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOKEY
Header SHA256 digest: OK
Header SHA1 digest: OK
Payload SHA256 digest: OK
RSA signature: NOTFOUND
DSA signature: NOTFOUND
RSA signature: NOTFOUND
MD5 digest: OK
1
Importing key:
Expand All @@ -466,24 +466,24 @@ Checking package after importing key:
RPMOUTPUT_LEGACY([error: Subkey 1f71177215217ee0 of key b3a771bfeb04e625 (Alice <[email protected]>) has been revoked])dnl
RPMOUTPUT_SEQUOIA([error: Verifying a signature using certificate B6542F92F30650C36B6F41BCB3A771BFEB04E625 (Alice <[email protected]>):])dnl
RPMOUTPUT_SEQUOIA([ Key 1F71177215217EE0 is invalid: key is revoked])dnl
Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOTTRUSTED
Header DSA signature: NOTFOUND
Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOTTRUSTED
Header SHA256 digest: OK
Header SHA1 digest: OK
Payload SHA256 digest: OK
RSA signature: NOTFOUND
DSA signature: NOTFOUND
RSA signature: NOTFOUND
MD5 digest: OK
1
Checking package after importing key, no digest:
/data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm:
RPMOUTPUT_LEGACY([error: Subkey 1f71177215217ee0 of key b3a771bfeb04e625 (Alice <[email protected]>) has been revoked])dnl
RPMOUTPUT_SEQUOIA([error: Verifying a signature using certificate B6542F92F30650C36B6F41BCB3A771BFEB04E625 (Alice <[email protected]>):])dnl
RPMOUTPUT_SEQUOIA([ Key 1F71177215217EE0 is invalid: key is revoked])dnl
Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOTTRUSTED
Header DSA signature: NOTFOUND
RSA signature: NOTFOUND
Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOTTRUSTED
DSA signature: NOTFOUND
RSA signature: NOTFOUND
1
Checking package after importing key, no signature:
/data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm:
Expand Down Expand Up @@ -864,8 +864,8 @@ runroot rpmkeys -Kv /tmp/${pkg}
Header SHA1 digest: OK
Payload SHA256 digest: BAD (Expected 84a7338287bf19715c4eed0243f5cdb447eeb0ade37b2af718d4060aefca2f7c != bea903609dceac36e1f26a983c493c98064d320fdfeb423034ed63d649b2c8dc)
Payload SHA256 ALT digest: NOTFOUND
V4 RSA/SHA256 Signature, key ID 1964c5fc: BAD
DSA signature: NOTFOUND
V4 RSA/SHA256 Signature, key ID 1964c5fc: BAD
MD5 digest: BAD (Expected 137ca1d8b35cca02a1854ba301c5432e != d662cd0d81601a7107312684ad1ddf38)
],
[])
Expand Down Expand Up @@ -904,8 +904,8 @@ dorpm -Kv
Header SHA256 digest: OK
Payload SHA256 digest: NOTFOUND
Payload SHA256 ALT digest: NOTFOUND
RSA signature: NOTFOUND
DSA signature: NOTFOUND
RSA signature: NOTFOUND
MD5 digest: OK
]],
[])
Expand Down
6 changes: 3 additions & 3 deletions tests/rpmvfylevel.at
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,8 @@ noplds
Header SHA1 digest: OK
Payload SHA256 digest: NOTFOUND
Payload SHA256 ALT digest: NOTFOUND
RSA signature: NOTFOUND
DSA signature: NOTFOUND
RSA signature: NOTFOUND
MD5 digest: OK
1
nohdrs
Expand All @@ -346,13 +346,13 @@ nohdrs
0
nosig
/data/RPMS/hello-2.0-1.x86_64-signed.rpm:
Header RSA signature: NOTFOUND
Header DSA signature: NOTFOUND
Header RSA signature: NOTFOUND
Header SHA256 digest: OK
Header SHA1 digest: OK
Payload SHA256 digest: OK
RSA signature: NOTFOUND
DSA signature: NOTFOUND
RSA signature: NOTFOUND
MD5 digest: OK
1
],
Expand Down
Loading