Row permission check short-circuits on insert failure

[pankgeorg]

Version Information

Server Version: Tested on 2.20, 2.29 and 2.32 CLI Version (for CLI related issue):

Environment

Docker

What is the current behaviour?

Insert mutation fails with uniqueness_violation

What is the expected behaviour?

Insert mutation to fail with row permission error

How to reproduce the issue?

1. Load the attached metadata and postgres schema pgdump and metadata.json.zip
2. try the insert in the screenshot below
3. Expect the hasura to check the permissions on a "visual" row. In this scenario, it would fail because the "label group" is not owned by the user.

This allows someone with knowledge of someone else's user_id to scan the database for that user's labels. A bit brute force, but 🤷🏾‍♂️

Screenshots or Screencast

Text to copy paste

```gql mutation A($obj: user_label_insert_input!){ insert_user_label_one(object:$obj){ label_group_id } } ``` ```json { "obj": { "label": "Secret", "user_id": "855c2296-e296-4c07-bc5f-d36386b1f82f", "label_group_id": "ab679f79-2109-4b89-bb47-db62b18d9771" } } ```

Please provide any traces or logs that could help here.

N/A

Any possible solutions/workarounds you're aware of?

N/A

Keywords

hasura, brute force, row permissions, ai, generative ai (last two labels are just for prioritization)

[manasag]

Hi @pankgeorg , the behavior in this case is because its not realistic for Hasura to be able to tell if the user has sufficient permissions before actually trying to insert the data and observing what comes out. Our engineering team will add more details on why of this in sometime.

[pankgeorg]

Hi @pankgeorg , the behavior in this case is because its not realistic for Hasura to be able to tell if the user has sufficient permissions before actually trying to insert the data and observing what comes out.

I kindly disagree. The permissions check is a SQL query and you can run that on a 'virtual' row (SELECT '1234' AS user_id, 4 AS group_id). Some queries won't be computable and the performance tradeoff may not be acceptable for the goals of Hasura, but that's different than "not realistic", at least based on how I perceive "realistic". Note that the current behaviour is "leaking" potentially confidential data.

Additionally, there is another mitigation of this issue: add an option to (smartly) filter the errors somehow

Our engineering team will add more details on why of this in sometime.

Looking forward to it!

[plcplc]

The issue with this is that the only way we can know if inserted rows passed a permissions check is to let the database actually perform the insert and then expect the resulting rows against the permission expression (which we do via the RETURNING sub-clause of INSERT INTO).

The reason this is necessary is because of column value defaulting, which can include arbitrary SQL scalar expressions as well as identity columns.

The best we could hope to do would be to build a new feature which always issues the permission-check-failed error (or a more vague error message even) whenever an insert fails for any reason. While that doesn't leak this bit of information, it's also very unhelpful for the use cases where it's not considered a security issue. As such, it ought to be speficiable by the user then, perhaps on a role-by-role basis whether or not to use this feature.

[pankgeorg]

The issue with this is that the only way we can know if inserted rows passed a permissions check is to let the database actually perform the insert and then expect the resulting rows against the permission expression (which we do via the RETURNING sub-clause of INSERT INTO).

The reason this is necessary is because of column value defaulting, which can include arbitrary SQL scalar expressions as well as identity columns.

I understand why it's convenient to use the RETURNING clause. But if the permissions check doesn't contain any IDs, then you can calculate the hypothetical row: It's user defined data + defaults (hasura only allows static values either way) and you can check it. So "only way we can know" is not exactly accurate, if my understanding is correct.

Having said that, I understand the hasura team not wanting to do this, for performance, style, code complexity or other internal reasons aligned with your mission. But let's agree it is within the realm of what is possible.

I think a better way out of this would be to integrate with an Open Policy Agent solution: the row permissions could then request access to specific resources for a specific user, and that would be computable more easily (in the attached case, a query like "user owns the group and user owns the label" or something similar). Which of course would need to be calculated before the insert otherwise it's the same issue.

[plcplc]

Sorry, I was writing my comment in an un-refreshed, day-old browser tab, so I didn't see your reply about using 'virtual' rows.

(hasura only allows static values either way)

The problem is not so much what defaults you can specify in Hasura, but what the database lets you specify.

But let's agree it is within the realm of what is possible.

It is certainly possible from a theoretical standpoint, but complex to execute on correctly in practise.