Обсуждение: Extended test coverage and docs for SSL passphrase commands

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

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?

> 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
systemand have everything work.  So I think that code needs to extract the actual EXEC_BACKEND setting somehow, instead
ofusing 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
thecode as well.  At least a comment near default_openssl_tls_init() maybe?  I haven't traced the code through, but I
wouldbe curious about what is different in an EXEC_BACKEND environment.  For example, is the argument isServerStart
alsotrue 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

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")

> 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