Re: [HACKERS] GnuTLS support

[Andreas Karlsson]

On 09/18/2017 07:04 PM, Jeff Janes wrote:> You fixed the first issue, 
but I still get the second one:


be-secure-gnutls.c: In function 'get_peer_certificate':
be-secure-gnutls.c:667: error: 'GNUTLS_X509_CRT_LIST_SORT' undeclared 
(first use in this function)
be-secure-gnutls.c:667: error: (Each undeclared identifier is reported 
only once

be-secure-gnutls.c:667: error: for each function it appears in.)


Thanks again for testing the code. I have now rebased the patch and 
fixed the second issue. I tested that it works on CentOS 6.


Work which remains:

- sslinfo
- pgcrypto
- Documentation
- Decide if what I did with the config is a good idea

Andreas
diff --git a/configure b/configure
index 4ecd2e1922..1ba34dfced 100755
--- a/configure
+++ b/configure
@@ -709,6 +709,7 @@ UUID_EXTRA_OBJS
 with_uuid
 with_systemd
 with_selinux
+with_gnutls
 with_openssl
 krb_srvtab
 with_python
@@ -837,6 +838,7 @@ with_bsd_auth
 with_ldap
 with_bonjour
 with_openssl
+with_gnutls
 with_selinux
 with_systemd
 with_readline
@@ -1531,6 +1533,7 @@ Optional Packages:
   --with-ldap build with LDAP support
   --with-bonjour  build with Bonjour support
   --with-openssl  build with OpenSSL support
+  --with-gnutls   build with GnuTS support
   --with-selinux  build with SELinux support
   --with-systemd  build with systemd support
   --without-readline  do not use GNU Readline nor BSD Libedit for editing
@@ -5997,6 +6000,41 @@ fi
 $as_echo "$with_openssl" >&6; }
 
 
+#
+# GnuTLS
+#
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to build with GnuTLS support" >&5
+$as_echo_n "checking whether to build with GnuTLS support... " >&6; }
+
+
+
+# Check whether --with-gnutls was given.
+if test "${with_gnutls+set}" = set; then :
+  withval=$with_gnutls;
+  case $withval in
+yes)
+
+$as_echo "#define USE_GNUTLS 1" >>confdefs.h
+
+  ;;
+no)
+  :
+  ;;
+*)
+  as_fn_error $? "no argument expected for --with-gnutls option" "$LINENO" 5
+  ;;
+  esac
+
+else
+  with_gnutls=no
+
+fi
+
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $with_gnutls" >&5
+$as_echo "$with_gnutls" >&6; }
+
+
 #
 # SELinux
 #
@@ -10164,6 +10202,94 @@ done
 
 fi
 
+if test "$with_gnutls" = yes ; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing gnutls_init" >&5
+$as_echo_n "checking for library containing gnutls_init... " >&6; }
+if ${ac_cv_search_gnutls_init+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ac_func_search_save_LIBS=$LIBS
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char gnutls_init ();
+int
+main ()
+{
+return gnutls_init ();
+  ;
+  return 0;
+}
+_ACEOF
+for ac_lib in '' gnutls; do
+  if test -z "$ac_lib"; then
+ac_res="none required"
+  else
+ac_res=-l$ac_lib
+LIBS="-l$ac_lib  $ac_func_search_save_LIBS"
+  fi
+  if ac_fn_c_try_link "$LINENO"; then :
+  ac_cv_search_gnutls_init=$ac_res
+fi
+rm -f core conftest.err conftest.$ac_objext \
+conftest$ac_exeext
+  if ${ac_cv_search_gnutls_init+:} false; then :
+  break
+fi
+done
+if ${ac_cv_search_gnutls_init+:} false; then :
+
+else
+  ac_cv_search_gnutls_init=no
+fi
+rm conftest.$ac_ext
+LIBS=$ac_func_search_save_LIBS
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_gnutls_init" >&5
+$as_echo "$ac_cv_search_gnutls_init" >&6; }
+ac_res=$ac_cv_search_gnutls_init
+if test "$ac_res" != no; then :
+  test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
+
+else
+  as_fn_error $? "library 'gnutls' is required for GnuTLS" "$LINENO" 5
+fi
+
+  # GnuTLS versions before 3.4.0 do not support sorting incorrectly sorted
+  # certificate chains.
+  ac_fn_c_check_decl "$LINENO" "GNUTLS_X509_CRT_LIST_SORT" "ac_cv_have_decl_GNUTLS_X509_CRT_LIST_SORT" "#include 
+#include 
+
+"
+if test "x$ac_cv_have_decl_GNUTLS_X509_CRT_LIST_SORT" = xyes; then :
+  ac_have_decl=1
+else
+  ac_have_decl=0
+fi
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE_DECL_GNUTLS_X509_CRT_LIST_SORT $ac_have_decl
+_ACEOF
+
+  for ac_func in gnutls_pkcs11_set_pin_function
+do :
+  ac_fn_c_check_func "$LINENO" "gnutls_pkcs11_set_pin_function" "ac_cv_func_gnutls_pkcs11_set_pin_function"
+if test "x$ac_cv_func_gnutls_pkcs11_set_pin_function" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_GNUTLS_PKCS11_SET_PIN_FUNCTION 1
+_ACEOF
+
+fi
+done
+
+fi
+
 if test "$with_pam" = yes ; then
   { $as_echo "$as_me:${as_lineno-$LINENO}: checking for pam_start in -lpam" >&5
 $as_echo_n "checking for pam_start in -lpam... " >&6; }
