YunoHost-Apps / my_webapp_ynh

Custom Web app with SFTP access
GNU General Public License v3.0
48 stars 39 forks source link

PasswordAuthentication yes in the upgrade script #21

Closed anmol26s closed 3 years ago

anmol26s commented 6 years ago

If authentication through is disabled in ssh, sftp fails. Adding PasswordAuthentication yes for the webapp user makes it work only for the weapp.


Problem

Solution

PR Status

Work finished. Package_check, basic tests and upgrade from last version OK.
Could be reviewed and tested.

Validation

Minor decision

anmol26s commented 6 years ago

Any update on it? In my case I have to add this line every time the app is installed because I have disabled password login in the ssh.

maniackcrudelis commented 6 years ago

Password login isn't disable by default on YunoHost. I don't think we should add this in the config. I've also some issues with my own ssh config, I handle it myself because I've changed my config.

But, I don't use that app myself. Any other point of view ?

anmol26s commented 6 years ago

If there is no issue regarding this to anyone else, I am ok with it.

zamentur commented 5 years ago

PasswordAuthentication is disable by default on several yunohost setup, currently it depends of your way to install yunohost. A PR, try to standardize the SSHD config on yunohost, but you can't consider passwordauthentication is disable or authorize.

So your proposition to specifically authorize this user seems to be the proper way

zamentur commented 5 years ago

Note this PR conflict with https://github.com/YunoHost/yunohost/pull/518

Precisely: here: https://github.com/YunoHost/yunohost/pull/518/files#diff-eae476b86ee57954ecbc550786e69a27R57

With this PR, it will be possible to use a sshd_config.d directory, like that change are manage in a proper way : https://github.com/YunoHost/yunohost/pull/518/files#diff-ca404b72f582b63a6e02136bd1726d6aR53

alexAubin commented 5 years ago

With this PR, it will be possible to use a sshd_config.d directory, like that change are manage in a proper way : https://github.com/YunoHost/yunohost/pull/518/files#diff-ca404b72f582b63a6e02136bd1726d6aR53

Update about this : this doesn't work :s

So not sure what to do exactly about this ... But there are some movement on the whole ssh config (to be standardized by the migration included in 3.4). Haven't checked the whole SFTP thing though :/

Naively I would refrain from doing brutal seds to the sshd_config. If we really want to extend the ssh conf, then there might be some trick with the conf_regen. I did this kind of thing with mailman because I wanted to extend the postfix conf without breaking the regenconf.

I added this hook : https://github.com/YunoHost-Apps/mailman_ynh/blob/master/sources/hooks/conf_regen/98-postfix_mailman

Which is called postfix_something and therefore is to be ran each time a postfix regenconf is triggered. You should be able to achieve the same with ssh.

The alternative can also be to add an "AllowGroup sftpusers" or something like this .. This is related to Josue's work on group permissions.

maniackcrudelis commented 5 years ago

Looks like we should have a sshd_config.d, https://github.com/YunoHost/yunohost/pull/518/files#diff-843b7c9e9160b9d943fc5cb50163d9c9R49 now. This PR should be reworked to use it, at least. Still don't know about the whole idea if it's good or bad.

alexAubin commented 5 years ago

Looks like we should have a sshd_config.d

N.B. : There is no ssh_config.d, and in fact no mechanism in sshd to create such a thing. I dunno how ljf came up with this thing but my understanding is that it simply does not exist...

For an alternative clean solution, c.f. my previous comment though it's a bit technical to create and manipulate those hooks, but at least it won't break the regenconf like manually editing the file does :s

maniackcrudelis commented 5 years ago

Should I understand that the migration do create the .d directory, but sshd doesn't use it ?

alexAubin commented 5 years ago

Hmmm nope I don't think the migration creates any .d ... There is just simply no possibility to include files in a sshd_config file ... This is apparently possible for client configuration but not server (sshd) configuration

alexAubin commented 5 years ago

c.f. those for instance : https://lists.gt.net/openssh/dev/68180 https://bugs.launchpad.net/ubuntu/+source/openssh/+bug/1701298

maniackcrudelis commented 5 years ago

Ok. But this line says "Create sshd_config.d dir".

Anyway, I trust you, I was just saying that because of the comment and the mkdir after, mkdir(SSHD_CONF + '.d'

alexAubin commented 5 years ago

Meh, yes indeed ... Didn't saw that line, should also have removed it in this commit : https://github.com/YunoHost/yunohost/pull/518/commits/25efab7f2a91243ef55438fc76cd2deb13fec3dd

I'll do it naow

maniackcrudelis commented 5 years ago

By the way, that's really too bad that sshd doesn't use a .d That would be better that way...

alexAubin commented 3 years ago

Not relevant anymore, was done in the app at some point, now managed by the core