Re: [PATCHES] Merge pg_shadow && pg_group -- UNTESTED

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > Ok. Can I get some help defining what the New Truth will look like
> > then? I understand users and groups pretty well but I'm not 100% sure
> > about roles.
>
> I looked through SQL99 a bit (see 4.31 "Basic security model") and think

Ah, I was looking through SQL2003 recently, I don't think much has
changed in that area though.

> I now have some handle on this. According to the spec a "role" is
> more or less exactly what we think of as a "group", with the extension
> that roles can have other roles as members (barring circularity).

Right, ok.

> In particular the spec draws a distinction between "user identifiers"
> and "role identifiers", although this distinction seems very nearly 100%
> useless because the two sorts of identifiers can be used almost
> interchangeably (an "authorization identifier" means either one, and in
> most places "authorization identifier" is what is relevant). AFAICT the
> only really solid reason for the distinction is that you have to log in
> initially as a user and not as a role. That strikes me as a security
> policy --- it's analogous to saying you can't log in directly as root
> but have to su to root from your personal login --- which may be a good
> thing for a given site to enforce but IMHO it should not be hard-wired
> into the security mechanism.

Ok, I agree, though personally I don't like the idea of permitting
role-logins, but no need to have the security system force it.

> The implementation reason for not having a hard distinction is mainly
> that we want to have a single unique-identifier space for both users and
> roles. This simplifies representation of ACLs (which will no longer
> need extra bits to identify whether an entry references a user or a
> group) and allows us to have groups as members of other groups without
> messy complication there.

The other difference would seem to be that "user identifiers" can't be
granted to users whereas "role identifiers" can be. Following this,
"rolmembers" must be NULL if rolcanlogin is true, no? That breaks if
roles can log in though. Or should we just allow granting of "user
identifiers" to other users- but if we do should the user be permitted
to do that?

> It's not entirely clear to me whether the spec allows roles to be
> directly owners of objects, but I think we should allow it.

I agree, and in fact group/role ownership is what I'm specifically
interested in, though I'd like role support too and so I'm happy to
implement it along the way. :)

> So I'm envisioning something like
>
> CREATE TABLE pg_role (
> rolname name, -- name of role
> rolsuper boolean, -- superuser?
> rolcreateuser boolean, -- can create more users?
> rolcreatedb boolean, -- can create databases?
> rolcatupdate boolean, -- can hack system catalogs?
> rolcanlogin boolean, -- can log in as this role?
> rolvaliduntil timestamptz, -- password
> rolpassword text, -- password expiration time
> rolmembers oid[], -- OIDs of members, if any
> roladmin boolean[], -- do members have ADMIN OPTION
> rolconfig text[] -- ALTER USER SET guc = value
> ) WITH OIDS;
>
> It might be better to call this by some name other than "pg_role",
> since what it defines is not exactly roles in the sense that SQL99
> uses; but I don't have a good idea what to use instead.
> "pg_authorization" would work but it's unwieldy.

Hmmm, I agree pg_role isn't quite right. pg_auth would be shorter than
pg_authorization, but it isn't intuitive what it is. How about
pg_ident? It's not users or roles, but it's identifiers. Perhaps
pg_authid?

> OIDs of rows in this table replace AclIds.

ok.

> I'm supposing that we should separate "superuserness" from "can create
> users" (presumably a non-superuser with rolcreateuser would only be
> allowed to create non-super users). The lack of distinction on this
> point has been a conceptual problem for newbies for a long time, and an
> admin issue too. As long as we are hacking this table we should fix it.

Agreed.

> If you want to enforce a hard distinction between users and roles (groups)
> then you'd prohibit rolcanlogin from being true when rolmembers is
> nonempty, but as said above I'm not sure the system should enforce that.

Right, but there's still the issue of granting "users" to users.

> rolpassword, rolvaliduntil, and rolconfig are irrelevant if not rolcanlogin.

Right.

> The roladmin[] bool array indicates whether members were granted
> admission WITH ADMIN OPTION, which means they can grant membership to
> others (analogous to WITH GRANT OPTION for individual privileges).
> I'm not sure this is sufficient ... we may need to record who granted
> membership to each member as well, in order to process revocation.

I think we'll probably need to record who granted membership too, to
prevent circulation as well as revocation processing..

> It might be better to lose the rolmembers/roladmin columns and instead
> represent membership in a separate table, roughly
>
> CREATE TABLE pg_role_members (
> role oid,
> member oid,
> grantor oid,
> admin_option bool,
> primary key (role, member, grantor)
> );
>
> This is cleaner from a relational theory point of view but is probably
> harder for the system to process. One advantage is that it is easier to
> find out "which roles does user X belong to?" ... but I'm not sure we
> care about making that fast.

I like this approach more.

> One thing that needs to be thought about before going too far is exactly
> how ACL rights testing will work, particularly in the face of roles
> being members of other roles. That is the one performance-critical
> operation that uses these data structures, so we ought to design around
> making it fast.

I agree that it should be fast- but I think it should be possible to
implement it in such a way that if you don't make roles members of other
roles then you won't pay a performance penelty for the fact that we
support that ability. If you use it, it'll be a bit more expensive to
check permissions where you do.

> > Ok, I probably will. Should I be concerned with trying to make
> > 'smallish' patches that build upon each other (ie: change to pg_role
> > first, then change AclId to Oid, or whatever) or will one larger patch
> > that takes care of it all be ok?
>
> Smaller patches are easier to review, for sure. Also, you'll need to
> coordinate with Alvaro's work on dependencies for global objects.

Right, ok. I'll look at what Alvaro's got and think about the approach
and milestones/patches to get from where we're at now to what we want.

Stephen