@@ -10961,6 +11087,17 @@ else
 fi
 
 
+fi
+
+if test "$with_gnutls" = yes ; then
+  ac_fn_c_check_header_mongrel "$LINENO" "gnutls/gnutls.h" "ac_cv_header_gnutls_gnutls_h" "$ac_includes_default"
+if test 

Re: [HACKERS] GnuTLS support

[Jeff Janes]

On Sun, Sep 17, 2017 at 2:17 PM, Andreas Karlsson  wrote:

> On 09/15/2017 06:55 PM, Jeff Janes wrote:
>
>> I can't build against gnutls-2.12.23-21.el6.x86_64 from CentOS 6.9
>>
>
> Thanks for testing my patch. I have fixed both these issues plus some of
> the other feedback. A new version of my patch is attached which should, at
> least on theory, support all GnuTLS versions >= 2.11.
>

You fixed the first issue, but I still get the second one:

be-secure-gnutls.c: In function 'get_peer_certificate':
be-secure-gnutls.c:667: error: 'GNUTLS_X509_CRT_LIST_SORT' undeclared
(first use in this function)
be-secure-gnutls.c:667: error: (Each undeclared identifier is reported only
once
be-secure-gnutls.c:667: error: for each function it appears in.)

Cheers,

Jeff

Re: [HACKERS] GnuTLS support

[Andreas Karlsson]

On 09/15/2017 06:55 PM, Jeff Janes wrote:

I can't build against gnutls-2.12.23-21.el6.x86_64 from CentOS 6.9


Thanks for testing my patch. I have fixed both these issues plus some of 
the other feedback. A new version of my patch is attached which should, 
at least on theory, support all GnuTLS versions >= 2.11.


I just very quickly fixed the broken SSL tests, as I am no fan of how 
the SSL tests currently are written and think they should be cleaned up.


Andreas
diff --git a/configure b/configure
index 0d76e5ea42..33b1f00bff 100755
--- a/configure
+++ b/configure
@@ -709,6 +709,7 @@ UUID_EXTRA_OBJS
 with_uuid
 with_systemd
 with_selinux
+with_gnutls
 with_openssl
 krb_srvtab
 with_python
@@ -838,6 +839,7 @@ with_bsd_auth
 with_ldap
 with_bonjour
 with_openssl
+with_gnutls
 with_selinux
 with_systemd
 with_readline
@@ -1534,6 +1536,7 @@ Optional Packages:
   --with-ldap build with LDAP support
   --with-bonjour  build with Bonjour support
   --with-openssl  build with OpenSSL support
+  --with-gnutls   build with GnuTS support
   --with-selinux  build with SELinux support
   --with-systemd  build with systemd support
   --without-readline  do not use GNU Readline nor BSD Libedit for editing
@@ -6051,6 +6054,41 @@ fi
 $as_echo "$with_openssl" >&6; }
 
 
+#
+# GnuTLS
+#
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to build with GnuTLS support" >&5
+$as_echo_n "checking whether to build with GnuTLS support... " >&6; }
+
+
+
+# Check whether --with-gnutls was given.
+if test "${with_gnutls+set}" = set; then :
+  withval=$with_gnutls;
+  case $withval in
+yes)
+
+$as_echo "#define USE_GNUTLS 1" >>confdefs.h
+
+  ;;
+no)
+  :
+  ;;
+*)
+  as_fn_error $? "no argument expected for --with-gnutls option" "$LINENO" 5
+  ;;
+  esac
+
+else
+  with_gnutls=no
+
+fi
+
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $with_gnutls" >&5
+$as_echo "$with_gnutls" >&6; }
+
+
 #
 # SELinux
 #
@@ -10218,6 +10256,83 @@ done
 
 fi
 
