openssh: add UCI support

[mhusaam]

Update init script to handle UCI and add a default config

Signed-off-by: Mohd Husaam Mehdi husaam.mehdi@iopsys.eu

[stokito]

The PR should be useful given that vendors of modern routers prefer the OpenSSH.

Currently we already have the /etc/config/dropbear config that has similar keys:
https://github.com/openwrt/openwrt/blob/main/package/network/services/dropbear/files/dropbear.init#L165

Maybe we can migrate the dropbear config to the sshd. E.g. first check if the dropbear exists and if not read the sshd. And similarly for the OpenSSH but it will install own config and that will cause a conflict which config to use.

Users may change the ssh server e.g. have the Dropbear and then install the OpenSSH. But old settings won't be used now e.g. Port number. Of course a user should know what he doing but still some people may lost an access.

Anyway, that would be extremely useful to support same options that the dropbear supports currently. Well, maybe not all, like the BannerFile is not critical at all.

Basically since the Dropbear imitates the sshd. It should be possible to just take its init file and just replace PROG and it must work. If something is not compatible we may patch the Dropbear as I did recently for the ssh-keygen.

[mhusaam]

The PR should be useful given that vendors of modern routers prefer the OpenSSH.

Currently we already have the /etc/config/dropbear config that has similar keys: https://github.com/openwrt/openwrt/blob/main/package/network/services/dropbear/files/dropbear.init#L165

Maybe we can migrate the dropbear config to the sshd. E.g. first check if the dropbear exists and if not read the sshd. And similarly for the OpenSSH but it will install own config and that will cause a conflict which config to use.

Users may change the ssh server e.g. have the Dropbear and then install the OpenSSH. But old settings won't be used now e.g. Port number. Of course a user should know what he doing but still some people may lost an access.

Anyway, that would be extremely useful to support same options that the dropbear supports currently. Well, maybe not all, like the BannerFile is not critical at all.

Basically since the Dropbear imitates the sshd. It should be possible to just take its init file and just replace PROG and it must work. If something is not compatible we may patch the Dropbear as I did recently for the ssh-keygen.

Hello, thank you for reviewing this PR. I am sorry for the delayed response.

I have updated the PR as follows:

• I have added support for migrating from dropbear to sshd. So, if someone has an image with dropbear and they upgrade their image to one which has sshd, sshd will look for dropbear config file, and if it is found, it will try to generate sshd config from that, and also try to copy the authorized_keys file.
• I have added support for more dropbear options.
• I have changed the approach from passing command line arguments to sshd daemon, to generating a sshd config file and passing that config file.

Kindly review.

[stokito]

Oh, sorry, I didn't noticed that the IdleTimeout is a Dropbear's thing https://www.mankier.com/8/dropbear#-I

One thing that I see here that if in a future we'll add the ClientAliveCountMax then we may have a conflict and add some priority for them. Also this may confuse that the option from the Dropbear is used for OpenSSH.
What if we simply rename it to ClientAliveInterval during migration? Yes, technically this will be not exactly the same and will effectively make the timeout triple bigger but that should not break anything. But code will be much simpler and smaller.

The LUCI doesn't show an option for the IdleTimeout and it's also not in the default Dropbraer's config. Not sure how many users use it. So maybe it's not that important.

[stokito]

I grepped the option enable , config_get enable and config_get_bool enable in packages and we do have a few configs that are using the enable instead of enabled. So to avoid accidental SSH shell loss after editing of the conf I think it would be better to check both enable and enabled.
In a documentation always use the enabled. I also think it would be good to make same for other places that use the enable: atftpd, collectd, dnsmasq, dropbear, natmap, nut, socat, sslh, openvpn. But in documentation warn that the enabled works only for new versions.
Please let me know what you think about this.

For bool options must be used config_get_bool instead of just config_get. It will parse no/off yes/on.

[stokito]

I checked and the Dropbear also has some options that weren't migrated:

• ForceCommand
• rsakeyfile but it was deprecated
• keyfile corresponds to HostKeyFiles
• SSHKeepAlive corresponds to (again) the ClientAliveInterval
• RecvWindowSize doesn't have a similar option in sshd

This is not critical at all, the PR can be merged and we can back to thos later.

[stokito]

I updated https://openwrt.org/docs/guide-user/base-system/dropbear with more detailed description of the SSHKeepAlive and IdleTimeout. Please check

[mhusaam]

Oh, sorry, I didn't noticed that the IdleTimeout is a Dropbear's thing https://www.mankier.com/8/dropbear#-I

One thing that I see here that if in a future we'll add the ClientAliveCountMax then we may have a conflict and add some priority for them. Also this may confuse that the option from the Dropbear is used for OpenSSH. What if we simply rename it to ClientAliveInterval during migration? Yes, technically this will be not exactly the same and will effectively make the timeout triple bigger but that should not break anything. But code will be much simpler and smaller.

