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.

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

[IPU 8->9] Detect RPMs with RSA/SHA1 signatures #854

Merged
merged 1 commit into from Mar 16, 2022

Conversation

pirat89
Copy link
Member

@pirat89 pirat89 commented Mar 10, 2022

tl;dr;

  • get the currennt crypto policies
  • inhibit upgrade when rpms with RSA/SHA1 signature are installed
    and used crypto policies will not allow it on the upgrade system
    • the only allowed crypto policy in such a case is LEGACY or
      DEFAULT:SHA1
  • set the crypto policy in the target userspace container if
    it is not DEFAULT

The SHA-1 hash algorithm is considered as unsafe and it is not longer
allowed to be used on RHEL 9 system using default crypto policies.
One of side effects is problem with installed packages with RSA/SHA1
signatures, as these packages could cause fail of the in-place upgrade
because such packages cannot be read, nor removed by dnf/rpm so the
tests of the transaction already fails in case we want to remove
(include replace) such packages. Additionally, even when we do not
touch such packages during the IPU, the final system will be still
malfunction as rpm will not be able to handle it by default.

Exception are packages signed by RSA/SHA1 keys that are not imported
in the system, so such signatures are not checked and are ignored
like in the case no signature is present at all.

To prevent unwanted situations regarding the installed packages
(packages that will be installed during the upgrade will be handled
in separate commit):

  • inhibit the upgrade when an RSA/SHA1 rpm is installed and
    current crypto policy is not LEGACY or DEFAULT:SHA1 (e.g.
    DEFAULT:SHA1).
    • NOTE: SHA1 subpolicy have to be created by user manually on the
      the system (and correctly!!) to make it working,
      so rather do not suggest it in remediation instructions
  • just report presence of such packages with HIGH risk so users
    are informed about the changed policy on RHEL 9

The current crypto policy is obtained from:

  • /etc/crypto-policies/state/current

In case the policy is not DEFAULT, set the crypto policy inside
the targetuserspace container. However, in case of custom policies
it can be broken (policies have to be copied/installed into the
container as well).

This is currently expected limitation. It is expected to improve it
in future.

@pirat89 pirat89 added this to the release-86_90 milestone Mar 10, 2022
@github-actions
Copy link

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergable.
If you want to re-run tests or request review, you can use following commands as a comment:

  • leapp-ci build to run copr build and e2e tests in OAMG CI
  • review please to notify leapp developers of review request

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please consider rerunning the CI by commenting leapp-ci build (might require several comments). If the problem persists, contact leapp-infra.

@pirat89 pirat89 marked this pull request as draft March 10, 2022 16:46
@pirat89 pirat89 force-pushed the deprecated-sha1-alg branch 6 times, most recently from ba196a4 to 4359365 Compare March 10, 2022 19:18
@pirat89
Copy link
Member Author

pirat89 commented Mar 11, 2022

@abadger thanks!! applied.

@pirat89 pirat89 force-pushed the deprecated-sha1-alg branch 2 times, most recently from 0f8a653 to ce0ef5c Compare March 11, 2022 09:12
@abadger
Copy link
Member

abadger commented Mar 11, 2022

The code looks good!

I added some minor grammatical suggestions. Other than those, I wonder if we should include something that tells the user to set the crypto policy back to default after the upgrade? (Or tells them the pros and cons of leaving it on legacy vs setting it back to default). If we don't encourage them to set it back after the upgrade, they probably will leave it on legacy which will circumvent one of rhel9's security enhancements.

@pirat89
Copy link
Member Author

pirat89 commented Mar 11, 2022

@abadger I would like to add a link to a RH article that gives people more information about the impacts. But at least some summary we should add in the code as well. Of course, FIPS systems could not use anything like that, as they become non-FIPS when they do it. Their only solution could be to replace/remove the packages prior the upgrade. But let's keep that as good enough for the first release and improvement could be delivered with the next one.

Also I checked that we have to do additional stuff in the container really as I was afraid. It means, we will restrict the possible values just to DEFAULT:SHA-1 and LEGACY right now so we know we can apply "same" policies inside the container. Other stuff will require additional handling and from my POV this could be pretty fragile, so I want to keep it on SMEs from crypto SST,

Copy link
Member

@fernflower fernflower left a comment

Choose a reason for hiding this comment

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

No smart remarks, just some nits.
Just please run make lint_fix

@pirat89
Copy link
Member Author

pirat89 commented Mar 11, 2022

Just please run make lint_fix

I did. however I can run it just with Py3 only nowadays on my system. We should add run of tests inside containers

Ahh. Ignore my comment. I read make lint.

@pirat89
Copy link
Member Author

pirat89 commented Mar 11, 2022