+if test "$with_gnutls" = yes ; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing gnutls_init" >&5
+$as_echo_n "checking for library containing gnutls_init... " >&6; }
+if ${ac_cv_search_gnutls_init+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ac_func_search_save_LIBS=$LIBS
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char gnutls_init ();
+int
+main ()
+{
+return gnutls_init ();
+  ;
+  return 0;
+}
+_ACEOF
+for ac_lib in '' gnutls; do
+  if test -z "$ac_lib"; then
+ac_res="none required"
+  else
+ac_res=-l$ac_lib
+LIBS="-l$ac_lib  $ac_func_search_save_LIBS"
+  fi
+  if ac_fn_c_try_link "$LINENO"; then :
+  ac_cv_search_gnutls_init=$ac_res
+fi
+rm -f core conftest.err conftest.$ac_objext \
+conftest$ac_exeext
+  if ${ac_cv_search_gnutls_init+:} false; then :
+  break
+fi
+done
+if ${ac_cv_search_gnutls_init+:} false; then :
+
+else
+  ac_cv_search_gnutls_init=no
+fi
+rm conftest.$ac_ext
+LIBS=$ac_func_search_save_LIBS
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_gnutls_init" >&5
+$as_echo "$ac_cv_search_gnutls_init" >&6; }
+ac_res=$ac_cv_search_gnutls_init
+if test "$ac_res" != no; then :
+  test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
+
+else
+  as_fn_error $? "library 'gnutls' is required for GnuTLS" "$LINENO" 5
+fi
+
+  # GnuTLS versions before 3.4.0 do not support sorting incorrectly sorted
+  # certificate chains.
+  ac_fn_c_check_decl "$LINENO" "GNUTLS_X509_CRT_LIST_SORT" "ac_cv_have_decl_GNUTLS_X509_CRT_LIST_SORT" "#include 
+#include 
+
+"
+if test "x$ac_cv_have_decl_GNUTLS_X509_CRT_LIST_SORT" = xyes; then :
+  ac_have_decl=1
+else
+  ac_have_decl=0
+fi
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE_DECL_GNUTLS_X509_CRT_LIST_SORT $ac_have_decl
+_ACEOF
+
+fi
+
 if test "$with_pam" = yes ; then
   { $as_echo "$as_me:${as_lineno-$LINENO}: checking for pam_start in -lpam" >&5
 $as_echo_n "checking for pam_start in -lpam... " >&6; }
@@ -11015,6 +11130,17 @@ else
 fi
 
 
+fi
+
+if test "$with_gnutls" = yes ; then
+  ac_fn_c_check_header_mongrel "$LINENO" "gnutls/gnutls.h" "ac_cv_header_gnutls_gnutls_h" "$ac_includes_default"
+if test "x$ac_cv_header_gnutls_gnutls_h" = xyes; then :
+
+else
+  as_fn_error $? "header file  is required for GnuTLS" "$LINENO" 5
+fi
+
+
 fi
 
 if test "$with_pam" = yes ; then
@@ -15522,9 +15648,11 @@ fi
 # in the template or configure command line.
 
 # If not selected manually, try to select a source automatically.
-if test "$enable_strong_random" = "yes" && test x"$USE_OPENSSL_RANDOM" = x"" && test x"$USE_WIN32_RANDOM" = x"" && test x"$USE_DEV_URANDOM" = x"" ; then
+if test "$enable_strong_random" = "yes" && test 

Re: [HACKERS] GnuTLS support

[Jeff Janes]

On Thu, Aug 31, 2017 at 10:52 AM, Andreas Karlsson 
wrote:

>
>
>
> = Work left to do
>
> - Test code with older versions of GnuTLS
>


I can't build against gnutls-2.12.23-21.el6.x86_64 from CentOS 6.9

be-secure-gnutls.c: In function 'be_tls_init':
be-secure-gnutls.c:168: warning: implicit declaration of function
'gnutls_certificate_set_pin_function'
be-secure-gnutls.c: In function 'get_peer_certificate':
be-secure-gnutls.c:656: error: 'GNUTLS_X509_CRT_LIST_SORT' undeclared
(first use in this function)
be-secure-gnutls.c:656: error: (Each undeclared identifier is reported only
once
be-secure-gnutls.c:656: error: for each function it appears in.)

But I can build on ubuntu 16.04, using whatever baffling salami of package
versions they turned gnutls into on that system.

I can interoperate in both direction gnutls client to openssl server and
the reverse (and gnutls to gnutls).

Cheers,

Jeff

Re: [HACKERS] GnuTLS support

[Robert Haas]

On Thu, Sep 7, 2017 at 10:35 PM, Tom Lane  wrote:
> I think we might be best off just playing it straight and providing
> a config file that contains a section along these lines:
>
> # Parameters for OpenSSL.  Leave these commented out if not using OpenSSL.
> #
> #ssl_ciphers = 'HIGH:MEDIUM:+3DES:!aNULL' # allowed SSL ciphers
> #ssl_prefer_server_ciphers = on
> #ssl_ecdh_curve = 'prime256v1'
> #ssl_dh_params_file = ''
> #ssl_cert_file = 'server.crt'
> #ssl_key_file = 'server.key'
> #ssl_ca_file = ''
> #ssl_crl_file = ''
> #
> # Parameters for GnuTLS.  Leave these commented out if not using GnuTLS.
> #
> #gnufoo=...
> #gnubar=...
> #
> # Parameters for macOS TLS.  ... you get the idea.
>
> As previously noted, it'd be a good idea to rename the existing
> ssl_xxx parameters to openssl_xxx, except maybe ones that we think
> will be universal.  (But even if we do think that, it might be
> simpler in the long run to just have three or four totally independent
> sections of the config file, instead of some common and some library-
> specific parameters.)

