subject:"\[HACKERS\] Is this a bug\?"

Re: [HACKERS] Row Level Security Bug ?

2017-11-12 Thread Joe Conway

On 11/12/2017 10:17 AM, Andrea Adami wrote:
> if i do:
> 
> SET ROLE 'manage...@scuola-1.it '

[SELECT from table]

> i see only one row (as expected)
> 
> but when i do:

[SELECT from VIEWs]

> I see all the rows always
> 
> this way i lack all the row level security i defined
> 
> is this either a bug or it's made by design ?
> if it's made by design why ?
> Is there  a way to write view that respect the row level security ?
> For my point of view is a nonsense make a row level security that
> doesn't work with the view.

See:
https://www.postgresql.org/docs/10/static/sql-createview.html
In particular: "Access to tables referenced in the view is determined by
permissions of the view owner."

And:
https://www.postgresql.org/docs/10/static/ddl-rowsecurity.html
"Superusers and roles with the BYPASSRLS attribute always bypass the row
security system when accessing a table. Table owners normally bypass row
security as well, though a table owner can choose to be subject to row
security with ALTER TABLE ... FORCE ROW LEVEL SECURITY."

HTH,

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

[HACKERS] Row Level Security Bug ?

2017-11-12 Thread Andrea Adami

Hello,
i have a db with a couple of tables
(enclosed the script to recreate it, please have a look before to proceed)
i enabled the row level security and all seem to work fine

if i do it (connected in as superuser like, usualy, postgres is):

select school, description, example
from schools

i can see all the rows

if i do:

SET ROLE 'manage...@scuola-1.it'

select school, description, example
from school

i see only one row (as expected)

but when i do:

select *
from _rls_test

select *
FROM _rls_test_security_barrier

select *
from _rls_test_with_check_local

select *
from _rls_test_with_check_local_cascade

I see all the rows always

this way i lack all the row level security i defined

is this either a bug or it's made by design ?
if it's made by design why ?
Is there  a way to write view that respect the row level security ?
For my point of view is a nonsense make a row level security that doesn't
work with the view.

Thanks to all the spend time to answer me.

here:
https://github.com/scuola247/postgresql
you can have a look at the complete database

Andrea Adami

===
===
===

CREATE DATABASE test
  WITH OWNER = postgres
   ENCODING = 'UTF8'
   TABLESPACE = pg_default
   CONNECTION LIMIT = -1;


CREATE SEQUENCE public.pk_seq
  INCREMENT 1
  MINVALUE 1
  MAXVALUE 9223372036854775807
  START 736220
  CACHE 1;


CREATE TABLE public.schools
(
  school bigint NOT NULL DEFAULT nextval('pk_seq'::regclass), -- Uniquely
identifies the table row
  description character varying(160) NOT NULL, -- Description for the school
  processing_code character varying(160) NOT NULL, -- A code that identify
the school on the government information system
  mnemonic character varying(30) NOT NULL, -- Short description to be use
as code
  example boolean NOT NULL DEFAULT false, -- It indicates that the data
have been inserted to be an example of the use of the data base
  behavior bigint, -- Indicates the subject used for the behavior
  CONSTRAINT schools_pk PRIMARY KEY (school),
  CONSTRAINT schools_uq_description UNIQUE (description),
  CONSTRAINT schools_uq_mnemonic UNIQUE (mnemonic),
  CONSTRAINT schools_uq_processing_code UNIQUE (processing_code, example)
);

-- Index: public.schools_fk_behavior

CREATE INDEX schools_fk_behavior
  ON public.schools
  USING btree
  (behavior);


CREATE TABLE public.usenames_schools
(
  usename_school bigint NOT NULL DEFAULT nextval('pk_seq'::regclass), --
Unique identification code for the row
  usename name NOT NULL, -- The session's usename
  school bigint NOT NULL, -- School enabled for the the usename
  CONSTRAINT usenames_schools_pk PRIMARY KEY (usename_school),
  CONSTRAINT usenames_schools_fk_school FOREIGN KEY (school)
  REFERENCES public.schools (school) MATCH SIMPLE
  ON UPDATE CASCADE ON DELETE CASCADE,
  CONSTRAINT usenames_schools_uq_usename_school UNIQUE (usename, school) --
Foe every usename one school can be enabled only one time
);

