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

[GPG] Run update process with checking RPM signatures #910

Merged
merged 5 commits into from
Nov 29, 2022

Conversation

Jakuje
Copy link
Contributor

@Jakuje Jakuje commented Jun 16, 2022

To summarize what we are trying to achieve:

  • Currently the update process runs without gpgcheck, which is potential thread model if somebody would be able to mitm repositories for the customer.
  • In RHEL9, if some package has SHA1 signatures (installed with --nogpgcheck), it will break rpm as it will not be able to operate on the existing db
  • The RHEL9 userspace can not import RHEL8 gpg keys, because it contains SHA-1 signature of auxiliary key. But once the keys are in the rpmdb, it can use them just ok (actually the key is still the same, the only difference is in the aux key). The SHA1 signature is verified only during the import and is not represented in the rpm db (or is not checked later?)

Checking gpg signatures can be changed in dnf configuration (default False) and per-repository in repo files. It can be used only to strengthen the value set by /usr/lib/rpm/macros with %_pkgverify_level digest (default). We already check for existing packages having SHA-1 signatures in #854 so after this change, no new SHA-1 signed package should be installed (if the custom repository has gpgcheck enabled)

Now, why I believe we do not need to run the import in other steps and why we do not need to import custmer certs here:

  • This step installs just dnf package, which should not depend on any other customer packages:
2022-06-16 14:03:18.451 DEBUG    PID: 24535 leapp.workflow.TargetTransactionFactsCollection.target_userspace_creator: External command has started: ['systemd-nspawn', '--register=no', '--quiet', '--keep-unit', '--capability=all', '-D', '/var/lib/leapp/scratch/mounts/root_/system_overlay', '--bind=/etc/dnf/dnf.conf:/etc/dnf/dnf.conf', [...], 'dnf', 'install', '-y', '--setopt=module_platform_id=platform:el9', '--setopt=keepcache=1', '--releasever', '9.0', '--installroot', '/el9target', '--disablerepo', '*', '--enablerepo', [...], 'dnf-command(config-manager)', 'dnf', '--disableplugin', 'subscription-manager']

The next steps do already operate on the source system RPMDB with all the imported keys and just use the dnf from the target OS.


Note: Upgrade description (some stuff above is still valid, so keeping this separated):


Current implementation description

Previously the gpgcheck=0 has been enforced during the IPU as we have not have any reasonable solution doing upgrade with allowed gpgcheck. Previously such upgrades automatically imported any gpgkeys without any possible check whether the actual keys are valid or not, which could lead to the automatical import of compromised/spurious gpg keys and user would not know about that. So using the original solution, user was asked for the import of new keys when installing additional content from new repositories (if keys have been different from those
used in the original system).

To do the upgrade in the original way (without gpgcheck), execute leapp with the --nogpgcheck option, or specify LEAPP_NOGPGCHECK=1 (envar). In such a case, all actions described below are skipped.

The current solution enables the GPG check by default but also could require additional actions from user to be able to upgrade. The goal is to ensure that no additional GPG keys are imported by DNF automatically during any action (partially resolved, read bellow). To be able to achive this, we are importing gpg keys automatically from a trusted directory before any dnf transaction is executed, so:
a) into the rpmdb of the target userspace container, before the
actual installation of the target userspace container
b) into overlayed rpmdb when calculating/testing the upgrade
transaction
c) into the system rpmdb right before the execution of the DNF
upgrade transaction

The case a) is handled directly in the target_userspace_creator actor. The other cases are handled via DNFWorkaround msg, using the importrpmgpgkeys tool script.

The trusted directory is in this case located under files/rpm-gpg/, where the directory name is major release of the
target system in case of production releases, in other cases it has the beta suffix. So e.g.:

  • files/rpm-gpg/8
  • files/rpm-gpg/8beta

That's because production and beta repositories have different gpg
keys and it is not wanted to mix production and beta keys. Beta
repositories are used only when the target production type is beta:

LEAPP_DEVEL_TARGET_PRODUCT_TYPE=beta

Introducing the missinggpgkeysinhibitor actor that checks gpg keys based on gpgkey specified in repofiles per each used target repository which does not explicitly specify gpgcheck=0. Such a key is compared with the currently installed gpg keys in the host rpmdb and keys inside the trusted directory. If the key is not stored in any of those places, the upgrade is inhibited with the corresponding report. User can resolve the problem installing the missing gpg keys or storing them to the trusted directory.

Currently supported protocols for the gpg keys are

  • file:///
  • http://
  • https://

