Обсуждение: Question: CREATE EXTENSION and create schema permission?

CreateExtension() possibly creates a new schema when the supplied
extension was not relocatable and the target schema was given by
control file of the extension.
However, it allows users to create a new schema with his ownership,
even if current user does not have permission to create a new schema.
   Oid         extowner = GetUserId();     :   else if (control->schema != NULL)   {       /*        * The extension is
notrelocatable and the author gave us a schema        * for it.  We create the schema here if it does not already
exist.       */       schemaName = control->schema;       schemaOid = get_namespace_oid(schemaName, true);
 
       if (schemaOid == InvalidOid)       {           schemaOid = NamespaceCreate(schemaName, extowner);           /*
Advancecmd counter to make the namespace visible */           CommandCounterIncrement();       }   }
 

It seems to me that we should inject permission checks here like as
CreateSchemaCommand() doing.
   /*    * To create a schema, must have schema-create privilege on the current    * database and must be able to
becomethe target role (this does not    * imply that the target role itself must have create-schema privilege).    *
Thelatter provision guards against "giveaway" attacks.  Note that a    * superuser will always have both of these
privilegesa fortiori.    */   aclresult = pg_database_aclcheck(MyDatabaseId, saved_uid, ACL_CREATE);   if (aclresult !=
ACLCHECK_OK)      aclcheck_error(aclresult, ACL_KIND_DATABASE,                      get_database_name(MyDatabaseId));
 

I didn't follow the discussion about extension so much when it got merged.
Please tell me, if it was a topic already discussed before.

Thanks,
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>

Kohei KaiGai <kaigai@kaigai.gr.jp> writes:
> However, it allows users to create a new schema with his ownership,
> even if current user does not have permission to create a new schema.
[...]
> It seems to me that we should inject permission checks here like as
> CreateSchemaCommand() doing.

It seems to me the code has been written this way before we relaxed the
superuser only check in CREATE EXTENSION.  I'm not enough into security
to convince myself there's harm to protect against here, but I would
agree there's a sound logic into refusing to create the schema if the
current role isn't granted that operation.

Please note, though, that you're effectively forbidding the role to
create the extension.  As it's not relocatable, the role will not be
able to install it into another schema.  Which could be exactly what you
wanted to achieve.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support

2011/8/21 Dimitri Fontaine <dimitri@2ndquadrant.fr>:
> Kohei KaiGai <kaigai@kaigai.gr.jp> writes:
>> However, it allows users to create a new schema with his ownership,
>> even if current user does not have permission to create a new schema.
> [...]
>> It seems to me that we should inject permission checks here like as
>> CreateSchemaCommand() doing.
>
> It seems to me the code has been written this way before we relaxed the
> superuser only check in CREATE EXTENSION.  I'm not enough into security
> to convince myself there's harm to protect against here, but I would
> agree there's a sound logic into refusing to create the schema if the
> current role isn't granted that operation.
>
> Please note, though, that you're effectively forbidding the role to
> create the extension.  As it's not relocatable, the role will not be
> able to install it into another schema.  Which could be exactly what you
> wanted to achieve.
>
The current implementation set the current user as owner of the new schema.
The default permission check of schema allows owner to create several kinds
of underlying objects.

In the result, we may consider a scenario that a user without permissions to
create new objects possibly get a schema created by CREATE EXTENSION
that allows him to create new objects (such as table, function, ...).

I don't think it is a desirable behavior. :-(

Thanks,
--
KaiGai Kohei <kaigai@kaigai.gr.jp>

Kohei KaiGai <kaigai@kaigai.gr.jp> writes:
> The current implementation set the current user as owner of the new schema.
> The default permission check of schema allows owner to create several kinds
> of underlying objects.
>
> In the result, we may consider a scenario that a user without permissions to
> create new objects possibly get a schema created by CREATE EXTENSION
> that allows him to create new objects (such as table, function, ...).
>
> I don't think it is a desirable behavior. :-(

Agreed,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support

The attached patch adds permission check at the scenario that I
explained bellow.

Unlike CreateSchemaCommand(), we don't have check_is_member_of_role() here
because the extowner is obviously same with the current user in this code path.

I hope this patch being also back ported to v9.1 tree, not only v9.2
development.

Thanks,

2011/8/21 Dimitri Fontaine <dimitri@2ndquadrant.fr>:
> Kohei KaiGai <kaigai@kaigai.gr.jp> writes:
>> The current implementation set the current user as owner of the new schema.
>> The default permission check of schema allows owner to create several kinds
>> of underlying objects.
>>
>> In the result, we may consider a scenario that a user without permissions to
>> create new objects possibly get a schema created by CREATE EXTENSION
>> that allows him to create new objects (such as table, function, ...).
>>
>> I don't think it is a desirable behavior. :-(
>
> Agreed,
> --
> Dimitri Fontaine
> http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support
>
--
KaiGai Kohei <kaigai@kaigai.gr.jp>

Kohei KaiGai <kaigai@kaigai.gr.jp> writes:
> The attached patch adds permission check at the scenario that I
> explained bellow.

Instead of using this patch, I changed the code to call
CreateSchemaCommand itself.  The test that was still missing was the one
to restrict the schema name to not start with "pg_".  It seemed to me
that if we were treating this as a basically nonprivileged schema
creation operation, that rule ought to be enforced too, as well as any
other restrictions that we might someday add to CREATE SCHEMA execution.
        regards, tom lane