support ssh configuration logic through sshd_config.d on google backend#155
support ssh configuration logic through sshd_config.d on google backend#155sergiocazzolato wants to merge 3 commits intocanonical:masterfrom
Conversation
Some images (like ubuntu kinetic) override the ssh configuration by using the .d logic This change makes sure these systems will have the needed sh configuration to run spread. As the first value wins, we need to use 00 for the confiugartion file to make sure this is the first file with that config
mvo5
left a comment
There was a problem hiding this comment.
Thank you for fixing spread with Ubuntu 22.10!
spread/google.go
Outdated
| echo root:%s | chpasswd | ||
|
|
||
| sed -i 's/^\s*#\?\s*\(PermitRootLogin\|PasswordAuthentication\)\>.*/\1 yes/' /etc/ssh/sshd_config | ||
| test -d /etc/ssh/sshd_config.d && echo -e 'PermitRootLogin=yes\nPasswordAuthentication=yes' > /etc/ssh/sshd_config.d/00-spread-settings.conf |
There was a problem hiding this comment.
Juerg just pointed that we probably need the above tweak in client.go:SetupRootAccess() too and probably also in lxd.go ?
There was a problem hiding this comment.
Look reasonable. Only two comments:
-
What happens if there are other entries in that directory which list these options? Is it last match wins, or first match wins? Should we comment them out first? How about this: if the directory exists, comment out the options in all files that exist there, and then add a new file with our options.
-
The -settings is redundant with the .conf, and given (1) above, I think we can name this nicely as just "spread.conf", assuming that's the pattern there.
|
Updated the change. |
Comemnt the lines in case there is a file with the PermitRootLogin or PasswordAuthentication. As the command could fail when there is not files in sshd_config.d dir it is added a || true to make sure the script is not going to fail because of that. Also it is being updated the name of the config file
| sed -i 's/^PermitRootLogin=/#PermitRootLogin=/g' /etc/ssh/sshd_config.d/* || true | ||
| sed -i 's/^PasswordAuthentication=/#PasswordAuthentication=/g' /etc/ssh/sshd_config.d/* || true | ||
| test -d /etc/ssh/sshd_config.d && echo -e 'PermitRootLogin=yes\nPasswordAuthentication=yes' > /etc/ssh/sshd_config.d/00-spread.conf |
There was a problem hiding this comment.
According to the sshd_config(5) manpage, the config file usually doesn't have a = between the key/value pairs. For example:
% grep = /etc/ssh/sshd_config
# This sshd was compiled with PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
| sed -i 's/^PermitRootLogin=/#PermitRootLogin=/g' /etc/ssh/sshd_config.d/* || true | ||
| sed -i 's/^PasswordAuthentication=/#PasswordAuthentication=/g' /etc/ssh/sshd_config.d/* || true |
There was a problem hiding this comment.
Config files might have whitespace before the the entry. also as @thp-canonical mentioned, the entry usually doesn't have a =.
| sed -i 's/^PermitRootLogin=/#PermitRootLogin=/g' /etc/ssh/sshd_config.d/* || true | |
| sed -i 's/^PasswordAuthentication=/#PasswordAuthentication=/g' /etc/ssh/sshd_config.d/* || true | |
| sed -i 's/^\s*\(PermitRootLogin\|PasswordAuthentication\)\>.*/# COMMENTED OUT BY SPREAD: \0/' /etc/ssh/sshd_config.d/* || true |
|
@sergiocazzolato could you please also extend the fix to the LXD backend?
|
Some images (like ubuntu kinetic) override the ssh configuration by using the new
sshd_config.ddirectory and no longer ship a/etc/sshd_configfile.This change makes sure these systems will have the needed shell configuration to run spread.
As the first value wins, we need to use
00-for the configuration file to make sure this is the first file with that config