Discussion:
Directory/File Access Permissions for COPY and Generic File Access Functions
Brightwell, Adam
2014-10-16 03:21:34 UTC
Permalink
All,

The attached patch for review implements a directory permission system that
allows for providing a directory read/write capability to directories for
COPY TO/FROM and Generic File Access Functions to non-superusers. This is
not a complete solution as it does not currently contain documentation or
regression tests. Though it is my hopes to get some feedback as I am sure
there are some aspects of it that need to be reworked. So I thought I'd
put it out there for comments/review.

The approach taken is to create "directory aliases" that have a unique name
and path, as well as an associated ACL list. A superuser can create a new
alias to any directory on the system and then provide READ or WRITE
permissions to any non-superuser. When a non-superuser then attempts to
execute a COPY TO/FROM or any one of the generic file access functions, a
permission check is performed against the aliases for the user and target
directory. Superusers are allowed to bypass all of these checks. All
alias paths must be an absolute path in order to avoid potential risks.
However, in the generic file access functions superusers are still allowed
to execute the functions with a relative path where non-superusers are
required to provide an absolute path.

- Implementation Details -

System Catalog:
pg_diralias
- dirname - the name of the directory alias
- dirpath - the directory path - must be absolute
- diracl - the ACL for the directory

Syntax:
CREATE DIRALIAS <name> AS '<path>'
ALTER DIRALIAS <name> AS '<path>'
ALTER DIRALIAS <name> RENAME TO <new_name>
DROP DIRALIAS <name>

This is probably the area that I would really appreciate your thoughts and
recommendations. To GRANT permissions to a directory alias, I had to create
a special variant of GRANT since READ and WRITE are not reserved keywords
and causes grammar issues. Therefore, I chose to start with the following
syntax:

GRANT ON DIRALIAS <name> <permissions> TO <roles>

where <permissions> is either READ, WRITE or ALL.

Any comments, suggestions or feedback would be greatly appreciated.

Thanks,
Adam
--
Adam Brightwell - ***@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.com
Tom Lane
2014-10-16 03:34:22 UTC
Permalink
Post by Brightwell, Adam
The attached patch for review implements a directory permission system that
allows for providing a directory read/write capability to directories for
COPY TO/FROM and Generic File Access Functions to non-superusers.
TBH, this sounds like it's adding a lot of mechanism and *significant*
risk of unforeseen security issues in order to solve a problem that we
do not need to solve. The field demand for such a feature is just about
indistinguishable from zero.

regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-***@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas
2014-10-16 15:15:55 UTC
Permalink
Post by Tom Lane
Post by Brightwell, Adam
The attached patch for review implements a directory permission system that
allows for providing a directory read/write capability to directories for
COPY TO/FROM and Generic File Access Functions to non-superusers.
TBH, this sounds like it's adding a lot of mechanism and *significant*
risk of unforeseen security issues in order to solve a problem that we
do not need to solve. The field demand for such a feature is just about
indistinguishable from zero.
I am also not convinced that we need this. If we need to allow
non-superusers COPY permission at all, can we just exclude certain
"unsafe" directories (like the data directory, and tablespaces) and
let them access anything else? Or can we have a whitelist of
directories stored as a PGC_SUSER GUC? This seems awfully heavyweight
for what it is.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-***@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Stephen Frost
2014-10-16 16:01:28 UTC
Permalink
Post by Robert Haas
Post by Tom Lane
Post by Brightwell, Adam
The attached patch for review implements a directory permission system that
allows for providing a directory read/write capability to directories for
COPY TO/FROM and Generic File Access Functions to non-superusers.
TBH, this sounds like it's adding a lot of mechanism and *significant*
risk of unforeseen security issues in order to solve a problem that we
do not need to solve. The field demand for such a feature is just about
indistinguishable from zero.
I am also not convinced that we need this. If we need to allow
non-superusers COPY permission at all, can we just exclude certain
"unsafe" directories (like the data directory, and tablespaces) and
let them access anything else?
Wow.. I'd say 'no' to this, certainly. Granularity is required here.
I want to give a non-superuser the ability to slurp data off a specific
NFS mount, not read /etc/passwd..
Post by Robert Haas
Or can we have a whitelist of
directories stored as a PGC_SUSER GUC? This seems awfully heavyweight
for what it is.
Hrm, perhaps this would work though..

