> On 16 Mar 2018, at 17:07, Peter Eisentraut <peter.eisentr...@2ndquadrant.com>
> wrote:
>
> On 3/15/18 12:13, Daniel Gustafsson wrote:
>> * 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.
It makes sense for the OpenSSL code to know, I’m mostly wondering why
run_ssl_passphrase_command() isn’t doing the same thing wrt the leading ‘-‘?
ISTM that it should consider is_server_start as well to match the
documentation.
> Maybe this is a bit too cute. We could instead add another setting
> "ssl_passphrase_command_support_reload”.
I think thats a good idea, this feels like an easy thing to be confused about
and get wrong (which I might have done with the above).
Either way, there is a clear path forward on this (the new setting or just
ignoring me due to being confused) so I am going to mark this ready for
committer as the remaining work is known and small.
cheers ./daniel
