[bugfix] sepgsql didn't follow the latest core API changes
This patch fixes a few portions on which sepgsql didn't follow the latest
core API changes.
1) Even though the prototype of ProcessUtility_hook was recently changed,
sepgsql side didn't follow this update, so it made build failed.
2) sepgsql internally uses GETSTRUCT() and HeapTupleGetOid() macro
these were moved to htup_details.h, so it needs an additional #include
for "access/htup_defails.h".
3) sepgsql internally used a bool typed variable named "abort".
I noticed it conflicts with ereport macro because it internally expanded to
ereport_domain that contains invocation of "abort()". So, it renamed this
variables to abort_on_violation.
#define ereport_domain(elevel, domain, rest) \
(errstart(elevel, __FILE__, __LINE__, PG_FUNCNAME_MACRO, domain) ? \
(errfinish rest) : (void) 0), \
((elevel) >= ERROR ? abort() : (void) 0)
This does not affect to v9.2, so please apply it on the master branch.
Thanks,
--
KaiGai Kohei <kaigai@kaigai.gr.jp>
Attachments:
sepgsql-fixbug-follow-core-apis.patchapplication/octet-stream; name=sepgsql-fixbug-follow-core-apis.patchDownload
contrib/sepgsql/database.c | 1 +
contrib/sepgsql/dml.c | 15 ++++++++-------
contrib/sepgsql/hooks.c | 8 ++++----
contrib/sepgsql/label.c | 1 +
contrib/sepgsql/proc.c | 2 ++
contrib/sepgsql/relation.c | 2 ++
contrib/sepgsql/schema.c | 1 +
contrib/sepgsql/selinux.c | 6 +++---
contrib/sepgsql/sepgsql.h | 8 ++++----
contrib/sepgsql/uavc.c | 12 +++++++-----
10 files changed, 33 insertions(+), 23 deletions(-)
diff --git a/contrib/sepgsql/database.c b/contrib/sepgsql/database.c
index 5a42467..c15f2d0 100644
--- a/contrib/sepgsql/database.c
+++ b/contrib/sepgsql/database.c
@@ -12,6 +12,7 @@
#include "access/genam.h"
#include "access/heapam.h"
+#include "access/htup_details.h"
#include "access/sysattr.h"
#include "catalog/dependency.h"
#include "catalog/pg_database.h"
diff --git a/contrib/sepgsql/dml.c b/contrib/sepgsql/dml.c
index 47a1087..49502f5 100644
--- a/contrib/sepgsql/dml.c
+++ b/contrib/sepgsql/dml.c
@@ -10,6 +10,7 @@
*/
#include "postgres.h"
+#include "access/htup_details.h"
#include "access/sysattr.h"
#include "access/tupdesc.h"
#include "catalog/catalog.h"
@@ -148,7 +149,7 @@ check_relation_privileges(Oid relOid,
Bitmapset *selected,
Bitmapset *modified,
uint32 required,
- bool abort)
+ bool abort_on_violation)
{
ObjectAddress object;
char *audit_name;
@@ -194,7 +195,7 @@ check_relation_privileges(Oid relOid,
SEPG_CLASS_DB_TABLE,
required,
audit_name,
- abort);
+ abort_on_violation);
break;
case RELKIND_SEQUENCE:
@@ -205,7 +206,7 @@ check_relation_privileges(Oid relOid,
SEPG_CLASS_DB_SEQUENCE,
SEPG_DB_SEQUENCE__GET_VALUE,
audit_name,
- abort);
+ abort_on_violation);
break;
case RELKIND_VIEW:
@@ -213,7 +214,7 @@ check_relation_privileges(Oid relOid,
SEPG_CLASS_DB_VIEW,
SEPG_DB_VIEW__EXPAND,
audit_name,
- abort);
+ abort_on_violation);
break;
default:
@@ -264,7 +265,7 @@ check_relation_privileges(Oid relOid,
SEPG_CLASS_DB_COLUMN,
column_perms,
audit_name,
- abort);
+ abort_on_violation);
pfree(audit_name);
if (!result)
@@ -279,7 +280,7 @@ check_relation_privileges(Oid relOid,
* Entrypoint of the DML permission checks
*/
bool
-sepgsql_dml_privileges(List *rangeTabls, bool abort)
+sepgsql_dml_privileges(List *rangeTabls, bool abort_on_violation)
{
ListCell *lr;
@@ -351,7 +352,7 @@ sepgsql_dml_privileges(List *rangeTabls, bool abort)
if (!check_relation_privileges(tableOid,
selectedCols,
modifiedCols,
- required, abort))
+ required, abort_on_violation))
return false;
}
list_free(tableIds);
diff --git a/contrib/sepgsql/hooks.c b/contrib/sepgsql/hooks.c
index 9145191..f3cf1c5 100644
--- a/contrib/sepgsql/hooks.c
+++ b/contrib/sepgsql/hooks.c
@@ -265,9 +265,9 @@ static void
sepgsql_utility_command(Node *parsetree,
const char *queryString,
ParamListInfo params,
- bool isTopLevel,
DestReceiver *dest,
- char *completionTag)
+ char *completionTag,
+ ProcessUtilityContext context)
{
sepgsql_context_info_t saved_context_info = sepgsql_context_info;
ListCell *cell;
@@ -328,10 +328,10 @@ sepgsql_utility_command(Node *parsetree,
if (next_ProcessUtility_hook)
(*next_ProcessUtility_hook) (parsetree, queryString, params,
- isTopLevel, dest, completionTag);
+ dest, completionTag, context);
else
standard_ProcessUtility(parsetree, queryString, params,
- isTopLevel, dest, completionTag);
+ dest, completionTag, context);
}
PG_CATCH();
{
diff --git a/contrib/sepgsql/label.c b/contrib/sepgsql/label.c
index 23577b5..3ebf273 100644
--- a/contrib/sepgsql/label.c
+++ b/contrib/sepgsql/label.c
@@ -11,6 +11,7 @@
#include "postgres.h"
#include "access/heapam.h"
+#include "access/htup_details.h"
#include "access/genam.h"
#include "access/xact.h"
#include "catalog/catalog.h"
diff --git a/contrib/sepgsql/proc.c b/contrib/sepgsql/proc.c
index b68314d..fbd358a 100644
--- a/contrib/sepgsql/proc.c
+++ b/contrib/sepgsql/proc.c
@@ -12,12 +12,14 @@
#include "access/genam.h"
#include "access/heapam.h"
+#include "access/htup_details.h"
#include "access/sysattr.h"
#include "catalog/dependency.h"
#include "catalog/indexing.h"
#include "catalog/pg_namespace.h"
#include "catalog/pg_proc.h"
#include "commands/seclabel.h"
+#include "lib/stringinfo.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
diff --git a/contrib/sepgsql/relation.c b/contrib/sepgsql/relation.c
index e759a7d..4ab7fc8 100644
--- a/contrib/sepgsql/relation.c
+++ b/contrib/sepgsql/relation.c
@@ -12,6 +12,7 @@
#include "access/genam.h"
#include "access/heapam.h"
+#include "access/htup_details.h"
#include "access/sysattr.h"
#include "catalog/indexing.h"
#include "catalog/dependency.h"
@@ -20,6 +21,7 @@
#include "catalog/pg_namespace.h"
#include "commands/seclabel.h"
#include "utils/fmgroids.h"
+#include "utils/catcache.h"
#include "utils/lsyscache.h"
#include "utils/syscache.h"
#include "utils/tqual.h"
diff --git a/contrib/sepgsql/schema.c b/contrib/sepgsql/schema.c
index 230449d..e063e39 100644
--- a/contrib/sepgsql/schema.c
+++ b/contrib/sepgsql/schema.c
@@ -12,6 +12,7 @@
#include "access/genam.h"
#include "access/heapam.h"
+#include "access/htup_details.h"
#include "access/sysattr.h"
#include "catalog/dependency.h"
#include "catalog/indexing.h"
diff --git a/contrib/sepgsql/selinux.c b/contrib/sepgsql/selinux.c
index baf92b6..7df9817 100644
--- a/contrib/sepgsql/selinux.c
+++ b/contrib/sepgsql/selinux.c
@@ -893,7 +893,7 @@ sepgsql_compute_create(const char *scontext,
* tclass: class code (SEPG_CLASS_*) of the object being referenced
* required: a mask of required permissions (SEPG_<class>__<perm>)
* audit_name: a human readable object name for audit logs, or NULL.
- * abort: true, if caller wants to raise an error on access violation
+ * abort_on_violation: true, if error shall be raised on access violation
*/
bool
sepgsql_check_perms(const char *scontext,
@@ -901,7 +901,7 @@ sepgsql_check_perms(const char *scontext,
uint16 tclass,
uint32 required,
const char *audit_name,
- bool abort)
+ bool abort_on_violation)
{
struct av_decision avd;
uint32 denied;
@@ -937,7 +937,7 @@ sepgsql_check_perms(const char *scontext,
audit_name);
}
- if (!result && abort)
+ if (!result && abort_on_violation)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("SELinux: security policy violation")));
diff --git a/contrib/sepgsql/sepgsql.h b/contrib/sepgsql/sepgsql.h
index 479b136..9c89eaa 100644
--- a/contrib/sepgsql/sepgsql.h
+++ b/contrib/sepgsql/sepgsql.h
@@ -247,7 +247,7 @@ extern bool sepgsql_check_perms(const char *scontext,
uint16 tclass,
uint32 required,
const char *audit_name,
- bool abort);
+ bool abort_on_violation);
/*
* uavc.c
@@ -257,12 +257,12 @@ extern bool sepgsql_avc_check_perms_label(const char *tcontext,
uint16 tclass,
uint32 required,
const char *audit_name,
- bool abort);
+ bool abort_on_violation);
extern bool sepgsql_avc_check_perms(const ObjectAddress *tobject,
uint16 tclass,
uint32 required,
const char *audit_name,
- bool abort);
+ bool abort_on_violation);
extern char *sepgsql_avc_trusted_proc(Oid functionId);
extern void sepgsql_avc_init(void);
@@ -285,7 +285,7 @@ extern Datum sepgsql_restorecon(PG_FUNCTION_ARGS);
/*
* dml.c
*/
-extern bool sepgsql_dml_privileges(List *rangeTabls, bool abort);
+extern bool sepgsql_dml_privileges(List *rangeTabls, bool abort_on_violation);
/*
* database.c
diff --git a/contrib/sepgsql/uavc.c b/contrib/sepgsql/uavc.c
index 9641a17..04ec305 100644
--- a/contrib/sepgsql/uavc.c
+++ b/contrib/sepgsql/uavc.c
@@ -335,7 +335,7 @@ sepgsql_avc_lookup(const char *scontext, const char *tcontext, uint16 tclass)
*
* It returns 'true', if the security policy suggested to allow the required
* permissions. Otherwise, it returns 'false' or raises an error according
- * to the 'abort' argument.
+ * to the 'abort_on_violation' argument.
* The 'tobject' and 'tclass' identify the target object being referenced,
* and 'required' is a bitmask of permissions (SEPG_*__*) defined for each
* object classes.
@@ -345,7 +345,8 @@ sepgsql_avc_lookup(const char *scontext, const char *tcontext, uint16 tclass)
bool
sepgsql_avc_check_perms_label(const char *tcontext,
uint16 tclass, uint32 required,
- const char *audit_name, bool abort)
+ const char *audit_name,
+ bool abort_on_violation)
{
char *scontext = sepgsql_get_client_label();
avc_cache *cache;
@@ -415,7 +416,7 @@ sepgsql_avc_check_perms_label(const char *tcontext,
audit_name);
}
- if (abort && !result)
+ if (abort_on_violation && !result)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("SELinux: security policy violation")));
@@ -426,14 +427,15 @@ sepgsql_avc_check_perms_label(const char *tcontext,
bool
sepgsql_avc_check_perms(const ObjectAddress *tobject,
uint16 tclass, uint32 required,
- const char *audit_name, bool abort)
+ const char *audit_name,
+ bool abort_on_violation)
{
char *tcontext = GetSecurityLabel(tobject, SEPGSQL_LABEL_TAG);
bool rc;
rc = sepgsql_avc_check_perms_label(tcontext,
tclass, required,
- audit_name, abort);
+ audit_name, abort_on_violation);
if (tcontext)
pfree(tcontext);
Excerpts from Kohei KaiGai's message of dom sep 02 15:53:22 -0300 2012:
This patch fixes a few portions on which sepgsql didn't follow the latest
core API changes.
I think you should get a buildfarm animal installed that builds and
tests sepgsql, to avoid this kind of problem in the future.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
2012/9/3 Alvaro Herrera <alvherre@2ndquadrant.com>:
Excerpts from Kohei KaiGai's message of dom sep 02 15:53:22 -0300 2012:
This patch fixes a few portions on which sepgsql didn't follow the latest
core API changes.I think you should get a buildfarm animal installed that builds and
tests sepgsql, to avoid this kind of problem in the future.
Thanks for your suggestion. I'm interested in.
http://wiki.postgresql.org/wiki/PostgreSQL_Buildfarm_Howto
Does it test only build-correctness? Or, is it possible to include
result of regression test for result to be alarmed?
--
KaiGai Kohei <kaigai@kaigai.gr.jp>
Excerpts from Kohei KaiGai's message of mié sep 05 08:30:37 -0300 2012:
2012/9/3 Alvaro Herrera <alvherre@2ndquadrant.com>:
Excerpts from Kohei KaiGai's message of dom sep 02 15:53:22 -0300 2012:
This patch fixes a few portions on which sepgsql didn't follow the latest
core API changes.I think you should get a buildfarm animal installed that builds and
tests sepgsql, to avoid this kind of problem in the future.Thanks for your suggestion. I'm interested in.
http://wiki.postgresql.org/wiki/PostgreSQL_Buildfarm_Howto
Does it test only build-correctness? Or, is it possible to include
result of regression test for result to be alarmed?
Yes, regression test diffs are also reported and can cause failures.
As far as I know, you can construct your own test steps, if you want to
do something customized that's not present in regular BF animals.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On 09/05/2012 09:11 AM, Alvaro Herrera wrote:
Excerpts from Kohei KaiGai's message of mié sep 05 08:30:37 -0300 2012:
2012/9/3 Alvaro Herrera <alvherre@2ndquadrant.com>:
Excerpts from Kohei KaiGai's message of dom sep 02 15:53:22 -0300 2012:
This patch fixes a few portions on which sepgsql didn't follow the latest
core API changes.I think you should get a buildfarm animal installed that builds and
tests sepgsql, to avoid this kind of problem in the future.Thanks for your suggestion. I'm interested in.
http://wiki.postgresql.org/wiki/PostgreSQL_Buildfarm_Howto
Does it test only build-correctness? Or, is it possible to include
result of regression test for result to be alarmed?Yes, regression test diffs are also reported and can cause failures.
As far as I know, you can construct your own test steps, if you want to
do something customized that's not present in regular BF animals.
Looking at SEPgsql testing is on my long TODO list. I'll have to set up
a separate VM for it, as I don't habitually run SELinux.
cheers
andrew
On Sun, Sep 2, 2012 at 2:53 PM, Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
This patch fixes a few portions on which sepgsql didn't follow the latest
core API changes.1) Even though the prototype of ProcessUtility_hook was recently changed,
sepgsql side didn't follow this update, so it made build failed.2) sepgsql internally uses GETSTRUCT() and HeapTupleGetOid() macro
these were moved to htup_details.h, so it needs an additional #include
for "access/htup_defails.h".3) sepgsql internally used a bool typed variable named "abort".
I noticed it conflicts with ereport macro because it internally expanded to
ereport_domain that contains invocation of "abort()". So, it renamed this
variables to abort_on_violation.#define ereport_domain(elevel, domain, rest) \
(errstart(elevel, __FILE__, __LINE__, PG_FUNCNAME_MACRO, domain) ? \
(errfinish rest) : (void) 0), \
((elevel) >= ERROR ? abort() : (void) 0)This does not affect to v9.2, so please apply it on the master branch.
I have committed this untested. It seems pretty mechanical and I
assume that you tested it. Anyway, it's certainly broken without the
patch.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
2012/9/5 Andrew Dunstan <andrew@dunslane.net>:
On 09/05/2012 09:11 AM, Alvaro Herrera wrote:
Excerpts from Kohei KaiGai's message of mié sep 05 08:30:37 -0300 2012:
2012/9/3 Alvaro Herrera <alvherre@2ndquadrant.com>:
Excerpts from Kohei KaiGai's message of dom sep 02 15:53:22 -0300 2012:
This patch fixes a few portions on which sepgsql didn't follow the
latest
core API changes.I think you should get a buildfarm animal installed that builds and
tests sepgsql, to avoid this kind of problem in the future.Thanks for your suggestion. I'm interested in.
http://wiki.postgresql.org/wiki/PostgreSQL_Buildfarm_Howto
Does it test only build-correctness? Or, is it possible to include
result of regression test for result to be alarmed?Yes, regression test diffs are also reported and can cause failures.
As far as I know, you can construct your own test steps, if you want to
do something customized that's not present in regular BF animals.Looking at SEPgsql testing is on my long TODO list. I'll have to set up a
separate VM for it, as I don't habitually run SELinux.
If you are available to provide a VM environment for sepgsql, let me help
set up its build and regression test environment.
As you may know, the regression test of sepgsql requires some additional
configurations on operating system level, such as security policy load and
so on. I expect we have to add something special stuff onto the buildfirm
system.
Thanks,
--
KaiGai Kohei <kaigai@kaigai.gr.jp>
2012/9/5 Robert Haas <robertmhaas@gmail.com>:
On Sun, Sep 2, 2012 at 2:53 PM, Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
This patch fixes a few portions on which sepgsql didn't follow the latest
core API changes.1) Even though the prototype of ProcessUtility_hook was recently changed,
sepgsql side didn't follow this update, so it made build failed.2) sepgsql internally uses GETSTRUCT() and HeapTupleGetOid() macro
these were moved to htup_details.h, so it needs an additional #include
for "access/htup_defails.h".3) sepgsql internally used a bool typed variable named "abort".
I noticed it conflicts with ereport macro because it internally expanded to
ereport_domain that contains invocation of "abort()". So, it renamed this
variables to abort_on_violation.#define ereport_domain(elevel, domain, rest) \
(errstart(elevel, __FILE__, __LINE__, PG_FUNCNAME_MACRO, domain) ? \
(errfinish rest) : (void) 0), \
((elevel) >= ERROR ? abort() : (void) 0)This does not affect to v9.2, so please apply it on the master branch.
I have committed this untested. It seems pretty mechanical and I
assume that you tested it. Anyway, it's certainly broken without the
patch.
Thanks, I'd like to pay attention to core API changes more.
I still have one other bug fix for v9.2 and master branch.
Isn't it obvious to apply?
http://archives.postgresql.org/message-id/CADyhKSVWKjCKY3cDeQG6qp7OczqsbJtT9cihk3HB7TkvcEDD+Q@mail.gmail.com
--
KaiGai Kohei <kaigai@kaigai.gr.jp>
Kohei KaiGai wrote:
2012/9/5 Andrew Dunstan <andrew@dunslane.net>:
Looking at SEPgsql testing is on my long TODO list. I'll have to set up a
separate VM for it, as I don't habitually run SELinux.If you are available to provide a VM environment for sepgsql, let me help
set up its build and regression test environment.As you may know, the regression test of sepgsql requires some additional
configurations on operating system level, such as security policy load and
so on. I expect we have to add something special stuff onto the buildfirm
system.
Did this go anywhere?
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
2012/10/10 Alvaro Herrera <alvherre@2ndquadrant.com>:
Kohei KaiGai wrote:
2012/9/5 Andrew Dunstan <andrew@dunslane.net>:
Looking at SEPgsql testing is on my long TODO list. I'll have to set up a
separate VM for it, as I don't habitually run SELinux.If you are available to provide a VM environment for sepgsql, let me help
set up its build and regression test environment.As you may know, the regression test of sepgsql requires some additional
configurations on operating system level, such as security policy load and
so on. I expect we have to add something special stuff onto the buildfirm
system.Did this go anywhere?
Andrew offered a test environment, but I could not have enough time to
set up it yet...
Thanks,
--
KaiGai Kohei <kaigai@kaigai.gr.jp>
Kohei KaiGai wrote:
2012/10/10 Alvaro Herrera <alvherre@2ndquadrant.com>:
Kohei KaiGai wrote:
2012/9/5 Andrew Dunstan <andrew@dunslane.net>:
Looking at SEPgsql testing is on my long TODO list. I'll have to set up a
separate VM for it, as I don't habitually run SELinux.If you are available to provide a VM environment for sepgsql, let me help
set up its build and regression test environment.As you may know, the regression test of sepgsql requires some additional
configurations on operating system level, such as security policy load and
so on. I expect we have to add something special stuff onto the buildfirm
system.Did this go anywhere?
Andrew offered a test environment, but I could not have enough time to
set up it yet...
I will snail-mail you a round tuit I have at home. I would feel more
comfortable committing sepgsql stuff if we had it.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services