Discussions about sudo use and fix/expansion of whitelist

[andrew-c-tran]

What happened (or feature request):

Command whitelisting is not correctly implemented, the sudo-prompt library is still accessible to the entire ddev-shell module, and the current sudo command whitelisting currently checks for whitelisted commands, but does not check all arguments for banned characters.

Furthermore, the current approach is to sudo ddev version to confirm user has sudo access prior to allowing them to enter the add site from CMS/add site from existing files flow. The intention here is that we don't want to allow the user to enter information for a site, unpack and create files on their system, only to kick them out during the hostname phase if they don't have sudo access, either leaving them with orphan files on their system or having to do a file deletion on the user's machine. This, however, is causing an additional sudo prompt during the flow when the actual hostname command is run on *nix systems that have tty_tickets disabled, or on windows systems. In short, do we want to risk annoying the user with potentially two sudo prompts, or to annoy them by potentially letting them unpack files onto their system and kick them out of the flow if they don't have sudo.

What you expected to happen:

• The sudo-prompt library needs to be abstracted into a higher level module that implements filtering, and that module to expose only the filtered method to the modules that depend on sudo. This eliminates the chances of accidentally calling the wrong sudo-module from within modules that require sudo.

• The aforementioned abstraction needs to not only check that the command being run is included in the whitelist, but also filter every argument for banned characters.

• A decision needs to be made on when it's appropriate to prompt the user for sudo.

How to reproduce this:

• notice that in ddev-shell.js there is a function named "sudo" that handles the whitelisting and the filtering. However, the full 'sudo-prompt' library has been loaded at this point, and it's public methods freely available to be accidentally called by any function in this module.

• no way to do this through the UI, but should -j ; rm -rf / be passed as an argument to 'hostname', the command would be accepted, and the bad -j argument would not be sanitized.

[andrew-c-tran]

I've made a bit of a compromise here.

On launch of the add flow, the info message warning that they may be prompted for permissions comes up. After they OK to that, they are allowed into the flow, and are prompted for sudo when hostname attempts to execute. I believe the info message warning is sufficient and should they abandon at this point, the files left on their drive are fair game as they were already warned they would need privs.

[andrew-c-tran]

no longer needed, issue addressed in pull #72

[rickmanelius]

I believe there was quite a bit of discussion in Slack that would be nice to pull into the issue as the updated punchlist. Thanks!

[rfay]

Per slack discussion starting at https://drud.slack.com/archives/C45DF87JT/p1517271861000124:

• We'd like to avoid some more of the priv escalation (and not escalate for existing hostnames, a common occurrence)
• Randy created https://github.com/drud/ddev/pull/626 - if we can get that in, then this PR can be updated to probe ddev hostname -j authoritatively (without privs) and if it gets WRITEERROR, then run it with privs.