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

curl: add support for a "--rootme" command line parameter #2444

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
@lamby

lamby commented Apr 1, 2018

Passing this parameter will download the specified URLs and execute them via sudo(8) using sh(1), saving countless keystrokes when installing modern software.

For example:

$ curl --rootme https://people.debian.org/~lamby/install.sh

Is equivalent to:

$ curl -qs https://people.debian.org/~lamby/install.sh | sudo sh -

Errors that occur when executing sudo(8) are marked with new CURLE_ROOTME internal error code (94), otherwise we wrap the return code of the executed script itself.

curl: add support for a "--rootme" command line parameter
Passing this parameter will download the specified URLs and
execute them via sudo(8) using sh(1), saving countless keystrokes
when installing modern software.

For example:

  $ curl --rootme https://people.debian.org/~lamby/install.sh

Is equivalent to:

  $ curl -qs https://people.debian.org/~lamby/install.sh | sudo sh -

Errors that occur when executing sudo(8) are marked with new
CURLE_ROOTME internal error code (94), otherwise we wrap the return
code of the executed script itself.

Signed-off-by: Chris Lamb <lamby@debian.org>
@badboy

This comment has been minimized.

Show comment
Hide comment
@badboy

badboy Apr 1, 2018

This would simplify the installation of rustup.rs quite a bit.

badboy commented Apr 1, 2018

This would simplify the installation of rustup.rs quite a bit.

@cym13

This comment has been minimized.

Show comment
Hide comment
@cym13

cym13 Apr 1, 2018

What if sudo isn't present on the machine? Shouldn't curl be less platform dependent and write its own escalation function? That could also lead to having this scheme work on windows with non-trivial effects on the market. Imagine if everybody could just "curl --rootme URL" instead of dealing with lengthy installers and such?

cym13 commented Apr 1, 2018

What if sudo isn't present on the machine? Shouldn't curl be less platform dependent and write its own escalation function? That could also lead to having this scheme work on windows with non-trivial effects on the market. Imagine if everybody could just "curl --rootme URL" instead of dealing with lengthy installers and such?

@CuriousA62A2A

This comment has been minimized.

Show comment
Hide comment
@CuriousA62A2A

CuriousA62A2A Apr 1, 2018

Wow, this is not only a bad idea it's also completely solveable with a very simple bash script for those who want to go this route.
Let's not put this trap into curl, it's simply a really bad idea.

CuriousA62A2A commented Apr 1, 2018

Wow, this is not only a bad idea it's also completely solveable with a very simple bash script for those who want to go this route.
Let's not put this trap into curl, it's simply a really bad idea.

@Anntoin

This comment has been minimized.

Show comment
Hide comment
@Anntoin

Anntoin Apr 1, 2018

Could have curl -rm as the short form.

Anntoin commented Apr 1, 2018

Could have curl -rm as the short form.

@imnotbad

This comment has been minimized.

Show comment
Hide comment
@imnotbad

imnotbad Apr 1, 2018

I honestly can't tell if this is a joke or not.

Not only is this a horrific idea to begin with, it also makes cURL more platform dependant. If you want to download random scripts from the internet and execute them as root automatically you can do that very simply already using any type of shell scripting (even windows cmd allows this). Adding a command that does this automatically is borderline out of scope for cURL and presents a large security risk to users anyway.

I'm honestly not sure why anyone would think this is a good idea. even if it saves you ~1 second of typing it's at the expensive of all of your system security. This is without mentioning the fact that a lot of shell scripts are not designed to be run as the root/admin user anyway.

imnotbad commented Apr 1, 2018

I honestly can't tell if this is a joke or not.

Not only is this a horrific idea to begin with, it also makes cURL more platform dependant. If you want to download random scripts from the internet and execute them as root automatically you can do that very simply already using any type of shell scripting (even windows cmd allows this). Adding a command that does this automatically is borderline out of scope for cURL and presents a large security risk to users anyway.

I'm honestly not sure why anyone would think this is a good idea. even if it saves you ~1 second of typing it's at the expensive of all of your system security. This is without mentioning the fact that a lot of shell scripts are not designed to be run as the root/admin user anyway.

@citrus-it

This comment has been minimized.

Show comment
Hide comment
@citrus-it

citrus-it Apr 1, 2018

Could have curl -rm as the short form.

Maybe have that for the form that execs sudo or pfexec (platform dependant) but have a faster internal implementation --root-me-really-fast, or -rm-rf for short.

citrus-it commented Apr 1, 2018

Could have curl -rm as the short form.

Maybe have that for the form that execs sudo or pfexec (platform dependant) but have a faster internal implementation --root-me-really-fast, or -rm-rf for short.

@ajgon

This comment has been minimized.

Show comment
Hide comment
@ajgon

ajgon Apr 1, 2018

but have a faster internal implementation --root-me-really-fast, or -rmrf for short.

I like the idea! Also, let's add --no-preserve-root to ensure, when command is finished the user will be still in a root shell.

ajgon commented Apr 1, 2018

but have a faster internal implementation --root-me-really-fast, or -rmrf for short.

I like the idea! Also, let's add --no-preserve-root to ensure, when command is finished the user will be still in a root shell.

@dw

This comment has been minimized.

Show comment
Hide comment
@dw

dw Apr 1, 2018

I find the naming of the parameter somewhat unnecessarily abrasive, perhaps we can encourage more inclusivity and comfort with a name like "--easy-install". There's no need to scare away new users!

dw commented Apr 1, 2018

I find the naming of the parameter somewhat unnecessarily abrasive, perhaps we can encourage more inclusivity and comfort with a name like "--easy-install". There's no need to scare away new users!

@CuriousA62A2A

This comment has been minimized.

Show comment
Hide comment
@CuriousA62A2A

CuriousA62A2A Apr 1, 2018

Oh I get it now. This was the best April's fools in quite a while. I totally fell for it.

CuriousA62A2A commented Apr 1, 2018

Oh I get it now. This was the best April's fools in quite a while. I totally fell for it.

@MarcelRaad

Great idea! I wonder why we haven't had that in TODO.

@stephenreay

This comment has been minimized.

Show comment
Hide comment
@stephenreay

stephenreay Apr 1, 2018

let's add --no-preserve-root

How about scanning the script for any invocations of rm or /bin/rm and passing --no-preserve-root to them too, while we're at it.

stephenreay commented Apr 1, 2018

let's add --no-preserve-root

How about scanning the script for any invocations of rm or /bin/rm and passing --no-preserve-root to them too, while we're at it.

@diooktput

This comment has been minimized.

Show comment
Hide comment
@diooktput

diooktput Apr 1, 2018

I hope that option will give me a critical warning first, although it can be overridden.

diooktput commented Apr 1, 2018

I hope that option will give me a critical warning first, although it can be overridden.

@stephenreay

This comment has been minimized.

Show comment
Hide comment
@stephenreay

stephenreay Apr 1, 2018

I hope that option will give me a critical warning first, although it can be overridden.

We're past the point of warnings here.

stephenreay commented Apr 1, 2018

I hope that option will give me a critical warning first, although it can be overridden.

We're past the point of warnings here.

@namtabmai

This comment has been minimized.

Show comment
Hide comment
@namtabmai

namtabmai Apr 1, 2018

There seem to be a few people who are concerned with security around this issue. Wouldn't it be wise to implement the parameter

--no-rootme

which completely disables this feature.

You can then make the --rootme the default action.

namtabmai commented Apr 1, 2018

There seem to be a few people who are concerned with security around this issue. Wouldn't it be wise to implement the parameter

--no-rootme

which completely disables this feature.

You can then make the --rootme the default action.

@lorkki

This comment has been minimized.

Show comment
Hide comment
@lorkki

lorkki Apr 1, 2018

This would simplify the installation of rustup.rs quite a bit.

Alas, rustup won't benefit from this feature since it's not meant to be installed as root.

lorkki commented Apr 1, 2018

This would simplify the installation of rustup.rs quite a bit.

Alas, rustup won't benefit from this feature since it's not meant to be installed as root.

@imnotbad

This comment has been minimized.

Show comment
Hide comment
@imnotbad

imnotbad Apr 1, 2018

i hate this day

imnotbad commented Apr 1, 2018

i hate this day

@badboy

This comment has been minimized.

Show comment
Hide comment
@badboy

badboy Apr 1, 2018

@lorkki you speak for you. Things not meant to be used does not mean people are not using it at all.

badboy commented Apr 1, 2018

@lorkki you speak for you. Things not meant to be used does not mean people are not using it at all.

@moparisthebest

This comment has been minimized.

Show comment
Hide comment
@moparisthebest

moparisthebest Apr 1, 2018

Contributor

I think this should imply --insecure to save even more keystrokes.

Contributor

moparisthebest commented Apr 1, 2018

I think this should imply --insecure to save even more keystrokes.

@Anntoin

This comment has been minimized.

Show comment
Hide comment
@Anntoin

Anntoin Apr 1, 2018

I think the general consensus is clear. Not only would this change be a great usability improvement. It sets the groundwork for curl to become the de-facto linux & osx package management tool.

Anntoin commented Apr 1, 2018

I think the general consensus is clear. Not only would this change be a great usability improvement. It sets the groundwork for curl to become the de-facto linux & osx package management tool.

@ryukinix

This comment has been minimized.

Show comment
Hide comment
@ryukinix

ryukinix Apr 1, 2018

HOLY FUCK, this is serious???? What are you doing?

ryukinix commented Apr 1, 2018

HOLY FUCK, this is serious???? What are you doing?

@rohan-molloy

This comment has been minimized.

Show comment
Hide comment
@rohan-molloy

rohan-molloy Apr 1, 2018

I like the idea of having a built in option that does the following

  1. Download to temporary file
  2. sh $temp
  3. delete temporary file

The option should also have an optional argument for integrity checking (hash)

I think the sudo component is very problematic and anything relating to privilege escalation is well outside of curl's scope

(lol forgot it's April 1 on the other side of the world)

rohan-molloy commented Apr 1, 2018

I like the idea of having a built in option that does the following

  1. Download to temporary file
  2. sh $temp
  3. delete temporary file

The option should also have an optional argument for integrity checking (hash)

I think the sudo component is very problematic and anything relating to privilege escalation is well outside of curl's scope

(lol forgot it's April 1 on the other side of the world)

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Apr 1, 2018

I can't thing of a single better addition to curl than this pull request. This will fast track YOTLD tenfold and we will all look back and know it started today, on April 1st, 2018.

ghost commented Apr 1, 2018

I can't thing of a single better addition to curl than this pull request. This will fast track YOTLD tenfold and we will all look back and know it started today, on April 1st, 2018.

@jlozadad

This comment has been minimized.

Show comment
Hide comment
@jlozadad

jlozadad Apr 1, 2018

@CuriousA62A2A woosh if not then u forgot /s lol

jlozadad commented Apr 1, 2018

@CuriousA62A2A woosh if not then u forgot /s lol

@jochem

This comment has been minimized.

Show comment
Hide comment
@jochem

jochem Apr 1, 2018

Regarding the comment of @dw about naming of the parameter "--easy-install": easy_install has been available as software to install Python packages over HTTP since 2004. I'm afraid naming a parameter for installing software using curl after another software installer would lead to mass confusal, unless we actually make curl able to run easy_install as well on the URL provided. Giving this more thought, I'd be absolutely delighted to also be able to do things like curl --dpkg-install http://somethingsomething.deb (of course sudo is implied here).

jochem commented Apr 1, 2018

Regarding the comment of @dw about naming of the parameter "--easy-install": easy_install has been available as software to install Python packages over HTTP since 2004. I'm afraid naming a parameter for installing software using curl after another software installer would lead to mass confusal, unless we actually make curl able to run easy_install as well on the URL provided. Giving this more thought, I'd be absolutely delighted to also be able to do things like curl --dpkg-install http://somethingsomething.deb (of course sudo is implied here).

@CuriousA62A2A

This comment has been minimized.

Show comment
Hide comment
@CuriousA62A2A

CuriousA62A2A Apr 1, 2018

@jlozadad I got much clearer when I had a nap and checked the date ;)

CuriousA62A2A commented Apr 1, 2018

@jlozadad I got much clearer when I had a nap and checked the date ;)

@ryukinix

Close this cancer, please

@beren12

This comment has been minimized.

Show comment
Hide comment
@beren12

beren12 Apr 1, 2018

Make it simple for people. Just use the exploit du jour and auto escalate to root every time.

beren12 commented Apr 1, 2018

Make it simple for people. Just use the exploit du jour and auto escalate to root every time.

@NeilHanlon

This comment has been minimized.

Show comment
Hide comment
@NeilHanlon

NeilHanlon Apr 1, 2018

lgtm. Merge it

NeilHanlon commented Apr 1, 2018

lgtm. Merge it

@jay jay added the cmdline tool label Apr 1, 2018

Show outdated Hide outdated src/tool_operate.c
@@ -1744,8 +1761,14 @@ static CURLcode operate_do(struct GlobalConfig *global,
strerror(errno));
}
if(config->rootme) {
int rc = WEXITSTATUS(pclose(outs.stream));

This comment has been minimized.

@geofft

geofft Apr 1, 2018

This is failing CI on macOS because WEXITSTATUS apparently expects its argument to be an lvalue (??). Also more relevantly, a) WEXITSTATUS isn't meaningfully defined if WIFEXITED isn't true: if it died from a signal you'll get some mangled version of the signal number and b) it's not usable as a curl error anyway. Try something like

int wstatus = pclose(outs.stream);
if (!result && (!WIFEXITED(wstatus) || WEXITSTATUS(wstatus) != 0)) {
    result = CURLE_WRITE_ERROR;
}

which matches the cases below.

@geofft

geofft Apr 1, 2018

This is failing CI on macOS because WEXITSTATUS apparently expects its argument to be an lvalue (??). Also more relevantly, a) WEXITSTATUS isn't meaningfully defined if WIFEXITED isn't true: if it died from a signal you'll get some mangled version of the signal number and b) it's not usable as a curl error anyway. Try something like

int wstatus = pclose(outs.stream);
if (!result && (!WIFEXITED(wstatus) || WEXITSTATUS(wstatus) != 0)) {
    result = CURLE_WRITE_ERROR;
}

which matches the cases below.

src/tool_operate.c: EXITSTATUS is not defined if !WIFEXITED.
We should naturally be as robust as possible in such security-minded code.

This also fixes the build on MacOS, as WEXITSTATUS apparently expects its
argument to be an lvalue on that platform. This is especially important as I
would suspect Mac users would utilise this feature more than *Nix or BSD
folks...

Thanks to Geoffrey Thomas for the pointers.
@feakuru

This comment has been minimized.

Show comment
Hide comment
@feakuru

feakuru Apr 1, 2018

This thread is so awesome! I love github so much! You are the best, guys! Haven't had this kind of laugh in a long while

P. S. There's something white on your shoulder 😉

feakuru commented Apr 1, 2018

This thread is so awesome! I love github so much! You are the best, guys! Haven't had this kind of laugh in a long while

P. S. There's something white on your shoulder 😉

@gfriloux

This comment has been minimized.

Show comment
Hide comment
@gfriloux

gfriloux Apr 1, 2018

This is so wrong.
It's not curl that should call sudo, but sudo that should call curl :
sudo http://victim.org/foo.sh
And sudo would detect its an URL, download script, and execute.
This would be a lot more convenient.

It may even be the shell that should be able to execute commands based on scripts/binaries that are stored remotely, so people not using sudo could execute them too, when already in a root shell.

gfriloux commented Apr 1, 2018

This is so wrong.
It's not curl that should call sudo, but sudo that should call curl :
sudo http://victim.org/foo.sh
And sudo would detect its an URL, download script, and execute.
This would be a lot more convenient.

It may even be the shell that should be able to execute commands based on scripts/binaries that are stored remotely, so people not using sudo could execute them too, when already in a root shell.

@beren12

This comment has been minimized.

Show comment
Hide comment
@beren12

beren12 Apr 1, 2018

The sad part of this really is that people are told to do this with certain software, and others think it's ok.

beren12 commented Apr 1, 2018

The sad part of this really is that people are told to do this with certain software, and others think it's ok.

@vaygr

This comment has been minimized.

Show comment
Hide comment
@vaygr

vaygr Apr 1, 2018

Just make sure that OpenBSD's doas is also supported. Portability matters! Plus, 1 extra port is always better than 2 extra ports :)

EDIT And Window' runas. Could actually be useful to lower the access-rights.

vaygr commented Apr 1, 2018

Just make sure that OpenBSD's doas is also supported. Portability matters! Plus, 1 extra port is always better than 2 extra ports :)

EDIT And Window' runas. Could actually be useful to lower the access-rights.

Show outdated Hide outdated src/tool_operate.c
@@ -487,6 +488,7 @@ static CURLcode operate_do(struct GlobalConfig *global,
outs.fopened = TRUE;
outs.s_isreg = FALSE;
}
#ifdef

This comment has been minimized.

@gfriloux

gfriloux Apr 1, 2018

Replace with #endif

@gfriloux

gfriloux Apr 1, 2018

Replace with #endif

@ahmadnassri

This comment has been minimized.

Show comment
Hide comment
@ahmadnassri

ahmadnassri commented Apr 2, 2018

🍿

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Apr 2, 2018

Member

Thank you everyone for playing. I'm happy to see that the CI finally turned green for this set and that so many of you found your way to the curl github repo, expressed your opinions and shared your concerns. I hope this will lead to some real PRs down the line in addition to more April Fool's pranks!

This particular PR is hereby closed.

Member

bagder commented Apr 2, 2018

Thank you everyone for playing. I'm happy to see that the CI finally turned green for this set and that so many of you found your way to the curl github repo, expressed your opinions and shared your concerns. I hope this will lead to some real PRs down the line in addition to more April Fool's pranks!

This particular PR is hereby closed.

@bagder bagder closed this Apr 2, 2018

@lamby

This comment has been minimized.

Show comment
Hide comment
@lamby

lamby Apr 2, 2018

@bagder My pleasure, and thanks for being such a good sport :)

lamby commented Apr 2, 2018

@bagder My pleasure, and thanks for being such a good sport :)

@arjamizo

This comment has been minimized.

Show comment
Hide comment
@arjamizo

arjamizo Apr 2, 2018

Can we also have this for mobiles devices (Android and iOS)?

arjamizo commented Apr 2, 2018

Can we also have this for mobiles devices (Android and iOS)?

@xdgc

This comment has been minimized.

Show comment
Hide comment
@xdgc

xdgc Apr 2, 2018

Also, as a representative of the financial service sector, I would like TLS1.3 session keys automatically sent to a designated multicast address.

xdgc commented Apr 2, 2018

Also, as a representative of the financial service sector, I would like TLS1.3 session keys automatically sent to a designated multicast address.

@tzvetkoff

This comment has been minimized.

Show comment
Hide comment
@tzvetkoff

tzvetkoff Apr 3, 2018

Also, please integrate known (and not known) rootkits so we don't bother setting up sudoers.d or making the binary SUID or whatever. 🤣

tzvetkoff commented Apr 3, 2018

Also, please integrate known (and not known) rootkits so we don't bother setting up sudoers.d or making the binary SUID or whatever. 🤣

@lock lock bot locked as resolved and limited conversation to collaborators Jul 2, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.