The LUCI doesn't show an option for the IdleTimeout and it's also not in the default Dropbraer's config. Not sure how many users use it. So maybe it's not that important.

Hello,

• I think what is necessary is that the meaning of the option is preserved.
• Also, like you said, IdleTimeout is only for dropbear and ClientAliveCountMax and ClientAliveInterval are only for sshd (OpenSSH).
• So either the user can use dropbear option or the explicit sshd options.
• Accordingly I have updated the logic to give higher priority to sshd options if they are set explicitly.

[mhusaam]

I checked and the Dropbear also has some options that weren't migrated:

• ForceCommand
• rsakeyfile but it was deprecated
• keyfile corresponds to HostKeyFiles
• SSHKeepAlive corresponds to (again) the ClientAliveInterval
• RecvWindowSize doesn't have a similar option in sshd

This is not critical at all, the PR can be merged and we can back to thos later.

I have added support for the option keyfile in init script.
But I did not add it to migration script because uci_validate checks the presence of the file and if the file is not present then init script will not start sshd.
So, if the option is set and somehow the file is removed during upgrade (maybe because it was present in non-persistent storage, I know the chance of that is low) then ssh will not be possible to the device unless defaultreset is done.

As you said, maybe we can come back to this later.

[mhusaam]

Hello,

Sorry for the delayed response and if there seems to be many changes in the script. As the number of options increased, I felt that I could optimize the script. The following updates have been done in the latest patch:

• update config file location (review comment)
• use config_get_bool for boolean (review comment)
• check for both enable and enabled options to see if section is enabled (review comment)
• reduce the number of functions by introducing generic function for handling list values (set_option_list_comma and set_option_list_space)
• reduce shellcheck warnings by using && and || instead of -a and -o
• add support for more sshd options

Kindly review.

[mhusaam]

I grepped the option enable , config_get enable and config_get_bool enable in packages and we do have a few configs that are using the enable instead of enabled. So to avoid accidental SSH shell loss after editing of the conf I think it would be better to check both enable and enabled. In a documentation always use the enabled. I also think it would be good to make same for other places that use the enable: atftpd, collectd, dnsmasq, dropbear, natmap, nut, socat, sslh, openvpn. But in documentation warn that the enabled works only for new versions. Please let me know what you think about this.

For bool options must be used config_get_bool instead of just config_get. It will parse no/off yes/on.

For other packages that you have mentioned, yes maybe this could be added. But I don't have enough availability right now to raise more PRs. :(

[stokito]

When installing the sshd it will copy options from dropbear and remove (!) the dropbear config. But the dropbear will remain started and the port 22 will be busy.

I think that on migration we should copy options from the dropbear but disable the sshd e.g. uci set sshd.main.enabled=0.
Then a user can itself go and disable dropbear and enable the sshd and make the uci commit and restart the sshd. At least he will be in control and know what he is doing.

Also it looks like we need a protection from double migration after OpenWrt firmware upgade:

https://openwrt.org/docs/guide-developer/uci-defaults#ensuring_scripts_don_t_overwrite_custom_settingsimplementing_checks

The least but not least. What if a user just manualy configured the /etc/ssh/sshd_config and then upgreade the openssh-server package and now instead of his config the /etc/config/sshd is used.

We may need to add an option config_path /etc/ssh/sshd_config that will translated to Include /etc/ssh/sshd_config.

[mhusaam]

When installing the sshd it will copy options from dropbear and remove (!) the dropbear config. But the dropbear will remain started and the port 22 will be busy.

I think that on migration we should copy options from the dropbear but disable the sshd e.g. uci set sshd.main.enabled=0. Then a user can itself go and disable dropbear and enable the sshd and make the uci commit and restart the sshd. At least he will be in control and know what he is doing.

Also it looks like we need a protection from double migration after OpenWrt firmware upgade:

https://openwrt.org/docs/guide-developer/uci-defaults#ensuring_scripts_don_t_overwrite_custom_settingsimplementing_checks

The least but not least. What if a user just manualy configured the /etc/ssh/sshd_config and then upgreade the openssh-server package and now instead of his config the /etc/config/sshd is used.

We may need to add an option config_path /etc/ssh/sshd_config that will translated to Include /etc/ssh/sshd_config.

Hello,

Sorry for the delayed response.

We need to consider mirgation on upgrade too.

If we disable sshd, and someone upgrades with an image which has sshd (openssh), then sshd will get disabled and there will be no ssh possible to the device.

I have added support for option IncludeConfigFile.

Kindly review.

[feckert]

• please rebase the changes
• stash to one commit
• please bump PKG_RELEASE number by one

[mhusaam]

Updated, kindly review.