If a key cannot be obtained (including use of an unsupported protocol, e.g. ftp://) the actor prompt a log, but does not generate a report about that (so the upgrade can continue, which could later lead into a failure during the download of packages - one of TODOs).

This is not the final commit for this feature and additional work is expected before the new release is introduced. Regarding that, see the code for new TODO / FIXME notes that are put into the code.

Summary of some TODOs planned to address in followup PR:

  • add checks that DNF does not import additional GPG keys during
    any action
  • report GPG keys that could not be checked, informing user about
    possible consequences - the report should not inhibit the upgrade
  • possibly introduce fallback for getting file:///... gpg keys
    as currently they are obtained from the target userspace container
    but if not present, the host system should be possibly checked:
    • Note that if the file has been created manually (custom repo file)
      most likely the gpgkey will be stored only on the host system
      • and in such a case the file would need to be copied from the
        host system into the container.

@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 request a review or rebuild a package in copr, you can use following commands as a comment:

  • review please to notify leapp developers of review request
  • /packit copr-build to submit a public copr build using packit

To launch regression testing public members of oamg organization can leave the following comment:

  • /rerun to schedule basic regression tests using this pr build and leapp*master* as artifacts
  • /rerun 42 to schedule basic regression tests using this pr build and leapp*PR42* as artifacts
  • /rerun-all to schedule all tests (including sst) using this pr build and leapp*master* as artifacts
  • /rerun-all 42 to schedule all tests (including sst) using this pr build and leapp*PR42* as artifacts

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 self-assigned this Jun 16, 2022
@Jakuje Jakuje force-pushed the gpgcheck branch 2 times, most recently from 409e229 to bf69e87 Compare June 16, 2022 14:30
@leapp-bot
Copy link
Collaborator

This PR has been linked in issue tracker (#OAMG-7062).

@packit-as-a-service
Copy link

Your git-forge project github.com/oamg/leapp-repository has permissions to build in @oamg/leapp Copr project configured in Packit. However, we migrated to the solution where you can configure the allowed git-forge projects in Copr yourself and will remove the configuration in Packit for the allowed projects soon. Therefore, please, add this git-forge project github.com/oamg/leapp-repository to Packit allowed forge projectsin the Copr project settings.

@packit-as-a-service
Copy link

Your git-forge project github.com/oamg/leapp-repository has permissions to build in @oamg/leapp Copr project configured in Packit. However, we migrated to the solution where you can configure the allowed git-forge projects in Copr yourself and will remove the configuration in Packit for the allowed projects soon. Therefore, please, add this git-forge project github.com/oamg/leapp-repository to Packit allowed forge projectsin the Copr project settings.

@packit-as-a-service
Copy link

Your git-forge project github.com/oamg/leapp-repository has permissions to build in @oamg/leapp Copr project configured in Packit. However, we migrated to the solution where you can configure the allowed git-forge projects in Copr yourself and will remove the configuration in Packit for the allowed projects soon. Therefore, please, add this git-forge project github.com/oamg/leapp-repository to Packit allowed forge projectsin the Copr project settings.

@packit-as-a-service
Copy link

Your git-forge project github.com/oamg/leapp-repository has permissions to build in @oamg/leapp Copr project configured in Packit. However, we migrated to the solution where you can configure the allowed git-forge projects in Copr yourself and will remove the configuration in Packit for the allowed projects soon. Therefore, please, add this git-forge project github.com/oamg/leapp-repository to Packit allowed forge projectsin the Copr project settings.

@packit-as-a-service
Copy link

Your git-forge project github.com/oamg/leapp-repository has permissions to build in @oamg/leapp Copr project configured in Packit. However, we migrated to the solution where you can configure the allowed git-forge projects in Copr yourself and will remove the configuration in Packit for the allowed projects soon. Therefore, please, add this git-forge project github.com/oamg/leapp-repository to Packit allowed forge projectsin the Copr project settings.

@packit-as-a-service
Copy link

Your git-forge project github.com/oamg/leapp-repository has permissions to build in @oamg/leapp Copr project configured in Packit. However, we migrated to the solution where you can configure the allowed git-forge projects in Copr yourself and will remove the configuration in Packit for the allowed projects soon. Therefore, please, add this git-forge project github.com/oamg/leapp-repository to Packit allowed forge projectsin the Copr project settings.

@packit-as-a-service
Copy link

Your git-forge project github.com/oamg/leapp-repository has permissions to build in @oamg/leapp Copr project configured in Packit. However, we migrated to the solution where you can configure the allowed git-forge projects in Copr yourself and will remove the configuration in Packit for the allowed projects soon. Therefore, please, add this git-forge project github.com/oamg/leapp-repository to Packit allowed forge projectsin the Copr project settings.

@packit-as-a-service
Copy link

Your git-forge project github.com/oamg/leapp-repository has permissions to build in @oamg/leapp Copr project configured in Packit. However, we migrated to the solution where you can configure the allowed git-forge projects in Copr yourself and will remove the configuration in Packit for the allowed projects soon. Therefore, please, add this git-forge project github.com/oamg/leapp-repository to Packit allowed forge projectsin the Copr project settings.

@packit-as-a-service
Copy link

Your git-forge project github.com/oamg/leapp-repository has permissions to build in @oamg/leapp Copr project configured in Packit. However, we migrated to the solution where you can configure the allowed git-forge projects in Copr yourself and will remove the configuration in Packit for the allowed projects soon. Therefore, please, add this git-forge project github.com/oamg/leapp-repository to Packit allowed forge projectsin the Copr project settings.

@packit-as-a-service
Copy link

Your git-forge project github.com/oamg/leapp-repository has permissions to build in @oamg/leapp Copr project configured in Packit. However, we migrated to the solution where you can configure the allowed git-forge projects in Copr yourself and will remove the configuration in Packit for the allowed projects soon. Therefore, please, add this git-forge project github.com/oamg/leapp-repository to Packit allowed forge projectsin the Copr project settings.

@pirat89
Copy link
Member

pirat89 commented Nov 25, 2022

@Jakuje that's problem of incorrect test. let's discuss it during the sync

@pirat89
Copy link
Member

pirat89 commented Nov 25, 2022

This is also not resolved yet, but we already talked about this that we should be able to get the path to the container when the file:/// protocol is used

and that's another thing I want to sync about. I was thinking already about the solution in the actor but I want to discuss it with you

@pirat89 pirat89 added the enhancement New feature or request label Nov 25, 2022
@t184256
Copy link
Contributor

t184256 commented Nov 25, 2022

/packit copr-build

@packit-as-a-service
Copy link

Account t184256 has no write access nor is author of PR!

@t184256
Copy link
Contributor

t184256 commented Nov 25, 2022

Can I have access? =) Or just builds.

@pirat89
Copy link
Member

pirat89 commented Nov 25, 2022

@t184256 hi, builds are created automatically, see details in rpm--build* test tasks to get the link to the builds. or you can just activate the copr repository (epel7 or epel8, depends on where you want to install the build) and then install the latest build for this PR e.g. by:

# dnf install "leapp-upgrade-*pr910*"

let me know in case of any troubles with obtaining builds.

)


