Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Please reduce use of sudo in REAMDE #11875

Closed
madduck opened this issue Jan 29, 2017 · 8 comments
Closed

Please reduce use of sudo in REAMDE #11875

madduck opened this issue Jan 29, 2017 · 8 comments

Comments

@madduck
Copy link

@madduck madduck commented Jan 29, 2017

The README.md file uses a lot of sudo, e.g. to invoke curl or wget, but also to actually invoke youtube-dl. But I am sure you'll agree that use of sudo should be kept to an absolute minimum.

So I suggest to use curl/wget as user and then copy the file to /usr/local/bin using install (which also takes care of chmod), if that's even necessary. Often, the use will be single-user, and then it'd be better to use something like ~/bin.

@dstftw
Copy link
Collaborator

@dstftw dstftw commented Jan 29, 2017

but also to actually invoke youtube-dl

No it does not. No sudo is used to actually run youtube-dl in README.md (apart from built-in updating).

So I suggest to use curl/wget as user and then copy the file to /usr/local/bin using install (which also takes care of chmod), if that's even necessary.

Don't see much difference. You are still using sudo but also will need to deal with downloading as user somewhere in user directory and ensure not to overwrite if such file already exists. Just unnecessary clutter for nothing.

Often, the use will be single-user, and then it'd be better to use something like ~/bin.

You can't know the intention beforehand. Those who aware can handle this themselves. We provide generic instructions for average user that will just work.

@dstftw dstftw closed this Jan 29, 2017
@madduck
Copy link
Author

@madduck madduck commented Jan 29, 2017

Yeah, I just noticed it's used only for the updating. But still…

The difference is simple: at the moment, you are running complex toosl like curl and wget as root. You are also implicitly trusting the entire set of available CA certificates, which makes it a whole lot easier to launch a MITM-attack. Given the binary nature of youtube-dl, that'll go unnoticed, for sure, and the next thing is that you'll run my code as root…

Sure, it works for the average user, but you are also suggesting highly insecure practices and fueling average user naïvite about it.

And of course, you can't know the intentions beforehand, but I'd be surprised if the majority of the people doing manual installations do so for multiple users. If they just do it for themselves, then no root privilege is ever needed and it'd be a whole lot safer to not advocate for the use of sudo. Contrary to what you write, those who want to install it for multiple users will know what to do…

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Feb 1, 2017

Users are expected to verify downloaded files with GPG keys, so MITM attacks won't work. Indeed README.md should mention GPG keys...

@madduck
Copy link
Author

@madduck madduck commented Feb 3, 2017

Yes, they should do so in addition. However, I suspect that the kind of user for which this documentation is written (i.e. the one that's best told to use sudo to get the job done because they clearly don't know better themselves) will likely not even know what GPG is…

Whatever.

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Feb 3, 2017

IMO downloading files using sudo is safer - it prevents unauthorized modifications to downloaded files. youtube-dl checks GPG signatures during updates, too. (see youtube_dl/update.py), so it's not a big problem.

@madduck
Copy link
Author

@madduck madduck commented Feb 3, 2017

IMO downloading files using sudo is safer - it prevents unauthorized modifications to downloaded files.

How so?

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Feb 4, 2017

If youtube-dl is owned by the current user, all user processes can modify it at any time. You may argue that downloading with local user and chown to root solves the problem, but there's inotify, which allows user processes monitor file system changes and take actions.

If you have a Mac, you may want to compare Homebrew and common Linux package managers like apt-get/yum/pacman. The former is more fragile as installed files may change without a notice.

@madduck
Copy link
Author

@madduck madduck commented Feb 5, 2017

Your view of security is, uh, just too different for me to be able to participate in this any longer. Good luck.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.