Обсуждение: [DOCS] Default names for CRL and CA files in the backend

Commit a445cb92ef5b3a31313ebce30e18cc1d6e0bdecb removed the default names for
serverside CRL and CA files, but the defaults were left in the "SSL Server File
Usage” table with a small note.  I completely missed the note, even after
having been fiddling about with the code in question.  Removing the filenames
from the table, and altering the note per the attached patch, makes the docs
clearer IHMO.

cheers ./daniel

On Thu, Aug 17, 2017 at 7:31 AM, Daniel Gustafsson <daniel@yesql.se> wrote:
> Commit a445cb92ef5b3a31313ebce30e18cc1d6e0bdecb removed the default names for
> serverside CRL and CA files, but the defaults were left in the "SSL Server File
> Usage” table with a small note.  I completely missed the note, even after
> having been fiddling about with the code in question.  Removing the filenames
> from the table, and altering the note per the attached patch, makes the docs
> clearer IHMO.

Here are additional notes on the matter.

From libpq.sgml:
  <para>
   In some cases, the client certificate might be signed by an
   <quote>intermediate</> certificate authority, rather than one that is
   directly trusted by the server.  To use such a certificate, append the
   certificate of the signing authority to the <filename>postgresql.crt</>
   file, then its parent authority's certificate, and so on up to a certificate
   authority, <quote>root</> or <quote>intermediate</>, that is trusted by
   the server, i.e. signed by a certificate in the server's
   <filename>root.crt</filename> file.
  </para>

Am I reading that correctly? The last sentence should not mention
root.crt as well. The paragraph after that assume that ssl_ca_file is
set to root.crt so it looks fine to use it. But that's not assumed
here.

In sslinfo.sgml:
    <para>
     This function is really useful only if you have more than one trusted CA
     certificate in your server's <filename>root.crt</> file, or if this CA
     has issued some intermediate certificate authority certificates.
    </para>

In runtime.sgml:
  <para>
   Note that the server's <filename>root.crt</filename> lists the top-level
   CAs that are considered trusted for signing client certificates.
   In principle it need
   not list the CA that signed the server's certificate, though in most cases
   that CA would also be trusted for client certificates.
  </para>
Perhaps this should be changed as well.

In config.sgml:
       <para>
        In previous releases of PostgreSQL, the name of this file was
    hard-coded as <filename>root.crt</filename>.
       </para>
[...]
       <para>
        In previous releases of PostgreSQL, the name of this file was
        hard-coded as <filename>root.crt</filename>.
       </para>
Why not mentioning the version of Postgres where the change has begun?
I find confusing not to precise such level of details.
--
Michael

> On 17 Aug 2017, at 03:26, Michael Paquier <michael.paquier@gmail.com> wrote:
>
> On Thu, Aug 17, 2017 at 7:31 AM, Daniel Gustafsson <daniel@yesql.se> wrote:
>> Commit a445cb92ef5b3a31313ebce30e18cc1d6e0bdecb removed the default names for
>> serverside CRL and CA files, but the defaults were left in the "SSL Server File
>> Usage” table with a small note.  I completely missed the note, even after
>> having been fiddling about with the code in question.  Removing the filenames
>> from the table, and altering the note per the attached patch, makes the docs
>> clearer IHMO.
>
> Here are additional notes on the matter.

Thanks, I should learn to not hit send before having coffee.

> From libpq.sgml:
>  <para>
>   In some cases, the client certificate might be signed by an
>   <quote>intermediate</> certificate authority, rather than one that is
>   directly trusted by the server.  To use such a certificate, append the
>   certificate of the signing authority to the <filename>postgresql.crt</>
>   file, then its parent authority's certificate, and so on up to a certificate
>   authority, <quote>root</> or <quote>intermediate</>, that is trusted by
>   the server, i.e. signed by a certificate in the server's
>   <filename>root.crt</filename> file.
>  </para>
>
> Am I reading that correctly? The last sentence should not mention
> root.crt as well.

Agreed.