-- Index: public.usenames_schools_fx_school

CREATE INDEX usenames_schools_fx_school
  ON public.usenames_schools
  USING btree
  (school);

  CREATE OR REPLACE VIEW public._rls_test AS
 SELECT schools.school,
schools.description,
schools.example
   FROM schools;


CREATE OR REPLACE VIEW public._rls_test_security_barrier WITH
(security_barrier=true) AS
 SELECT schools.school,
schools.description,
schools.example
   FROM schools;

CREATE OR REPLACE VIEW public._rls_test_with_check_local WITH
(check_option=local) AS
 SELECT schools.school,
schools.description,
schools.example
   FROM schools;

CREATE OR REPLACE VIEW public._rls_test_with_check_local_cascade WITH
(check_option=cascaded) AS
 SELECT schools.school,
schools.description,
schools.example
   FROM schools;

-- now same data
-- now same data
-- now same data

INSERT INTO
public.schools(school,description,processing_code,mnemonic,example) VALUES
('289610','Istituto comprensivo "Voyager"','ZZIC1Z','IC
VOYAGER','t');
INSERT INTO
public.schools(school,description,processing_code,mnemonic,example) VALUES
('20','Istituto Tecnico Tecnologico "Leonardo da
Vinci"','ZZITTZ','ITT DAVINCI','t');
INSERT INTO
public.schools(school,description,processing_code,mnemonic,example) VALUES
('10','Istituto comprensivo ''Andromeda''','ZZIC8Z','IC
ANDROMEDA','t');


INSERT INTO public.usenames_schools(usename_school,usename,school) VALUES
('7266330','manage...@scuola-1.it','10');


-- THEN ENABLE ROW LEVEL SECURITY
-- THEN ENABLE ROW LEVEL SECURITY
-- THEN ENABLE ROW LEVEL SECURITY


  ALTER TABLE usenames_schools ENABLE ROW LEVEL SECURITY;

 ALTER TABLE schools ENABLE ROW LEVEL SECURITY;

CREATE POLICY usenames_schools_pl_usename ON usenames_schools TO public
 USING (usename = current_user)
  WITH CHECK (usename = current_user);

CREATE POLICY schools_pl_school ON schools

[HACKERS] Proposed fix for bug #14826 (the invalid empty array business)

2017-09-23 Thread Tom Lane

I poked around among the callers of construct_[md_]array() and found at
least two more besides ts_lexize() that could build zero-element arrays;
one of very ancient standing is in the pg_prepared_statements view.

So I think we've got a clear hazard here that justifies changing the
behavior of the low-level array functions, rather than expecting every
call site to be on its guard about empty arrays.  Accordingly, I propose
the attached patch which makes construct_md_array responsible for getting
this right.

In principle we could now remove code from the places that do take care to
call construct_empty_array instead.  But I found only one place where it
really seemed worth removing anything, in ExecEvalArrayExpr.

I'm a little bit scared about back-patching this, as it seems at least
possible that some external code could be broken by the change in what
construct_[md_]array could hand back.  Given that some of these bugs
have been in place for ~ten years and nobody noticed till now, seems
like it might be good enough to fix them in HEAD only.

Comments?

regards, tom lane

diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index bd8a15d..09abd46 100644
*** a/src/backend/executor/execExprInterp.c
--- b/src/backend/executor/execExprInterp.c
*** ExecEvalArrayExpr(ExprState *state, Expr
*** 2131,2144 
  		Datum	   *dvalues = op->d.arrayexpr.elemvalues;
  		bool	   *dnulls = op->d.arrayexpr.elemnulls;
  
- 		/* Shouldn't happen here, but if length is 0, return empty array */
- 		if (nelems == 0)
- 		{
- 			*op->resvalue =
- PointerGetDatum(construct_empty_array(element_type));
- 			return;
- 		}
- 
  		/* setup for 1-D array of the given length */
  		ndims = 1;
  		dims[0] = nelems;
--- 2131,2136 
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index e4101c9..d1f2fe7 100644
*** a/src/backend/utils/adt/arrayfuncs.c
--- b/src/backend/utils/adt/arrayfuncs.c
*** array_map(FunctionCallInfo fcinfo, Oid r
*** 3297,3302 
--- 3297,3303 
   *
   * A palloc'd 1-D array object is constructed and returned.  Note that
   * elem values will be copied into the object even if pass-by-ref type.
+  * Also note the result will be 0-D not 1-D if nelems = 0.
   *
   * NOTE: it would be cleaner to look up the elmlen/elmbval/elmalign info
   * from the system catalogs, given the elmtype.  However, the caller is
*** construct_array(Datum *elems, int nelems
*** 3331,3336 
--- 3332,3338 
   *
   * A palloc'd ndims-D array object is constructed and returned.  Note that
   * elem values will be copied into the object even if pass-by-ref type.
+  * Also note the result will be 0-D not ndims-D if any dims[i] = 0.
   *
   * NOTE: it would be cleaner to look up the elmlen/elmbval/elmalign info
   * from the system catalogs, given the elmtype.  However, the caller is
*** construct_md_array(Datum *elems,
*** 3362,3373 
   errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)",
  		ndims, MAXDIM)));
  
- 	/* fast track for empty array */
- 	if (ndims == 0)
- 		return construct_empty_array(elmtype);
- 
  	nelems = ArrayGetNItems(ndims, dims);
  
  	/* compute required space */
  	nbytes = 0;
  	hasnulls = false;
--- 3364,3375 
   errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)",
  		ndims, MAXDIM)));
  
  	nelems = ArrayGetNItems(ndims, dims);
  
