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

[ssh] Stop collecting 'ls' information from non-local home dirs #3807

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions sos/report/plugins/ssh.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,25 @@ def user_ssh_files_permissions(self):
"""
users_data = pwd.getpwall()

fs_mount_info = {}
try:
with open('/proc/mounts', "r", encoding='UTF-8') as mounts_file:
for line in mounts_file:
(fs_file, fs_vstype) = line.split()[1:3]
fs_mount_info[fs_file] = fs_vstype
except Exception:
self._log_error("Couldn't read /proc/mounts")
return
non_local_fs = {'nfs', 'nfs4', 'autofs'}
# Read the home paths of users in the system and check the ~/.ssh dirs
for user in users_data:
if user.pw_dir in fs_mount_info and \
fs_mount_info[user.pw_dir] in non_local_fs:
self._log_info(
f"Skipping capture in {user.pw_dir}"
" because it's a remote directory"
)
continue
Copy link
Member

Choose a reason for hiding this comment

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

We should log something so that end users aren't left wondering why this wasn't collected when previously they may have been expecting it.

Copy link
Contributor

Choose a reason for hiding this comment

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

What verbosity level? Info? (or debug? isnt it too hidden..?)

Copy link
Member Author

Choose a reason for hiding this comment

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

With warn I get this:

 Setting up archive ...
 Setting up plugins ...
[plugin:ssh] Skipping capture in /home/jcastill because it's a remote directory
 Running plugins. Please wait ...

  Starting 1/1   ssh             [Running: ssh]

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, and I dont know if that is not too "vocal". I think info verbosity is the best, but I am ok with any.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right, in the specific scenario of tens or hundreds of NFS mounted homedirs, the output would be huge.

Copy link
Contributor

Choose a reason for hiding this comment

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

So the verbosity should not be warning, right..?

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly. I'm pushing a change with info now

home_dir = self.path_join(user.pw_dir, '.ssh')
self.add_dir_listing(home_dir)

Expand Down
Loading