Thread: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
From
Ashutosh Sharma
Date:
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.
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.
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
From
Ashutosh Bapat
Date:
On Fri, May 24, 2024 at 12:52 PM Ashutosh Sharma <ashu.coek88@gmail.com> 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.
Best Wishes,
Ashutosh Bapat
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
From
Ashutosh Sharma
Date:
On Fri, May 24, 2024 at 2:25 PM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:
On Fri, May 24, 2024 at 12:52 PM Ashutosh Sharma <ashu.coek88@gmail.com> 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.
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.
Attachment
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
From
Jeff Davis
Date:
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
From
Ashutosh Sharma
Date:
Hi,
On Thu, Jun 6, 2024 at 2:36 AM Jeff Davis <pgsql@j-davis.com> 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
From
Jeff Davis
Date:
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
From
Isaac Morland
Date:
On Thu, 6 Jun 2024 at 12:53, Jeff Davis <pgsql@j-davis.com> 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
From
Jelte Fennema-Nio
Date:
On Thu, 6 Jun 2024 at 20:10, Isaac Morland <isaac.morland@gmail.com> wrote: > > On Thu, 6 Jun 2024 at 12:53, Jeff Davis <pgsql@j-davis.com> 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 problemsthat are different than for security invoker functions, it's still weird for the behaviour of a function to dependon the caller's search_path. It’s even weirder for the default search path behaviour to be different depending on whetheror 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
From
Jeff Davis
Date:
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
From
Ashutosh Sharma
Date:
On Thu, Jun 6, 2024 at 2:36 AM Jeff Davis <pgsql@j-davis.com> 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.
Attachment
Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions
From
Jelte Fennema-Nio
Date:
On Tue, 11 Jun 2024 at 11:54, Ashutosh Sharma <ashu.coek88@gmail.com> 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
From
Ashutosh Sharma
Date:
Hi, On Tue, Jun 11, 2024 at 5:02 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote: > > On Tue, 11 Jun 2024 at 11:54, Ashutosh Sharma <ashu.coek88@gmail.com> 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
From
Alexander Kukushkin
Date:
Hi,
On Tue, 11 Jun 2024 at 14:50, Ashutosh Sharma <ashu.coek88@gmail.com> 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
From
Ashutosh Sharma
Date:
Hi Alexander, On Tue, Jun 11, 2024 at 6:26 PM Alexander Kukushkin <cyberdemn@gmail.com> wrote: > > Hi, > > On Tue, 11 Jun 2024 at 14:50, Ashutosh Sharma <ashu.coek88@gmail.com> 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
From
Jeff Davis
Date:
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
From
Jeff Davis
Date:
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
From
Ashutosh Sharma
Date:
Hi Jeff, On Wed, Jun 12, 2024 at 3:07 AM Jeff Davis <pgsql@j-davis.com> 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
From
Ashutosh Sharma
Date:
On Wed, Jun 12, 2024 at 9:35 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote: > > Hi Jeff, > > On Wed, Jun 12, 2024 at 3:07 AM Jeff Davis <pgsql@j-davis.com> 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
From
Ashutosh Bapat
Date:
On Wed, Jun 12, 2024 at 9:51 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
On Wed, Jun 12, 2024 at 9:35 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
>
> Hi Jeff,
>
> On Wed, Jun 12, 2024 at 3:07 AM Jeff Davis <pgsql@j-davis.com> 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
From
Alexander Kukushkin
Date:
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
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 thatan 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
From
Jeff Davis
Date:
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
From
Robert Haas
Date:
On Wed, Jun 12, 2024 at 2:05 PM John H <johnhyvr@gmail.com> 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
From
Jeff Davis
Date:
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
From
Ashutosh Sharma
Date:
Hi, On Wed, Jun 12, 2024 at 11:35 PM John H <johnhyvr@gmail.com> wrote: > > > But, I also agree with Jelte, it should be a property of a control file, rather than a user controlled parameter, sothat 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
From
Ashutosh Sharma
Date:
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.
Attachment
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 <ashu.coek88@gmail.com> 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
From
Ashutosh Sharma
Date:
Hi John, On Tue, Jun 18, 2024 at 2:35 AM John H <johnhyvr@gmail.com> 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.
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
From
Ashutosh Sharma
Date:
Hi, On Wed, Jun 19, 2024 at 6:06 AM John H <johnhyvr@gmail.com> 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
From
Jeff Davis
Date:
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
From
Ashutosh Sharma
Date:
On Tue, Jun 25, 2024 at 12:40 AM Jeff Davis <pgsql@j-davis.com> 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
From
Robert Haas
Date:
On Mon, Jun 24, 2024 at 3:10 PM Jeff Davis <pgsql@j-davis.com> 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
From
Ashutosh Sharma
Date:
Hi Robert,
On Fri, Jul 12, 2024 at 9:02 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Mon, Jun 24, 2024 at 3:10 PM Jeff Davis <pgsql@j-davis.com> 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
From
Robert Haas
Date:
On Mon, Jul 15, 2024 at 8:05 AM Ashutosh Sharma <ashu.coek88@gmail.com> 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
From
Jeff Davis
Date:
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
From
Robert Haas
Date:
On Mon, Jul 15, 2024 at 2:33 PM Jeff Davis <pgsql@j-davis.com> 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
From
Jeff Davis
Date:
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
From
Ashutosh Sharma
Date:
Hi Robert. On Mon, Jul 15, 2024 at 11:15 PM Robert Haas <robertmhaas@gmail.com> wrote: > > On Mon, Jul 15, 2024 at 8:05 AM Ashutosh Sharma <ashu.coek88@gmail.com> 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
From
Robert Haas
Date:
On Tue, Jul 16, 2024 at 1:55 AM Ashutosh Sharma <ashu.coek88@gmail.com> 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
From
Ashutosh Sharma
Date:
Hi Robert, On Tue, Jul 16, 2024 at 9:40 PM Robert Haas <robertmhaas@gmail.com> wrote: > > On Tue, Jul 16, 2024 at 1:55 AM Ashutosh Sharma <ashu.coek88@gmail.com> 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
From
Ashutosh Sharma
Date:
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.