+ 	/* if ndims <= 0 or any dims[i] == 0, return empty array */
+ 	if (nelems <= 0)
+ 		return construct_empty_array(elmtype);
+ 
  	/* compute required space */
  	nbytes = 0;
  	hasnulls = false;

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: [BUGS] Re: BUG #14680: startup process on standby encounter a deadlock of TwoPhaseStateLock when redo 2PC xlog

2017-06-10 Thread Alvaro Herrera

Noah Misch wrote:

> IMMEDIATE ATTENTION REQUIRED.  This PostgreSQL 10 open item is long past due
> for your status update.  Please reacquaint yourself with the policy on open
> item ownership[1] and then reply immediately.  If I do not hear from you by
> 2017-06-11 07:00 UTC, I will transfer this item to release management team
> ownership without further notice.

I volunteer to own this item.  I'll report on Wednesday 14th at the latest.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Potential hot-standby bug around xacts committed but in xl_running_xacts

2017-05-02 Thread Simon Riggs

On 2 May 2017 at 18:06, Andres Freund  wrote:

>> What I suggest is that with logical decoding in mind we do this
>> 1. Inject a new record XLOG_SNAPSHOT_START at the start of
>> LogStandbySnapshot(). We start logical decoding from there.
>> 2. Record any transactions that end
>> 3. Now the full XLOG_RUNNING_XACTS record arrives. We apply all xacts
>> that are seen as running, minus any ended between 1 and 3
>
>> This avoids the problems for the race but without holding locks while
>> we log XLOG_RUNNING_XACTS, something that was considered painful for
>> Hot Standby.
>
> I don't think that really solves it, because other transactions could
> just be stuck just after the XLogInsert() forever.  And it'd have the
> issue of having to backpatch a new record.  I'm working on an
> alternative approach, let's hope that that works out.

Backpatchable approach.

1. Record CurrentInsertPosition()
2. LogStandbySnapshot()
3. Insert custom logical wal message containing currentinsertposition
and LSN of (2)

When we decode
1. Read standby snapshot
2. Scan forwards until we see message written by step (3) above, which
is identifiable because it contains LSN of snapshot.
3. Read initial LSN from message then re-scan from LSN until
xl_running_xacts message collecting any commits/aborts and removing
them from snapshot.

No new WAL messages, no locking problem, race condition handled.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Potential hot-standby bug around xacts committed but in xl_running_xacts

2017-05-02 Thread Craig Ringer

On 2 May 2017 at 13:12, Simon Riggs  wrote:

> What I suggest is that with logical decoding in mind we do this
> 1. Inject a new record XLOG_SNAPSHOT_START at the start of
> LogStandbySnapshot(). We start logical decoding from there.
> 2. Record any transactions that end
> 3. Now the full XLOG_RUNNING_XACTS record arrives. We apply all xacts
> that are seen as running, minus any ended between 1 and 3
>
> This avoids the problems for the race but without holding locks while
> we log XLOG_RUNNING_XACTS, something that was considered painful for
> Hot Standby.

