Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi All, Here is the v4 patch with the following new changes: 1) Now allows users to explicitly set search_path to $extension_schema. 2) Includes updates to the documentation. 3) Adds comments where previously absent. Note: The new control file parameter is currently named as "protected" which is actually not the precise name knowing that it just solves a small portion of security problems related to extensions. I intend to rename it to something more appropriate later; but any suggestions are welcome. Besides, should we consider restricting the installation of extensions in schemas where a UDF with the same name that the extension intends to create already exists? Additionally, similar restrictions can also be applied if UDF being created shares its name with a function already created by an extension in that schema? I haven't looked at the feasibility part, but just thought of sharing it just because something of this sort would probably solve most of the issues related to extensions. -- With Regards, Ashutosh Sharma. v4-0001-Introduce-new-control-file-parameter-protected-to-de.patch Description: Binary data
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi Robert, On Tue, Jul 16, 2024 at 9:40 PM Robert Haas wrote: > > On Tue, Jul 16, 2024 at 1:55 AM Ashutosh Sharma wrote: > > Just to confirm, are you suggesting to remove the protected flag and > > set the default search_path (as $extension_schema,) for all functions > > within an extension where no explicit search_path is set? > > No, I'm not saying that. In fact I'm not sure we should have the > protected flag at all. > Based on our previous discussions in this thread - [1], [2], we wanted to give extension authors the ability to decide if they would like to go with the current behavior or if they would like to adopt the new behavior where the default search_path will be enforced for functions that do not have search_path explicitly set. That is why we considered introducing this flag. > > In addition > > to that, also allow users to explicitly set $extension_schema as the > > search_path and bypass resolution of $extension_schema for objects > > outside the extension? > > Yes, I'm saying that. > Sure, thanks for confirming. I'll make sure to address this in the next patch version. [1] - https://www.postgresql.org/message-id/340cd4a0c30b46a185e66b1c7e91535921137da8.camel%40j-davis.com [2] - https://www.postgresql.org/message-id/CAGECzQSms%2BikWo7E0E1QAVvhM2%2B9FQydEywyCLztPaAYr9s%2BBw%40mail.gmail.com -- With Regards, Ashutosh Sharma.
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Tue, Jul 16, 2024 at 1:55 AM Ashutosh Sharma wrote: > Just to confirm, are you suggesting to remove the protected flag and > set the default search_path (as $extension_schema,) for all functions > within an extension where no explicit search_path is set? No, I'm not saying that. In fact I'm not sure we should have the protected flag at all. > In addition > to that, also allow users to explicitly set $extension_schema as the > search_path and bypass resolution of $extension_schema for objects > outside the extension? Yes, I'm saying that. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi Robert. On Mon, Jul 15, 2024 at 11:15 PM Robert Haas wrote: > > On Mon, Jul 15, 2024 at 8:05 AM Ashutosh Sharma wrote: > > I've added these changes to restrict users from explicitly setting the > > $extension_schema in the search_path. This ensures that > > $extension_schema can only be set implicitly for functions created by > > the extension when the "protected" flag is enabled. > > But ... why? I mean, what's the point of prohibiting that? In fact, > maybe we should have *that* and forget about the protected flag in the > control file. > Just to confirm, are you suggesting to remove the protected flag and set the default search_path (as $extension_schema,) for all functions within an extension where no explicit search_path is set? In addition to that, also allow users to explicitly set $extension_schema as the search_path and bypass resolution of $extension_schema for objects outside the extension? -- With Regards, Ashutosh Sharma.
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Mon, 2024-07-15 at 16:04 -0400, Robert Haas wrote: > Oh, I had the opposite idea: I wasn't proposing ignoring it. I was > proposing making it work. I meant: ignore $extension_schema if the search_path has nothing to do with an extension. In other words, if it's in a search_path for the session, or on a function that's not part of an extension. On re-reading, I see that you mean it should work if they explicitly set it as a part of a function that *is* part of an extension. And I agree with that -- just make it work. Regards, Jeff Davis
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Mon, Jul 15, 2024 at 2:33 PM Jeff Davis wrote: > On Mon, 2024-07-15 at 13:44 -0400, Robert Haas wrote: > > But ... why? I mean, what's the point of prohibiting that? > > Agreed. We ignore all kinds of stuff in search_path that doesn't make > sense, like non-existent schemas. Simpler is better. Oh, I had the opposite idea: I wasn't proposing ignoring it. I was proposing making it work. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Mon, 2024-07-15 at 13:44 -0400, Robert Haas wrote: > But ... why? I mean, what's the point of prohibiting that? Agreed. We ignore all kinds of stuff in search_path that doesn't make sense, like non-existent schemas. Simpler is better. Regards, Jeff Davis
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Mon, Jul 15, 2024 at 8:05 AM Ashutosh Sharma wrote: > I've added these changes to restrict users from explicitly setting the > $extension_schema in the search_path. This ensures that > $extension_schema can only be set implicitly for functions created by > the extension when the "protected" flag is enabled. But ... why? I mean, what's the point of prohibiting that? In fact, maybe we should have *that* and forget about the protected flag in the control file. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi Robert,
On Fri, Jul 12, 2024 at 9:02 PM Robert Haas wrote:
>
> On Mon, Jun 24, 2024 at 3:10 PM Jeff Davis wrote:
> > A special search_path variable "$extension_schema" would be a better
> > solution to this problem. We need something like that anyway, in case
> > the extension is relocated, so that we don't have to dig through the
> > catalog and update all the settings.
>
> +1. I think we need that in order for this proposal to make any progress.
>
> And it seems like the patch has something like that, but I don't
> really understand exactly what's going on here, because it's also got
> hunks like this in a bunch of places:
>
> + if (strcmp($2, "$extension_schema") == 0)
> + ereport(ERROR,
> + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> + errmsg("search_path cannot be set to $extension_schema"),
> + parser_errposition(@2)));
>
> So it seems like the patch is trying to support $extension_schema in
> some cases but not others, which seems like an odd choice that, as far
> as I can see, is not explained anywhere: not in the commit message,
> not in the comments (which are pretty minimally updated by the patch),
> and not in the documentation (which the patch doesn't update at all).
>
> Ashutosh, can we please get some of that stuff updated, or at least a
> clearer explanation of what's going on with $extension_schema here?
>
I've added these changes to restrict users from explicitly setting the
$extension_schema in the search_path. This ensures that
$extension_schema can only be set implicitly for functions created by
the extension when the "protected" flag is enabled.
I apologize for not commenting on this change initially. I'll review
the patch, add comments where needed, and submit an updated version.
--
With Regards,
Ashutosh Sharma.
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Mon, Jun 24, 2024 at 3:10 PM Jeff Davis wrote:
> A special search_path variable "$extension_schema" would be a better
> solution to this problem. We need something like that anyway, in case
> the extension is relocated, so that we don't have to dig through the
> catalog and update all the settings.
+1. I think we need that in order for this proposal to make any progress.
And it seems like the patch has something like that, but I don't
really understand exactly what's going on here, because it's also got
hunks like this in a bunch of places:
+ if (strcmp($2, "$extension_schema") == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("search_path cannot be set to $extension_schema"),
+ parser_errposition(@2)));
So it seems like the patch is trying to support $extension_schema in
some cases but not others, which seems like an odd choice that, as far
as I can see, is not explained anywhere: not in the commit message,
not in the comments (which are pretty minimally updated by the patch),
and not in the documentation (which the patch doesn't update at all).
Ashutosh, can we please get some of that stuff updated, or at least a
clearer explanation of what's going on with $extension_schema here?
--
Robert Haas
EDB: http://www.enterprisedb.com
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Tue, Jun 25, 2024 at 12:40 AM Jeff Davis wrote: > > On Wed, 2024-06-19 at 08:53 +0530, Ashutosh Sharma wrote: > > For standalone functions, users can easily adjust the search_path > > settings as needed. However, managing this becomes challenging for > > functions created via extensions. Extensions are relocatable, making > > it difficult to determine and apply search_path settings in advance > > within the extension_name--*.sql file when defining functions or > > procedures. > > A special search_path variable "$extension_schema" would be a better > solution to this problem. We need something like that anyway, in case > the extension is relocated, so that we don't have to dig through the > catalog and update all the settings. > > > Introducing a new control flag option allows users to set > > implicit search_path settings for functions created by the extension, > > if needed. The main aim here is to enhance security for functions > > created by extensions by setting search_path at the function level. > > This ensures precise control over how objects are accessed within > > each > > function, making behavior more predictable and minimizing security > > risks, > > That leaves me wondering whether it's being addressed at the right > level. > > For instance, did you consider just having a GUC to control the default > search_path for a function? That may be too magical, but if so, why > would an extension-only setting be better? > I haven't given any such thought, my current focus is on extensions, specifically increasing their security with respect to superuser elevation. Regarding your first question which you had raised earlier : "What exactly is the right search_path for a function defined in an extension?" As I understand it, we cannot precisely determine the search_path for functions at the time of function creation in the code. The most accurate path (or something close to it) we can identify for functions created by extensions is the search_path set by the extension during its creation, which is what we aim to achieve with $extension_schema. This setting is up to the discretion of the author as to whether they are comfortable with this implicit search_path configuration at the function level. If not, they have the option to disable it. However, AFAIU, in most cases, especially when the extension can be relocated, this situation is unlikely to occur. -- With Regards, Ashutosh Sharma,
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Wed, 2024-06-19 at 08:53 +0530, Ashutosh Sharma wrote: > For standalone functions, users can easily adjust the search_path > settings as needed. However, managing this becomes challenging for > functions created via extensions. Extensions are relocatable, making > it difficult to determine and apply search_path settings in advance > within the extension_name--*.sql file when defining functions or > procedures. A special search_path variable "$extension_schema" would be a better solution to this problem. We need something like that anyway, in case the extension is relocated, so that we don't have to dig through the catalog and update all the settings. > Introducing a new control flag option allows users to set > implicit search_path settings for functions created by the extension, > if needed. The main aim here is to enhance security for functions > created by extensions by setting search_path at the function level. > This ensures precise control over how objects are accessed within > each > function, making behavior more predictable and minimizing security > risks, That leaves me wondering whether it's being addressed at the right level. For instance, did you consider just having a GUC to control the default search_path for a function? That may be too magical, but if so, why would an extension-only setting be better? > especially for SECURITY DEFINER functions associated with > extensions created by superusers. I'm not sure that it's specific to SECURITY DEFINER functions. Regards, Jeff Davis
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi, On Wed, Jun 19, 2024 at 6:06 AM John H wrote: > > Hi Ashutosh, > > Thanks for clarifying. > > > not standalone functions created independently > > I'm wondering why we wouldn't want to extend that functionality so > that if users (including superusers) do want to automatically > configure search_path > when they're creating functions it's available. > > > The difference is that installing extensions typically requires superuser > > privileges, > > which is not the case with standalone functions. > > Yes, but inadvertent access to different roles can still occur even if > it's not a superuser. > > It's not clear to me who the audience of this commit is aimed at, > cause I see two sort of > different use cases? > > 1. Make it easier for extension authors to configure search_path when > creating functions > > The proposed mechanism via control file makes sense, although I would > like to understand > why specifying it today in CREATE FUNCTION doesn't work. Is it an > awareness issue? > I suppose it's easier if you only need to set it once in the control file? > Is that ease-of-use what we want to solve? > > 2. Make it easier to avoid inadvertent escalations when functions are created > (e.g CREATE EXTENSION/CREATE FUNCTION) > > Then I think it's better to provide users a way to force the > search_path on functions when > extensions are created so superusers aren't reliant on extension authors. > For standalone functions, users can easily adjust the search_path settings as needed. However, managing this becomes challenging for functions created via extensions. Extensions are relocatable, making it difficult to determine and apply search_path settings in advance within the extension_name--*.sql file when defining functions or procedures. Introducing a new control flag option allows users to set implicit search_path settings for functions created by the extension, if needed. The main aim here is to enhance security for functions created by extensions by setting search_path at the function level. This ensures precise control over how objects are accessed within each function, making behavior more predictable and minimizing security risks, especially for SECURITY DEFINER functions associated with extensions created by superusers. -- With Regards, Ashutosh Sharma.
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi Ashutosh, Thanks for clarifying. > not standalone functions created independently I'm wondering why we wouldn't want to extend that functionality so that if users (including superusers) do want to automatically configure search_path when they're creating functions it's available. > The difference is that installing extensions typically requires superuser > privileges, > which is not the case with standalone functions. Yes, but inadvertent access to different roles can still occur even if it's not a superuser. It's not clear to me who the audience of this commit is aimed at, cause I see two sort of different use cases? 1. Make it easier for extension authors to configure search_path when creating functions The proposed mechanism via control file makes sense, although I would like to understand why specifying it today in CREATE FUNCTION doesn't work. Is it an awareness issue? I suppose it's easier if you only need to set it once in the control file? Is that ease-of-use what we want to solve? 2. Make it easier to avoid inadvertent escalations when functions are created (e.g CREATE EXTENSION/CREATE FUNCTION) Then I think it's better to provide users a way to force the search_path on functions when extensions are created so superusers aren't reliant on extension authors. Thanks, -- John Hsu - Amazon Web Services
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi John, On Tue, Jun 18, 2024 at 2:35 AM John H wrote: > > Hi Ashutosh, > > Thinking about this more, could you clarify the problem/issue at hand? > I think it's still not clear to me. > Yes, CREATE EXTENSION can create functions that lead to unexpected > privilege escalation, regardless > if they are SECURITY DEFINER or SECURITY INVOKER (if the function is > inadvertently executed by superuser). > But that's also true for a general CREATE FUNCTION call outside of extensions. > This specifically applies to extension functions, not standalone functions created independently. The difference is that installing extensions typically requires superuser privileges, which is not the case with standalone functions. -- With Regards, Ashutosh Sharma.
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi Ashutosh, Thinking about this more, could you clarify the problem/issue at hand? I think it's still not clear to me. Yes, CREATE EXTENSION can create functions that lead to unexpected privilege escalation, regardless if they are SECURITY DEFINER or SECURITY INVOKER (if the function is inadvertently executed by superuser). But that's also true for a general CREATE FUNCTION call outside of extensions. If I read the commit message I see: > This flag controls PostgreSQL's behavior in setting the implicit > search_path within the proconfig for functions created by an extension > that does not have the search_path explicitly set in proconfig If that's the "general" issue you're trying to solve, I would wonder why we we wouldn't for instance be extending the functionality to normal CREATE FUNCTION calls (ie, schema qualify based off search_path) Thanks, John H On Thu, Jun 13, 2024 at 1:09 AM Ashutosh Sharma wrote: > > Hi, > > Please find the attached patch addressing all the comments. I kindly > request your review and feedback. Your thoughts are greatly > appreciated. > > -- > With Regards, > Ashutosh Sharma. -- John Hsu - Amazon Web Services
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi, Please find the attached patch addressing all the comments. I kindly request your review and feedback. Your thoughts are greatly appreciated. -- With Regards, Ashutosh Sharma. v3-0001-Introduce-a-new-control-file-flag-called-protected-f.patch Description: Binary data
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi, On Wed, Jun 12, 2024 at 11:35 PM John H wrote: > > > But, I also agree with Jelte, it should be a property of a control file, > > rather than a user controlled parameter, so that an attacker can't opt out. > This will be addressed in the next patch version. > +1. Also curious what happens if an extension author has search_path > already set in proconfig for a function that doesn't match what's in > the control file. I'm guessing the function one should take > precedence. > Yes, if the author has explicitly set the proconfig, it will take precedence. -- With Regards, Ashutosh Sharma.
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Wed, 2024-06-12 at 15:36 -0400, Robert Haas wrote: > But I think there's another problem, which is > that if the extension is relocatable, how do you set a secure > search_path? You could say SET search_path = foo, pg_catalog if you > know the extension will be installed in schema foo, but if you don't > know in what schema the extension will be installed, then what are > you > supposed to do? The proposal of litting $extension_schema could help > with that ... > > ...except I'm not sure that's really a full solution either, because > what if the extension is installed into a schema that's writable by > others, like public? Jelte proposed something to fix that here: https://www.postgresql.org/message-id/CAGECzQQzDqDzakBkR71ZkQ1N1ffTjAaruRSqppQAKu3WF%2B6rNQ%40mail.gmail.com Regards, Jeff Davis
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Wed, Jun 12, 2024 at 2:05 PM John H wrote:> > I'm sympathetic to the problem of potential privilege escalation when > executing functions. At the same time I'm not sure if there's that > much of a difference between 'CREATE EXTENSION' vs superusers copying > a series of 'CREATE FUNCTION' where they don't understand these same > nuances. +1. > CREATE FUNCTION already provides an ability to set the search_path. So > I'm wondering what the problem we want to solve here is. Is it that > there's too much friction for extension authors to have to set > search_path as part of the function definition and we want to make it > easier for them to "set once and forget"? If that's the problem we want to solve, I'm unconvinced that it's worth doing anything. But I think there's another problem, which is that if the extension is relocatable, how do you set a secure search_path? You could say SET search_path = foo, pg_catalog if you know the extension will be installed in schema foo, but if you don't know in what schema the extension will be installed, then what are you supposed to do? The proposal of litting $extension_schema could help with that ... ...except I'm not sure that's really a full solution either, because what if the extension is installed into a schema that's writable by others, like public? If $extension_schema resolves to public and public allows CREATE access to normal users, you're in a lot of trouble again already, because now an attacker can try to capture operator references within your function, or function calls that are approximate matches to some existing function by introducing perfect matches that take priority. Perhaps we want to consider the case where the attacker can write to the schema containing the extension as an unsupported scenario, but if so, we'd better be explicit about that. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Wed, 2024-06-12 at 12:13 +0530, Ashutosh Bapat wrote: > > Alternatively, we could leverage the extension dependency > > information > > to determine whether the function is created by an extension or > > not. > > That will be simpler. We do that sort of thing for identity > sequences. So there's a precedent to do that kind of stuff. I did not look at the details, but +1 for using information we already have. There's a little bit of extra work to resolve it, but thanks to the search_path cache it should only need to be done once per unique search_path setting per session. Regards, Jeff Davis
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi, > I know about the problem and have seen the original email. I'm sympathetic to the problem of potential privilege escalation when executing functions. At the same time I'm not sure if there's that much of a difference between 'CREATE EXTENSION' vs superusers copying a series of 'CREATE FUNCTION' where they don't understand these same nuances. CREATE FUNCTION already provides an ability to set the search_path. So I'm wondering what the problem we want to solve here is. Is it that there's too much friction for extension authors to have to set search_path as part of the function definition and we want to make it easier for them to "set once and forget"? > But, I also agree with Jelte, it should be a property of a control file, > rather than a user controlled parameter, so that an attacker can't opt out. +1. Also curious what happens if an extension author has search_path already set in proconfig for a function that doesn't match what's in the control file. I'm guessing the function one should take precedence. -- John Hsu - Amazon Web Services
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi Ashutosh, Apologies for any confusion, but I'm not entirely following your > explanation. Could you kindly provide further clarification? > Additionally, would you mind reviewing the problem description > outlined in the initial email? > I know about the problem and have seen the original email. What confused me, is that your email didn't specify that SET SEARCH_PATH in the CREATE EXTENSION is a boolean flag, hence I made an assumption that it is a TEXT (similar to GUC with the same name). Now after looking at your code it makes more sense. Sorry about the confusion. But, I also agree with Jelte, it should be a property of a control file, rather than a user controlled parameter, so that an attacker can't opt out. Regards, -- Alexander Kukushkin
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Wed, Jun 12, 2024 at 9:51 AM Ashutosh Sharma wrote: > On Wed, Jun 12, 2024 at 9:35 AM Ashutosh Sharma > wrote: > > > > Hi Jeff, > > > > On Wed, Jun 12, 2024 at 3:07 AM Jeff Davis wrote: > > > > > > On Tue, 2024-06-11 at 15:24 +0530, Ashutosh Sharma wrote: > > > > 3) When the ALTER EXTENSION SET SCHEMA command is executed and if the > > > > function's search_path contains the old schema of the extension, it > > > > is > > > > updated with the new schema. > > > > > > I don't think it's reasonable to search-and-replace within a function's > > > SET clause at ALTER time. > > > > > > I believe we need a new special search_path item, like > > > "$extension_schema", to mean the schema of the extension owning the > > > function. It would, like "$user", automatically adjust to the current > > > value when changed. > > > > > > That sounds like a useful and non-controversial change. > > > > I've definitely thought about it, and I agree that this approach could > > help simplify things. However, we do have some challenges with the > > resolution of $extension_schema variable compared to $user. Firstly, > > we need something like CurrentExtensionId to resolve > > $extension_schema, and we need to decide where to manage it > > (CurrentExtensionId). From what I understand, we should set > > CurrentExtensionId when we're getting ready to execute a function, as > > that's when we recompute the namespace path. But to set > > CurrentExtensionId at this point, we need information in pg_proc to > > distinguish between extension-specific and non-extension functions. If > > it's an extension specific function, it should have the extension OID > > in pg_proc, which we can use to find the extension's namespace and > > eventually resolve $extension_schema. So, to summarize this at a high > > level, here's is what I think can be done: > > > > 1) Include extension-specific details, possibly the extension OID, for > > functions in pg_proc during function creation. > > > > Alternatively, we could leverage the extension dependency information > to determine whether the function is created by an extension or not. > > That will be simpler. We do that sort of thing for identity sequences. So there's a precedent to do that kind of stuff. -- Best Wishes, Ashutosh Bapat
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Wed, Jun 12, 2024 at 9:35 AM Ashutosh Sharma wrote: > > Hi Jeff, > > On Wed, Jun 12, 2024 at 3:07 AM Jeff Davis wrote: > > > > On Tue, 2024-06-11 at 15:24 +0530, Ashutosh Sharma wrote: > > > 3) When the ALTER EXTENSION SET SCHEMA command is executed and if the > > > function's search_path contains the old schema of the extension, it > > > is > > > updated with the new schema. > > > > I don't think it's reasonable to search-and-replace within a function's > > SET clause at ALTER time. > > > > I believe we need a new special search_path item, like > > "$extension_schema", to mean the schema of the extension owning the > > function. It would, like "$user", automatically adjust to the current > > value when changed. > > > > That sounds like a useful and non-controversial change. > > I've definitely thought about it, and I agree that this approach could > help simplify things. However, we do have some challenges with the > resolution of $extension_schema variable compared to $user. Firstly, > we need something like CurrentExtensionId to resolve > $extension_schema, and we need to decide where to manage it > (CurrentExtensionId). From what I understand, we should set > CurrentExtensionId when we're getting ready to execute a function, as > that's when we recompute the namespace path. But to set > CurrentExtensionId at this point, we need information in pg_proc to > distinguish between extension-specific and non-extension functions. If > it's an extension specific function, it should have the extension OID > in pg_proc, which we can use to find the extension's namespace and > eventually resolve $extension_schema. So, to summarize this at a high > level, here's is what I think can be done: > > 1) Include extension-specific details, possibly the extension OID, for > functions in pg_proc during function creation. > Alternatively, we could leverage the extension dependency information to determine whether the function is created by an extension or not. > 2) Check if a function is extension-specific while preparing for > function execution and set CurrentExtensionId accordingly. > > 3) Utilize CurrentExtensionId to resolve the namespace path during > recomputation. > > 4) Above steps will automatically repeat if the function is nested. > > 5) Upon completion of function execution, reset CurrentExtensionId to > InvalidOid. > > I think this should effectively tackle the outlined challenges with > resolution of $extension_schema during namespace path recomputation. > What are your thoughts on it? > > Thanks, > > -- > With Regards, > Ashutosh Sharma.
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi Jeff, On Wed, Jun 12, 2024 at 3:07 AM Jeff Davis wrote: > > On Tue, 2024-06-11 at 15:24 +0530, Ashutosh Sharma wrote: > > 3) When the ALTER EXTENSION SET SCHEMA command is executed and if the > > function's search_path contains the old schema of the extension, it > > is > > updated with the new schema. > > I don't think it's reasonable to search-and-replace within a function's > SET clause at ALTER time. > > I believe we need a new special search_path item, like > "$extension_schema", to mean the schema of the extension owning the > function. It would, like "$user", automatically adjust to the current > value when changed. > > That sounds like a useful and non-controversial change. I've definitely thought about it, and I agree that this approach could help simplify things. However, we do have some challenges with the resolution of $extension_schema variable compared to $user. Firstly, we need something like CurrentExtensionId to resolve $extension_schema, and we need to decide where to manage it (CurrentExtensionId). From what I understand, we should set CurrentExtensionId when we're getting ready to execute a function, as that's when we recompute the namespace path. But to set CurrentExtensionId at this point, we need information in pg_proc to distinguish between extension-specific and non-extension functions. If it's an extension specific function, it should have the extension OID in pg_proc, which we can use to find the extension's namespace and eventually resolve $extension_schema. So, to summarize this at a high level, here's is what I think can be done: 1) Include extension-specific details, possibly the extension OID, for functions in pg_proc during function creation. 2) Check if a function is extension-specific while preparing for function execution and set CurrentExtensionId accordingly. 3) Utilize CurrentExtensionId to resolve the namespace path during recomputation. 4) Above steps will automatically repeat if the function is nested. 5) Upon completion of function execution, reset CurrentExtensionId to InvalidOid. I think this should effectively tackle the outlined challenges with resolution of $extension_schema during namespace path recomputation. What are your thoughts on it? Thanks, -- With Regards, Ashutosh Sharma.
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Tue, 2024-06-11 at 15:24 +0530, Ashutosh Sharma wrote: > 3) When the ALTER EXTENSION SET SCHEMA command is executed and if the > function's search_path contains the old schema of the extension, it > is > updated with the new schema. I don't think it's reasonable to search-and-replace within a function's SET clause at ALTER time. I believe we need a new special search_path item, like "$extension_schema", to mean the schema of the extension owning the function. It would, like "$user", automatically adjust to the current value when changed. That sounds like a useful and non-controversial change. Regards, Jeff Davis
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Tue, 2024-06-11 at 14:56 +0200, Alexander Kukushkin wrote: > Now attackers can just set search_path for the current session. IIUC, the proposal is that only the function's "SET" clause can override the behavior, not a top-level SET command. Regards, Jeff Davis
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi Alexander, On Tue, Jun 11, 2024 at 6:26 PM Alexander Kukushkin wrote: > > Hi, > > On Tue, 11 Jun 2024 at 14:50, Ashutosh Sharma wrote: >> >> If the author has configured the search_path for any desired function, >> using this option with the CREATE EXTENSION command will not affect >> those functions. > > > Then effectively this feature is useless. > Now attackers can just set search_path for the current session. > With this feature they will also be able to influence search_path of not > protected functions when they create an extension. > Apologies for any confusion, but I'm not entirely following your explanation. Could you kindly provide further clarification? Additionally, would you mind reviewing the problem description outlined in the initial email? -- With Regards, Ashutosh Sharma.
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi, On Tue, 11 Jun 2024 at 14:50, Ashutosh Sharma wrote: > If the author has configured the search_path for any desired function, > using this option with the CREATE EXTENSION command will not affect > those functions. > Then effectively this feature is useless. Now attackers can just set search_path for the current session. With this feature they will also be able to influence search_path of not protected functions when they create an extension. Regards, -- Alexander Kukushkin
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi, On Tue, Jun 11, 2024 at 5:02 PM Jelte Fennema-Nio wrote: > > On Tue, 11 Jun 2024 at 11:54, Ashutosh Sharma wrote: > > 1) Extends the CREATE EXTENSION command to support a new option, SET > > SEARCH_PATH. > > > I don't think it makes sense to add such an option to CREATE EXTENSION. > I feel like such a thing should be part of the extension control file > instead. That way the extension author controls the search path, not > the person that installs the extension. If the author has configured the search_path for any desired function, using this option with the CREATE EXTENSION command will not affect those functions. -- With Regards, Ashutosh Sharma.
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Tue, 11 Jun 2024 at 11:54, Ashutosh Sharma wrote: > 1) Extends the CREATE EXTENSION command to support a new option, SET > SEARCH_PATH. I don't think it makes sense to add such an option to CREATE EXTENSION. I feel like such a thing should be part of the extension control file instead. That way the extension author controls the search path, not the person that installs the extension.
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Thu, Jun 6, 2024 at 2:36 AM Jeff Davis wrote: > > On Wed, 2024-06-05 at 14:36 +0530, Ashutosh Sharma wrote: > > Thank you, Ashutosh, for the quick response. I've drafted a patch > > aimed at addressing this issue. The patch attempts to solve this > > issue by configuring the search_path for all security definer > > functions created by the extension. > > I like the general direction you propose, but I think it needs more > discussion about the details. > > * What exactly is the right search_path for a function defined in an > extension? > > * Do we need a new magic search_path value of "$extension_schema" that > resolves to the extension's schema, so that it can handle ALTER > EXTENSION ... SET SCHEMA? > > * What do we do for functions that want the current behavior and how do > we handle migration issues? > > * What about SECURITY INVOKER functions? Those can still be vulnerable > to manipulation by the caller by setting search_path, which can cause > an incorrect value to be returned. That can matter in some contexts > like a CHECK constraint. > Attached is the new version of patch addressing aforementioned comments. It implements the following changes: 1) Extends the CREATE EXTENSION command to support a new option, SET SEARCH_PATH. 2) If the SET SEARCH_PATH option is specified with the CREATE EXTENSION command, the implicit search_path for functions created by an extension is set, if not already configured. This is true for both SECURITY DEFINER and SECURITY INVOKER functions. 3) When the ALTER EXTENSION SET SCHEMA command is executed and if the function's search_path contains the old schema of the extension, it is updated with the new schema. Please have a look and let me know your comments. -- With Regards, Ashutosh Sharma. v2-0001-Implement-implicit-search_path-assignment-for-extens.patch Description: Binary data
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Fri, 2024-06-07 at 00:19 +0200, Jelte Fennema-Nio wrote: > Even by default making the search_path "pg_catalog, pg_temp" for > functions created by extensions would be very useful. Right now there's no syntax to override that. We'd need something to say "get the search_path from the session". Regards, Jeff Davis
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Thu, 6 Jun 2024 at 20:10, Isaac Morland wrote: > > On Thu, 6 Jun 2024 at 12:53, Jeff Davis wrote: > >> >> > I didn't get you completely here. w.r.t extensions how will this have >> > an impact if we set the search_path for definer functions. >> >> If we only set the search path for SECURITY DEFINER functions, I don't >> think that solves the whole problem. > > > Indeed. While the ability for a caller to set the search_path for a security > definer functions introduces security problems that are different than for > security invoker functions, it's still weird for the behaviour of a function > to depend on the caller's search_path. It’s even weirder for the default > search path behaviour to be different depending on whether or not the > function is security definer. +1 And +1 to the general idea and direction this thread is going in. I definitely think we should be making extensions more secure by default, and this is an important piece of it. Even by default making the search_path "pg_catalog, pg_temp" for functions created by extensions would be very useful.
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Thu, 6 Jun 2024 at 12:53, Jeff Davis wrote: > > I didn't get you completely here. w.r.t extensions how will this have > > an impact if we set the search_path for definer functions. > > If we only set the search path for SECURITY DEFINER functions, I don't > think that solves the whole problem. Indeed. While the ability for a caller to set the search_path for a security definer functions introduces security problems that are different than for security invoker functions, it's still weird for the behaviour of a function to depend on the caller's search_path. It’s even weirder for the default search path behaviour to be different depending on whether or not the function is security definer.
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Thu, 2024-06-06 at 21:17 +0530, Ashutosh Sharma wrote: > That can be controlled via some GUC if needed, I guess. That's a possibility, but it's easy to create a mess that way. I don't necessarily oppose it, but we'd need some pretty strong agreement that we are somehow moving users in a better direction and not just creating two behaviors that last forever. I also think there should be a way to explicitly request the old behavior -- obtaining search_path from the session -- regardless of how the GUC is set. > I didn't get you completely here. w.r.t extensions how will this have > an impact if we set the search_path for definer functions. If we only set the search path for SECURITY DEFINER functions, I don't think that solves the whole problem. Regards, Jeff Davis
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi, On Thu, Jun 6, 2024 at 2:36 AM Jeff Davis wrote: > On Wed, 2024-06-05 at 14:36 +0530, Ashutosh Sharma wrote: > > Thank you, Ashutosh, for the quick response. I've drafted a patch > > aimed at addressing this issue. The patch attempts to solve this > > issue by configuring the search_path for all security definer > > functions created by the extension. > I like the general direction you propose, but I think it needs more > discussion about the details. > I agree. > > * What exactly is the right search_path for a function defined in an > extension? > Determining the precise value can be challenging. However, since it's a function installed by an extension, typically, the search_path should include the extension's search_path and the schema where the function resides. If the function relies on a schema other than the one we set in its search_path, which would usually be the one created by the extension, this approach will enforce the extension developers to set the extension's specific search_path in the create function statement, if it's not set. The primary goal here is to ensure that the security definer functions created by an extension do not refer to any untrusted schema(s). > > * Do we need a new magic search_path value of "$extension_schema" that > resolves to the extension's schema, so that it can handle ALTER > EXTENSION ... SET SCHEMA? > Possibly yes, we can think about it, I think it would be something like the $user we have now. > > * What do we do for functions that want the current behavior and how do > we handle migration issues? > That can be controlled via some GUC if needed, I guess. > > * What about SECURITY INVOKER functions? Those can still be vulnerable > to manipulation by the caller by setting search_path, which can cause > an incorrect value to be returned. That can matter in some contexts > like a CHECK constraint. > I didn't get you completely here. w.r.t extensions how will this have an impact if we set the search_path for definer functions. -- With Regards, Ashutosh Sharma.
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Wed, 2024-06-05 at 14:36 +0530, Ashutosh Sharma wrote: > Thank you, Ashutosh, for the quick response. I've drafted a patch > aimed at addressing this issue. The patch attempts to solve this > issue by configuring the search_path for all security definer > functions created by the extension. I like the general direction you propose, but I think it needs more discussion about the details. * What exactly is the right search_path for a function defined in an extension? * Do we need a new magic search_path value of "$extension_schema" that resolves to the extension's schema, so that it can handle ALTER EXTENSION ... SET SCHEMA? * What do we do for functions that want the current behavior and how do we handle migration issues? * What about SECURITY INVOKER functions? Those can still be vulnerable to manipulation by the caller by setting search_path, which can cause an incorrect value to be returned. That can matter in some contexts like a CHECK constraint. Regards, Jeff Davis
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Fri, May 24, 2024 at 2:25 PM Ashutosh Bapat wrote: > > > On Fri, May 24, 2024 at 12:52 PM Ashutosh Sharma > wrote: > >> Hi All, >> >> We all know that installing an extension typically requires superuser >> privileges, which means the database objects it creates are owned by the >> superuser. >> >> If the extension creates any SECURITY DEFINER functions, it can introduce >> security vulnerabilities. For example, consider an extension that creates >> the following functions, outer_func and inner_func, in the schema s1 when >> installed: >> >> CREATE OR REPLACE FUNCTION s1.inner_func(data text) >> RETURNS void AS $$ >> BEGIN >> INSERT INTO tab1(data_column) VALUES (data); >> END; >> $$ LANGUAGE plpgsql; >> >> CREATE OR REPLACE FUNCTION s1.outer_func(data text) >> RETURNS void AS $$ >> BEGIN >> PERFORM inner_func(data); >> END; >> $$ LANGUAGE plpgsql SECURITY DEFINER; >> >> If a regular user creates another function with name "inner_func" with >> the same signature in the public schema and sets the search path to public, >> s1, the function created by the regular user in the public schema takes >> precedence when outer_func is called. Since outer_func is marked as >> SECURITY DEFINER, the inner_func created by the user in the public schema >> is executed with superuser privileges. This allows the execution of any >> statements within the function block, leading to potential security issues. >> >> To address this problem, one potential solution is to adjust the function >> resolution logic. For instance, if the caller function belongs to a >> specific schema, functions in the same schema should be given preference. >> Although I haven’t reviewed the feasibility in the code, this is one >> possible approach. >> >> Another solution could be to categorize extension-created functions to >> avoid duplication. This might not be an ideal solution, but it's another >> consideration worth sharing. >> >> > Function call should schema qualify it. That's painful, but it can be > avoided by setting a search path from inside the function. There was some > discussion about setting a search path for a function at [1]. But the last > message there is non-conclusive. We may want to extend it to extensions > such that all the object references in a given extension are resolved using > extension specific search_path. > > [1] > https://www.postgresql.org/message-id/2710f56add351a1ed553efb677408e51b060e67c.camel%40j-davis.com > Thank you, Ashutosh, for the quick response. I've drafted a patch aimed at addressing this issue. The patch attempts to solve this issue by configuring the search_path for all security definer functions created by the extension. It ensures they are set to trusted schemas, which includes the schema where the extension and the function are created. PFA patch. -- With Regards, Ashutosh Sharma. v1-0001-Ensure-security-definer-functions-created-by-extensi.patch Description: Binary data
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
On Fri, May 24, 2024 at 12:52 PM Ashutosh Sharma wrote: > Hi All, > > We all know that installing an extension typically requires superuser > privileges, which means the database objects it creates are owned by the > superuser. > > If the extension creates any SECURITY DEFINER functions, it can introduce > security vulnerabilities. For example, consider an extension that creates > the following functions, outer_func and inner_func, in the schema s1 when > installed: > > CREATE OR REPLACE FUNCTION s1.inner_func(data text) > RETURNS void AS $$ > BEGIN > INSERT INTO tab1(data_column) VALUES (data); > END; > $$ LANGUAGE plpgsql; > > CREATE OR REPLACE FUNCTION s1.outer_func(data text) > RETURNS void AS $$ > BEGIN > PERFORM inner_func(data); > END; > $$ LANGUAGE plpgsql SECURITY DEFINER; > > If a regular user creates another function with name "inner_func" with the > same signature in the public schema and sets the search path to public, s1, > the function created by the regular user in the public schema takes > precedence when outer_func is called. Since outer_func is marked as > SECURITY DEFINER, the inner_func created by the user in the public schema > is executed with superuser privileges. This allows the execution of any > statements within the function block, leading to potential security issues. > > To address this problem, one potential solution is to adjust the function > resolution logic. For instance, if the caller function belongs to a > specific schema, functions in the same schema should be given preference. > Although I haven’t reviewed the feasibility in the code, this is one > possible approach. > > Another solution could be to categorize extension-created functions to > avoid duplication. This might not be an ideal solution, but it's another > consideration worth sharing. > > Function call should schema qualify it. That's painful, but it can be avoided by setting a search path from inside the function. There was some discussion about setting a search path for a function at [1]. But the last message there is non-conclusive. We may want to extend it to extensions such that all the object references in a given extension are resolved using extension specific search_path. [1] https://www.postgresql.org/message-id/2710f56add351a1ed553efb677408e51b060e67c.camel%40j-davis.com -- Best Wishes, Ashutosh Bapat
Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Hi All, We all know that installing an extension typically requires superuser privileges, which means the database objects it creates are owned by the superuser. If the extension creates any SECURITY DEFINER functions, it can introduce security vulnerabilities. For example, consider an extension that creates the following functions, outer_func and inner_func, in the schema s1 when installed: CREATE OR REPLACE FUNCTION s1.inner_func(data text) RETURNS void AS $$ BEGIN INSERT INTO tab1(data_column) VALUES (data); END; $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION s1.outer_func(data text) RETURNS void AS $$ BEGIN PERFORM inner_func(data); END; $$ LANGUAGE plpgsql SECURITY DEFINER; If a regular user creates another function with name "inner_func" with the same signature in the public schema and sets the search path to public, s1, the function created by the regular user in the public schema takes precedence when outer_func is called. Since outer_func is marked as SECURITY DEFINER, the inner_func created by the user in the public schema is executed with superuser privileges. This allows the execution of any statements within the function block, leading to potential security issues. To address this problem, one potential solution is to adjust the function resolution logic. For instance, if the caller function belongs to a specific schema, functions in the same schema should be given preference. Although I haven’t reviewed the feasibility in the code, this is one possible approach. Another solution could be to categorize extension-created functions to avoid duplication. This might not be an ideal solution, but it's another consideration worth sharing. Thoughts? -- With Regards, Ashutosh Sharma.