+1 to all of that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] GnuTLS support

[Tom Lane]

Andreas Karlsson  writes:
> On 09/07/2017 11:34 PM, Tomas Vondra wrote:
>> Well, people won't be able to set the inactive options, just like you
>> can't set ssl=on when you build without OpenSSL support. But perhaps we
>> could simply not include the inactive options into the config file, no?

> Yeah, I have been thinking about how bad it would be to dynamically 
> generate the config file. I think I will try this.

I'm not exactly convinced that dynamically inserting some parameters
and not others is a great idea.  Remember that people tend to copy
postgresql.conf files forward from one installation to another.  Or
they might decide to rebuild the postmaster for an existing installation
with a different SSL library.  In any scenario like that, you've not
done them any real favor if the config file they have contains no trace
of the SSL parameters they need.

I think we might be best off just playing it straight and providing
a config file that contains a section along these lines:

# Parameters for OpenSSL.  Leave these commented out if not using OpenSSL.
#
#ssl_ciphers = 'HIGH:MEDIUM:+3DES:!aNULL' # allowed SSL ciphers
#ssl_prefer_server_ciphers = on
#ssl_ecdh_curve = 'prime256v1'
#ssl_dh_params_file = ''
#ssl_cert_file = 'server.crt'
#ssl_key_file = 'server.key'
#ssl_ca_file = ''
#ssl_crl_file = ''
#
# Parameters for GnuTLS.  Leave these commented out if not using GnuTLS.
#
#gnufoo=...
#gnubar=...
#
# Parameters for macOS TLS.  ... you get the idea.

As previously noted, it'd be a good idea to rename the existing
ssl_xxx parameters to openssl_xxx, except maybe ones that we think
will be universal.  (But even if we do think that, it might be
simpler in the long run to just have three or four totally independent
sections of the config file, instead of some common and some library-
specific parameters.)

regards, tom lane


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

Re: [HACKERS] GnuTLS support

[Andreas Karlsson]

On 09/07/2017 11:34 PM, Tomas Vondra wrote:

I am worried about having 3x version of TLS controls in
postgresql.conf, and only one set being active. Perhaps we need to
break out the TLS config to separate files or something. Anyway, this
needs more thought.


Well, people won't be able to set the inactive options, just like you
can't set ssl=on when you build without OpenSSL support. But perhaps we
could simply not include the inactive options into the config file, no?


Yeah, I have been thinking about how bad it would be to dynamically 
generate the config file. I think I will try this.


Daniel: What options does Secure Transport need for configuring ciphers, 
ECDH, and cipher preference? Does it need any extra options (I think I 
saw something about the keychain)?


Andreas


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

Re: [HACKERS] GnuTLS support

[Magnus Hagander]

On Thu, Sep 7, 2017 at 2:34 PM, Tomas Vondra 
wrote:

> Hi,
>
> On 09/04/2017 04:24 PM, Bruce Momjian wrote:
> > On Fri, Sep  1, 2017 at 12:09:35PM -0400, Robert Haas wrote:
> >> I think that what this shows is that the current set of GUCs is overly
> >> OpenSSL-centric.  We created a set of GUCs that are actually specific
> >> to one particular implementation but named them as if they were
> >> generic.  My idea about this would be to actually rename the existing
> >> GUCs to start with "openssl" rather than "ssl", and then add new GUCs
> >> as needed for other SSL implementations.
> >>
> >> Depending on what we think is best, GUCs for an SSL implementation
> >> other than the one against which we compiled can either not exist at
> >> all, or can exist but be limited to a single value (e.g. "none", as we
> >> currently do when the compile has no SSL support at all).  Also, we
> >> could add a read-only GUC with a name like ssl_library that exposes
> >> the name of the underlying SSL implementation - none, openssl, gnutls,
> >> or whatever.
> >>
> >> I think if we go the route of insisting that every SSL implementation
> >> has to use the existing GUCs, we're just trying to shove a square peg
> >> into a round hole, and there's no real benefit for users.  If the
> >> string that has to be stuffed into ssl_ciphers differs based on which
> >> library was chosen at compile time, then you can't have a uniform
> >> default configuration for all libraries anyway.  I think it'll be
> >> easier to explain and document this if there's separate documentation
> >> for openssl_ciphers, gnutls_ciphers, etc. rather than one giant
> >> documentation section that tries to explain every implementation
> >> separately.
> >
> > I am worried about having 3x version of TLS controls in
> > postgresql.conf, and only one set being active. Perhaps we need to
> > break out the TLS config to separate files or something. Anyway, this
> > needs more thought.
> >
>
> Well, people won't be able to set the inactive options, just like you
> can't set ssl=on when you build without OpenSSL support. But perhaps we
> could simply not include the inactive options into the config file, no?
>

We build the pg_hba.conf file dynamically depending on if we have ipv6
support, IIRC. Maybe we need to implement that type of support into
postgresql.conf as well?

It will still be a mess though -- documentation, and tutorials around and
whatnot, will be dependent on library. But I'm not sure we can really get
around that.

