[systemd-devel] [PATCH][RFC] bus-proxy: add support for "GetConnectionCredentials" method

[David Herrmann]

Hi

On Fri, Feb 20, 2015 at 1:20 PM, Simon McVittie
<simon.mcvittie at collabora.co.uk> wrote:
> On 19/02/15 21:26, Djalal Harouni wrote:
>>
>> On Thu, Feb 19, 2015 at 05:44:34PM +0100, Djalal Harouni wrote:
>>>
>>> On Thu, Feb 19, 2015 at 01:05:22PM +0000, Simon McVittie wrote:
>>>>
>>>> On 19/02/15 12:43, Lukasz Skalski wrote:
>>>>>
>>>>> r = get_creds_by_message(a, m, SD_BUS_CREDS_PID|SD_BUS_CREDS_EUID,
>>>>> &creds, &error);
>>>>
>>>>
>>>> Can this ever return "unknown" (-1?) for creds->pid or creds->euid?
>
>>>
>>>
>>> So, I'm missing lot of bits, but pid can be 0, euid can perhaps be
>>> (uid_t)-1 which is also a valid value... that maps to the INVALID_UID
>
>>
>>
>> btw you will hit this when you cross pid and user namespaces, and it
>> depends on the process receiving the creds if it has the rights to read
>> or map these fields, which may be the case of modern containers...
>
>
> Then the logic should be more like (pseudocode)
>
>     if (creds->pid > 0 && creds->pid < UINT32_MAX) {
>         r = ...append (..., (uint32_t) creds->pid);
>         if (r < 0)
>             ... error ...
>     }
>
> and the same for euid being in-range and not INVALID_UID (or whatever the
> placeholder value there is).
>
> GetConnectionCredentials does not specify a "bad value": anything that is
> added to the a{sv} is meant to be valid.
>
> As an implementation detail, the reference implementation in dbus-daemon
> will never put ((whatever_t) -1) in the a{sv}, even if ((uid_t) -1) or
> ((pid_t) -1) is somehow a valid uid/pid, because it uses that as the "bad
> value" internally. However, this is not part of the specification for
> GetConnectionCredentials, and the internal representation could be changed
> to a pair { uid_t id, bool valid } in a future dbus-daemon/libdbus version
> if it became necessary.
>
> In practice, I think there are Unix APIs that use ((uid_t) -1) as an error
> return (although I can't immediately remember what they are), and negative
> PIDs are used in kill(2), so if future Linux made these values available to
> real processes it would be an ABI break anyway.

-1 is not a valid value. It should be silently dropped, so we're fine here.

Thanks
David