After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 617483 - Credentials passing
Credentials passing
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: network
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2010-05-02 23:38 UTC by David Zeuthen (not reading bugmail)
Modified: 2010-09-14 16:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Update GCredentials (22.75 KB, patch)
2010-07-20 18:29 UTC, David Zeuthen (not reading bugmail)
reviewed Details | Review
Patch with fixes (on top of previous one) (3.95 KB, patch)
2010-07-20 20:44 UTC, David Zeuthen (not reading bugmail)
none Details | Review

Description David Zeuthen (not reading bugmail) 2010-05-02 23:38:58 UTC
For GDBus I need to pass and receive credentials over transports like Unix Domain Sockets for authentication. Here's what I got so far:

First I wrote a GCredentials class that looks like [1]. I opted for the non-subclass approach where a GCredentials is basically a "bag of various credentials". It might be useful for a lot of other stuff (such as polkit). It is also useful because it varies _a lot_ what kind of credentials various POSIX implementations will give you.

For example on Linux, it is this (bits/socket.h)

 struct ucred
 {
   pid_t pid;                    /* PID of sending process.  */
   uid_t uid;                    /* UID of sending process.  */
   gid_t gid;                    /* GID of sending process.  */
 };

On Mac OS X it is this (sys/ucred.h)

 struct xucred {
        u_int   cr_version;             /* structure layout version */
        uid_t   cr_uid;                 /* effective user id */
        short   cr_ngroups;             /* number of advisory groups */
        gid_t   cr_groups[NGROUPS];     /* advisory group list */
 };

and honestly I don't know what it looks like on the BSDs or Solaris... but I don't think it's hyper important - with a loose enough abstraction we should be able to handle anyway.

Anyway, then I wrote a GUnixCredentialsMessage class that looks like [0] - this is a GSocketControlMessage subclass.

Then I added convenience functions like this

 GCredentials *
 g_unix_connection_receive_credentials (GUnixConnection      *connection,
                                        GCancellable         *cancellable,
                                        GError              **error);

 gboolean
 g_unix_connection_send_credentials (GUnixConnection      *connection,
                                     GCredentials         *credentials,
                                     GCancellable         *cancellable,
                                     GError              **error);

Then I wired up my GDBus code to use this (the client sends credentials as the very first step of the authentication process and the server readers it). That didn't work.

To actually get things to work on Linux, I needed to do this

    int opt_val = 1;
    if (setsockopt (g_socket_get_fd (socket),
                    SOL_SOCKET,
                    SO_PASSCRED,
                    &opt_val,
                    sizeof opt_val) != 0)
      {
        g_warning ("boo, error setting SO_PASSCRED: %m");
      }

and then things worked really well. I was also able to verify that the non-uid-0 clients gets EPERM if trying to fake the credentials and that uid 0 can fake them. So far so good.

There's a problem with the Linux approach. The problem  is that the kernel will offer credentials in recvmsg() calls even when the peer didn't attach them. 

However, if the peer *did* attach fake credentials you get those instead e.g. 

 # peer attached fake credentials (e.g. uid 501)
 recvmsg(11, {msg_name(0)=NULL, msg_iov(1)=[{"\0", 1}], msg_controllen=32,
 {cmsg_len=28, cmsg_level=SOL_SOCKET, cmsg_type=SCM_CREDENTIALS{pid=21705,
 uid=501, gid=0}}, msg_flags=0}, 0) = 1

vs.

 # peer didn't attach credentials at all
 recvmsg(8, {msg_name(0)=NULL, msg_iov(1)=[{"\0", 1}], msg_controllen=32,
 {cmsg_len=28, cmsg_level=SOL_SOCKET, cmsg_type=SCM_CREDENTIALS{pid=27249,
 uid=0, gid=0}}, msg_flags=0}, 0) = 1

