Do not send data with DBUS Auth mechanisms#265
Merged
jsouthworth merged 1 commit intogodbus:masterfrom Aug 5, 2021
Merged
Conversation
When running inside a container using systemd dynamic user to run as an actual non-root user outside the container (i.e. UID 12345), the DBUS refuses to allow the godbus code to connect. This is because the godbus code sends the UID of the user. The sdbus C library sends "AUTH EXTERNAL\r\n", whereas godbus sends "AUTH EXTERNAL 30\r\n". Since godbus goes the extra step to tell DBUS that the current UID is 0 (hex 0x30, getuid() == 0, i,e, root), DBUS refuses to allow the process to connect because it sees the process is actually running as UID 12345 (for example) outside of the container. When the UID is not specified by just sending "AUTH EXTERNAL\r\n", DBUS sends back a line of "DATA\r\n" and expects a reply of "DATA\r\n". The godbus code specifically checks for "DATA" when waitingForOk in tryAuth(), but sends a "CANCEL\r\n" to the DBUS and errors out. Instead of erroring out, the tryAuth() code in godbus should reply with "DATA\r\n" when the status is 'waitingForOk' and the line read back from the DBUS begins with "DATA". Fixes godbus#264
Member
|
Looks good, thanks. |
joanbm
reviewed
Aug 26, 2021
| if name, _, status := m.FirstData(); bytes.Equal(v, name) { | ||
| var ok bool | ||
| err = authWriteLine(conn.transport, []byte("AUTH"), v, data) | ||
| err = authWriteLine(conn.transport, []byte("AUTH"), v) |
There was a problem hiding this comment.
Doesn't this break DBUS_COOKIE_SHA1 / AuthCookieSha1 authentication? data is the authentication payload including the cookie here.
If so perhaps it's better to make the change only affect AuthExternal, as far as I can tell reverting those lines and calling AuthExternal("") instead of AuthExternal(uid) should be enough.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When running inside a container using systemd dynamic user to run as an
actual non-root user outside the container (i.e. UID 12345), the DBUS
refuses to allow the godbus code to connect. This is because the godbus
code sends the UID of the user.
The sdbus C library sends "AUTH EXTERNAL\r\n", whereas godbus sends
"AUTH EXTERNAL 30\r\n". Since godbus goes the extra step to tell DBUS
that the current UID is 0 (hex 0x30, getuid() == 0, i,e, root), DBUS
refuses to allow the process to connect because it sees the process is
actually running as UID 12345 (for example) outside of the container.
When the UID is not specified by just sending "AUTH EXTERNAL\r\n", DBUS
sends back a line of "DATA\r\n" and expects a reply of "DATA\r\n". The
godbus code specifically checks for "DATA" when waitingForOk in
tryAuth(), but sends a "CANCEL\r\n" to the DBUS and errors out.
Instead of erroring out, the tryAuth() code in godbus should reply with
"DATA\r\n" when the status is 'waitingForOk' and the line read back from
the DBUS begins with "DATA".
Fixes #264