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

3006.0: Prevent _pygit2.GitError: error loading known_hosts when $HOME is not set (bsc#1210994) #588

Merged

Conversation

meaksh
Copy link
Member

@meaksh meaksh commented Jun 27, 2023

What does this PR do?

This PR partially backport saltstack/salt#64510. For now we only backport the fix of the HOME env to avoid the problem with "pygit2" backend, but not the general fix of HOME for the whole stack (as upstream PR is not yet ready).


This PR should fix a problem that happens in some libgit2 versions, where there is an strict check for ssh "known_hosts" done by the "libgit2" library, in the context of fixing a CVE issue:

libgit2/libgit2@42e5db9

We noticed this CVE fix has been backported to older "libgit2" versions in some distributions.

Without the fix on this PR, we see _pygit2.GitError: error loading known_hosts errors when dealing with git ssh repositories.

What issues does this PR fix or reference?

Fixes: saltstack/salt#64121

Previous Behavior

There are gitfs errors we can see in the Salt master logs.

[ERROR   ] Error occurred fetching gitfs remote 'ssh://[email protected]/myorg/mysalt.git': error loading known_hosts:
Traceback (most recent call last):
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/utils/gitfs.py", line 1870, in _fetch
    fetch_results = origin.fetch(**fetch_kwargs)
  File "/opt/saltstack/salt/extras-3.10/pygit2/remote.py", line 146, in fetch
    payload.check_error(err)
  File "/opt/saltstack/salt/extras-3.10/pygit2/callbacks.py", line 98, in check_error
    check_error(error_code)
  File "/opt/saltstack/salt/extras-3.10/pygit2/errors.py", line 65, in check_error
    raise GitError(message)
_pygit2.GitError: error loading known_hosts:

New Behavior

There are no errors in Salt master logs and gitfs works as expected.

What issues does this PR fix or reference?

Fixes: https://github.com/SUSE/spacewalk/issues/21805

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

Copy link
Contributor

@vzhestkov vzhestkov left a comment

Choose a reason for hiding this comment

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

Looks good 👍

Copy link
Contributor

@ycedres ycedres left a comment

Choose a reason for hiding this comment

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

Just out of curiosity (probably already answered somewhere I cannot find). How are we sure that $HOME is going to point to a valid working directory to create/look up for the known_hosts file if the user running salt is a system user?

@meaksh
Copy link
Member Author

meaksh commented Jun 27, 2023

Thanks for the reviews!

So, this fix is just taking care that "HOME" is set while initializating "pygit2" backend, to prevent "libgit2" to raise error directly, even if valid known host, simply because HOME is not set: https://github.com/libgit2/libgit2/blob/42e5db98b963ae503229c63e44e06e439df50e56/src/libgit2/transports/ssh.c#L439-L440

Now, with this fix in place, if "known_hosts" cannot be read because of any other reason, then of course there would be an error produced and the user should take care of it.

This part of the code (salt.utils.gitfs) is used by Salt Master and it should run with the user you configured via Salt Master settings: https://docs.saltproject.io/en/latest/ref/configuration/master.html#user

Hth!

@meaksh meaksh merged commit 40a57af into openSUSE/release/3006.0 Jun 27, 2023
@meaksh meaksh deleted the openSUSE/fix/3006.0-fix-gitfs-known_hosts branch June 27, 2023 10:26
meaksh added a commit that referenced this pull request Oct 29, 2024
…E is not set (bsc#1210994) (#588)

* Prevent _pygit2.GitError: error loading known_hosts when $HOME is not set

* Add unit test to cover case of unset home
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.

[BUG] onedir blocks installation of shared libraries for pygit2 gitfs in 3006.0
3 participants