From: | Alex Shulgin <ash(at)commandprompt(dot)com> |
---|---|
To: | Dag-Erling Smørgrav <des(at)des(dot)no> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] add ssl_protocols configuration option |
Date: | 2014-11-19 15:34:39 |
Message-ID: | 87ioiburu8.fsf@commandprompt.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Dag-Erling Smørgrav <des(at)des(dot)no> writes:
>
> The attached patches add an ssl_protocols configuration option which
> control which versions of SSL or TLS the server will use. The syntax is
> similar to Apache's SSLProtocols directive, except that the list is
> colon-separated instead of whitespace-separated, although that is easy
> to change if it proves unpopular.
Hello,
Here is my review of the patch against master branch:
* The patch applies and compiles cleanly, except for sgml docs:
postgres.xml:66141: element varlistentry: validity error : Element
varlistentry content does not follow the DTD, expecting (term+ ,
listitem), got (term indexterm listitem)
</para></listitem></varlistentry><varlistentry
^
The fix is to move <indexterm> under the <term> tag in the material
added to config.sgml by the patch.
* The patch works as advertised, though the only way to verify that
connections made with the protocol disabled by the GUC are indeed
rejected is to edit fe-secure-openssl.c to only allow specific TLS
versions. Adding configuration on the libpq side as suggested in the
original discussion might help here.
* The code allows specifying SSLv2 and SSLv3 in the GUC, but removes
them forcibly after parsing the complete string (a warning is issued).
Should we also add a note about this to the documentation?
* In case the list of allowed protocols comes empty a FATAL error
message is logged: "could not set the protocol list (no valid
protocols available)". I think it's worth changing that to "could not
set SSL protocol list..." All other related error/warning logs
include "SSL".
* The added code follows conventions and looks good to me. I didn't
spot any problems in the parser. I've tried a good deal of different
values and all seem to be parsed correctly.
I would try to apply patches for older branches if there is consensus
that we really need this patch and we want to back-patch it.
Thanks.
--
Alex
From | Date | Subject | |
---|---|---|---|
Next Message | Mike Blackwell | 2014-11-19 15:42:53 | Re: proposal: plpgsql - Assert statement |
Previous Message | Robert Haas | 2014-11-19 15:26:50 | Re: alternative model for handling locking in parallel groups |