Do we have some examples of how other products that support multiple
libraries do to handle this?


>
> I don't see how breaking the TLS config into separate files improves the
> situation - instead of dealing with GUCs in a single file you'll be
> dealing with the same GUCs in multiple files. No?
>

+1. I don't think splitting them up into different files makes it in any
way better -- if anything, it makes it worse.


-- 
 Magnus Hagander
 Me: https://www.hagander.net/ 
 Work: https://www.redpill-linpro.com/ 

Re: [HACKERS] GnuTLS support

[Tomas Vondra]

Hi,

On 09/04/2017 04:24 PM, Bruce Momjian wrote:
> On Fri, Sep  1, 2017 at 12:09:35PM -0400, Robert Haas wrote:
>> I think that what this shows is that the current set of GUCs is overly
>> OpenSSL-centric.  We created a set of GUCs that are actually specific
>> to one particular implementation but named them as if they were
>> generic.  My idea about this would be to actually rename the existing
>> GUCs to start with "openssl" rather than "ssl", and then add new GUCs
>> as needed for other SSL implementations.
>>
>> Depending on what we think is best, GUCs for an SSL implementation
>> other than the one against which we compiled can either not exist at
>> all, or can exist but be limited to a single value (e.g. "none", as we
>> currently do when the compile has no SSL support at all).  Also, we
>> could add a read-only GUC with a name like ssl_library that exposes
>> the name of the underlying SSL implementation - none, openssl, gnutls,
>> or whatever.
>>
>> I think if we go the route of insisting that every SSL implementation
>> has to use the existing GUCs, we're just trying to shove a square peg
>> into a round hole, and there's no real benefit for users.  If the
>> string that has to be stuffed into ssl_ciphers differs based on which
>> library was chosen at compile time, then you can't have a uniform
>> default configuration for all libraries anyway.  I think it'll be
>> easier to explain and document this if there's separate documentation
>> for openssl_ciphers, gnutls_ciphers, etc. rather than one giant
>> documentation section that tries to explain every implementation
>> separately.
> 
> I am worried about having 3x version of TLS controls in
> postgresql.conf, and only one set being active. Perhaps we need to
> break out the TLS config to separate files or something. Anyway, this
> needs more thought.
> 

Well, people won't be able to set the inactive options, just like you
can't set ssl=on when you build without OpenSSL support. But perhaps we
could simply not include the inactive options into the config file, no?

I don't see how breaking the TLS config into separate files improves the
situation - instead of dealing with GUCs in a single file you'll be
dealing with the same GUCs in multiple files. No?

regards

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


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

Re: [HACKERS] GnuTLS support

[David Fetter]

On Fri, Sep 01, 2017 at 10:33:37PM +0200, Alvaro Herrera wrote:
> Tom Lane wrote:
> > Robert Haas  writes:
> > > On Thu, Aug 31, 2017 at 1:52 PM, Andreas Karlsson  
> > > wrote:
> 
> > >> There are currently two failing SSL tests which at least to me seems more
> > >> like they test specific OpenSSL behaviors rather than something which 
> > >> need
> > >> to be true for all SSL libraries.
> > 
> > > I don't know what we should do about these issues.
> > 
> > Maybe the SSL test suite needs to be implementation-specific as well.
> 
> If only two tests fail currently, I suggest that the thing to do is to
> split it up in generic vs. library-specific test files.  It should be
> easy to do with the TAP framework -- just move the library-specific
> tests to their own file and mark it as skipped at the start of the file
> when a different library is detected.

This seems like a much smarter and more reliable way to test.

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


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

Re: [HACKERS] GnuTLS support

[Bruce Momjian]

On Fri, Sep  1, 2017 at 12:09:35PM -0400, Robert Haas wrote:
> I think that what this shows is that the current set of GUCs is overly
> OpenSSL-centric.  We created a set of GUCs that are actually specific
> to one particular implementation but named them as if they were
> generic.  My idea about this would be to actually rename the existing
> GUCs to start with "openssl" rather than "ssl", and then add new GUCs
> as needed for other SSL implementations.
> 
> Depending on what we think is best, GUCs for an SSL implementation
> other than the one against which we compiled can either not exist at
> all, or can exist but be limited to a single value (e.g. "none", as we
> currently do when the compile has no SSL support at all).  Also, we
> could add a read-only GUC with a name like ssl_library that exposes
> the name of the underlying SSL implementation - none, openssl, gnutls,
> or whatever.
> 
> I think if we go the route of insisting that every SSL implementation
> has to use the existing GUCs, we're just trying to shove a square peg
> into a round hole, and there's no real benefit for users.  If the
> string that has to be stuffed into ssl_ciphers differs based on which
> library was chosen at compile time, then you can't have a uniform
> default configuration for all libraries anyway.  I think it'll be
> easier to explain and document this if there's separate documentation
> for openssl_ciphers, gnutls_ciphers, etc. rather than one giant
> documentation section that tries to explain every implementation
> separately.