So either Linux apps has to be smart and turn credentials on/off as appropriate or they need to deal with a bunch of unwanted GUnixCredentialsMessage objects (they can't ignore socket control messages at all - someone might send a file descriptor!)....

I'm not sure how to proceed here.... I'm pretty sure we need a GCredentials abstraction. I'm also pretty sure we need the GSocketControlMessage subclass... Thanks for any insight!

[0] :

GSocketControlMessage *g_unix_credentials_message_new                  (void);
GSocketControlMessage *g_unix_credentials_message_new_with_credentials (GCredentials *credentials);
GCredentials          *g_unix_credentials_message_get_credentials      (GUnixCredentialsMessage *message);

gboolean               g_unix_credentials_message_is_supported         (void);


[1] :

GCredentials    *g_credentials_new               (void);
GCredentials    *g_credentials_new_for_process   (void);

GCredentialType  g_credentials_get_credentials   (GCredentials    *credentials);
gboolean         g_credentials_has_credential    (GCredentials    *credentials,
                                                  GCredentialType  credential);
void             g_credentials_unset_credential  (GCredentials    *credentials,
                                                  GCredentialType  credential);

gboolean         g_credentials_same_user         (GCredentials    *credentials,
                                                  GCredentials    *other);

gboolean         g_credentials_has_unix_user     (GCredentials    *credentials);
guint64          g_credentials_get_unix_user     (GCredentials    *credentials);
void             g_credentials_set_unix_user     (GCredentials    *credentials,
                                                  guint64          user_id);

gboolean         g_credentials_has_unix_group    (GCredentials    *credentials);
guint64          g_credentials_get_unix_group    (GCredentials    *credentials);
void             g_credentials_set_unix_group    (GCredentials    *credentials,
                                                  guint64          group_id);

gboolean         g_credentials_has_unix_process  (GCredentials    *credentials);
guint64          g_credentials_get_unix_process  (GCredentials    *credentials);
void             g_credentials_set_unix_process  (GCredentials    *credentials,
                                                  guint64          process_id);

gboolean         g_credentials_has_windows_user  (GCredentials    *credentials);
const gchar     *g_credentials_get_windows_user  (GCredentials    *credentials);
void             g_credentials_set_windows_user  (GCredentials    *credentials,
                                                  const gchar     *user_sid);
Comment 1 David Zeuthen (not reading bugmail) 2010-05-02 23:41:51 UTC
Adding some relevant people. Thanks for any insight!
Comment 2 David Zeuthen (not reading bugmail) 2010-05-03 00:04:06 UTC
I think my gut feeling is that we want

 1. the GCredentials class (minus *_windows_user* methods for now)

 2. the GUnixCredentialsMessage subclass

 3. the g_unix_connection_receive_credentials() method +
    - on Linux, if SO_PASSCRED isn't enabled turn it on, then recvmsg()
      then turn it off

    - have docs state that this method is the preferred way of receiving
      creds since some platforms may do weird things

    - for platforms where you can get the creds from the peer without
      having the peer pass a message, we can still do something sensible
      here
Comment 3 David Zeuthen (not reading bugmail) 2010-05-03 00:26:11 UTC
Maybe we want the APIs to work like this

 - if we don't support credentials messages, then

    - g_unix_connection_send_credentials() write a NUL byte

    - g_unix_connection_receive_credentials() will use an out-of-band
      mechanism (such as Solaris' getpeerucred()) to get the credentials.
      If no such out-of-band mechanism is available, we fail with
      G_IO_ERROR_NOT_SUPPORTED so the client has a way of dealing with it.

Note that the app can always use g_unix_credentials_message_is_supported() to figure out if it can influence what credentials the remote peer will receive.

So with this, in the app client, the code is always like this

  credentials = g_credentials_new_for_process ();
  if (!g_unix_connection_send_credentials (socket_connection,
                                           credentials,
                                           cancellable,
                                           error))
    goto fail;

And in the app server it's like this:

  credentials = g_unix_connection_receive_credentials (socket_connection
                                                       cancellable,
                                                       error);
  if (credentials == NULL)
    {
      if (error->domain == G_IO_ERROR ||
          error->code == G_IO_ERROR_NOT_SUPPORTED)
        g_critical ("Return OS to dealer for upgrade");
      else
        goto fail; /* some genuine IO error */
    }
Comment 4 David Zeuthen (not reading bugmail) 2010-05-03 00:30:39 UTC
Also note that Solaris' struct ucred isn't very different what what I've envisioned for GCredentials (which is also inspired by dbus/dbus-credentials.c)

http://docs.sun.com/app/docs/doc/816-5168/ucred-get-3c
Comment 5 Dan Winship 2010-05-06 18:55:57 UTC
(In reply to comment #4)
> Also note that Solaris' struct ucred isn't very different what what I've
> envisioned for GCredentials (which is also inspired by dbus/dbus-credentials.c)
> 
> http://docs.sun.com/app/docs/doc/816-5168/ucred-get-3c

FTR FreeBSD has pid, uid, euid, gid, and groups (http://www.freebsd.org/cgi/man.cgi?query=recvmsg).

(In reply to comment #2)
> I think my gut feeling is that we want
> 
>  1. the GCredentials class (minus *_windows_user* methods for now)

definitely minus windows stuff. And since this is unix-specific is there any reason to not just use unixy names (uid, pid, etc, rather than unix_user, unix_process, etc).

What is g_credential_new() as opposed to new_for_process()? It just doesn't fill anything in, in case you're spoofing? Do you really need that? Can't you just start with the filled-in one and overwrite the parts you want to spoof?

BTW, the OS X docs (http://developer.apple.com/mac/library/documentation/Darwin/Reference/ManPages/man4/unix.4.html) say "there is no way for either party to influence the credentials presented to its peer", so if spoofing is supposed to be part of the API, you need some way to indicate failure.

Random thought: GCredentials could be a boxed type which was just an opaque version of the system creds type. (Eg, a GCredentials would be a struct ucred on Linux.)

>  2. the GUnixCredentialsMessage subclass

You'll need this as an implementation detail to make g_socket_control_message_deserialize() work, but I'm not sure it should be a public type, since really people should be using the GUnixConnection methods anyway, right?

>  3. the g_unix_connection_receive_credentials() method +

yup, the way you describe in comment 3.

Though really, making the sender create and then free a GCredentials is a little annoying since the normal case is to send unspoofed ones. You could just allow NULL for the creds. Or have separate g_unix_connection_send_credentials() and g_unix_connection_send_spoofed_credentials(). (Which could then return G_IO_ERROR_NOT_SUPPORTED on systems that don't support spoofing.) Or does anyone *actually* use spoofing anyway?
Comment 6 Colin Walters 2010-05-06 19:44:56 UTC
Spoofing would be useful to allow root to access user session buses.
Comment 7 David Zeuthen (not reading bugmail) 2010-05-06 22:35:47 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Also note that Solaris' struct ucred isn't very different what what I've
> > envisioned for GCredentials (which is also inspired by dbus/dbus-credentials.c)
> > 
> > http://docs.sun.com/app/docs/doc/816-5168/ucred-get-3c
> 
> FTR FreeBSD has pid, uid, euid, gid, and groups
> (http://www.freebsd.org/cgi/man.cgi?query=recvmsg).
> 
> (In reply to comment #2)
> > I think my gut feeling is that we want
> > 
> >  1. the GCredentials class (minus *_windows_user* methods for now)
> 
> definitely minus windows stuff. And since this is unix-specific is there any
> reason to not just use unixy names (uid, pid, etc, rather than unix_user,
> unix_process, etc).

(see below)

> What is g_credential_new() as opposed to new_for_process()? It just doesn't
> fill anything in, in case you're spoofing? Do you really need that? Can't you
> just start with the filled-in one and overwrite the parts you want to spoof?

Sure.

> BTW, the OS X docs
> (http://developer.apple.com/mac/library/documentation/Darwin/Reference/ManPages/man4/unix.4.html)
> say "there is no way for either party to influence the credentials presented to
> its peer", so if spoofing is supposed to be part of the API, you need some way
> to indicate failure.

Sure. On Linux, too. What happens is that g_socket_receive_message() will return G_IO_ERROR_PERMISSION_DENIED (because sendmsg() fails with EPERM) unless you are uid 0 (as per comment 0).

> Random thought: GCredentials could be a boxed type which was just an opaque
> version of the system creds type. (Eg, a GCredentials would be a struct ucred
> on Linux.)

Probably should do that.. I mean, the fewer abstractions we're trying to build, the better (I think). I don't want to make it a truly boxed type because I think we still want reference counting. So we'd extend the native credentials type through aggregation.

With this, we'd then always export

 g_credentials_has_unix_uid()
 g_credentials_get_unix_uid()
 g_credentials_set_unix_uid()

 g_credentials_has_unix_euid()
 g_credentials_get_unix_euid()
 g_credentials_set_unix_euid()

but g_credentials_has_unix_euid() would always return FALSE on Linux because there is no euid in Linux's struct ucred. I suppose that's OK. We'd also need to make all setters failable e.g. they need to take a GError and return a gboolean so we can return NOT_SUPPORTED.

(Or we could allow setting it but then fail if trying to send such a credentials object over the wire.)

Most importantly, I think we do want a g_credentials_same_user() method. In particular, I need this in GDBus to do this

 my_creds = g_credentials_new();
 if (!g_credentials_same_user (my_creds, creds_from_peer))
   /* disconnect peer */
 else
   /* peer is the same user, all good! */

without special-casing all over the place. I believe many other apps (in particular system-level services etc.) probably wants to do something similar.

For debugging we definitely want a g_credentials_to_string() method - in particular I need it for things like

 GDBus-debug:Auth: CLIENT: sent credentials `GCredentials:unix-user=500,unix-group=500,unix-process=17428'
 GDBus-debug:Auth: SERVER: received credentials `GCredentials:unix-user=500,unix-group=500,unix-process=17428'

which you get to see if you set G_DBUS_DEBUG=authentication.

> >  2. the GUnixCredentialsMessage subclass
> 
> You'll need this as an implementation detail to make
> g_socket_control_message_deserialize() work, but I'm not sure it should be a
> public type, since really people should be using the GUnixConnection methods
> anyway, right?

Well, there are various corner cases where, in order to implement a protocol, you want to send credentials as part of a message, e.g. you want to still be able to send credentials and not being forced to send a NUL byte.

(For example, this is the case for the D-Bus protocol and passing file descriptors. The spec says

 "The actual file descriptors need to be transferred via platform
  specific mechanism out-of-band. They must be sent at the same time
  as part of the message itself. They may not be sent before the first
  byte of the message itself is transferred or after the last byte
  of the message itself."

so in this case you can't use g_unix_connection_send_fd() because there might not be a NUL byte at all! Same reasons could exist for sending credentials.)

> >  3. the g_unix_connection_receive_credentials() method +
> 
> yup, the way you describe in comment 3.
> 
> Though really, making the sender create and then free a GCredentials is a
> little annoying since the normal case is to send unspoofed ones. You could just
> allow NULL for the creds. Or have separate g_unix_connection_send_credentials()
> and g_unix_connection_send_spoofed_credentials(). (Which could then return
> G_IO_ERROR_NOT_SUPPORTED on systems that don't support spoofing.) Or does
> anyone *actually* use spoofing anyway?

I'm not sure it's such a big deal. You normally only send credentials once per connection setup.

While on the topic, do we want something like this

 g_socket_get_peer_credentials (GSocket *, GCancellable *, GError **);

I think we do since we want to use it in the fallback implementation of g_unix_connection_get_credentials().

(Btw, I also think we need to do more research to figure out if Windows has a credentials concept and see how it fits in with GCredentials - IIRC you can definitely get the non-spoofed identity of the other end of a Win32 named pipe.)
Comment 8 David Zeuthen (not reading bugmail) 2010-05-07 01:35:41 UTC
(In reply to comment #7)
> (Btw, I also think we need to do more research to figure out if Windows has a
> credentials concept and see how it fits in with GCredentials - IIRC you can
> definitely get the non-spoofed identity of the other end of a Win32 named
> pipe.)

http://en.wikipedia.org/wiki/Access_token seems to provide a good overview about how this works on Windows....
Comment 9 Dan Winship 2010-05-07 14:26:28 UTC
(In reply to comment #7)
> Sure. On Linux, too. What happens is that g_socket_receive_message() will
> return G_IO_ERROR_PERMISSION_DENIED (because sendmsg() fails with EPERM) unless
> you are uid 0 (as per comment 0).

Yeah, but (assuming that I read the man pages correctly) on OS X even root
can't spoof (because the other end receives your credentials without you
actually *sending* anything, so you never have a chance to tell it that you
want to spoof. Presumably the same applies on Solaris.)

btw, do your comments in comment 0 about SO_PASSCRED imply that if you try to
do g_unix_connection_receive_credentials() on Linux, and the other side just
does 'send(sock, "\0", 1, 0)' instead of calling
g_unix_connection_send_credentials(), then the receiving end will receive an
apparently valid GCredentials object claiming that the remote end is root?

> (Or we could allow setting it but then fail if trying to send such a
> credentials object over the wire.)

Depends on how the API will be used, which I don't have a good sense of. 

> Most importantly, I think we do want a g_credentials_same_user() method. In
> particular, I need this in GDBus to do this
> 
>  my_creds = g_credentials_new();
>  if (!g_credentials_same_user (my_creds, creds_from_peer))

if (!g_credentials_is_self (creds_from_peer)) ?

either way you need to document very precisely what "same user" means, given that some platforms have spoofing, and some have separate uid/euid.

> > Though really, making the sender create and then free a GCredentials is a
> > little annoying....
> 
> I'm not sure it's such a big deal. You normally only send credentials once per
> connection setup.

OK, but if credentials are only being manipulated "occasionally" then there's
not a lot of advantage to making them refcounted instead of copied either.

> While on the topic, do we want something like this
> 
>  g_socket_get_peer_credentials (GSocket *, GCancellable *, GError **);
> 
> I think we do since we want to use it in the fallback implementation of
> g_unix_connection_get_credentials().

hm... and it returns G_IO_ERROR_NOT_SUPPORTED on platforms that don't let you
do that without sending a message? I guess we probably want this method if and only if GUnixCredentialsMessage is a public type.

> (Btw, I also think we need to do more research to figure out if Windows has a
> credentials concept and see how it fits in with GCredentials - IIRC you can
> definitely get the non-spoofed identity of the other end of a Win32 named
> pipe.)

Ah, that's why you had the windows stuff in the API before... I figured that
since this was a GUnixConnection API that you were talking about wacky samba
PAM stuff or something.

If we want this to be more of a generic identity thing then my arguments about
it being a boxed version of ucred, etc, are less relevant.
Comment 10 David Zeuthen (not reading bugmail) 2010-05-07 16:06:29 UTC
(In reply to comment #9)
> (In reply to comment #7)
> > Sure. On Linux, too. What happens is that g_socket_receive_message() will
> > return G_IO_ERROR_PERMISSION_DENIED (because sendmsg() fails with EPERM) unless
> > you are uid 0 (as per comment 0).

(Btw, I meant g_socket_send_message(), not _receive_)

> Yeah, but (assuming that I read the man pages correctly) on OS X even root
> can't spoof (because the other end receives your credentials without you
> actually *sending* anything, so you never have a chance to tell it that you
> want to spoof. Presumably the same applies on Solaris.)

Neither Mac OS X nor Solaris allows sending credentials via control messages AFAIK - but they do provide a way to read the credentials from the socket via e.g. getpeerucred(3C) on Solaris or LOCAL_PEERCRED since OS X 10.4.

> btw, do your comments in comment 0 about SO_PASSCRED imply that if you try to
> do g_unix_connection_receive_credentials() on Linux, and the other side just
> does 'send(sock, "\0", 1, 0)' instead of calling
> g_unix_connection_send_credentials(), then the receiving end will receive an
> apparently valid GCredentials object claiming that the remote end is root?

Yup, that's exactly what I meant (assuming the remote end is actually root). It's only if you actually send fake credentials (which requires you to be uid 0) that the other end will receive these fake credentials.

> > Most importantly, I think we do want a g_credentials_same_user() method. In
> > particular, I need this in GDBus to do this
> > 
> >  my_creds = g_credentials_new();
> >  if (!g_credentials_same_user (my_creds, creds_from_peer))
> 
> if (!g_credentials_is_self (creds_from_peer)) ?

_is_self() could also mean comparing process ids...

> either way you need to document very precisely what "same user" means, given
> that some platforms have spoofing, and some have separate uid/euid.

Right. It really depends what we want to use GCredentials for. It would probably be worth thinking about this from a 50,000 feet point of view including how such a data type can be used in other contexts than D-Bus.

Here's some food for thought: PolicyKit has two abstract data-types: Identity and Subject with a couple of concrete implementations

 Identity
   UnixUser
   UnixGroup

 Subject
   UnixSession
   UnixProcess
   SystemBusName

 (see http://hal.freedesktop.org/docs/polkit/ for more details)

The idea is roughly that

 Subject ~= running process

and

 Identity ~= "something that you are (a part of)"

See the various polkit API for example usage.

This whole polkit setup is not that different from various kernel-credential mechanisms, except that it allows for identifiers that the kernel knows nothing about. For example, a unique system bus name or a ConsoleKit session identifier.

So one way I like to think about the GCredentials type is that it should be versatile enough to represent such concepts.

I think the main question really is this: Do we want to build a new (useful) abstraction (with all the benefits and risks that entails) or do we just box the existing (but not as useful) abstraction provided by the OS kernel? I honestly don't know.

(Random thought: we probably want a g_credentials_check() method that verifies  by looking in /proc, speaking to the ConsoleKit daemon etc., that the credentials are correct and not spoofed.)

> OK, but if credentials are only being manipulated "occasionally" then there's
> not a lot of advantage to making them refcounted instead of copied either.

Good point.

> > While on the topic, do we want something like this
> > 
> >  g_socket_get_peer_credentials (GSocket *, GCancellable *, GError **);
> > 
> > I think we do since we want to use it in the fallback implementation of
> > g_unix_connection_get_credentials().
> 
> hm... and it returns G_IO_ERROR_NOT_SUPPORTED on platforms that don't let you
> do that without sending a message?

Yep.

> > (Btw, I also think we need to do more research to figure out if Windows has a
> > credentials concept and see how it fits in with GCredentials - IIRC you can
> > definitely get the non-spoofed identity of the other end of a Win32 named
> > pipe.)
> 
> Ah, that's why you had the windows stuff in the API before... I figured that
> since this was a GUnixConnection API that you were talking about wacky samba
> PAM stuff or something.
> 
> If we want this to be more of a generic identity thing then my arguments about
> it being a boxed version of ucred, etc, are less relevant.

That's the question. I don't really know. I'm tempted to just make GCredentials a simple GObject-derived type with this API

 /* returns struct ucred on Linux, ucred_t on Solaris,
  * struct xucred on BSD and so on
  */
 gpointer g_credentials_get_native    (GCredentials *credentials);

 gboolean g_credentials_is_same_user  (GCredentials  *credentials,
                                       GCredentials  *other,
                                       GError       **error);

e.g. basically boxing the native type (but doing so through aggregation) since it doesn't try to create a new abstraction. Notably

 - get_native() may return NULL if not implemented

 - is_same_user() can return G_IO_ERROR_NOT_IMPLEMENTED if we don't know
   how to compare stuff or if not implemented at all
Comment 11 Dan Winship 2010-05-07 16:28:50 UTC
(In reply to comment #10)
f> Neither Mac OS X nor Solaris allows sending credentials via control messages
> AFAIK - but they do provide a way to read the credentials

right, but my point was, there is no way to *spoof* credentials on those systems, even if you are root. So any program that depends on the ability of root to spoof as other users will not work right on OS X and Solaris. Eg, if there is some program that is running as root but needs to connect to the user's session bus, it will be able to do that on Linux, but not Solaris.

(Actually, it might be able to do it if it does a seteuid before connecting, and then changes back after it knows the other side has read its credentials? But that's a fundamentally different sort of thing than just writing a false uid into a data structure.)

> > btw, do your comments in comment 0 about SO_PASSCRED imply that if you try to
> > do g_unix_connection_receive_credentials() on Linux, and the other side just
> > does 'send(sock, "\0", 1, 0)' instead of calling
> > g_unix_connection_send_credentials(), then the receiving end will receive an
> > apparently valid GCredentials object claiming that the remote end is root?
> 
> Yup, that's exactly what I meant (assuming the remote end is actually root).

oh, good, that's not how I parsed your original comment. I thought you were saying that if the sender didn't pass SO_PASSCRED, then the receipient would always receive uid=0 *regardless of whether or not the sender was root*.
Comment 12 David Zeuthen (not reading bugmail) 2010-05-09 14:12:38 UTC
OK, I reworked GCredentials as we discussed on IRC, see

 http://git.gnome.org/browse/glib/tree/gio/gcredentials.h?h=gdbus-merge
 http://git.gnome.org/browse/glib/tree/gio/gcredentials.c?h=gdbus-merge

Some notes

 - GCredentials need to be available on all platforms so higher-level
   API like GDBusConnection, GSocket etc. can use it

 - We don't want to build our own abstraction - instead we want to rely
   on the native types (struct ucred, struct xucred) and allow apps to
   set/get this data. IOW, GCredentials is a simple wrapper around the
   native type

 - To write cross-platform apps we have two simple "abstractions"

    - is_same_user() - to compare users
    - get|set_unix_user - to spoof credentials

   that may not even be implemented. This should cover 95%-99% of
   all applications.

 - For logging, we have a simple g_credentials_to_string() method
   with very few guarantees.

Thoughts?
Comment 13 David Zeuthen (not reading bugmail) 2010-05-09 14:16:16 UTC
Note to self: still need to do

 - Add g_socket_get_peer_credentials() method

 - Add quirks described in comment 3 to the
   g_unix_connection_send|receive_credentials() methods
  (these are already in the gdbus-merge branch)
Comment 14 David Zeuthen (not reading bugmail) 2010-05-19 22:41:38 UTC
Bug 619142 comment 3 is related to this.
Comment 15 David Zeuthen (not reading bugmail) 2010-07-20 18:29:13 UTC
Created attachment 166231 [details] [review]
Update GCredentials

Here's an update for GCredentials

 - Make GCredentials instance and class structures private so it can't
   be subclassed and we don't have to worry about ABI compat
   issues. This also allows us to get rid of the GCredentialsPrivate
   struct.

 - Add a GCredentialsType enumeration that is used whenever exchanging
   pointers with the user. This allows us to support OSes with
   multiple native credential types. In particular, it allows
   supporting OSes where the native credential evolves or even changes
   over time.

 - Add g_socket_get_credentials() method.

 - Add tests for g_socket_get_credentials(). Right now this is in the
   GDBus peer-to-peer test case but we can change that later.

 - Move GTcpConnection into a separate gtk-doc page as was already
   half-done with GUnixConnection. Also finish the GUnixConnection
   move and ensure send_credentials() and receive_credentials()
   methods are in the docs. Also nuke comment about GTcpConnection
   being empty compared to its superclass.
Comment 16 Dan Winship 2010-07-20 19:17:07 UTC
Comment on attachment 166231 [details] [review]
Update GCredentials

Yeah, looks good.

>+ * - see the <literal>unix(7)</literal> man page for details. This

if you want to win the DocBook Guru badge, try
<citerefentry><refentrytitle>unix</refentrytitle><manvolnum>7</manvolnum></citerefentry>
(though I don't know that gtk-doc will DTRT with that, either typographically or href-ly).

>  * g_credentials_get_native:

>+ * Returns: The pointer to native credentials or %NULL if the
>+ * operation there is no #GCredentials support for the OS or if
>+ * @native_type isn't supported by the OS.

Presumably it will also return an error if the OS supports multiple types and the type in @credentials can't be converted to @native_type. Which suggests maybe you should have g_credentials_get_credentials_type() too?

>+/* for struct ucred */
>+#include <stdlib.h>
>+#ifdef __linux__
>+#define __USE_GNU
>+#include <sys/types.h>
>+#include <sys/socket.h>
>+#endif

I tried to hide as much of this stuff as possible in gnetworkingprivate.h, which you can feel free to tweak to make the right symbols show up rather than adding lots of #defines and #ifdefs to each file.

>+/**
>+ * g_socket_get_credentials:

you should include a reference to g_unix_connection_receive_credentials() from here (and vice versa).
Comment 17 David Zeuthen (not reading bugmail) 2010-07-20 20:44:08 UTC
Created attachment 166239 [details] [review]
Patch with fixes (on top of previous one)

(In reply to comment #16)
> (From update of attachment 166231 [details] [review])
> Yeah, looks good.

Thanks for the quick review! I've committed the patch with this one on top.

> >+ * - see the <literal>unix(7)</literal> man page for details. This
> 
> if you want to win the DocBook Guru badge, try
> <citerefentry><refentrytitle>unix</refentrytitle><manvolnum>7</manvolnum></citerefentry>
> (though I don't know that gtk-doc will DTRT with that, either typographically
> or href-ly).

Done. No href's in my local build but I think some sites run a script to fix such things up, I don't know.

> >  * g_credentials_get_native:
> 
> >+ * Returns: The pointer to native credentials or %NULL if the
> >+ * operation there is no #GCredentials support for the OS or if
> >+ * @native_type isn't supported by the OS.
> 
> Presumably it will also return an error if the OS supports multiple types and
> the type in @credentials can't be converted to @native_type. Which suggests
> maybe you should have g_credentials_get_credentials_type() too?

I was thinking that the stuff stored in GCredentials is always the richest native type and that there are always conversion functions to other native types (and from any native type to another). So the idea is that get_native() will always work for any GCredentialsType. So g_credentials_get_type() wouldn't be necessary.

I'm not sure exactly how all this is going to work out, I mainly added the GCredentialsType enumeration so we could be future proof in the (unlikely) event that something better than 'struct ucred' comes along. I don't expect this to happen though.

> I tried to hide as much of this stuff as possible in gnetworkingprivate.h,
> which you can feel free to tweak to make the right symbols show up rather than
> adding lots of #defines and #ifdefs to each file.

Neat. Done.

> 
> >+/**
> >+ * g_socket_get_credentials:
> 
> you should include a reference to g_unix_connection_receive_credentials() from
> here (and vice versa).

Done.
Comment 18 David Zeuthen (not reading bugmail) 2010-07-20 20:45:39 UTC
Here's the patch that I committed

(patches from comment 15 and comment 17 squashed together)

 http://git.gnome.org/browse/glib/commit/?id=7eba41346e014649d8f9cf8ab675d1f091f7cf38
Comment 19 Dan Winship 2010-07-20 20:55:18 UTC
(In reply to comment #17)
> I was thinking that the stuff stored in GCredentials is always the richest
> native type and that there are always conversion functions to other native
> types (and from any native type to another).

I was thinking of a case where you have something like:

  struct old_credentials_type {
      uid_t uid;
  }

  struct new_credentials_type {
      uid_t uid;
      gid_t gid;
  }

If some API hands you an old_credentials_type, and then the app asks for a new_credentials_type, what do you put in for the gid?

(I don't know if this situation is actually going to come up, but...)
Comment 20 David Zeuthen (not reading bugmail) 2010-07-20 21:09:00 UTC
(In reply to comment #19)
> (In reply to comment #17)
> > I was thinking that the stuff stored in GCredentials is always the richest
> > native type and that there are always conversion functions to other native
> > types (and from any native type to another).
> 
> I was thinking of a case where you have something like:
> 
>   struct old_credentials_type {
>       uid_t uid;
>   }
> 
>   struct new_credentials_type {
>       uid_t uid;
>       gid_t gid;
>   }

Yeah, that's what I had in mind too - e.g. we probably only want to support (telescopic) extensions of the credentials type.

> If some API hands you an old_credentials_type, and then the app asks for a
> new_credentials_type, what do you put in for the gid?

Either 0 or some "unset value" sentinel (e.g. -1) or some algorithm including looking it up (primary group) that could fail...

> (I don't know if this situation is actually going to come up, but...)

I don't think it will. And if it does, we can always add the g_credentials_get_native_type() call. I also think that even if the native credential type grows, then we'll always the biggest type at the sites where we need to create a GCredentials object (GUnixCredentialsMessage, g_socket_get_credentials(), g_credentials_new()). So maybe we should just make g_credentials_set_native() private (for now at least)?

Btw, I guess the way Linux works is that most people use the pid_t in struct ucred and look up stuff in /proc/$pid using that - maybe that's why struct ucred is so tiny on Linux compared to other UNIX implementations?
Comment 21 David Zeuthen (not reading bugmail) 2010-09-14 15:35:49 UTC
After talking to Ryan on IRC, we identified some painpoints

 - it's not nice to have get|set_unix_user on the GCredentials class
  - and is this the uid or euid?

 - it's problematic that GCredentials only holds one native credentials
   as it's not far fetched that in the future one could send both
   'struct ucred' and e.g. 'struct selinux_stuff'

 - does is_same_user() user the uid or the euid?

 - often you really just want the pid_t so you can ask the PolicyKit
   authority (cf. http://hal.freedesktop.org/docs/polkit/ ) to make
   the authorization decision for you

We also agreed that g_socket_get_credentials() should be called g_socket_get_peer_credentials().

We didn't really agree on how to best fix this. So here's a concrete proposal from me

 - Make GCredentials an abstract type
   - with is_same_user(creds, other_creds)
   - with is_self(creds)
     - with the note that this compares the "effective identity" (e.g.
       euid instead of uid)
   - Make it possible for GCredentials to hold multiple native
     credentials, e.g.
     - (actually the API is already set up like that)
   - Make it possible to get the GPid of the other peer. This should
     be able to fail.
   - could have failable g_credentials_new(GError *error) that
     returns whatever is "natural" and fails if GCredentials isn't
     supported
     - maybe not

 - Make GUnixCredentials an abstract subtype of GCredentials
   - getters/setters for
     - uid, euid
     - gid, egid
     - pid
   - warning: these getters/setters can fail! That's fine!
   - constructor: new() - gives you the effective credentials for the process
   - constructor: new_real() - gives you real credentials for the process

 - Make GLinuxCredentials a private concrete type that is a subclass
   of GUnixCredentials
   - for now it only accepts 'struct ucred' credentials
     - e.g. these are the only you can set/get
   - in the future it could accept 'struct selinux_stuff' credentials too
   - we can always make the type public if we need to expose SELinux stuff

 - Ditto with GFreeBSDCredentials
Comment 22 Allison Karlitskaya (desrt) 2010-09-14 15:47:12 UTC
I think these two points stand in direct conflict:

   - Make it possible for GCredentials to hold multiple native
     credentials, e.g.

 - Make GUnixCredentials an abstract subtype of GCredentials

You can't give a multiply-subclassed GCredentials.


My idea (here for documentation) is that GCredentials is a random collection of pieces of information that may or may not be set:

 - maybe has a uid  (actually, maybe we can guarantee at least this one)
 - maybe has a euid
 - maybe has a primary gid
 - maybe has a list of groups
 - maybe has a pid
 - maybe has a label
 - maybe has a DBus system bus name
 - maybe has a session ID

Something like a GFileInfo, actually.  Anything that isn't set could be -1 or NULL or similar.

In no way does a GCredentials contain a socket control message of any kind.  No part of the GCredentials code would be unix-specific (even in the implementation).

We would, however, have some call for transfering the contents of a socket control message into an existing GPermission.

  - on Linux, that would fill in the uid, gid, pid fields of the GCredentials.
  - on BSDs, it could fill the uid, euid, gid, pid and group list.
  - on other platforms, it can do whatever those other platforms do.

The GUnixCredentialsMessage code would certainly contain many #ifdefs.  In fact, it might actually make sense to have separate GUnixCredentialMessage types for separate platforms (and a virtual "transfer what you know into a GCredentials" call).

Note that we could transfer data from multiple sources into the GCredentials.  So we can use a 'credentials' socket control message to get uid/gid/pid and some other (trusted) source to get DBus system bus name, for example.
Comment 23 Allison Karlitskaya (desrt) 2010-09-14 15:55:27 UTC
A couple of other notes:

Expanding on my analogy above, I think GCredentials is to 'struct ucred' as GFileInfo is to 'struct stat'.  It does not contain it, but it contains a lot of the information from it (and possibly from other sources).

As an interesting side note, imagine being able to obtain GCredentials from a GDBusMethodInvocation -- could at least fill in the dbus system bus ID and uid/pid/selinux-context/ADT-data (if you do some IO).  Definitely useful, and it has absolutely nothing to do with socket control messages.
Comment 24 David Zeuthen (not reading bugmail) 2010-09-14 16:05:50 UTC
(In reply to comment #22)
> I think these two points stand in direct conflict:
> 
>    - Make it possible for GCredentials to hold multiple native
>      credentials, e.g.
> 
>  - Make GUnixCredentials an abstract subtype of GCredentials
> 
> You can't give a multiply-subclassed GCredentials.

You won't ever need to (why do you think we need to?). In fact, you can also just skip the GUnixCredentials intermediate class if you don't want to pretend you are a Unix implementation.

The point about current GCredentials and the enhancements proposed in comment 21 is simple: don't try to abstract something that can't be abstracted.

> My idea (here for documentation) is that GCredentials is a random collection of
> pieces of information that may or may not be set:
> 
>  - maybe has a uid  (actually, maybe we can guarantee at least this one)
>  - maybe has a euid
>  - maybe has a primary gid
>  - maybe has a list of groups
>  - maybe has a pid
>  - maybe has a label
>  - maybe has a DBus system bus name
>  - maybe has a session ID
> 
> Something like a GFileInfo, actually.  Anything that isn't set could be -1 or
> NULL or similar.

I don't like this - it's an abstraction and, i think, an unnecessary one at that. It will create a lot of work down the road (same way GFileInfo is a lot of code) - for example, if we abstract things this way there is the risk that we need to add API and code for lots of random items down the road - for example the horror in

 http://docs.sun.com/app/docs/doc/816-5168/ucred-get-3c

quickly comes to mind. That's why the current API just punts it to the user to use the native type.

Also, while it makes sense to do this for files (because files are so frequently used) it's simply overkill for socket communication. 99.9% of all apps does one of two things

 - checks if the other peer is the same uid

 - gets the pid of the other peer and checks with polkit where
   service should be rendered

So it's *completely* fine to require #ifdef-hell for the few apps that do wants to check things like SELinux labels (which we can't even transfer right now! So the app would just get the pid_t *anyway* and look up in Linux-specific /proc anyway) or Trusted Solaris labels or FreeBSD group membership or whatever.

> Note that we could transfer data from multiple sources into the GCredentials. 
> So we can use a 'credentials' socket control message to get uid/gid/pid and
> some other (trusted) source to get DBus system bus name, for example.

You can do exactly the same things with what I proposed in comment 21. For example, g_unix_connection_receive_credentials() if there are two messages and the first is SCM_CREDENTIALS we set the received 'struct ucred' on the GLinuxCredentials. If there's also a SCM_SELINUX_LABEL_STUFF then we set 'struct selinux_stuff' too.