> The paragraph after that assume that ssl_ca_file is
> set to root.crt so it looks fine to use it. But that's not assumed
> here.

Right, it should perhaps be made clearer that root.crt is a proposed filename
in this example which could’ve been chosen as something else, but I can’t see a
good way off the cuff.  Did a tiny amount of wordsmithing here though to
indicate that it’s not a file the user should expect to have already.

> In sslinfo.sgml:
>    <para>
>     This function is really useful only if you have more than one trusted CA
>     certificate in your server's <filename>root.crt</> file, or if this CA
>     has issued some intermediate certificate authority certificates.
>    </para>
>
> In runtime.sgml:
>  <para>
>   Note that the server's <filename>root.crt</filename> lists the top-level
>   CAs that are considered trusted for signing client certificates.
>   In principle it need
>   not list the CA that signed the server's certificate, though in most cases
>   that CA would also be trusted for client certificates.
>  </para>
> Perhaps this should be changed as well.

Agreed.

> In config.sgml:
>       <para>
>        In previous releases of PostgreSQL, the name of this file was
>    hard-coded as <filename>root.crt</filename>.
>       </para>
> [...]
>       <para>
>        In previous releases of PostgreSQL, the name of this file was
>        hard-coded as <filename>root.crt</filename>.
>       </para>
> Why not mentioning the version of Postgres where the change has begun?
> I find confusing not to precise such level of details.

Since all supported versions have this as a parameter, this seems to mainly
serve as a help for anyone upgrading from 9.1 (or earlier) so mentioning when
the change happened makes sense.  I added a note here (and on root.crl) stating
the version.

cheers ./daniel

On Thu, Aug 17, 2017 at 4:37 PM, Daniel Gustafsson <daniel@yesql.se> wrote:
>> On 17 Aug 2017, at 03:26, Michael Paquier <michael.paquier@gmail.com> wrote:
> Since all supported versions have this as a parameter, this seems to mainly
> serve as a help for anyone upgrading from 9.1 (or earlier) so mentioning when
> the change happened makes sense.  I added a note here (and on root.crl) stating
> the version.

Thanks for the new version.

-   the server, i.e. signed by a certificate in the server's
-   <filename>root.crt</filename> file.
+   the server, i.e. signed by a certificate in the server's root certificate
+   file.
   </para>
Do you think it would be worth adding a mention to ssl_ca_file in the
server's postgresql.conf? With a link to it?

(Spotted a transation issue, so added Álvaro in the loop)
Álvaro, I think that those translations are incorrect:
src/backend/po/fr.po:#~ msgid "Make sure the root.crt file is present
and readable."
src/backend/po/ja.po:#~ msgid "Make sure the root.crt file is present
and readable."
src/backend/po/ru.po:#~ msgid "Make sure the root.crt file is present
and readable."
This error message does not exist anymore.

+        In earlier versions of PostgreSQL, the name of this file was
+        hard-coded as <filename>root.crl</filename>. As of
+        <productname>PostgreSQL</> 9.2 it is a configuration parameter.
No need to mention PostgreSQL twice here? Or the first one should use
the markup productname.
--
Michael

> On 18 Aug 2017, at 09:28, Michael Paquier <michael.paquier@gmail.com> wrote:
>
> On Thu, Aug 17, 2017 at 4:37 PM, Daniel Gustafsson <daniel@yesql.se> wrote:
>>> On 17 Aug 2017, at 03:26, Michael Paquier <michael.paquier@gmail.com> wrote:
>> Since all supported versions have this as a parameter, this seems to mainly
>> serve as a help for anyone upgrading from 9.1 (or earlier) so mentioning when
>> the change happened makes sense.  I added a note here (and on root.crl) stating
>> the version.
>
> Thanks for the new version.

Thanks for reviewing!

> -   the server, i.e. signed by a certificate in the server's
> -   <filename>root.crt</filename> file.
> +   the server, i.e. signed by a certificate in the server's root certificate
> +   file.
>   </para>
> Do you think it would be worth adding a mention to ssl_ca_file in the
> server's postgresql.conf? With a link to it?