I am worried about having 3x version of TLS controls in postgresql.conf,
and only one set being active.  Perhaps we need to break out the TLS
config to separate files or something.  Anyway, this needs more thought.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


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

Re: [HACKERS] GnuTLS support

[Alvaro Herrera]

Tom Lane wrote:
> Robert Haas  writes:
> > On Thu, Aug 31, 2017 at 1:52 PM, Andreas Karlsson  wrote:

> >> There are currently two failing SSL tests which at least to me seems more
> >> like they test specific OpenSSL behaviors rather than something which need
> >> to be true for all SSL libraries.
> 
> > I don't know what we should do about these issues.
> 
> Maybe the SSL test suite needs to be implementation-specific as well.

If only two tests fail currently, I suggest that the thing to do is to
split it up in generic vs. library-specific test files.  It should be
easy to do with the TAP framework -- just move the library-specific
tests to their own file and mark it as skipped at the start of the file
when a different library is detected.

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


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

Re: [HACKERS] GnuTLS support

[Daniel Gustafsson]

> On 01 Sep 2017, at 20:00, Robert Haas  wrote:
> 
> On Fri, Sep 1, 2017 at 1:10 PM, Tom Lane  wrote:
>> Robert Haas  writes:
>>> On Thu, Aug 31, 2017 at 1:52 PM, Andreas Karlsson  wrote:
 I have seen discussions from time to time about OpenSSL and its licensing
 issues so I decided to see how much work it would be to add support for
 another TLS library, and  I went with GnuTLS since it is the library I know
 best after OpenSSL and it is also a reasonably popular library.
>> 
>>> Thanks for working on this.  I think it's good for PostgreSQL to have
>>> more options in this area.
>> 
>> +1.  We also have a patch in the queue to support macOS' TLS library,
>> and I suppose that's going to be facing similar issues.  It would be
>> a good plan, probably, to try to push both of these to conclusion in
>> the same development cycle.
> 
> The thing which I think would save the most aggravation - at least for
> my employer - is a Windows SSL implementation.

In 53ea546e.6020...@vmware.com, an early version of SChannel support was posted
by Heikki.  If anyone is keen to pick up the effort that would most likely be a
good starting point.

> Relying on OpenSSL
> means that every time OpenSSL puts out a critical security fix, we've
> got to rewrap all the Windows installers to pick up the new version.
> If we were relying on what's built into Windows, it would be
> Microsoft's problem.  Granted, it's not anybody's job to solve
> EnterpriseDB's problems except EnterpriseDB, but users might like it
> too -- and anyone else who is building Windows installers for
> PostgreSQL.
> 
> Depending on macOS TLS instead of OpenSSL has similar advantages, of
> course, just for a somewhat less common platform.

I think providing alternatives to OpenSSL on platforms where OpenSSL can’t be
relied on to be already available (Windows and macOS come to mind) would be a
great thing for many users and app developers.

cheers ./daniel

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

Re: [HACKERS] GnuTLS support

[Robert Haas]

On Fri, Sep 1, 2017 at 1:10 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Thu, Aug 31, 2017 at 1:52 PM, Andreas Karlsson  wrote:
>>> I have seen discussions from time to time about OpenSSL and its licensing
>>> issues so I decided to see how much work it would be to add support for
>>> another TLS library, and  I went with GnuTLS since it is the library I know
>>> best after OpenSSL and it is also a reasonably popular library.
>
>> Thanks for working on this.  I think it's good for PostgreSQL to have
>> more options in this area.
>
> +1.  We also have a patch in the queue to support macOS' TLS library,
> and I suppose that's going to be facing similar issues.  It would be
> a good plan, probably, to try to push both of these to conclusion in
> the same development cycle.

The thing which I think would save the most aggravation - at least for
my employer - is a Windows SSL implementation.  Relying on OpenSSL
means that every time OpenSSL puts out a critical security fix, we've
got to rewrap all the Windows installers to pick up the new version.
If we were relying on what's built into Windows, it would be
Microsoft's problem.  Granted, it's not anybody's job to solve
EnterpriseDB's problems except EnterpriseDB, but users might like it
too -- and anyone else who is building Windows installers for
PostgreSQL.

Depending on macOS TLS instead of OpenSSL has similar advantages, of
course, just for a somewhat less common platform.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] GnuTLS support

[Daniel Gustafsson]

> On 01 Sep 2017, at 19:10, Tom Lane  wrote:
> 
> Robert Haas  writes:
>> On Thu, Aug 31, 2017 at 1:52 PM, Andreas Karlsson  wrote:
> 
>>> There are currently two failing SSL tests which at least to me seems more
>>> like they test specific OpenSSL behaviors rather than something which need
>>> to be true for all SSL libraries.
> 
>> I don't know what we should do about these issues.
> 
> Maybe the SSL test suite needs to be implementation-specific as well.

To properly test the macOS Secure Transport support we will need to use
Keychain files on top of plain PEM files, so I think we have to.  That being
said, we should probably define a (as large possible) minimum set which applies
to all to ensure compatability between different frontends and backends.

cheers ./daniel

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

Re: [HACKERS] GnuTLS support

[Tom Lane]

Robert Haas  writes:
> On Thu, Aug 31, 2017 at 1:52 PM, Andreas Karlsson  wrote:
>> I have seen discussions from time to time about OpenSSL and its licensing
>> issues so I decided to see how much work it would be to add support for
>> another TLS library, and  I went with GnuTLS since it is the library I know
>> best after OpenSSL and it is also a reasonably popular library.

> Thanks for working on this.  I think it's good for PostgreSQL to have
> more options in this area.

+1.  We also have a patch in the queue to support macOS' TLS library,
and I suppose that's going to be facing similar issues.  It would be
a good plan, probably, to try to push both of these to conclusion in
the same development cycle.

> I think that what this shows is that the current set of GUCs is overly
> OpenSSL-centric.  We created a set of GUCs that are actually specific
> to one particular implementation but named them as if they were
> generic.  My idea about this would be to actually rename the existing
> GUCs to start with "openssl" rather than "ssl", and then add new GUCs
> as needed for other SSL implementations.

Works for me.

>> There are currently two failing SSL tests which at least to me seems more
>> like they test specific OpenSSL behaviors rather than something which need
>> to be true for all SSL libraries.

> I don't know what we should do about these issues.

Maybe the SSL test suite needs to be implementation-specific as well.

regards, tom lane


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

Re: [HACKERS] GnuTLS support

[Robert Haas]

On Thu, Aug 31, 2017 at 1:52 PM, Andreas Karlsson  wrote:
> I have seen discussions from time to time about OpenSSL and its licensing
> issues so I decided to see how much work it would be to add support for
> another TLS library, and  I went with GnuTLS since it is the library I know
> best after OpenSSL and it is also a reasonably popular library.

Thanks for working on this.  I think it's good for PostgreSQL to have
more options in this area.

> = Design questions
>
> == GnuTLS priority strings vs OpenSSL cipher lists
>
> GnuTLS uses a different format for specifying ciphers. Instead of setting
> ciphers, protocol versions, and ECDH curves separately GnuTLS instead uses a
> single priority string[1].
>
> For example the default settings of PostgreSQL (which include disabling
> SSLv3)
>
> ssl_ciphers = 'HIGH:MEDIUM:+3DES:!aNULL' # allowed SSL ciphers
> ssl_prefer_server_ciphers = on
> ssl_ecdh_curve = 'prime256v1'
>
> is represented with a string similar to
>
> SECURE128:+3DES-CBC:+GROUP-SECP256R1:%SERVER_PRECEDENCE
>
> So the two libraries have decided on different ways to specify things.

I think that what this shows is that the current set of GUCs is overly
OpenSSL-centric.  We created a set of GUCs that are actually specific
to one particular implementation but named them as if they were
generic.  My idea about this would be to actually rename the existing
GUCs to start with "openssl" rather than "ssl", and then add new GUCs
as needed for other SSL implementations.

Depending on what we think is best, GUCs for an SSL implementation
other than the one against which we compiled can either not exist at
all, or can exist but be limited to a single value (e.g. "none", as we
currently do when the compile has no SSL support at all).  Also, we
could add a read-only GUC with a name like ssl_library that exposes
the name of the underlying SSL implementation - none, openssl, gnutls,
or whatever.

I think if we go the route of insisting that every SSL implementation
has to use the existing GUCs, we're just trying to shove a square peg
into a round hole, and there's no real benefit for users.  If the
string that has to be stuffed into ssl_ciphers differs based on which
library was chosen at compile time, then you can't have a uniform
default configuration for all libraries anyway.  I think it'll be
easier to explain and document this if there's separate documentation
for openssl_ciphers, gnutls_ciphers, etc. rather than one giant
documentation section that tries to explain every implementation
separately.