def _get_test_targuserspaceinfo(path='nopath'):
Copy link
Member

Choose a reason for hiding this comment

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

@Jakuje for now, I prepared the function for the mocking.

@pirat89
Copy link
Member

pirat89 commented Nov 25, 2022

@Jakuje seems we have still problem. as part of actions around the contatiner is the copying of /etc/pki from the host into the container. So the detection of missing keys (even when it seems to be working as expected otherwise) is failing because it detects keys from the original /etc/pki/rpm-gpg directory instead of the real target one:

(Pdb) _read_gpg_fp_from_file('/etc/leapp/repos.d/system_upgrade/common/files/rpm-gpg/8/RPM-GPG-KEY-redhat-release')
2022-11-25 22:05:07.918 DEBUG    PID: 12256 leapp.workflow.TargetTransactionCheck.missing_gpg_keys_inhibitor: External command has started: ['gpg2', '--with-fingerprint', '--with-colons', '/etc/leapp/repos.d/system_upgrade/common/files/rpm-gpg/8/RPM-GPG-KEY-redhat-release']
pub:-:4096:1:199E2F91FD431D51:1256212795:::-:Red Hat, Inc. (release key 2) <security@redhat.com>:
fpr:::::::::567E347AD0044ADE55BA8A5F199E2F91FD431D51:
pub:-:4096:1:F76F66C3D4082792:1530059637:::-:Red Hat, Inc. (auxiliary key) <security@redhat.com>:
fpr:::::::::6A6AA7C97C8890AEC6AEBFE2F76F66C3D4082792:
sub:-:4096:1:81C640F91B5584D3:1530059637::::
2022-11-25 22:05:07.935 DEBUG    PID: 12256 leapp.workflow.TargetTransactionCheck.missing_gpg_keys_inhibitor: External command has finished: ['gpg2', '--with-fingerprint', '--with-colons', '/etc/leapp/repos.d/system_upgrade/common/files/rpm-gpg/8/RPM-GPG-KEY-redhat-release']
[u'fd431d51', u'd4082792']
(Pdb) _read_gpg_fp_from_file('/var/lib/leapp/el8userspace/etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release')
2022-11-25 22:05:51.231 DEBUG    PID: 12256 leapp.workflow.TargetTransactionCheck.missing_gpg_keys_inhibitor: External command has started: ['gpg2', '--with-fingerprint', '--with-colons', '/var/lib/leapp/el8userspace/etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release']
pub:-:4096:1:199E2F91FD431D51:1256212795:::-:Red Hat, Inc. (release key 2) <security@redhat.com>:
fpr:::::::::567E347AD0044ADE55BA8A5F199E2F91FD431D51:
pub:-:1024:17:45689C882FA658E0:1164971113:::-:Red Hat, Inc. (auxiliary key) <security@redhat.com>:
fpr:::::::::43A6E49C4A38F4BE9ABF2A5345689C882FA658E0:
2022-11-25 22:05:51.248 DEBUG    PID: 12256 leapp.workflow.TargetTransactionCheck.missing_gpg_keys_inhibitor: External command has finished: ['gpg2', '--with-fingerprint', '--with-colons', '/var/lib/leapp/el8userspace/etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release']
[u'fd431d51', u'2fa658e0']
(Pdb) _read_gpg_fp_from_file('/etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release')
2022-11-25 22:06:00.560 DEBUG    PID: 12256 leapp.workflow.TargetTransactionCheck.missing_gpg_keys_inhibitor: External command has started: ['gpg2', '--with-fingerprint', '--with-colons', '/etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release']
pub:-:4096:1:199E2F91FD431D51:1256212795:::-:Red Hat, Inc. (release key 2) <security@redhat.com>:
fpr:::::::::567E347AD0044ADE55BA8A5F199E2F91FD431D51:
pub:-:1024:17:45689C882FA658E0:1164971113:::-:Red Hat, Inc. (auxiliary key) <security@redhat.com>:
fpr:::::::::43A6E49C4A38F4BE9ABF2A5345689C882FA658E0:
2022-11-25 22:06:00.576 DEBUG    PID: 12256 leapp.workflow.TargetTransactionCheck.missing_gpg_keys_inhibitor: External command has finished: ['gpg2', '--with-fingerprint', '--with-colons', '/etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release']
[u'fd431d51', u'2fa658e0']

