prest / prest

PostgreSQL ➕ REST, low-code, simplify and accelerate development, ⚡ instant, realtime, high-performance on any Postgres application, existing or new
https://prestd.com
MIT License
4.21k stars 283 forks source link

Endpoint to execute custom query #439

Open avelino opened 4 years ago

avelino commented 4 years ago

Custom SQL query execution via POST, open text based on SELECT, will not be accepted ~INSERT, UPDATE and DELETE~.

This feature must be chosen enabled in the configuration file, by default it comes disabled.

We should page of query result.

Be aware of security flaw in sql injection - we need to write tests to avoid this security flaw.

ENDPOINT: /_QUERY/ METHOD: POST BODY:

{"query": "SELECT * FROM table_name WHERE number_field=123 AND char_field='abc'"}

example of use

"text area" postico-sql-query-view

aodhan-domhnaill commented 3 years ago

Without using Postgres roles, I believe such an endpoint could easily bypass your current authorization scheme. See my comment in https://github.com/prest/prest/issues/468#issuecomment-731658923.

For example,

ENDPOINT: /_QUERY/
METHOD: POST
BODY:
{"query": "CREATE USER SUPERUSER PASSWORD 'password'"}
ENDPOINT: /_QUERY/
METHOD: POST
BODY:
{"query": "DROP TABLE tablename"}

The above is easy to avoid by only launching prest with a non-admin user, but you could still SELECT from tables that you tried to lock down with https://docs.postgres.rest/permissions/.

I think this endpoint is valuable, but authorization needs to be protected using Postgres roles or something similar that is solid.

avelino commented 3 years ago

Good point, I had thought only of happy cumin (select friendly), to see your drop query left me worried about this feature. We need to evolve our permissions system to support this feature

worais commented 3 years ago

It could have an env-level flag that allows or does not allow writing query, EX: CUSTOM_QUERY=false|select|all But this is really quite dangerous :( In the elastic we use auth_basic. it is an alternative for the most sensitive cases.

avelino commented 3 years ago

the query execution can be turned on with JWT (it doesn't work if you have turned it off), but we need to generate the token with minimum expiration.

If the token has generated more than XX seconds it is not executed - forcing a low expiration time

aodhan-domhnaill commented 3 years ago

Still seems very dangerous without using Postgres roles to control possible actions. Basically anyone who can execute a query immediately has maximum level permissions and they can create admin users so they continue to have those permissions forever

On Fri, Nov 27, 2020, 07:44 Avelino notifications@github.com wrote:

the query execution can be turned on with JWT (it doesn't work if you have turned it off), but we need to generate the token with minimum expiration.

If the token has generated more than XX seconds it is not executed - forcing a low expiration time

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/prest/prest/issues/439#issuecomment-734891612, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTBOJ2GIVWQSYOCVH3V65TSR7COPANCNFSM4RSFVHQA .

avelino commented 3 years ago

I don't know how to use the permissions part of postgresql to manage this feature.

@aidan-plenert-macdonald could you help pREST with this part by creating an ACL model based on postgresql permissions? Do you think this is possible?

fabriziomello commented 3 years ago

@avelino the concerns pointed out by @aidan-plenert-macdonald are valid because this feature will open the doors of the database to simply run SQL statements using and REST api. Do we really need it?? And I wonder because the doors we're open here also for SQL injection.

Anyway thinking more about it what if to execute SELECT statements we create a special schema maybe called _prest and encapsulate the SELECT statements as VIEWs inside it? We can create a hash based on the query and name views like _prest.select_MD5HASH for example.

Doing something like that maybe we'll prevent users to send arbitrary statements to PostgreSQL.

But we'll bypass the pREST permission system as @aidan-plenert-macdonald pointed out before... and to do this correctly we should parse the SQL statement to get relations involved and is this a thing we really want to do?? Incorporate the Postgres parser can be hard... is not impossible but is hard.

avelino commented 3 years ago

@fabriziomello in the issue description I explain the reason for the existence - pREST Admin will have a sql queries executor

@aidan-plenert-macdonald @fabriziomello thinking about security we can limit execution of select command (the query starts with select), what is your opinion about this form of "security" reason ❓ Remembering that I said this in the description of the issue

Custom SQL query execution via POST, open text based on SELECT, will not be accepted ~INSERT, UPDATE and DELETE~.

aodhan-domhnaill commented 3 years ago

I really don't think there is any alternative to Postgres Roles

Just send this over in a single query.

SELECT * FROM table;
DROP TABLE table;

You could try to filter 100's of ways, but ultimately with the exception of rewriting Postgres, only Postgres can truly limit access.

avelino commented 3 years ago

I agree that view is the best solution (pREST already works with this kind of object), but as an engineer (developer) I don't like "things" inside the database, drop view, create view to change something bothers me, not to mention lack of version control for views, functions and etc.

In the SELECT and DROP example in the same query: we can limit the query in the first ;, that would limit the malicious scripts