I tried but couldn’t come up with anything that didn’t seem to confuse it
rather than make it clearer. Suggestions welcome, else we can leave it.

> +        In earlier versions of PostgreSQL, the name of this file was
> +        hard-coded as <filename>root.crl</filename>. As of
> +        <productname>PostgreSQL</> 9.2 it is a configuration parameter.
> No need to mention PostgreSQL twice here? Or the first one should use
> the markup productname.

From reading, it seems the common thing is to write the full name when
referencing a version, even when superfluous like here.  Personally I don’t
have strong opinions, I was just trying to follow the style.

Re the productname markup, that raises an interesting question.  There are more
than 2000 <productname>PostgreSQL</> in the docs, and somewhere just south of
250 plain PostgreSQL (not counting old release notes and titles etc).  Should
all occurrences of PostgreSQL, in text content, be wrapped in productname tags?
It’s probably more for consistency than anything else, and I’m happy to do the
work, but only if it’s deemed worthwhile to do so.

cheers ./daniel

Michael Paquier wrote:

> (Spotted a transation issue, so added Álvaro in the loop)
> Álvaro, I think that those translations are incorrect:
> src/backend/po/fr.po:#~ msgid "Make sure the root.crt file is present
> and readable."
> src/backend/po/ja.po:#~ msgid "Make sure the root.crt file is present
> and readable."
> src/backend/po/ru.po:#~ msgid "Make sure the root.crt file is present
> and readable."
> This error message does not exist anymore.

You're right, the message doesn't exist anywhere -- but note the #~
prefix in each line.  Those markers indicate that the translation is for
an outdated message.  If we were to ever re-introduce a similar message,
the software would use the old translations as suggestions, marked as
"fuzzy", i.e.  requiring translator review.

They don't do any harm, though personally I remove these lines from the
.po files that I am responsible for.  I think many translators don't
care.

--
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Fri, Aug 18, 2017 at 10:37 PM, Daniel Gustafsson <daniel@yesql.se> wrote:
>> On 18 Aug 2017, at 09:28, Michael Paquier <michael.paquier@gmail.com> wrote:
>> -   the server, i.e. signed by a certificate in the server's
>> -   <filename>root.crt</filename> file.
>> +   the server, i.e. signed by a certificate in the server's root certificate
>> +   file.
>>   </para>
>> Do you think it would be worth adding a mention to ssl_ca_file in the
>> server's postgresql.conf? With a link to it?
>
> I tried but couldn’t come up with anything that didn’t seem to confuse it
> rather than make it clearer. Suggestions welcome, else we can leave it.

By appending the following? "The server root's certificate is defined
in ssl_ca_file in its configuration". Though this makes the style of
the paragraph heavy I agree.

>> +        In earlier versions of PostgreSQL, the name of this file was
>> +        hard-coded as <filename>root.crl</filename>. As of
>> +        <productname>PostgreSQL</> 9.2 it is a configuration parameter.
>> No need to mention PostgreSQL twice here? Or the first one should use
>> the markup productname.
>
> From reading, it seems the common thing is to write the full name when
> referencing a version, even when superfluous like here.  Personally I don’t
> have strong opinions, I was just trying to follow the style.

OK, I am fine with your suggestion here.

> Re the productname markup, that raises an interesting question.  There are more
> than 2000 <productname>PostgreSQL</> in the docs, and somewhere just south of
> 250 plain PostgreSQL (not counting old release notes and titles etc).  Should
> all occurrences of PostgreSQL, in text content, be wrapped in productname tags?
> It’s probably more for consistency than anything else, and I’m happy to do the
> work, but only if it’s deemed worthwhile to do so.

That's another discussion anyway. Your patch looks fine to me for what
it focuses on.
--
Michael

Committed.

I have removed the mentions of the hard-coded defaults in previous
PostgreSQL releases.  Almost all settings probably did not exist or
behaved differently in a previous release, and we don't keep all of that
around, so this was a good chance to clean that up.

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