RFC: Use rpmkeys alone to verify the signature #1753
Conversation
|
This pull request introduces 3 alerts when merging ac20a40 into b3d2700 - view on LGTM.com new alerts:
|
|
Hello @DemiMarie! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2021-06-02 17:44:10 UTC |
DemiMarie
force-pushed
the
improved-sig-check
branch
from
afbbc17
to
41ac6c0
Compare
|
This pull request introduces 3 alerts when merging 41ac6c0 into b3d2700 - view on LGTM.com new alerts:
|
|
I filed rpm-software-management/ci-dnf-stack#983 for the CI failure. |
DemiMarie
force-pushed
the
improved-sig-check
branch
from
52a0815
to
2ca0d3f
Compare
DemiMarie
force-pushed
the
improved-sig-check
branch
4 times, most recently
from
4fcfbc8
to
10c309e
Compare
DemiMarie
force-pushed
the
improved-sig-check
branch
2 times, most recently
from
eed8599
to
23c48cc
Compare
DemiMarie
force-pushed
the
improved-sig-check
branch
4 times, most recently
from
895a8a3
to
8d2b69f
Compare
DemiMarie
force-pushed
the
improved-sig-check
branch
from
8d2b69f
to
1ac4fa4
Compare
|
@DemiMarie what is the status here? |
This PR should be ready for production use, but I would like a review first.
No, it is for consistency with the previous version. Changing |
I think the only thing we need is to fix the logger I mentioned in #1753 (comment). |
See my comment w.r.t. threading. |
DemiMarie
force-pushed
the
improved-sig-check
branch
from
1ac4fa4
to
20cbc7e
Compare
This avoids having to actually parse the package to check its signature, which reduces attack surface. If the output of rpmkeys cannot be parsed, we assume the package is corrupt (the most likely cause).
DemiMarie
force-pushed
the
improved-sig-check
branch
from
20cbc7e
to
e40e51a
Compare
You’re welcome! |
| # "--define=_pkgverify_flags 0x0" ensures that all signatures and digests | ||
| # are checked. | ||
| args = ('rpmkeys', '--checksig', '--root', installroot, '--verbose', | ||
| '--define=_pkgverify_level all', '--define=_pkgverify_flags 0x0', |
There was a problem hiding this comment.
_pkgverify_level all is wrong for this purpose. This is about checking signatures, and pulling digests into the picture WILL cause breakage of certain use-cases in FIPS mode.
There was a problem hiding this comment.
Right. I made #1775 for this.
There was a problem hiding this comment.
That belongs as a separate change, which is why I reverted it.
|
As a side-effect of this, there will be an extra rpmdb open + read for each package to be checked. It may not be an actual issue, but it's quite a bit of extra churn added and people should at least be aware of it. Using an external process will also affect some distro upgrade scenarios, but as long as its only ever used before starting a transaction, it's probably not much of an issue. |
We don't want to be veryfing digests as well when checking signatures. It would break legacy package installation in FIPS mode due to MD5 digest being unverifiable (see https://access.redhat.com/solutions/5221661) Follow up for rpm-software-management#1753
We don't want to be veryfing digests as well when checking signatures. It would break legacy package installation in FIPS mode due to MD5 digest being unverifiable (see https://access.redhat.com/solutions/5221661) Follow up for #1753
We don't want to be veryfing digests as well when checking signatures. It would break legacy package installation in FIPS mode due to MD5 digest being unverifiable (see https://access.redhat.com/solutions/5221661) Follow up for #1753
This avoids having to actually import the package to check its signature, which reduces attack surface. Marking as RFC because this is critical code and I have not tested this much.