Allow me to outline a few use-cases which I see for this though and
perhaps that'll help us make progress.

This started out as a request for a non-superuser to be able to review
the log files without needing access to the server. Now, things can
certainly be set up on the server to import *all* logs and then grant
access to a non-superuser, but generally it's "I need to review the log
from X to Y" and not *all* logs need to be stored or kept in PG.

In years past, I've wanted to be able to grant this ability out for
users to do loads without having to transfer the data through the user's
laptop or get them to log onto the Linux box from their Windows desktop
and pull the data in via psql (it's a bigger deal than some might
think..), and then there's the general ETL case where, without this, you
end up running something like Pentaho and having to pass all the data
through Java to get it into the database.

Building on that is the concept of *background* loads, with
pg_background. That's a killer capability, in my view. "Hey, PG, go
load all the files in this directory into this table, but don't make me
have to stick around and make sure my laptop is still connected for the
next 3 hours."

Next, the file_fdw could leverage this catalog to do its own checks and
allow non-superusers to use it, which would be fantastic and gets back
to the 'log file' use-case above.

And then there is the next-level item: CREATE TABLESPACE, which we
already see folks like RDS and others having to hack the codebase to
add as a non-superuser capability. It'd need to be an independently
grantable capability, of course.

Thanks!

Stephen
Bruce Momjian
2014-10-18 02:02:04 UTC
Permalink
Post by Stephen Frost
This started out as a request for a non-superuser to be able to review
the log files without needing access to the server. Now, things can
certainly be set up on the server to import *all* logs and then grant
access to a non-superuser, but generally it's "I need to review the log
from X to Y" and not *all* logs need to be stored or kept in PG.
Why is this patch showing up before being discussed? You are having to
back into the discusion because of this.
--
Bruce Momjian <***@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-***@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Stephen Frost
2014-10-18 02:29:51 UTC
Permalink
Post by Bruce Momjian
Post by Stephen Frost
This started out as a request for a non-superuser to be able to review
the log files without needing access to the server. Now, things can
certainly be set up on the server to import *all* logs and then grant
access to a non-superuser, but generally it's "I need to review the log
from X to Y" and not *all* logs need to be stored or kept in PG.
Why is this patch showing up before being discussed? You are having to
back into the discusion because of this.
For my part, I didn't actually see it as being a questionable use-case
from the start.. That was obviously incorrect, though I didn't know
that previously. The general idea has been discussed a couple of times
before, at least as far back as 2005:

http://www.postgresql.org/message-id/***@cs.concordia.ca

It's also a feature available in other databases (at least MySQL and
Oracle, but I'm pretty sure others also).

I can also recall chatting with folks about it a couple of times over
the years at various conferences. Still, perhaps it would have been
better to post about the idea before the patch, but hindsight is often
20/20.

Thanks!

Stephen
Peter Eisentraut
2014-10-21 19:06:21 UTC
Permalink
Post by Stephen Frost
This started out as a request for a non-superuser to be able to review
the log files without needing access to the server.
I think that can be done with a security-definer function.
--
Sent via pgsql-hackers mailing list (pgsql-***@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut
2014-10-21 19:14:13 UTC
Permalink
You patch is missing the files src/include/catalog/pg_diralias.h,
src/include/commands/diralias.h, and src/backend/commands/diralias.c.

(Hint: git add -N)
--
Sent via pgsql-hackers mailing list (pgsql-***@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Brightwell, Adam
2014-10-21 19:49:50 UTC
Permalink
Peter,

You patch is missing the files src/include/catalog/pg_diralias.h,
Post by Peter Eisentraut
src/include/commands/diralias.h, and src/backend/commands/diralias.c.
(Hint: git add -N)
Yikes, sorry about that, not sure how that happened. Attached is an
updated patch.

-Adam
--
Adam Brightwell - ***@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.com
Loading...