On Wed, Sep 2, 2015 at 1:19 PM, Stefan Kaltenbrunner <
ste...@kaltenbrunner.cc> wrote:
> On 09/02/2015 10:10 PM, dinesh kumar wrote:
> > On Tue, Sep 1, 2015 at 10:58 PM, Stefan Kaltenbrunner
> > <ste...@kaltenbrunner.cc <mailto:ste...@kaltenbrunner.cc>> wrote:
> >
> > On 07/25/2015 03:38 AM, dinesh kumar wrote:
> > >
> > >
> > > On Fri, Jul 24, 2015 at 10:22 AM, Robert Haas <
> robertmh...@gmail.com <mailto:robertmh...@gmail.com>
> > > <mailto:robertmh...@gmail.com <mailto:robertmh...@gmail.com>>>
> wrote:
> > >
> > > On Thu, Jul 23, 2015 at 8:15 PM, dinesh kumar
> > > <dineshkuma...@gmail.com <mailto:dineshkuma...@gmail.com>
> > <mailto:dineshkuma...@gmail.com <mailto:dineshkuma...@gmail.com>>>
> > wrote:
> > > > On Thu, Jul 23, 2015 at 9:21 AM, Robert Haas
> > > <robertmh...@gmail.com <mailto:robertmh...@gmail.com>
> > <mailto:robertmh...@gmail.com <mailto:robertmh...@gmail.com>>>
> wrote:
> > > >>
> > > >> On Thu, Jul 23, 2015 at 12:19 PM, dinesh kumar
> > > <dineshkuma...@gmail.com <mailto:dineshkuma...@gmail.com>
> > <mailto:dineshkuma...@gmail.com <mailto:dineshkuma...@gmail.com>>>
> > > >> wrote:
> > > >> > Sorry for my unclear description about the proposal.
> > > >> >
> > > >> > "WITH PERMISSIVE" is equal to our existing behavior. That
> > is, chmod=644
> > > >> > on
> > > >> > the created files.
> > > >> >
> > > >> > If User don't specify "PERMISSIVE" as an option, then the
> > chmod=600 on
> > > >> > created files. In this way, we can restrict the other
> > users from reading
> > > >> > these files.
> > > >>
> > > >> There might be some benefit in allowing the user to choose
> the
> > > >> permissions, but (1) I doubt we want to change the default
> > behavior
> > > >> and (2) providing only two options doesn't seem flexible
> > enough.
> > > >>
> > > >
> > > > Thanks for your inputs Robert.
> > > >
> > > > 1) IMO, we will keep the exiting behavior as it is.
> > > >
> > > > 2) As the actual proposal talks about the permissions of
> > group/others. So,
> > > > we can add few options as below to the WITH clause
> > > >
> > > > COPY
> > > > ..
> > > > ..
> > > > WITH
> > > > [
> > > > NO
> > > > (READ,WRITE)
> > > > PERMISSION TO
> > > > (GROUP,OTHERS)
> > > > ]
> > >
> > > If we're going to do anything here, it should use COPY's
> > > extensible-options syntax, I think.
> > >
> > >
> > > Thanks Robert. Let me send a patch for this.
> >
> >
> > how are you going to handle windows or unix ACLs here?
> > Its permission model is quite different and more powerful than
> (non-acl
> > based) unix in general, handling this in a flexible way might soon
> get
> > very complicated and complex for limited gain...
> >
> >
> > Hi Stefan,
> >
> > I had the same questions too. But, I believe, our initdb works in these
> > cases, after creating the data cluster. Isn't ?
>
> maybe - but having a fixed "default" is very different from baking a
> classic unix permission concept of user/group/world^others into actual
> DDL or into a COPY option. The proposed syntax might make some sense to
> a admin used to a unix style system but it is likely utterly
> incomprehensible to somebody who is used to (windows style) ACLs.
>
> I dont have a good answer on what to do else atm but I dont think we
> should embedded traditional/historical unix permission models in our
> grammer unless really really needed..
>
Yes, I agree with you. We shouldn't embed the unix style access model into
COPY.
COPY's default behaviour umask 644 on output files, which is giving read
access to other users.
I see, there is a good reason behind it. Also, it would be good to have a
control on the READ ACCESS of a file, where we can secure our dump files,
from the non instance ownership users.
Adding a trivial patch to control this read ACL.
--Sample Test Case
--Default behaviour
postgres=# COPY test_table TO '/tmp/readacs.txt';
COPY 1000
$ ls -l /tmp/readacs.txt
-rw-r--r-- /tmp/test1.txt
--With applied patch
postgres=# COPY test_table TO '/tmp/noreadacs.txt' NO READ ACCESS;
COPY 1000
$ ls -l /tmp/noreadacs.txt
-rw------- /tmp/noreadacs.txt
We can also use "PROGRAM 'cat > Output.csv' " to achieve this "NO READ
ACCESS", since the program is always running as a instance owner.
Let me know your inputs and thoughts.
Stefan
>
--
Regards,
Dinesh
manojadinesh.blogspot.com
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 2850b47..eb53ddf 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -44,6 +44,7 @@ COPY { <replaceable
class="parameter">table_name</replaceable> [ ( <replaceable
FORCE_NOT_NULL ( <replaceable class="parameter">column_name</replaceable>
[, ...] )
FORCE_NULL ( <replaceable class="parameter">column_name</replaceable> [,
...] )
ENCODING '<replaceable class="parameter">encoding_name</replaceable>'
+ NO READ ACCESS
</synopsis>
</refsynopsisdiv>
@@ -356,7 +357,17 @@ COPY { <replaceable
class="parameter">table_name</replaceable> [ ( <replaceable
</para>
</listitem>
</varlistentry>
-
+
+ <varlistentry>
+ <term><literal>NO READ ACCESS</></term>
+ <listitem>
+ <para>
+ This specifies, do not give any read access to non instance owner members.
+ If this option is omitted, copy gives read access to all members, which
is the default behaviour.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</refsect1>
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 8db1b35..367d430 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -133,7 +133,7 @@ typedef struct CopyStateData
bool convert_selectively; /* do selective binary
conversion? */
List *convert_select; /* list of column names (can be NIL) */
bool *convert_select_flags; /* per-column CSV/TEXT CS flags
*/
-
+ bool is_no_read_acc;
/* these are just for error messages, see CopyFromErrorCallback */
const char *cur_relname; /* table name for error messages */
int cur_lineno; /* line number for
error messages */
@@ -1153,6 +1153,14 @@ ProcessCopyOptions(CopyState cstate,
errmsg("argument to option
\"%s\" must be a valid encoding name",
defel->defname)));
}
+ else if (strcmp(defel->defname, "is_no_read_acc") == 0)
+ {
+ if (cstate->is_no_read_acc)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("conflicting or
redundant options")));
+ cstate->is_no_read_acc = true;
+ }
else
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -1302,6 +1310,11 @@ ProcessCopyOptions(CopyState cstate,
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("CSV quote character must not appear in
the NULL specification")));
+
+ if ( cstate->is_no_read_acc && is_from )
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("COPY owner access only available using
COPY TO file")));
}
/*
@@ -1723,7 +1736,11 @@ BeginCopyTo(Relation rel,
(errcode(ERRCODE_INVALID_NAME),
errmsg("relative path not allowed for
COPY to file")));
- oumask = umask(S_IWGRP | S_IWOTH);
+ if (cstate->is_no_read_acc)
+ oumask = umask(S_IWGRP | S_IRGRP | S_IWOTH |
S_IROTH);
+ else
+ oumask = umask(S_IWGRP | S_IWOTH);
+
cstate->copy_file = AllocateFile(cstate->filename,
PG_BINARY_W);
umask(oumask);
if (cstate->copy_file == NULL)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 1efc6d6..5dc5cbd 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2712,6 +2712,10 @@ copy_opt_item:
{
$$ = makeDefElem("encoding", (Node
*)makeString($2));
}
+ | NO READ ACCESS
+ {
+ $$ = makeDefElem("is_no_read_acc",
(Node *)makeInteger(TRUE));
+ }
;
/* The following exist for backward compatibility with very old versions */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
