Re: SE-PgSQL patch review

KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:

> > CREATE TABLE tbl (...) SECURITY CONTEXT '...'
> > * CREATE TABLE tbl (col integer AS SECURITY_CONTEXT = '...')
>
> We need to put a reserved token, such as "AS", prior to the SECURITY_CONTEXT
> to avoid syntax conflicts to "DEFAULT b_expr" option.

There might be another idea to put security context in WITH options:
1. CREATE TABLE tbl (...) WITH (security_context = '...')
2. CREATE TABLE tbl (col integer WITH (security_context = '...') PRIMARY KEY)
If we use the syntax, '_' and '=' is reasonable.

BTW, I like to reverse the order of constraints and WITH options in
column definitions (2), but I could not solve shift/reduce errors
-- it might conflict with "PRIMARY KEY WITH (index-parameters)".

> - sepgsql_template1_getcon -> pg_get_template1_secon
> - sepgsql_database_getcon -> pg_get_database_secon

Why do we need two version of functions for template1 and database?
The template1 database is the default template for CREATE DATABASE,
but we can also choose another one. Do we need to distinguish them?

> > * It uses dedicated 'SExxx' error codes, but I think they should belong to
> > the same family of ERRCODE_INSUFFICIENT_PRIVILEGE (42501).
> I already uses predefined error code, if exist.

What I meant was: there are no problem to add new error codes for SE-PgSQL,
but I think the values of the codes should be '42xxx' because those errors
are still "Class 42 - Access Rule Violation" from the view of users.

> > ==== Internal structures ====
> > * Are the security labels enough stable?
> > We store security labels as text for each object and column.
>
> If the security labels get invalid due to the modification of SELinux
> configuration or other reasons, it considers the database objects are
> unlabeled.

I believe you have a plan to add row-level security checking in the future
version. Do you have some documentation about how to implement security
context for each row? I'm worrying about the on-disk representation.
Security labels stored in text format takes 20-40 bytes per row. It is not
negligibly-small, and might be hard to be treated because of variable-length.

We store OIDs for each row at the end of tuple header. If we also
store securty labels in the same way, will we need some kinds of
"securty label to OID" converter in the future?

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center