## for readability:
 _read_gpg_fp_from_file('/etc/leapp/repos.d/system_upgrade/common/files/rpm-gpg/8/RPM-GPG-KEY-redhat-release')
[u'fd431d51', u'd4082792']
(Pdb) _read_gpg_fp_from_file('/var/lib/leapp/el8userspace/etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release')
[u'fd431d51', u'2fa658e0']
(Pdb) _read_gpg_fp_from_file('/etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release')
[u'fd431d51', u'2fa658e0']

@Jakuje I will need your help to check with me what PKI keys we should not copy to the container. Or possibly we will just do the exception around rpm-gpg keys in the targetuserspace creator to put them back after the /etc/pki is copied.

@pirat89
Copy link
Member

pirat89 commented Nov 28, 2022

@Jakuje I guess that regarding the copying of certs, we should always copy all of them but preserve those that are installed in the container from packages. currently it's handled only for rpm-gpg directory (with the exception of prod. cert)

@t184256
Copy link
Contributor

t184256 commented Nov 28, 2022

@pirat89, thanks! I was looking for leapp-upgrade-*PR910* with uppercase PR =/

# target userspace container? What if it's a file locally stored by user
# and the repository is defined like that as well? Possibly it's just
# a corner corner case. I guess it does not have a high prio tbh, but want
# to be sure.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thinking about this one a bit more, it might make sense to create a fallback in case the file is not in the target userspace container. It will be there only if it is provided by some of the packages that were installed initially together with the rpm/dnf and stuff requested by customer. In case it is just random file dropped in the filesystem (alongside with the random repo files), it will be only in the source filesystem and we will have to look there. I think this will be more common case and if this would not be that complicated, I would propose to log something here and fall-back.

Copy link
Member

Choose a reason for hiding this comment

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

yes. that would be probably best

@pirat89
Copy link
Member

pirat89 commented Nov 28, 2022

@Jakuje we have to preserve all certificates installed by rpms in the container... For now, make the copy of certificates (/etc/pki) conditional again (only with rhsm) until the preservation of all files is handled correctly . So:

  • we will not affect RHUI, which is always executed without RHSM
  • replacing the rpm-gpg directory happens in such a case when RHSM is used; if RHSM is not used, the original target certificates are installed as expected

The correct solution around that will be handled separately for CTC2

P.S. as the gpg2 is not mocked in tests, current test results are failing due to environment setup.

@pirat89
Copy link
Member

pirat89 commented Nov 29, 2022

@Jakuje @t184256 discovered we are missing checks whether dnf transactions are executed without additional imports of keys. Currently the target userspace container can be installed with additional import of unsecure keys.

target_userspace_creator: Using rpmkeys executable at /usr/bin/rpmkeys to verify signatures
target_userspace_creator: Microsoft Azure RPMs for Red Hat Enterprise Lin 960 kB/s | 983  B     00:00
target_userspace_creator: Importing GPG key 0xBE1229CF:
target_userspace_creator:  Userid     : "Microsoft (Release signing) <gpgsecurity@microsoft.com>"
target_userspace_creator:  Fingerprint: BC52 8686 B50D 79E3 39D3 721C EB3E 94AD BE12 29CF
target_userspace_creator:  From       : /etc/pki/rpm-gpg/RPM-GPG-KEY-microsoft-azure-release
target_userspace_creator: Key imported successfully
target_userspace_creator: Running transaction check

The problem is, that we do not know keys that are needed before the container is installed, when the key is stored on file:///.... path and so we cannot even tell people what keys they need. It does not need to be critical, as we are still checking the keys that are supposed to be used for the upgrade of the system itself. I am thinking in this case, what key actually has been used. Possibly it has been imported key from the /etc/pki/rpm-gpg directory of the host system? It's also raising question what to do when such an import happens. Currently we do not have any checks of the DNF output to see whether any keys have been imported during the transaction. I believe we should add such detections, but I also think it's ok to make it part of the solution for CTC2 and do not block the current merge.

