-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Information disclosure in RPC function error handling for inaccessible functions in exposedschemas #3980
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
In general, our "security model" is that of PostgreSQL. We depend on PostgreSQL roles, grants, RLS etc. PostgreSQL's philosophy is, that metadata like object names & co is not sensitive information. The system catalogs are public, a regular user can always see all the tables and functions. They might not be able to use them, though. Thus, I don't think this line of argument will fly for us - because we don't really want to implement our own security layer. That being said.. would it be cool, if we could give better error messages, where only accessible functions are suggested? Oh yeah, that would be great. But.. it would be hard. Because we'd need to actually know about the permissions of each role. But we don't - we rely on PostgreSQL to throw that error. So.. I don't think we can reasonably do anything about this. |
As stated in the Expected behavior part of my initial post, when I try to query tables or views in the |
PostgREST uses a schema cache, in which it keeps information about which functions exist. Before even hitting the database, we know whether the function exists or not. If it does not, we return the error suggesting alternatives, if possible. Only when we can match the RPC call to an existing function, we hit the database. At this time, PostgreSQL will return an error - in your case an error about the schema. This error is only about using the schema, i.e. referencing it in a query. It's not about "looking into the schema". You can look up the schema's contents in the system catalogs. We don't keep any role-related / permission-related data in the schema cache, thus we can not throw the permission error before the "not found" error. |
Thanks for the clarification @wolfgangwalther. If I understand correctly, the difference I observed in behavior exists because:
Can you confirm if this is accurate? Also, even with |
Correct.
Not sure about this. It might depend on the version you are using. But I'm not 100% sure whether we just discussed this somewhere or already changed it. In any case, it's likely that we will check the schema cache for tables and views first in the future as well. So the behavior might change regarding this. In general, we want to avoid hitting the database, when we can.
I don't think we are going to make any promises about that behavior - and tbh, I just don't know. I strongly suggest you don't rely on security through obscurity. |
Ok, I see. However, my concern here is not security per se, for which I rely on other strategies (schema usage grant/revoke is actually already good enough for security in this specific case). I am mostly interested in schema hiding as a feature of a broader IP protection strategy. Assuming part of the API to implement over PostgREST is only meant to be used by internal backend/service workers and not by third-party nor by user facing apps, it is reasonable to expect the schema to be somewhat protected from arbitrary users, following a least disclosure principle. I am not expecting such a solution to protect trade secrets, but I don't want to make it obvious either. One solution is of course to ensure the names do not reveal anything sensitive, but it's not necessary practical nor a real solution. One of the reasons why I decided to post this issue was @steve-chavez comment in the related issue where he states "I don't see harm in providing a bit of security through obscurity". Anyway, if PostgREST is actually not going to make any promise about schema concealing behaviors, I guess this becomes the concern of thid-party wrapper, proxy API, or full-blown separate API that can be put on top of PostgREST. |
Yes, Obscurity Is a Valid Security Layer. If we do implement #3824 we would need to cache some permissions anyway, so this issue would be an extension. Also, we might be able to avoid caching all permissions on functions to do this. By default, all db users have execute privilege on all functions in a schema (pg does something like So if we detect the "grant to PUBLIC" is not enabled, we could disable the Another simpler option might be just offering a config to disable the Edit: This would also solve #3824. |
I'd like to be proven wrong, but I'm quite sure that this will not turn out easy - at all.
It would only be a very small part of the bigger picture, though. What about error messages that come directly from PostgreSQL? For example constraint names are easily leaked, when they are violated. There are very likely many others, that we just don't see right now. I sure don't want to be responsible to keep track of that and provide solutions for all of that to hide this etc. - this is massive effort. |
Once more on this: I am very much -1 on adding permissions to the schema cache. This would probably lead to having to reload the schema cache in even more cases.. and give us real problems in scenarios with a lot of roles. For example, I don't want to keep the permissions of more than 100k roles in the schema cache in my current project. |
Right, I'm much more in favor of being able to customize the client error messages verbosity now (voiding the hint and maybe details). That should also solve #3824. |
If that works for those as well...
... then maybe it would cover most cases. I currently can't imagine a non-error case exposing things. Except for OpenAPI ofc ;) |
As far as I understand, configurable client error message verbosity would definitely help cover a lot of related concerns. I had already noted constraint name leakage as a possible concern. Do you know how such a feature would that work in practice? Would the verbosity be configurable by error code or some other query-time variable? There are some error messages/details/codes that the client let the end-user know about in order to fix their input, in particular custom exceptions intentionally raised from a postgres functions and maybe certain unique constraint violations; unless you tell me that recommended approach is to wrap such violations into custom exception. I would expect the dev environment to use the highest level of verbosity in general, but there would likely be a need to guarantee that important business logic does not depend on error details that could be hidden in production. Ideally, I could also imagine in a production environment to let certain clients get more error info under certain circumstances, like based on the postgres role used. One could also imagine error info verbosity not to depend on postgres role used but on execution environment: e.g., a query executed by the backend on behalf of a non-superuser gets more error details than if executed by the end-user client itself, but maybe that would break assumptions if the error details are necessary to display the correct error message to end-user and not just for logging, resulting in mismatch between client and server-side rendering. Also, this might not fit well within PostgREST execution model. |
@masda70 Related to customizing error messages: #2706. I think that should solve the concerns you mentioned.
I was thinking a new config option, or maybe we try to reuse client_min_messages so it can be set on a per-role basis. |
client-min-messages is like log-level - it says which messages to log, not what level of detail those would have, right? So I don't think this is a fit here. Also, this would need to be cached for each user, right? I don't think we should do that. I don't see us providing any configurability here, except "disable detail/hint for all errors". Everything else is way too much effort for little benefit. |
Just now I also had the request to replace the I believe we should solve this with #2706 instead, that way users have much more flexibility and can do role based logic too. |
I shared the wrong config, I knew there was something about error verboseness: log_error_verbosity. That only applies to server-side error messages though. Searching more I found the psql \errverbose, which internally uses PQresultVerboseErrorMessage. Which implies one can set an enum
Looks this can be controlled per-session with PQsetErrorVerbosity. There's no database config though, I believe it can only be done through code. So solving this issue should be orthogonal to #2706. |
Uh oh!
There was an error while loading. Please reload this page.
Problem
When a user attempts to call RPC functions in an exposed schema they don't have access to, PostgREST sometimes exposes information about the underlying schema through error messages. This creates an inconsistent security boundary and potential information leakage.
Current Behavior:
admin.custom_function2
):admin.missing_function
):admin.custom_function
):Expected Behavior
For security and consistency, PostgREST should return the same permission denied error message in all cases where a user attempts to access a schema they don't have permission for, regardless of whether the function exists or not:
This is consistent with the behavior when querying tables or views in protected schemas.
Related Issue
This issue is related to #3824
Additional notes
When trying to access an object (view/table/function) to which no GRANT has been given for the current role in an exposed schema (e.g.,
public
), PostgreSQL returns error messages likepermission denied for view xxxx
that may expose details about object existence, function parameters or table columns. This behavior appears to be expected when schema usage privileges are granted but specific object privileges are restricted.For more granular access control and to prevent information disclosure between roles, it seems the recommended approach is to organize database objects into multiple schemas and only grant
USAGE
privileges to each role for schemas they need to access. This architecture would likely ensure more complete information hiding between roles with different access requirements.The text was updated successfully, but these errors were encountered: