Dag-Erling Smørgrav <d...@des.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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to