Ignore NOTTRUSTED results in verification #4020
Conversation
|
One minimalistic approach for a better output could be changing the logging in rpmKeyringVerifySig2() to emit NOTTRUSTED results as RPMLOG_DEBUG by default and RPMLOG_WARNING when verbose. But this is far from optimal, as it spreads the semantics into multiple places in odd ways. I guess the real solution is going to be moving any logging out of the verification function, it cannot possibly know what the caller wants to do with that info. But them I'm loathe to add rpmKeyringVerifySig3() for this. |
| # EDDSA disabled | ||
| RPMTEST_CHECK([ | ||
| runroot cp -f /tmp/rpm-sequoia.config /etc/crypto-policies/back-ends/ | ||
| runroot sed -i '/^cv25519/s/always/never/g' /etc/crypto-policies/back-ends/rpm-sequoia.config |
There was a problem hiding this comment.
I think this should be ed25519. The cv25519 is key exchange/derive mechanism used in encryption:
| runroot sed -i '/^cv25519/s/always/never/g' /etc/crypto-policies/back-ends/rpm-sequoia.config | |
| runroot sed -i '/^ed25519/s/always/never/g' /etc/crypto-policies/back-ends/rpm-sequoia.config |
There was a problem hiding this comment.
This breaks the test though, the obvious looking thing does not actually disable the algorithm that rpm reports as EdDSA/SHA512. I realize now I did see this apparent mismatch while adding the test, but didn't really stop to listen the distant bell going off in my head.
There was a problem hiding this comment.
So it smells like a bug someplace...
There was a problem hiding this comment.
This is all Greek to me, but https://gitlab.com/sequoia-pgp/sequoia/-/blob/main/openpgp/src/policy.rs?ref_type=heads#L1574 looks suspicious:
[...]
Curve::Ed25519 => Cv25519,
Curve::Cv25519 => Cv25519,
[...]
There was a problem hiding this comment.
@nwalfield @teythoon - this looks to me like a bug in the sequoia stack someplace: disabling cv25519 in the rpm-sequoia policy ends up disabling ed25519 instead, whereas disabling ed25519 does not do so.
This also revealed some untested error returns because 'sed' was masking the actual rpm command return. Make the output predictable instead of trying to filter out, and let stdout/stderr go where they go.
This is wrong, NOTTRUSTED results shouldn't fail it
Shouldn't change behavior in itself, but makes it a little bit safer in principle and needed for the next step.
|
The work here will also let us fix #1057 finally, but I'll leave that to a separate PR because it's quite a noisy patch. |
|
Oh and, this includes #4033, so it's best to review + merge that before diving into this. |
We have currently no way of differentiating between signatures whose algorithm is disabled in system policy and ones that are otherwise no longer trustworthy, eg due to expiration. The peculiar thing about these cases is that they are neither failures nor are they positive verifications. It seems they are best dealt with by simply ignoring, except for the verbose output where they need to be shown for diagnostic purposes. This pointed out multiple flaws in the verification machinery which was still rather attuned to one signature per package, and making assumptions that never were really valid, but completely broken now. We simply can't make decisions after any single item in the callback, we need to process the whole lot to make sense of it, and we need to rank our failure codes to figure the most relevant for the final return. Also, we need to honor the configured verify level in the transaction verification step too. One of the flaws found by this was that a V4 signature NOKEY result was overriding a FAIL result from V3 signature. Adjust the original test for to only check for V4 signatures, and add another test exhibiting the new behavior where the failure shows up more prominently. Fixes: rpm-software-management#3996
|
Apparently getting late in the day 😑 Anyway, flagging this for review now to avoid this becoming too big. The verify logic seems to do what intended now, and that's the topic here. The output spew from disabled algorithms is a bit much, but lets deal with that in a separate PR. |
2 participants
We have currently no way of differentiating between signatures whose algorithm is disabled in system policy and ones that are otherwise no longer trustworthy, eg due to expiration. The peculiar thing about these cases is that they are neither failures nor are they positive verifications. It seems they are best dealt with by simply ignoring, except for the verbose output where they need to be shown for diagnostic purposes.
This fixes the verification result wrt #3996 but the output is just wrong as these spew a bunch of noisy errors on stderr, which is good for troubleshooting but not something that you'd want to see in these situations. So this is not a full fix for that issue but posting as draft for comments/feedback/ideas.