So simple detection whether we are successful when the whole upgrade passes:

# the space behind "key" is important :)
grep -r "Importing GPG key " /var/log/leapp/leapp-upgrade.log

If there is basically any other case of import than imports during creation of the container, I guess it should be considered as bigger problem.

@pirat89
Copy link
Member

pirat89 commented Nov 29, 2022

@t184256 @Jakuje so in case that for a repoid: gpgcheck=1 and gpgkey is not specified + key is not installed, upgrade is stopped by this error after the download of packages

(604/605): nss-tools-3.79.0-10.el8_6.x86_64.rpm 3.2 MB/s | 582 kB     00:00    
(605/605): nss-softokn-3.79.0-10.el8_6.x86_64.r 6.4 MB/s | 1.2 MB     00:00    
--------------------------------------------------------------------------------
Total                                            19 MB/s | 555 MB     00:29     
Using rpmkeys executable at /usr/bin/rpmkeys to verify signatures
The downloaded packages were saved in cache until the next successful transaction.
You can remove cached packages by executing 'dnf clean packages'.

STDERR:
Public key for my-little-dummy-package-0.1-2.el8.x86_64.rpm is not installed
Error: GPG check FAILED

Which is 'good'. So I will add a TODO note, to the related actor, to catch the error and when GPG check FAILED appears,
print a proper error msg giving a context why it possibly crashed, etc so user knows what's going on. Also it corresponds to one of other TODOs, to report the possible problem about the missing gpgkey

Tips for the reproducer:

echo my-little-dummy-package >> /etc/leapp/transaction/to_install
# cat /etc/leapp/files/leapp_upgrade_repositories.repo 
[leapp-test]
name=Copr repo for test-repo owned by pstodulk
baseurl=https://download.copr.fedorainfracloud.org/results/pstodulk/test-repo/epel-8-$basearch/
type=rpm-md
skip_if_unavailable=True
gpgcheck=1
## whatever=https://download.copr.fedorainfracloud.org/results/pstodulk/test-repo/pubkey.gpg
repo_gpgcheck=0
enabled=1
enabled_metadata=1

pirat89 and others added 5 commits November 29, 2022 18:19
The original solution copied /etc/pki from the host into the
target userspace container if the upgrade has been performed with
RHSM, which causes several negative impacts:

a) certificates are missing inside the container when upgrading
   without RHSM (still issue)
     - Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2040706
b) the target OS certificates are replaced by the original OS
   certificates when upgrading with RHSM (partially fixed)

This commit partially fixes the case b), so we preserve target
certificates inside the container only from the /etc/pki/rpm-gpg
directory when upgrading with RHSM. If files or directories with
the same name exists inside, prefered are those from the target OS.

For the full fix of this case. The full fix should preserve
all certificates owned by packages inside the container, and only
"new files" from the host should be applied. This is also prerequisite
to be able to fix the case a).

To be able to fix the case a) we would need to make this behaviour
unconditional (not dependent on the use of RHSM). Which most likely
should resolve the bug 2040706. Which needs the full fix of the case
b) first, as described above. The unconditional copy of /etc/pki
currently breaks upgrades on systems using RHUI (at least on
Azure for IPU 8 -> 9, other clouds could be affected also).
So postponing the sollution to a followup PR.
The original model provided a possibility to execute a script
that will handle problems before the DNF / RPM transaction,
in correct contexts (overlay, host system, ..) before any use
of the upgrade dnf plugin.

But current solution provided only the script_path field, which
suggests it should contain only the path to the script. The executed
command (inside a context) looked like this:
    bash -c script_path
However we have realized we need to be able to execute a script
with additional arguments. Regarding that, introducing
the script_args field. SO the final command looks like this:
    bash -c 'script_path arg1 arg2..'
when script_args are specified. The default is set to an empty
list.
The script can be used to import gpg keys from a particular file
or files inside a directory. Expected to be executed like:
   importrpmgpgkey <absolute-path>
Previously the gpgcheck=0 has been enforced during the IPU as we have
not have any reasonable solution doing upgrade with allowed gpgcheck.
Previously such upgrades automatically imported any gpgkeys without
any possible check whether the actual keys are valid or not, which
could lead to the automatical import of compromised/spurious gpg keys
and user would not know about that. So using the original solution,
user was asked for the import of new keys when installing additional
content from new repositories (if keys have been different from those
used in the original system).

To do the upgrade in the original way (without gpgcheck), execute
leapp with the `--nogpgcheck` option, or specify `LEAPP_NOGPGCHECK=1`
(envar). In such a case, all actions described below are skipped.

The current solution enables the GPG check by default but also could
require additional actions from user to be able to upgrade. The goal
is to ensure that no additional GPG keys are imported by DNF
automatically during any action (partially resolved, read bellow).
To be able to achive this, we are importing gpg keys automatically
from a *trusted directory* before any dnf transaction is executed, so:
 a) into the rpmdb of the target userspace container, before the
    actual installation of the target userspace container
 b) into overlayed rpmdb when calculating/testing the upgrade
    transaction
 c) into the system rpmdb right before the execution of the DNF
    upgrade transaction