> There are currently two failing SSL tests which at least to me seems more
> like they test specific OpenSSL behaviors rather than something which need
> to be true for all SSL libraries.
>
> The two tests:
>
> # Try with just the server CA's cert. This fails because the root file
> # must contain the whole chain up to the root CA.
> note "connect with server CA cert, without root CA";
> test_connect_fails("sslrootcert=ssl/server_ca.crt sslmode=verify-ca");
>
> # A CRL belonging to a different CA is not accepted, fails
> test_connect_fails(
> "sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca
> sslcrl=ssl/client.crl");
>
> For the missing root CA case GnuTLS seems to be happy enough with just an
> intermediate CA and as far as I can tell this behavior is not easy to
> configure.
>
> And for the CRL belonging to a different CA case GnuTLS can be configured to
> either just store such a non-validating CRL or to ignore it, but not to
> return an error.
>
> Personally I think thee two tests should just be removed but maybe I am
> missing something.

I don't know what we should do about these issues.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

[HACKERS] GnuTLS support

[Andreas Karlsson]

Hi,

I have seen discussions from time to time about OpenSSL and its 
licensing issues so I decided to see how much work it would be to add 
support for another TLS library, and  I went with GnuTLS since it is the 
library I know best after OpenSSL and it is also a reasonably popular 
library.


Attached is a work in progress patch which implements the basics. I send 
it the list because I want feedback on some design questions and to 
check with the community if this is a feature we want.


= What is implemented

- Backend
- Frontend
- Diffie-Hellmann support
- Using GnuTLS for secure random numbers
- Using GnuTLS for sha2

= Work left to do

- Add GnuTLS support to sslinfo
- Add GnuTLS support to pgcrypto
- Support for GnuTLS's version of engines
- Test code with older versions of GnuTLS
- Update documentation
- Add support for all postgresql.conf options (see design question)
- Fix two failing tests (see design question)

= Design questions

== GnuTLS priority strings vs OpenSSL cipher lists

GnuTLS uses a different format for specifying ciphers. Instead of 
setting ciphers, protocol versions, and ECDH curves separately GnuTLS 
instead uses a single priority string[1].


For example the default settings of PostgreSQL (which include disabling 
SSLv3)


ssl_ciphers = 'HIGH:MEDIUM:+3DES:!aNULL' # allowed SSL ciphers
ssl_prefer_server_ciphers = on
ssl_ecdh_curve = 'prime256v1'

is represented with a string similar to

SECURE128:+3DES-CBC:+GROUP-SECP256R1:%SERVER_PRECEDENCE

So the two libraries have decided on different ways to specify things.

One way to solve th issue would be to just let ssl_ciphers be the 
priority string and then add %SERVER_PRECEDENCE to it if 
ssl_prefer_server_ciphers is on. Adding the ssl_ecdh_curve is trickier 
since the curves have different names in GnuTLS (e.g. prime256v1 vs 
SECP256R1) and I would rather avoid having to add such a mapping to 
PostgreSQL. Thoughts?


== Potentially OpenSSL-specific  est cases

There are currently two failing SSL tests which at least to me seems 
more like they test specific OpenSSL behaviors rather than something 
which need to be true for all SSL libraries.


The two tests:

# Try with just the server CA's cert. This fails because the root file
# must contain the whole chain up to the root CA.
note "connect with server CA cert, without root CA";
test_connect_fails("sslrootcert=ssl/server_ca.crt sslmode=verify-ca");

# A CRL belonging to a different CA is not accepted, fails
test_connect_fails(
"sslrootcert=ssl/root+server_ca.crt sslmode=verify-ca 
sslcrl=ssl/client.crl");


For the missing root CA case GnuTLS seems to be happy enough with just 
an intermediate CA and as far as I can tell this behavior is not easy to 
configure.


And for the CRL belonging to a different CA case GnuTLS can be 
configured to either just store such a non-validating CRL or to ignore 
it, but not to return an error.


Personally I think thee two tests should just be removed but maybe I am 
missing something.


Notes:

1. https://gnutls.org/manual/html_node/Priority-Strings.html

Andreas
diff --git a/configure b/configure
index a2f9a256b4..8dcb26b532 100755
--- a/configure
+++ b/configure
@@ -709,6 +709,7 @@ UUID_EXTRA_OBJS
 with_uuid
 with_systemd
 with_selinux
+with_gnutls
 with_openssl
 krb_srvtab
 with_python
@@ -838,6 +839,7 @@ with_bsd_auth
 with_ldap
 with_bonjour
 with_openssl
+with_gnutls
 with_selinux
 with_systemd
 with_readline
@@ -1534,6 +1536,7 @@ Optional Packages:
   --with-ldap build with LDAP support
   --with-bonjour  build with Bonjour support
   --with-openssl  build with OpenSSL support
+  --with-gnutls   build with GnuTS support
   --with-selinux  build with SELinux support
   --with-systemd  build with systemd support
   --without-readline  do not use GNU Readline nor BSD Libedit for editing
@@ -6051,6 +6054,41 @@ fi
 $as_echo "$with_openssl" >&6; }
 
 
+#
+# GnuTLS
+#
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to build with GnuTLS support" >&5
+$as_echo_n "checking whether to build with GnuTLS support... " >&6; }
+
+
+
+# Check whether --with-gnutls was given.
+if test "${with_gnutls+set}" = set; then :
+  withval=$with_gnutls;
+  case $withval in
+yes)
+
+$as_echo "#define USE_GNUTLS 1" >>confdefs.h
+
+  ;;
+no)
+  :
+  ;;
+*)
+  as_fn_error $? "no argument expected for --with-gnutls option" "$LINENO" 5
+  ;;
+  esac
+
+else
+  with_gnutls=no
+
+fi
+
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $with_gnutls" >&5
+$as_echo "$with_gnutls" >&6; }
+
+
 #
 # SELinux
 #
@@ -10218,6 +10256,67 @@ done
 
 fi
 
+if test "$with_gnutls" = yes ; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing gnutls_init" >&5
+$as_echo_n "checking for library containing gnutls_init... " >&6; }
+if ${ac_cv_search_gnutls_init+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ac_func_search_save_LIBS=$LIBS
+cat confdefs.h -