If you want to test scenario with DEFAULT:SHA1 crypto policy, create
/usr/share/crypto-policies/policies/modules/SHA1.pmod:

# This subpolicy adds SHA1 hash and signature support

hash = SHA1+

sign = ECDSA-SHA1+ RSA-PSS-SHA1+ RSA-SHA1+

sha1_in_certs = 1

(from rhel 9 SHA1.pmod)

abadger
abadger previously approved these changes Mar 11, 2022
@pirat89
Copy link
Member Author

pirat89 commented Mar 11, 2022

The current implementation covers issue with already installed rpms. I've tested scenarios when we try to install rpms that are signed with RSA/SHA1 signatures - and when not bad rpms are present on the system, so DEFAULT crypto policies are set and we have these two scenarios:

  • installing bad RPM when the key is not imported on the system before the run of leapp - in this case nothing happens because
    • our upgrade dnf plugin is executed with gpgcheck = False, so the keys are not imported and so not checked, so the whole IPU passes and no problems are present on the system
  • installing bad RPM when the key is already imported on the system - in such a case dnf/rpm checks packages and informs about bad signatures of packages
Error: Transaction test error:
  package my-signed-sha1-package-10.0.9-0.202203111513Z.8619719a.deprecated_sha1_alg.el9.noarch does not verify: Header V3 RSA/SHA1 Signature, key ID efc5967f: BAD

Under current circumstances, I think this is good enough solution.

UPDATE: however, on the upgraded system, if user runs e.g. dnf update and import the key, the issue will be present immadiately. But the same can happen on the RHEL 9 without upgrades as well.

@pirat89 pirat89 force-pushed the deprecated-sha1-alg branch 2 times, most recently from 52544b7 to bf1c4f7 Compare March 12, 2022 08:58
@pirat89 pirat89 force-pushed the deprecated-sha1-alg branch 2 times, most recently from 9bccd72 to 5caa1fc Compare March 16, 2022 15:55
@pirat89 pirat89 marked this pull request as ready for review March 16, 2022 15:56
@pirat89 pirat89 marked this pull request as draft March 16, 2022 16:18
@pirat89 pirat89 marked this pull request as ready for review March 16, 2022 16:18
abadger
abadger previously approved these changes Mar 16, 2022
tl;dr;
 * get the currennt crypto policies
 * inhibit upgrade when rpms with RSA/SHA1 signature are installed
   and used crypto policies will not allow it on the upgrade system
   * the only allowed crypto policy in such a case is LEGACY or
     DEFAULT:SHA1
 * set the crypto policy in the target userspace container if
   it is not DEFAULT

The SHA-1 hash algorithm is considered as unsafe and it is not longer
allowed to be used on RHEL 9 system using default crypto policies.
One of side effects is problem with installed packages with RSA/SHA1
signatures, as these packages could cause fail of the in-place upgrade
because such packages cannot be read, nor removed by dnf/rpm so the
tests of the transaction already fails in case we want to remove
(include replace) such packages. Additionally, even when we do not
touch such packages during the IPU, the final system will be still
malfunction as rpm will not be able to handle it by default.

Exception are packages signed by RSA/SHA1 keys that are not imported
in the system, so such signatures are not checked and are ignored
like in the case no signature is present at all.

To prevent unwanted situations regarding the installed packages
(packages that will be installed during the upgrade will be handled
in separate commit):
 * inhibit the upgrade when an RSA/SHA1 rpm is installed and
   current crypto policy is not LEGACY or DEFAULT:SHA1
   * NOTE: SHA1 subpolicy have to be created by user manually on the
           the system (and correctly!!) to make it working,
           so rather do not suggest it in remediation instructions
 * just report presence of such packages with HIGH risk so users
   are informed about the changed policy on RHEL 9

The current crypto policy is obtained from
  /etc/crypto-policies/state/current

In case the policy is not "DEFAULT", set the crypto policy inside
the targetuserspace container. However, in case of custom policies
it can be broken (policies have to be copied/installed into the
container as well).
This is currently expected limitation. It is expected to improve it
in future.

Co-authored-by: Toshio Kuratomi <a.badger@gmail.com>
Co-authored-by: Michal Reznik (mreznik) <mreznik@redhat.com>
Co-authored-by: Alexander Sosedkin <monk@unboiled.info>
@pirat89 pirat89 marked this pull request as draft March 16, 2022 16:36
@pirat89 pirat89 marked this pull request as ready for review March 16, 2022 16:36
@Rezney Rezney merged commit 218c2e0 into oamg:master Mar 16, 2022
6 checks passed
@pirat89 pirat89 deleted the deprecated-sha1-alg branch March 10, 2023 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants