Disallow running Graphene as root

[boryspoplawski]

Description of the changes

Just don't.

---

This change is 

[boryspoplawski]

Jenkins, retest Jenkins-Debug-20.04 please (sendfile09 timed out, known issue)

[dimakuv]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @boryspoplawski)

a discussion (no related file):
But why?

What about Docker containers, where the user is typically root (though I'm not sure about accessibility of /proc/kallsyms inside Docker)? I don't know apps that restrict being run as root, is this something that people do?

Also, maybe just change to a warning instead of failing?

---

[woju]

Reviewed all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @boryspoplawski and @dimakuv)

a discussion (no related file):

is this something that people do?

Rarely, but this happens for several reasons. Wireshark bails out, because the corpus of packet dissectors is too big and probably vulnerable (and usually run over untrusted traffic), so only the capture process itself runs as root. X11 apps should not be run as root and some of them complain, because there will be problems with connecting to X server and friends (auth cookie, session dbus etc. -- while it can be done, it's not recommended).

I think the only reason people run graphene as root is because they can't configure their servers and sudo graphene-sgx is a magic incantation to overcome problems with permissions.

Also, maybe just change to a warning instead of failing?

Or better, an undocumented override like
if ! [ -z "$I_PROMISE_NOT_TO_SEND_SUPPORT_REQUESTS_ANYWHERE" ] && [ $(id ...) -eq 0 ].

---

---

Runtime/pal_loader, line 25 at r1 (raw file):

fi

if [ "00" -eq "0$(id -u 2>/dev/null)" ]

if [ 0"$(id ...)" -eq 0 ] is OK, -eq works on ints, not strings.

[boryspoplawski]

Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv and @woju)

a discussion (no related file):
@dimakuv

But why?

Because it's insecure and unsupported yet lots of people do that.

What about Docker containers, where the user is typically root (though I'm not sure about accessibility of /proc/kallsyms inside Docker)?

This checks if user is root and if they have CAP_SYSLOG, which is not true in default docker (but it would fail in --privileged one, but that's basically the same as running as root). Ideally we would check for CAP_SYSADMIN, but I couldn't find a way without manually parsing /proc/self/status and checking caps bits.

---

---

Runtime/pal_loader, line 25 at r1 (raw file):

Previously, woju (Wojtek Porczyk) wrote…

if [ 0"$(id ...)" -eq 0 ] is OK, -eq works on ints, not strings.

But I wanted to work on strings (in case id returns something thats not a number).
Should look better now.

[woju]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv)

---

Runtime/pal_loader, line 25 at r1 (raw file):

Previously, boryspoplawski (Borys Popławski) wrote…

But I wanted to work on strings (in case id returns something thats not a number).
Should look better now.

Yep.

[mkow]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @boryspoplawski and @dimakuv)

---

Runtime/pal_loader, line 25 at r2 (raw file):

fi

if [ -z "$I_AM_AWARE_OF_CONSEQUENCES_AND_I_WONT_SUBMIT_GRAPHENE_REPORTS_WITH_THIS_ENABLED" ] && \

GRAPHENE_REPORTS -> GRAPHENE_BUGREPORTS (it's not clear what "reports" you mean here)

[boryspoplawski]

Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv and @mkow)

---

Runtime/pal_loader, line 25 at r2 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

GRAPHENE_REPORTS -> GRAPHENE_BUGREPORTS (it's not clear what "reports" you mean here)

Any and all of them

[svenkata9]

Because it's insecure and unsupported yet lots of people do that.

@boryspoplawski I know executing as root in general is insecure. But, what is the specific issue w.r.t. Graphene?

[dimakuv]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @boryspoplawski and @mkow)

a discussion (no related file):

Previously, boryspoplawski (Borys Popławski) wrote…

@dimakuv

But why?

Because it's insecure and unsupported yet lots of people do that.

What about Docker containers, where the user is typically root (though I'm not sure about accessibility of /proc/kallsyms inside Docker)?

This checks if user is root and if they have CAP_SYSLOG, which is not true in default docker (but it would fail in --privileged one, but that's basically the same as running as root). Ideally we would check for CAP_SYSADMIN, but I couldn't find a way without manually parsing /proc/self/status and checking caps bits.

OK. I'm fine with the magic override I_AM_AWARE_OF_CONSEQUENCES_AND_I_WONT_SUBMIT_GRAPHENE_REPORTS_WITH_THIS_ENABLED.

---

a discussion (no related file):
From what I understand, @boryspoplawski wanted to add some documentation on how to config the system to NOT require root. So this PR is blocked on this.

---