Re: [systemd-devel] [PATCH][RFC] bus-proxy: add support for GetConnectionCredentials method
Hi
On Fri, Feb 20, 2015 at 1:20 PM, Simon McVittie
simon.mcvit...@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 +, 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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH][RFC] bus-proxy: add support for GetConnectionCredentials method
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 +, 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.
--
Simon McVittie
Collabora Ltd. http://www.collabora.com/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH][RFC] bus-proxy: add support for GetConnectionCredentials method
On 20/02/15 12:43, David Herrmann wrote:
-1 is not a valid value. It should be silently dropped, so we're fine here.
Perhaps I'm misunderstanding what you're saying here and we're actually
agreeing, but for clarity:
The specification for GetConnectionCredentials deliberately does not
document any invalid values; members of the returned a{sv} should only
appear if their values are valid. If ((uint32_t) -1) is in fact never a
valid value for a UnixUserID or ProcessID, then the pairs
{ UnixUserID: (uint32_t) -1 } and { ProcessID: (uint32_t) -1 }
should never appear in the a{sv} at all.
Similarly, for LinuxSecurityLabel, if getsockopt SO_PEERSEC (or the
closest kdbus equivalent) fails, or if the kdbus message does not
contain the necessary information, the keys of the a{sv} should not
include LinuxSecurityLabel: the absence of a security label looks like
{}, not { LinuxSecurityLabel: [] } or
{ LinuxSecurityLabel: [ '\0' ] }.
--
Simon McVittie
Collabora Ltd. http://www.collabora.com/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH][RFC] bus-proxy: add support for GetConnectionCredentials method
On Thu, Feb 19, 2015 at 01:05:22PM +, Simon McVittie wrote: On 19/02/15 12:43, Lukasz Skalski wrote: GetConnectionCredentials method was added to dbus-1 specification more than one year ago. This method should return [...] as many credentials as possible for the process connected to the server, but at this moment only UnixUserID and ProcessID are defined by the specification. We should add support for next credentials after extending dbus-1 spec. As of dbus master (soon to be 1.9.12), LinuxSecurityLabel is also defined. It's the bytestring from SO_PEERSEC, whatever that means for the current LSM(s), with a trailing '\0' appended if there wasn't already one there. AppArmor, SELinux and Smack developers have all told me this is valid for their LSMs. Spec patches welcome for others, but I don't think there's a great deal of point in adding GetConnectionCredentials support for additional credentials that can be transferred over kdbus but not (securely) over AF_UNIX: anything with enough kdbus knowledge to know about those might as well be using kdbus directly. +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 -- Djalal Harouni http://opendz.org ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