The case a) is handled directly in the target_userspace_creator actor.
The other cases are handled via DNFWorkaround msg, using the
`importrpmgpgkeys` tool script.

The *trusted directory* is in this case located under
`files/rpm-gpg/`, where the directory name is major release of the
target system in case of production releases, in other cases it has
the *beta* suffix. So e.g.:
    files/rpm-gpg/8
    files/rpm-gpg/8beta
That's because production and beta repositories have different gpg
keys and it is not wanted to mix production and beta keys. Beta
repositories are used only when the target production type is beta:
    LEAPP_DEVEL_TARGET_PRODUCT_TYPE=beta

Introducing the missinggpgkeysinhibitor actor that checks gpg keys
based on `gpgkey` specified in repofiles per each used target
repository which does not explicitly specify `gpgcheck=0`.
Such a key is compared with the currently installed gpg keys in the
host rpmdb and keys inside the *trusted directory*. If the key
is not stored in any of those places, the upgrade is inhibited with
the corresponding report. User can resolve the problem installing the
missing gpg keys or storing them to the trusted directory.

Currently supported protocols for the gpg keys are
  file:///
  http://
  https://
If a key cannot be obtained (including use of an unsupported protocol,
e.g. ftp://) the actor prompt a log, but does not generate a report
about that (so the upgrade can continue, which could later lead into
a failure during the download of packages - one of TODOs).

This is not the final commit for this feature and additional work
is expected before the new release is introduced. Regarding that,
see the code for new TODO / FIXME notes that are put into the code.

Summary of some TODOs planned to address in followup PR:
  - add checks that DNF does not import additional GPG keys during
    any action
  - report GPG keys that could not be checked, informing user about
    possible consequences - the report should not inhibit the upgrade
  - possibly introduce fallback for getting file:///... gpg keys
    as currently they are obtained from the target userspace container
    but if not present, the host system should be possibly checked:
    - Note that if the file has been created manually (custom repo file)
      most likely the gpgkey will be stored only on the host system
      - and in such a case the file would need to be copied from the
        host system into the container.

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Copy link
Member

@pirat89 pirat89 left a comment

Choose a reason for hiding this comment

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

Manually tested for various cases, other TODOs are mentioned in commit msgs, comments, the PR description, and in the code, that are expected to be delivered in an additional PR. Current solution seems good enough in this phase, even when additional changes are needed to be delivered before the new release. But regarding the current amount of work and the need to start to use this solution already to get feedback, we are merging it. In case of problems, people can use the --nogpgcheck option and in such a case they should not be affected by the new functionality. This can negatively affect the current CI. We expec to t update tests properly in upcoming days.

@pirat89 pirat89 merged commit 9ed7194 into oamg:master Nov 29, 2022
@pirat89
Copy link
Member

pirat89 commented Nov 29, 2022

I've missed to upgrade the report. Handled in #993

@pirat89 pirat89 added the changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant label Jan 20, 2023
pirat89 added a commit to pirat89/leapp-repository that referenced this pull request Feb 21, 2023
## Packaging
- Requires cpio (oamg#979)
- Requires python3-gobject-base, NetworkManager-libnm (oamg#969)
- Bump leapp-repository-dependencies to 9 (oamg#969, oamg#979)

## Upgrade handling
### Fixes
- Add leapp RHUI packages to an allowlist to drop confusing reports (oamg#995)
- Check only mounted XFS partitions (oamg#1027)
- Detect the kernel-core RPM instead of kernel to prevent an error during post-upgrade phases (oamg#1024)
- Disable the amazon-id DNF plugin on AWS during the upgrade stage to omit error messages during the upgrade process caused by missing network connection (oamg#990)
- Do not create new *pyc files when running leapp after the DNF upgrade transaction (oamg#1017)
- Enable upgrades on s390x when /boot is part of rootfs (oamg#991)
- Extend the allow list of RHUI clients by azure-sap-apps to omit confusing report (oamg#974)
- Filter out PES events unrelated for the used upgrade path and handle overlapping events (oamg#1008)
- Fix scan of ceph volumes on systems without ceph-osd (oamg#1011)
- Fix scan of ceph volumes when ceph-osd container is not found (oamg#986)
- Fix systemd symlinks that become incorrect during the IPU (oamg#972)
- Fix the check of memory (RAM) limits (oamg#984)
- Fix the upgrade of IBM Z machines configured with ZFCP (oamg#983)
- Ignore external accounts in /etc/passwd (oamg#958)
- Inhibit the upgrade when entries in /etc/fstab cause overshadowing during the upgrade (oamg#1009)
- Prevent leapp failures caused by re-run of leapp in the upgrade initramfs after previous failure, which causes additional confusing error message hiding original bugs (oamg#996)
- Prevent the upgrade with RHSM when a baseos and an appstream target repositories are not discovered (oamg#1001)
- RHUI(Azure) handle correctly various SAP images (oamg#1037)
- Rework the network configuration handling and parse the configuration data properly (oamg#969)
- Set RHSM release for non-ga and non-beta channels (oamg#1033)
- Use the "grub" library to find the GRUB device (oamg#989)
- [IPU 7 -> 8] Detect corrupted grubenv file (oamg#1012)
- [IPU 7 -> 8] Ensure that rsyncd stays enabled if it is enabled prior the upgrade(oamg#1043)
- [IPU 7 -> 8] Ensure that satellite metapackages are installed after the upgrade (oamg#994)
- [IPU 7 -> 8] Ensure the device_cio_free service stays enabled on s390x after the upgrade (oamg#977)
- [IPU 7 -> 8] Fixed checks for RHEL SAP IPU 7.9 -> 8.6 (oamg#978)
- [IPU 7 -> 8] Fixed migration of ntp to chrony when the ntp service is masked (oamg#966)
- [IPU 7 -> 8] Prevent the traceback during migration of sendmail configuration files when the package is not installed (oamg#1041)
- [IPU 7 -> 8] Satellite: reindex all related databases to fix issues due to new locales in RHEL 8 (oamg#1007, oamg#1018)
- [IPU 7 -> 8] Use the correct domain name in SSSD reports (oamg#1040)
- [IPU 8 -> 9] Added checks for RHEL SAP IPU 8.6 -> 9.0 (oamg#978)
- [IPU 8 -> 9] CheckVDO: Ask user for the confirmation only on failures and undetermined devices (oamg#961)
- [IPU 8 -> 9] Fix the kernel detection during initramfs creation for new kernel on RHEL 9.2+ (oamg#1048)
- [IPU 8 -> 9] Fix the upgrade on Azure using RHUI for SAP Apps images (oamg#975)
- [IPU 8 -> 9] Handle correctly firewalld version 0.8 (oamg#963)

### Enhancements
- Set new upgrade paths (oamg#988):
-- RHEL 7.9 -> 8.8, 8.6 (default: 8.8)
-- RHEL 8.6 -> 9.0
-- RHEL 8.8 -> 9.2
- Check that used leapp data are valid and compatible with the installed leapp-repository (oamg#1003)
- Detect a proxy configuration in YUM/DNF and adjust an error msg on issues caused by the configuration (oamg#914)
- Detect and report systemd symlinks that are broken before the upgrade (oamg#972)
- Drop obsoleted upgrade paths (oamg#1047)
- Improve remediation instructions for packages in unknown repositories (oamg#1010)
- Improve the error message to guide users when discovered more space is needed (oamg#956)
- Introduce --nogpgcheck option to skip checking of RPM signatures (oamg#910)
- Introduced an option to use an ISO file as a target RHEL version content source (oamg#979)
- Introduced possibility to specify what systemd services should be enabled/disabled on the upgraded system (oamg#964)
- Map the target repositories also based on the installed content (oamg#967)
- Provide common information about systemd services (oamg#959)
- Register subscribed systems automatically to Red Hat Insights unless --no-insights-register is used (oamg#1000)
- Remove obsoleted GPG keys provided by RH after the upgrade to prevent errors (oamg#1022)
- Run upgrade process with checking RPM signatures by default (oamg#910, oamg#993, oamg#1025)
- Save breadcrumbs results as RHSM facts (oamg#1002)
- Small improvements in various reports (oamg#1038, oamg#1039, oamg#1032)
- [IPU 8 -> 9] Detect CIFS also when upgrading from RHEL8 to RHEL9 (PR1035)
- [IPU 8 -> 9] Detect RoCE on IBM Z machines and check the configuration is safe for the upgrade (oamg#1030)
- [IPU 8 -> 9] Enable upgrades of RHEL 8 for SAP HANA to RHEL 9 on ppc64le (oamg#1042)
- [IPU 8 -> 9] Improve the handling of blocklisted certificates (oamg#992)

## Additional changes interesting for devels
- Started work on bringing up networking inside the upgrade initramfs - currently available for testing and development purposes when LEAPP_DEVEL_INITRAM_NETWORK is set (oamg#960)
- Add leapp debug tools to the upgrade initramfs - dracut upgrade module (oamg#997)
- Enable disabling of DNF plugins in the dnfplugin library (oamg#990)
@pirat89 pirat89 mentioned this pull request Feb 21, 2023
pirat89 added a commit that referenced this pull request Feb 21, 2023
## Packaging
- Requires cpio (#979)
- Requires python3-gobject-base, NetworkManager-libnm (#969)
- Bump leapp-repository-dependencies to 9 (#969, #979)

## Upgrade handling
### Fixes
- Add leapp RHUI packages to an allowlist to drop confusing reports (#995)
- Check only mounted XFS partitions (#1027)
- Detect the kernel-core RPM instead of kernel to prevent an error during post-upgrade phases (#1024)
- Disable the amazon-id DNF plugin on AWS during the upgrade stage to omit error messages during the upgrade process caused by missing network connection (#990)
- Do not create new *pyc files when running leapp after the DNF upgrade transaction (#1017)
- Enable upgrades on s390x when /boot is part of rootfs (#991)
- Extend the allow list of RHUI clients by azure-sap-apps to omit confusing report (#974)
- Filter out PES events unrelated for the used upgrade path and handle overlapping events (#1008)
- Fix scan of ceph volumes on systems without ceph-osd (#1011)
- Fix scan of ceph volumes when ceph-osd container is not found (#986)
- Fix systemd symlinks that become incorrect during the IPU (#972)
- Fix the check of memory (RAM) limits (#984)
- Fix the upgrade of IBM Z machines configured with ZFCP (#983)
- Ignore external accounts in /etc/passwd (#958)
- Inhibit the upgrade when entries in /etc/fstab cause overshadowing during the upgrade (#1009)
- Prevent leapp failures caused by re-run of leapp in the upgrade initramfs after previous failure, which causes additional confusing error message hiding original bugs (#996)
- Prevent the upgrade with RHSM when a baseos and an appstream target repositories are not discovered (#1001)
- RHUI(Azure) handle correctly various SAP images (#1037)
- Rework the network configuration handling and parse the configuration data properly (#969)
- Set RHSM release for non-ga and non-beta channels (#1033)
- Use the "grub" library to find the GRUB device (#989)
- [IPU 7 -> 8] Detect corrupted grubenv file (#1012)
- [IPU 7 -> 8] Ensure that rsyncd stays enabled if it is enabled prior the upgrade(#1043)
- [IPU 7 -> 8] Ensure that satellite metapackages are installed after the upgrade (#994)
- [IPU 7 -> 8] Ensure the device_cio_free service stays enabled on s390x after the upgrade (#977)
- [IPU 7 -> 8] Fixed checks for RHEL SAP IPU 7.9 -> 8.6 (#978)
- [IPU 7 -> 8] Fixed migration of ntp to chrony when the ntp service is masked (#966)
- [IPU 7 -> 8] Prevent the traceback during migration of sendmail configuration files when the package is not installed (#1041)
- [IPU 7 -> 8] Satellite: reindex all related databases to fix issues due to new locales in RHEL 8 (#1007, #1018)
- [IPU 7 -> 8] Use the correct domain name in SSSD reports (#1040)
- [IPU 8 -> 9] Added checks for RHEL SAP IPU 8.6 -> 9.0 (#978)
- [IPU 8 -> 9] CheckVDO: Ask user for the confirmation only on failures and undetermined devices (#961)
- [IPU 8 -> 9] Fix the kernel detection during initramfs creation for new kernel on RHEL 9.2+ (#1048)
- [IPU 8 -> 9] Fix the upgrade on Azure using RHUI for SAP Apps images (#975)
- [IPU 8 -> 9] Handle correctly firewalld version 0.8 (#963)

### Enhancements
- Set new upgrade paths (#988):
-- RHEL 7.9 -> 8.8, 8.6 (default: 8.8)
-- RHEL 8.6 -> 9.0
-- RHEL 8.8 -> 9.2
- Check that used leapp data are valid and compatible with the installed leapp-repository (#1003)
- Detect a proxy configuration in YUM/DNF and adjust an error msg on issues caused by the configuration (#914)
- Detect and report systemd symlinks that are broken before the upgrade (#972)
- Drop obsoleted upgrade paths (#1047)
- Improve remediation instructions for packages in unknown repositories (#1010)
- Improve the error message to guide users when discovered more space is needed (#956)
- Introduce --nogpgcheck option to skip checking of RPM signatures (#910)
- Introduced an option to use an ISO file as a target RHEL version content source (#979)
- Introduced possibility to specify what systemd services should be enabled/disabled on the upgraded system (#964)
- Map the target repositories also based on the installed content (#967)
- Provide common information about systemd services (#959)
- Register subscribed systems automatically to Red Hat Insights unless --no-insights-register is used (#1000)
- Remove obsoleted GPG keys provided by RH after the upgrade to prevent errors (#1022)
- Run upgrade process with checking RPM signatures by default (#910, #993, #1025)
- Save breadcrumbs results as RHSM facts (#1002)
- Small improvements in various reports (#1038, #1039, #1032)
- [IPU 8 -> 9] Detect CIFS also when upgrading from RHEL8 to RHEL9 (PR1035)
- [IPU 8 -> 9] Detect RoCE on IBM Z machines and check the configuration is safe for the upgrade (#1030)
- [IPU 8 -> 9] Enable upgrades of RHEL 8 for SAP HANA to RHEL 9 on ppc64le (#1042)
- [IPU 8 -> 9] Improve the handling of blocklisted certificates (#992)

## Additional changes interesting for devels
- Started work on bringing up networking inside the upgrade initramfs - currently available for testing and development purposes when LEAPP_DEVEL_INITRAM_NETWORK is set (#960)
- Add leapp debug tools to the upgrade initramfs - dracut upgrade module (#997)
- Enable disabling of DNF plugins in the dnfplugin library (#990)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants