Horton

Thread

Commits

  1. doc: Clarify passphrase command reloading on Windows

  2. ssl: Add connection and reload tests for key passphrases

  3. Add GUC to show EXEC_BACKEND state

  1. Extended test coverage and docs for SSL passphrase commands

    Daniel Gustafsson <daniel@yesql.se> — 2025-11-07T20:26:38Z

    When I was writing tests for the SSL SNI patch [0] I realized that the current
tests for ssl passphrase commands aren't fully exercising the feature, so I
extended them to better understand how it works.  Attached is an extended set
of tests for passphrase protected keys where connection and reloads are tested
as well as their different characteristics on Windows.

The patchset also contains a small doc addition which documents the fact that
passphrase command reloading must be on when running on Windows (EXEC_BACKEND)
since every backend will issue a SSL configuration reload.

--
Daniel Gustafsson

  2. Re: Extended test coverage and docs for SSL passphrase commands

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-12T14:15:20Z

    On 07.11.25 21:26, Daniel Gustafsson wrote:
> When I was writing tests for the SSL SNI patch [0] I realized that the current
> tests for ssl passphrase commands aren't fully exercising the feature, so I
> extended them to better understand how it works.  Attached is an extended set
> of tests for passphrase protected keys where connection and reloads are tested
> as well as their different characteristics on Windows.
> 
> The patchset also contains a small doc addition which documents the fact that
> passphrase command reloading must be on when running on Windows (EXEC_BACKEND)
> since every backend will issue a SSL configuration reload.

Your test code conflates $windows_os with EXEC_BACKEND.  It should work 
to enable EXEC_BACKEND on a non-Windows system and have everything work. 
  So I think that code needs to extract the actual EXEC_BACKEND setting 
somehow, instead of using the OS identity as a proxy.

About the behavior that your documentation patch describes, I would like 
to have some kind of reflection of that in the code as well.  At least a 
comment near default_openssl_tls_init() maybe?  I haven't traced the 
code through, but I would be curious about what is different in an 
EXEC_BACKEND environment.  For example, is the argument isServerStart 
also true if it's not a server start?  Or should the setting actually be 
enforced directly on the GUC system?

  3. Re: Extended test coverage and docs for SSL passphrase commands

    Daniel Gustafsson <daniel@yesql.se> — 2025-11-12T17:25:45Z

    > On 12 Nov 2025, at 15:15, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
> On 07.11.25 21:26, Daniel Gustafsson wrote:
>> When I was writing tests for the SSL SNI patch [0] I realized that the current
>> tests for ssl passphrase commands aren't fully exercising the feature, so I
>> extended them to better understand how it works.  Attached is an extended set
>> of tests for passphrase protected keys where connection and reloads are tested
>> as well as their different characteristics on Windows.
>> The patchset also contains a small doc addition which documents the fact that
>> passphrase command reloading must be on when running on Windows (EXEC_BACKEND)
>> since every backend will issue a SSL configuration reload.
> 
> Your test code conflates $windows_os with EXEC_BACKEND.  It should work to enable EXEC_BACKEND on a non-Windows system and have everything work.  So I think that code needs to extract the actual EXEC_BACKEND setting somehow, instead of using the OS identity as a proxy.

As far as I know the only way to programmatically learn that from the Perl
testcode would be to check for the presence of the CONFIG_EXEC_PARAMS file in
$self->data_dir, which should be easy enough to do.  Do you know of a better
way?

> About the behavior that your documentation patch describes, I would like to have some kind of reflection of that in the code as well.  At least a comment near default_openssl_tls_init() maybe?  I haven't traced the code through, but I would be curious about what is different in an EXEC_BACKEND environment.  For example, is the argument isServerStart also true if it's not a server start?  Or should the setting actually be enforced directly on the GUC system?

It is documented in src/backend/tcop/backend_startup.c with the following
comment in BackendMain():

#ifdef EXEC_BACKEND

    /*
     * Need to reinitialize the SSL library in the backend, since the context
     * structures contain function pointers and cannot be passed through the
     * parameter file.
     *
     * If for some reason reload fails (maybe the user installed broken key
     * files), soldier on without SSL; that's better than all connections
     * becoming impossible.
     *
     * XXX should we do this in all child processes?  For the moment it's
     * enough to do it in backend children.
     */
#ifdef USE_SSL
    if (EnableSSL)
    {
        if (secure_initialize(false) == 0)
            LoadedSSL = true;

Calling secure_initialize with isServerStart == false will force a reload which
in turn requires the passphrase command to be reloadable if it is to work at
all.

Not sure if we need too much more than that, but maybe a note could be added to
be_tls_init that isServerStart will reflect config reloads as well as
EXEC_BACKEND?

--
Daniel Gustafsson

  4. Re: Extended test coverage and docs for SSL passphrase commands

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-11-12T17:47:36Z

    On 2025-Nov-12, Daniel Gustafsson wrote:

> As far as I know the only way to programmatically learn that from the Perl
> testcode would be to check for the presence of the CONFIG_EXEC_PARAMS file in
> $self->data_dir, which should be easy enough to do.  Do you know of a better
> way?

We have check_pg_config(), which reads pg_config.h.  For EXEC_BACKEND
you need pg_config_manual.h, but I think you could go roughly in the
same direction ... for instance you could add an argument to
check_pg_config() to say which file to read -- though I think the
easiest is to read both files always and return the grep count in both.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
Officer Krupke, what are we to do?
Gee, officer Krupke, Krup you! (West Side Story, "Gee, Officer Krupke")

  5. Re: Extended test coverage and docs for SSL passphrase commands

    Daniel Gustafsson <daniel@yesql.se> — 2025-11-12T23:12:19Z

    > On 12 Nov 2025, at 18:47, Álvaro Herrera <alvherre@kurilemu.de> wrote:
> 
> On 2025-Nov-12, Daniel Gustafsson wrote:
> 
>> As far as I know the only way to programmatically learn that from the Perl
>> testcode would be to check for the presence of the CONFIG_EXEC_PARAMS file in
>> $self->data_dir, which should be easy enough to do.  Do you know of a better
>> way?
> 
> We have check_pg_config(), which reads pg_config.h.  For EXEC_BACKEND
> you need pg_config_manual.h,

Right, but they can't be treated the same since EXEC_BACKEND will always be
matched by such a grep and the presence of WIN32 and !__CYGWIN__ mst be tested
for.  Or am I thinking about it the wrong way?  This is why I figured checking
for the exec_params file could be an option, but with the drawback that it
would only work for a running cluster so it wouldn't be a generic function but
coded directly in the SSL TAP test file.

--
Daniel Gustafsson

  6. Re: Extended test coverage and docs for SSL passphrase commands

    Daniel Gustafsson <daniel@yesql.se> — 2025-11-21T22:38:02Z

    > On 13 Nov 2025, at 00:12, Daniel Gustafsson <daniel@yesql.se> wrote:
> 
>> On 12 Nov 2025, at 18:47, Álvaro Herrera <alvherre@kurilemu.de> wrote:
>> 
>> On 2025-Nov-12, Daniel Gustafsson wrote:
>> 
>>> As far as I know the only way to programmatically learn that from the Perl
>>> testcode would be to check for the presence of the CONFIG_EXEC_PARAMS file in
>>> $self->data_dir, which should be easy enough to do.  Do you know of a better
>>> way?
>> 
>> We have check_pg_config(), which reads pg_config.h.  For EXEC_BACKEND
>> you need pg_config_manual.h,
> 
> Right, but they can't be treated the same since EXEC_BACKEND will always be
> matched by such a grep and the presence of WIN32 and !__CYGWIN__ mst be tested
> for.

The attached v2 adds a GUC debug_exec_backend which can be used to get the
state of the running cluster, much like how debug_assertions will tell whether
or not assertions were compiled in or not.  (Per an idea off-list conversation
about this.) This will be operating system independent and reusable in other
tests as well.

The rest of the patches are the same, just adapted to use this GUC in the SSL
test.

--
Daniel Gustafsson

  7. Re: Extended test coverage and docs for SSL passphrase commands

    Chao Li <li.evan.chao@gmail.com> — 2025-11-22T09:30:11Z

    Hi Daniel,

I just reviewed the patch and got a few comments.

> On Nov 22, 2025, at 06:38, Daniel Gustafsson <daniel@yesql.se> wrote:
> 
> 
> The attached v2 adds a GUC debug_exec_backend which can be used to get the
> state of the running cluster, much like how debug_assertions will tell whether
> or not assertions were compiled in or not.  (Per an idea off-list conversation
> about this.) This will be operating system independent and reusable in other
> tests as well.
> 
> The rest of the patches are the same, just adapted to use this GUC in the SSL
> test.
> 
> --
> Daniel Gustafsson
> 
> <v2-0001-Add-GUC-to-show-EXEC_BACKEND-state.patch><v2-0002-doc-Clarify-passphrase-command-reloading-on-Windo.patch><v2-0003-ssl-Add-connection-and-reload-tests-for-key-passp.patch>

1 - 0001
```
+  short_desc => 'Shows whether the running server is running in EXEC_BACKEND mode.',
```

The GUC is added like a mirror of debug_assertions. However, I think a small difference is that, assertions will impact everything at runtime, while EXEC_BACKEND don’t really impact PG’s behavior, instead it only impacts how backend processes are spawned. Thus, I feel “running server is EXEC_BACKEND mode” is a little bit inaccurate, maybe just say “show whether the running server is built with EXEC_BACKEND”.

2 - 0002
```
+        This parameter must be set to <literal>on</literal> when running on
+        <systemitem class="osname">Windows</systemitem> since all connections
```

This is not a comment. I’m just thinking that, as EXEC_BACKEND is compile flag, when a server is started, it knows if EXEC_BACKEND is enabled or not. So that, if ssl_passphrase_command must be turned on, why cannot we automatically turn on it?

 3 - 0003
```
+$node->log_check(
+	"passhprase could reload private key",

```

Typo: passhprase -> passphrase

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  8. Re: Extended test coverage and docs for SSL passphrase commands

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-11-22T13:00:55Z

    On 2025-Nov-21, Daniel Gustafsson wrote:

> The attached v2 adds a GUC debug_exec_backend which can be used to get the
> state of the running cluster,

Nice idea.

I think the parallel to debug_assertions is not perfect, because you can
turn off debug_assertions in a server built with them included, but you
cannot turn off EXEC_BACKEND.  This says we shouldn't name the symbol
with the DEFAULT word: it should just be "EXEC_BACKEND_ENABLED".  The
value already cannot be changed, which is good, and there are tests for
this behavior of PGC_INTERNAL gucs in src/test/modules/unsafe_tests/sql/guc_privs.sql,
so I think we don't need anything additional.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Once again, thank you and all of the developers for your hard work on
PostgreSQL.  This is by far the most pleasant management experience of
any database I've worked on."                             (Dan Harris)
http://archives.postgresql.org/pgsql-performance/2006-04/msg00247.php

  9. Re: Extended test coverage and docs for SSL passphrase commands

    Daniel Gustafsson <daniel@yesql.se> — 2025-11-23T19:32:46Z

    > On 22 Nov 2025, at 10:30, Chao Li <li.evan.chao@gmail.com> wrote:

> I just reviewed the patch and got a few comments.

Thanks! An updated version will come downthread.

> The GUC is added like a mirror of debug_assertions. However, I think a small difference is that, assertions will impact everything at runtime, while EXEC_BACKEND don’t really impact PG’s behavior, instead it only impacts how backend processes are spawned. Thus, I feel “running server is EXEC_BACKEND mode” is a little bit inaccurate, maybe just say “show whether the running server is built with EXEC_BACKEND”.

That's a good point, the docementation hunk had it right where this part got it
wrong.  Fixed.

> This is not a comment. I’m just thinking that, as EXEC_BACKEND is compile flag, when a server is started, it knows if EXEC_BACKEND is enabled or not. So that, if ssl_passphrase_command must be turned on, why cannot we automatically turn on it?

Technically we might be able to, but I don't want to override the administrator
when it comes to sensitive configuration settings.  Better to document what
needs to be done and have the user make informed decisions.

> Typo: passhprase -> passphrase

Fixed.

--
Daniel Gustafsson

  10. Re: Extended test coverage and docs for SSL passphrase commands

    Daniel Gustafsson <daniel@yesql.se> — 2025-11-23T19:56:35Z

    > On 22 Nov 2025, at 14:00, Álvaro Herrera <alvherre@kurilemu.de> wrote:
> 
> On 2025-Nov-21, Daniel Gustafsson wrote:
> 
>> The attached v2 adds a GUC debug_exec_backend which can be used to get the
>> state of the running cluster,
> 
> Nice idea.
> 
> I think the parallel to debug_assertions is not perfect, because you can
> turn off debug_assertions in a server built with them included, but you
> cannot turn off EXEC_BACKEND.

debug_assertions cannot be disabled anymore, since 3bdcf6a5a755.

>  This says we shouldn't name the symbol
> with the DEFAULT word: it should just be "EXEC_BACKEND_ENABLED".

Regardless, I totally agree with that, fixed in the attached along with the
review comments from upthread.

--
Daniel Gustafsson

  11. Re: Extended test coverage and docs for SSL passphrase commands

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-11-24T17:08:24Z

    On 2025-Nov-23, Daniel Gustafsson wrote:

> > On 22 Nov 2025, at 14:00, Álvaro Herrera <alvherre@kurilemu.de> wrote:

> > I think the parallel to debug_assertions is not perfect, because you
> > can turn off debug_assertions in a server built with them included,
> > but you cannot turn off EXEC_BACKEND.
> 
> debug_assertions cannot be disabled anymore, since 3bdcf6a5a755.

Hah, 2014 -- goes to show that I don't try very often ;-)  (I do have a
separate build tree for no-assertions, which I don't use very often
either.)

> > This says we shouldn't name the symbol with the DEFAULT word: it
> > should just be "EXEC_BACKEND_ENABLED".
> 
> Regardless, I totally agree with that, fixed in the attached along
> with the review comments from upthread.

This looks reasonable to me, thanks.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"No nos atrevemos a muchas cosas porque son difíciles,
pero son difíciles porque no nos atrevemos a hacerlas" (Séneca)

  12. Re: Extended test coverage and docs for SSL passphrase commands

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-25T15:01:45Z

    On 23.11.25 20:56, Daniel Gustafsson wrote:
>> On 22 Nov 2025, at 14:00, Álvaro Herrera <alvherre@kurilemu.de> wrote:
>>
>> On 2025-Nov-21, Daniel Gustafsson wrote:
>>
>>> The attached v2 adds a GUC debug_exec_backend which can be used to get the
>>> state of the running cluster,
>>
>> Nice idea.
>>
>> I think the parallel to debug_assertions is not perfect, because you can
>> turn off debug_assertions in a server built with them included, but you
>> cannot turn off EXEC_BACKEND.
> 
> debug_assertions cannot be disabled anymore, since 3bdcf6a5a755.
> 
>>   This says we shouldn't name the symbol
>> with the DEFAULT word: it should just be "EXEC_BACKEND_ENABLED".
> 
> Regardless, I totally agree with that, fixed in the attached along with the
> review comments from upthread.

This looks good to me.