Re: SSL passphrase prompt external command - Mailing list pgsql-hackers

On 3/15/18 12:13, Daniel Gustafsson wrote:
> * In src/backend/libpq/be-secure-common.c:
> 
> +int
> +run_ssl_passphrase_command(const char *prompt, bool is_server_start, char *buf, int size)
> +{
>     [..]
> +   size_t      len = 0;
>     [..]
> +   return len;
> +}
> 
> run_ssl_passphrase_command() is defined to return a signed int but returns
> size_t which is unsigned.  It’s obviously a remote cornercase to hit, but
> wouldn’t it be better to return the same signedness?

The OpenSSL (an GnuTLS) callbacks return an int, so we need to convert
it to int at some point anyway.  Might as well make it here.

> * The documentation for the passphrase command format states:
> 
>     "If the setting starts with -, then it will be ignored during a reload and
>     the SSL configuration will not be reloaded if a passphrase is needed."
> 
> In run_ssl_passphrase_command() the leading ‘-‘ is however just shifted away
> regardless of the state of is_server_start.
> 
> +   p = ssl_passphrase_command;
> +
> +   if (p[0] == '-')
> +       p++;
> +
> 
> The OpenSSL code is instead performing this in be_tls_init() in the below hunk:
> 
> +       if (ssl_passphrase_command[0] && ssl_passphrase_command[0] != '-')
> 
> In order to make this generic, shouldn’t we in run_ssl_passphrase_command() do
> something along the lines of the below to ensure we aren’t executing the
> passphrase callback during reloads?
> 
>     if (p[0] == '-')
>     {
>         /*
>          * passphrase commands with a leading '-' are only invoked on server
>          * start, and are ignored on reload.
>          */
>         if (!is_server_start)
>             goto error;
>         p++;
>     }

We need the OpenSSL code to know whether the callback supports reloads,
so it can call the dummy callback if not.

Maybe this is a bit too cute.  We could instead add another setting
"ssl_passphrase_command_support_reload".

> * In src/tools/msvc/Mkvcbuild.pm
> 
>     # if building without OpenSSL
>     if (!$solution->{options}->{openssl})
>     {
> +       $postgres->RemoveFile('src/backend/libpq/be-secure-common.c');
>         $postgres->RemoveFile('src/backend/libpq/be-secure-openssl.c');
>     }
> 
> Is this because run_ssl_passphrase_command() would otherwise generate a warning
> due to not being used?

I have no idea. ;-)  It's the same thing I was told to do for
fe-secure-common.c a while back.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services