nextcloudpi icon indicating copy to clipboard operation
nextcloudpi copied to clipboard

multi factor authentication for ssh

Open theCalcaholic opened this issue 4 years ago • 7 comments

As proposed in #1035 I implemented an ncp app to manage multi (and single) factor authentication methods. Currently supported are:

  • password only (default)
  • public key authentication
  • public key + password
  • TOTP (google-authenticator based) + password

If multiple options are enabled, they will act as alternatives. However, if at least one mfa method is enabled, all single factor methods will be disabled automatically. Also, it is unfortunately impossible to use totp (or any PAM based) authentication methods with non-PAM password authentication. That makes some combinations invalid, e.g. having pubkey+pw and totp+pw both enabled.

Functionality wise I'm satisfied with the current state of the ncp app now. The only missing things that I'm aware of are documentation and localization at this point.

TODO:

  • [x] Implement provision of ssh public key
  • [x] Prevent invalid/broken combinations of parameters/authentication methods
  • [x] Fix terminal version (when run via ncp-config)
  • [x] ~Provide QR-Code in web interface (?)~
  • [x] ~Add localization~
  • [x] Test with debian buster

image

image

theCalcaholic avatar Dec 14 '19 02:12 theCalcaholic

Somehow the ncp app is currently broken, but only in the terminal (ncp-config). I'll have to investigate that...

theCalcaholic avatar Dec 14 '19 02:12 theCalcaholic

@nachoparker I have now implemented the ability to supply a public key. However, it get's caught by the sanitization, because SSH public keys contain spaces (specifically at this line). I know that it's hard to properly handle them in bash without security risks, but in some cases spaces are entirely valid. I have two ideas on how to approach this, please let me know what you think about it:

  1. Mark parameters in the nc-app config if they should be able to receive unsafe inputs. That way we only needed to pay attention when handling specific parameters and could apply extra caution.

  2. Sanitize parameters with spaces by replacing them (e.g. with %SPACE%). That way, if a script needs access to the unescaped variable it could just reverse the process with a simple search & replace (maybe a function for that should be provided via the library.sh).

  3. A combination of the two could also make sense, because it would probably be a good idea if a script knew when to revert potential sanitization.

theCalcaholic avatar Dec 14 '19 15:12 theCalcaholic

I have taken the liberty to implement the 3rd option as an example. If you are not fine with it, it can always be reverted.

theCalcaholic avatar Dec 14 '19 16:12 theCalcaholic

Hm, maybe it would be a good idea to add a few more text fields for ssh public keys...

I'll implement that in a bit (don't merge yet 😉)

theCalcaholic avatar Jan 02 '20 20:01 theCalcaholic

@nachoparker Alright, it's feature complete now

EDIT: Please note, that spaces (for fields which have 'allow_unsafe' set to true) will be shown as %SPACE% in ncp-config, because we can't pass parameters with spaces to dialog atm (possibly related to #1038).

theCalcaholic avatar Jan 10 '20 23:01 theCalcaholic

Thanks! I will take a look soon, but I don't know when since I have family visits for the following weeks.

nachoparker avatar Jan 11 '20 21:01 nachoparker

Sure, take your time :)

theCalcaholic avatar Jan 14 '20 11:01 theCalcaholic