Sounds like a sensible solution to me. It avoids the need for a rather
undesirable interlock between xlog and shmem commit.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Potential hot-standby bug around xacts committed but in xl_running_xacts

2017-05-01 Thread Simon Riggs

On 1 May 2017 at 22:38, Andres Freund  wrote:

> The thread below 
> http://archives.postgresql.org/message-id/f37e975c-908f-858e-707f-058d3b1eb214%402ndquadrant.com
> describes an issue in logical decoding that arises because
> xl_running_xacts' contents aren't necessarily coherent with the contents
> of the WAL, because RecordTransactionCommit() / RecordTransactionAbort()
> don't have any interlock against the procarray.  That means
> xl_running_xacts can contain transactions assumed to be running, that
> already have their commit/abort records WAL logged.

That is known/by design.

> I think that's not just problematic in logical decoding, but also
> Hot-Standby.  Consider the following:
>
> ProcArrayApplyRecoveryInfo() gets an xl_running_xacts record that's not
> suboverflowed, and thus will change to STANDBY_SNAPSHOT_READY.

Yes

> In that
> case it'll populate the KnownAssignedXids machinery using
> KnownAssignedXidsAdd().

No, because of this section of code in ProcArrayApplyRecoveryInfo()

/*
 * The running-xacts snapshot can contain xids that were still visible
 * in the procarray when the snapshot was taken, but were already
 * WAL-logged as completed. They're not running anymore, so ignore
 * them.
 */
 if (TransactionIdDidCommit(xid) || TransactionIdDidAbort(xid))
 continue;

> Am I missing something that protects against the above scenario?

It does seem so, yes. The locking issues in Hot Standby are complex,
but they do seem to be correct, thanks for reviewing them.

This is documented in multiple locations, including what I thought was
your own comment in LogStandbySnapshot().

What I suggest is that with logical decoding in mind we do this
1. Inject a new record XLOG_SNAPSHOT_START at the start of
LogStandbySnapshot(). We start logical decoding from there.
2. Record any transactions that end
3. Now the full XLOG_RUNNING_XACTS record arrives. We apply all xacts
that are seen as running, minus any ended between 1 and 3

This avoids the problems for the race but without holding locks while
we log XLOG_RUNNING_XACTS, something that was considered painful for
Hot Standby.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Potential hot-standby bug around xacts committed but in xl_running_xacts

2017-05-01 Thread Andres Freund

Hi,

The thread below
http://archives.postgresql.org/message-id/f37e975c-908f-858e-707f-058d3b1eb214%402ndquadrant.com
describes an issue in logical decoding that arises because
xl_running_xacts' contents aren't necessarily coherent with the contents
of the WAL, because RecordTransactionCommit() / RecordTransactionAbort()
don't have any interlock against the procarray. That means
xl_running_xacts can contain transactions assumed to be running, that
already have their commit/abort records WAL logged.

I think that's not just problematic in logical decoding, but also
Hot-Standby. Consider the following:

ProcArrayApplyRecoveryInfo() gets an xl_running_xacts record that's not
suboverflowed, and thus will change to STANDBY_SNAPSHOT_READY. In that
case it'll populate the KnownAssignedXids machinery using
KnownAssignedXidsAdd().

Once STANDBY_SNAPSHOT_READY, CheckRecoveryConsistency() will signal
postmaster to allow connections.

For HS, a snapshot will be built by GetSnapshotData() using
KnownAssignedXidsGetAndSetXmin(). That in turn uses the transactions
currently known to be running, to populate the snapshot.

Now, if transactions have committed before (in the "earlier LSN" sense)
the xl_running_xacts record, ExpireTreeKnownAssignedTransactionIds() in
xact_redo_commit() will already have run. Which means we'll assume
already committed transactions are still running. In other words, the
snapshot is corrupted.

Luckily this'll self-correct over time, fixed by
ExpireOldKnownAssignedTransactionIds().

Am I missing something that protects against the above scenario?

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

1 2 3 4 5 6 >

1 - 100 of 519 matches

Mail list logo