more ssh fixes #4675
more ssh fixes #4675
Conversation
After seeing 9a81078 it dawned on me that Arch Linux doesn't have /usr/lib/openssh, but uses /usr/lib/ssh instead. That's a different path than what's referenced in our current {allow-ssh,disable-common}.inc files. Some very superficial checks revealed that OpenSSH seems to be packaged quite differently, at least on Debian/Ubuntu and Arch Linux. And then there's version differences on non-rolling distro's to consider. All in all IMO it makes more sense to (no)blacklist /usr/lib/openssh and /usr/lib/ssh instead of referencing all the possible individual files that live under those paths.
Counterpart fix for changes in allow-ssh.inc.
Added Fedora path as per #4675 (review).
Added Fedora path as per #4675 (review). NOTE: there are several other profiles touching /usr/libexec, so untill someone on Fedora can shed some light on what files are installed under /usr/libexec, I only blacklisted ssh-keysign. I'll pick this up tomorrow, a bit pressed for time in the non-digital worlds...
|
@rusty-snake I've added a fix for Fedora in allow-ssh.inc. The disable-common.inc part might be a bit more complex, due to several other profiles touching /usr/libexec (and being unfamiliar with Fedora). Feel free to make suggestions! |
Suggested in #4675 (comment). Makes sense!
@reinerh Thanks for your review. I've brought in your suggestion. Not too clearheaded at the moment, great to see your in much better shape :-) |
There was a problem hiding this comment.
The code changes LGTM.
Could you fixup/squash the commits?
Example: With git rebase -i "$(git merge-base master HEAD)".
Also, the first commit has a (non-auto-generated) line that's over 500 chars
long, which makes it hard to read (and makes it stand out) on git log.
Suggestion: Replace the message with a formatted version:
disable-common.inc: blacklist more ssh lib paths
After seeing 9a81078 it dawned on me that Arch Linux doesn't have
/usr/lib/openssh, but uses /usr/lib/ssh instead. That's a different
path than what's referenced in our current
{allow-ssh,disable-common}.inc files. Some very superficial checks
revealed that OpenSSH seems to be packaged quite differently, at least
on Debian/Ubuntu and Arch Linux. And then there's version differences
on non-rolling distros to consider. All in all IMO it makes more sense
to (no)blacklist /usr/lib/openssh and /usr/lib/ssh instead of
referencing all the possible individual files that live under those
paths.
Also, add Fedora paths as per:
https://github.com/netblue30/firejail/pull/4675#pullrequestreview-802438767
(The above would be the message of the final commit, after all the other commits
are fixedup/squashed)
|
Fixed, thanks! |
See 9a81078#commitcomment-59921139 for details. Basically this PR (no)blacklists /usr/lib/{open,}ssh instead of referencing all possible individual files that might get installed there depending on OS and versioning.