Add support to TLS 1.3 cipher suites and curves lists

[Erica Zhang]

Hi All,

I’m a Postgres user and I’m looking into restricting the set of allowed ciphers 
on Postgres and configure a concrete set of curves on our postgres instances.

I see in current Postgres doc mentioned that only TLS1.2 and below cipher lists 
can be configured. And there is no setting that controls the cipher choices 
used by TLS1.3. 

As for ECDH keys currently postgres opts to support setting only a single 
elliptic group instead of setting a lists.
As described in below doc link:

https://www.postgresql.org/docs/devel/runtime-config-connection.html


Now I have a patch to support settings for TLS1.3 ciphersuites and expanding 
the configuration option for EC settings. With my patch we can do:
1. Added a new configuration option ssl_ciphers_suites to control the cipher 
choices used by TLS 1.3. 2. Extend the existing configuration option 
ssl_ecdh_curve to accept a list of curve names seperated by colon.

Could you please help to review to see if you are interested in having this 
change in upcoming Postgres major release(It's should be PG17)? 

Thanks in advance.

patch_support_tls1.3_curvelist.diff
Description: Binary data

Re: Add support to TLS 1.3 cipher suites and curves lists

[Peter Eisentraut]

On 07.06.24 08:10, Erica Zhang wrote:
I’m a Postgres user and I’m looking into restricting the set of allowed 
ciphers on Postgres and configure a concrete set of curves on our 
postgres instances.


Out of curiosity, why is this needed in practice?

Could you please help to review to see if you are interested in having 
this change in upcoming Postgres major release(It's should be PG17)?


It would be targetting PG18 now.

Re:Re: Add support to TLS 1.3 cipher suites and curves lists

[Erica Zhang]

Hi Peter,
Thanks a lot for the quick response. We are using Postgres instance in our 
product. For some security consideration, we prefer to use TLS1.3 cipher suites 
in our product with some customization values instead of default value 
"HIGH:MEDIUM:+3DES:!aNULL". Moreover we prefer to set a group of ecdh keys 
instead of a single value.


I see the https://commitfest.postgresql.org/48/ is still open, could it be 
possible to target for PG17? As I know PG17 is going to be release this year so 
that we can upgrade our instances to this new version accodingly.
   
Original Email
   
 

Sender:"Peter Eisentraut"< pe...@eisentraut.org >;

Sent Time:2024/6/7 16:55

To:"Erica Zhang"< ericazhangy2...@qq.com >;"pgsql-hackers"< 
pgsql-hackers@lists.postgresql.org >;

Subject:Re: Add support to TLS 1.3 cipher suites and curves lists


On 07.06.24 08:10, Erica Zhang wrote:
> I’m a Postgres user and I’m looking into restricting the set of allowed 
> ciphers on Postgres and configure a concrete set of curves on our 
> postgres instances.

Out of curiosity, why is this needed in practice?

> Could you please help to review to see if you are interested in having 
> this change in upcoming Postgres major release(It's should be PG17)?

It would be targetting PG18 now.

Re: Add support to TLS 1.3 cipher suites and curves lists

[Daniel Gustafsson]

> On 7 Jun 2024, at 19:14, Jacob Champion  
> wrote:

> - Could you separate the two features into two patches? That would
> make it easier for reviewers. (They can still share the same thread
> and CF entry.)

+1, please do.

> - The "curve" APIs have been renamed "group" in newer OpenSSLs for a
> while now, and we should probably use those if possible.

Agreed.  While not deprecated per se the curve API is considered obsolete and
is just aliased to the group API (OpenSSL using both the term obsolete and
deprecated to mean the same thing but with very different mechanics is quite
confusing).

> - I think parsing apart the groups list to check NIDs manually could
> lead to false negatives. From a docs skim, 3.0 allows providers to add
> their own group names, and 3.3 now supports question marks in the
> string to allow graceful fallbacks.

Parsing the list will likely risk false negatives as you say, but from skimming
the code there doesn't seem to be a good errormessage from SSL_set1_groups_list
to indicate if listitems were invalid (unless all of them were).  Maybe calling
the associated _get function to check the number of set groups can be used to
verify what happenend?

Regarding the ciphersuites portion of the patch.  I'm not particularly thrilled
about having a GUC for TLSv1.2 ciphers and one for TLSv1.3 ciphersuites, users
not all that familiar with TLS will likely find it confusing to figure out what
to do.

In which way is this feature needed since this can be achieved with the config
directive "Ciphersuites" in openssl.conf IIUC?

If we add this I think we should keep it blank and if so skip setting it at all
falling back on OpenSSL defaults.  The below default for the GUC does not match
the OpenSSL default and I think we are better off trusting them on this.

+   "TLS_AES_256_GCM_SHA384:TLS_AES_128_GCM_SHA256",

--
Daniel Gustafsson

Re: Add support to TLS 1.3 cipher suites and curves lists

[Jelte Fennema-Nio]

On Mon, 10 Jun 2024 at 12:31, Daniel Gustafsson  wrote:
> Regarding the ciphersuites portion of the patch.  I'm not particularly 
> thrilled
> about having a GUC for TLSv1.2 ciphers and one for TLSv1.3 ciphersuites, users
> not all that familiar with TLS will likely find it confusing to figure out 
> what
> to do.

I don't think it's easy to create a single GUC because OpenSSL has
different APIs for both. So we'd have to add some custom parsing for
the combined string, which is likely to cause some problems imho. I
think separating them is the best option from the options we have and
I don't think it matters much practice for users. Users not familiar
with TLS might indeed be confused, but those users shouldn't touch
these settings anyway, and just use the defaults. The users that care
about this probably already get two cipher strings from their
compliance teams, because many other applications also have two
separate options for specifying both.

Re: Add support to TLS 1.3 cipher suites and curves lists

[Peter Eisentraut]

On 12.06.24 10:51, Jelte Fennema-Nio wrote:

On Mon, 10 Jun 2024 at 12:31, Daniel Gustafsson  wrote:

Regarding the ciphersuites portion of the patch.  I'm not particularly thrilled
about having a GUC for TLSv1.2 ciphers and one for TLSv1.3 ciphersuites, users
not all that familiar with TLS will likely find it confusing to figure out what
to do.


I don't think it's easy to create a single GUC because OpenSSL has
different APIs for both. So we'd have to add some custom parsing for
the combined string, which is likely to cause some problems imho. I
think separating them is the best option from the options we have and
I don't think it matters much practice for users. Users not familiar
with TLS might indeed be confused, but those users shouldn't touch
these settings anyway, and just use the defaults. The users that care
about this probably already get two cipher strings from their
compliance teams, because many other applications also have two
separate options for specifying both.


Maybe some comparisons with other SSL-enabled server products would be 
useful.


Here is the Apache httpd setting:

https://httpd.apache.org/docs/current/mod/mod_ssl.html#sslciphersuite

They use a complex syntax to be able to set both via one setting.

Here is the nginx setting:

https://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_ciphers

This doesn't appear to support TLS 1.3?

Re:Re: Add support to TLS 1.3 cipher suites and curves lists

[Erica Zhang]

Hi All,
Frist of all really appreciate the review of my patch. I've seperated the patch 
into two: patch_support_tls1.3 for tls1.3 support and 
patch_support_curves_group for a set of curves list. 



TLS1.3 support - patch_support_tls1.3
I agree with Jelte that it's better to have different options for tls1.2 and 
lower(cipher list) and tls1.3(cipher suite) since openssl provided different 
APIs for each. As for users not faimilar with TLS（they don't care TLS,）we can 
still keep the default value as described here 
https://www.postgresql.org/docs/devel/runtime-config-connection.html. If TLS is 
critical to them(they should have figured out the different options in tls1.2 
and tls1.3), then they can set the values on-demand. Moreover we can add some 
description of these two options.

eg. 
ssl_ciphers                   
              | HIGH:MEDIUM:+3DES:!aNULL 
                     | 
Sets the list of allowed SSL ciphers for TLS1.2 and 
lower. 
 
ssl_ciphers_suites                 
         | HIGH:MEDIUM:+3DES:!aNULL     
            | Sets the list of allowed SSL cipher 
suites for TLS1.3.


Curves groups - patch_support_curves_group
Indeed SSL_CTX_set1_curves_list is deprecated it's bette to change to 
SSL_CTX_set1_groups_list instead.





   
Original Email
   
 

Sender:"Jelte Fennema-Nio"< postg...@jeltef.nl >;

Sent Time:2024/6/12 16:51

To:"Daniel Gustafsson"< dan...@yesql.se >;

Cc recipient:"Erica Zhang"< ericazhangy2...@qq.com >;"Jacob Champion"< 
jacob.champ...@enterprisedb.com >;"Peter Eisentraut"< pe...@eisentraut.org 
>;"pgsql-hackers"< pgsql-hackers@lists.postgresql.org >;

Subject:Re: Add support to TLS 1.3 cipher suites and curves lists


On Mon, 10 Jun 2024 at 12:31, Daniel Gustafsson  wrote:
> Regarding the ciphersuites portion of the patch.  I'm not particularly 
thrilled
> about having a GUC for TLSv1.2 ciphers and one for TLSv1.3 ciphersuites, 
users
> not all that familiar with TLS will likely find it confusing to figure out 
what
> to do.

I don't think it's easy to create a single GUC because OpenSSL has
different APIs for both. So we'd have to add some custom parsing for
the combined string, which is likely to cause some problems imho. I
think separating them is the best option from the options we have and
I don't think it matters much practice for users. Users not familiar
with TLS might indeed be confused, but those users shouldn't touch
these settings anyway, and just use the defaults. The users that care
about this probably already get two cipher strings from their
compliance teams, because many other applications also have two
separate options for specifying both.

patch_support_curves_list.diff
Description: Binary data


patch_support_tls1.3.diff
Description: Binary data

Re: Add support to TLS 1.3 cipher suites and curves lists

[Daniel Gustafsson]

> On 13 Jun 2024, at 09:07, Erica Zhang  wrote:

> How can I achieve the value for TLS1.3? Do you mean I can set the 
> Ciphersuites in openssl.conf, then Postgres will pick up and use this value 
> accordingly?

Yes, you should be able to restrict the ciphersuites for TLSv1.3 with
openssl.conf on your system.

--
Daniel Gustafsson

Re: Add support to TLS 1.3 cipher suites and curves lists

[Andres Freund]

Hi,

This thread was referenced by 
https://www.postgresql.org/message-id/48F0A1F8-E0B4-41F8-990F-41E6BA2A6185%40yesql.se

On 2024-06-13 14:34:27 +0800, Erica Zhang wrote:

> diff --git a/src/backend/libpq/be-secure-openssl.c 
> b/src/backend/libpq/be-secure-openssl.c
> index 39b1a66236..d097e81407 100644
> --- a/src/backend/libpq/be-secure-openssl.c
> +++ b/src/backend/libpq/be-secure-openssl.c
> @@ -1402,30 +1402,30 @@ static bool
>  initialize_ecdh(SSL_CTX *context, bool isServerStart)
>  {
>  #ifndef OPENSSL_NO_ECDH
> - EC_KEY *ecdh;
> - int nid;
> + char*curve_list = strdup(SSLECDHCurve);

ISTM we'd want to eventually rename the GUC variable to indicate it's a list?
I think the "ecdh" portion is actually not accurate anymore either, it's used
outside of ecdh if I understand correctly (probably I am not)?


> + char*saveptr;
> + char*token = strtok_r(curve_list, ":", &saveptr);
> + int nid;
>  
> - nid = OBJ_sn2nid(SSLECDHCurve);
> - if (!nid)
> + while (token != NULL)

It'd be good to have a comment explaining why we're parsing the list ourselves
instead of using just the builtin SSL_CTX_set1_groups_list().

>   {
> - ereport(isServerStart ? FATAL : LOG,
> + nid = OBJ_sn2nid(token);
> + if (!nid)
> + {ereport(isServerStart ? FATAL : LOG,
>   (errcode(ERRCODE_CONFIG_FILE_ERROR),
> -  errmsg("ECDH: unrecognized curve name: %s", 
> SSLECDHCurve)));
> +  errmsg("ECDH: unrecognized curve name: %s", 
> token)));
>   return false;
> + }
> + token = strtok_r(NULL, ":", &saveptr);
>   }
>  
> - ecdh = EC_KEY_new_by_curve_name(nid);
> - if (!ecdh)
> + if(SSL_CTX_set1_groups_list(context, SSLECDHCurve) !=1)
>   {
>   ereport(isServerStart ? FATAL : LOG,
>   (errcode(ERRCODE_CONFIG_FILE_ERROR),
> -  errmsg("ECDH: could not create key")));
> +  errmsg("ECDH: failed to set curve names")));

Probably worth including the value of the GUC here?



This also needs to change the documentation for the GUC.



Once we have this parameter we probably should add at least x25519 to the
allowed list, as that's the client side default these days.

But that can be done in a separate patch.

Greetings,

Andres Freund

Re:Re: Add support to TLS 1.3 cipher suites and curves lists

[Erica Zhang]

Hi,
Thanks a lot for the review.
Indeed the original ssl_ecdh_curve is used to set a single value of curve name. 
If we want to change it to indicate a list of curve names, is there any rule 
for naming in Postgres? like ssl_curve_groups?



   
Original Email
   
 

From:"Andres Freund"< and...@anarazel.de >;

Sent Time:2024/6/18 2:48

To:"Erica Zhang"< ericazhangy2...@qq.com >;

Cc recipient:"Jelte Fennema-Nio"< postg...@jeltef.nl >;"Daniel Gustafsson"< 
dan...@yesql.se >;"Jacob Champion"< jacob.champ...@enterprisedb.com 
>;"Peter Eisentraut"< pe...@eisentraut.org >;"pgsql-hackers"< 
pgsql-hackers@lists.postgresql.org >;

Subject:Re: Add support to TLS 1.3 cipher suites and curves lists


Hi,

This thread was referenced by 
https://www.postgresql.org/message-id/48F0A1F8-E0B4-41F8-990F-41E6BA2A6185%40yesql.se

On 2024-06-13 14:34:27 +0800, Erica Zhang wrote:

> diff --git a/src/backend/libpq/be-secure-openssl.c 
b/src/backend/libpq/be-secure-openssl.c
> index 39b1a66236..d097e81407 100644
> --- a/src/backend/libpq/be-secure-openssl.c
> +++ b/src/backend/libpq/be-secure-openssl.c
> @@ -1402,30 +1402,30 @@ static bool
>  initialize_ecdh(SSL_CTX *context, bool isServerStart)
>  {
>  #ifndef OPENSSL_NO_ECDH
> -  EC_KEY *ecdh;
> -  int nid;
> +  char*curve_list = strdup(SSLECDHCurve);

ISTM we'd want to eventually rename the GUC variable to indicate it's a list?
I think the "ecdh" portion is actually not accurate anymore either, it's used
outside of ecdh if I understand correctly (probably I am not)?


> +  char*saveptr;
> +  char*token = strtok_r(curve_list, ":", &saveptr);
> +  int nid;
>  
> -  nid = OBJ_sn2nid(SSLECDHCurve);
> -  if (!nid)
> +  while (token != NULL)

It'd be good to have a comment explaining why we're parsing the list ourselves
instead of using just the builtin SSL_CTX_set1_groups_list().

>{
> -  ereport(isServerStart ? FATAL : LOG,
> +  nid = OBJ_sn2nid(token);
> +  if (!nid)
> +  {ereport(isServerStart ? FATAL : LOG,
>(errcode(ERRCODE_CONFIG_FILE_ERROR),
> -   errmsg("ECDH: unrecognized curve name: %s", 
SSLECDHCurve)));
> +   errmsg("ECDH: unrecognized curve name: %s", 
token)));
>return false;
> +  }
> +  token = strtok_r(NULL, ":", &saveptr);
>}
>  
> -  ecdh = EC_KEY_new_by_curve_name(nid);
> -  if (!ecdh)
> +  if(SSL_CTX_set1_groups_list(context, SSLECDHCurve) !=1)
>{
>ereport(isServerStart ? FATAL : LOG,
>(errcode(ERRCODE_CONFIG_FILE_ERROR),
> -   errmsg("ECDH: could not create key")));
> +   errmsg("ECDH: failed to set curve names")));

Probably worth including the value of the GUC here?



This also needs to change the documentation for the GUC.



Once we have this parameter we probably should add at least x25519 to the
allowed list, as that's the client side default these days.

But that can be done in a separate patch.

Greetings,

Andres Freund

Re: Add support to TLS 1.3 cipher suites and curves lists

[Daniel Gustafsson]

I had a look at this patchset today and I think I've come around to the idea of
having a separate GUC for cipher suites.  I don't have strong opinions on
renaming ssl_ecdh_curve to reflect that it can take a list of multiple values,
there is merit to having descriptive names but it would also be an invasive
change for adding suffix 's'.

After fiddling a bit with the code and documentation I came up with the
attached version which also makes the testsuite use the list syntax in order to
test it.  It's essentially just polish and adding comments with the functional
changes that a) it parses the entire list of curves so all errors can be
reported instead of giving up at the first error; b) leaving the cipher suite
GUC blank will set the suites to the OpenSSL default vale.

This patch requires OpenSSL 1.1.1 as the minimum version, which in my view is
fine.  Removing support for older OpenSSL versions is being discussed already
and this makes a good case for requiring 1.1.1.  It does however mean that this
patch cannot be commmitted until that has been done though.  I have yet to test
this with LibreSSL.

As was suggested in a related thread I think we should change the default value
of the ECDH curves parameter, but that's for another patch.

--
Daniel Gustafsson



v3-0001-Support-multiple-ECDH-curves.patch
Description: Binary data


v3-0002-Support-TLSv1.3-cipher-suites.patch
Description: Binary data

Re: Add support to TLS 1.3 cipher suites and curves lists

[Peter Eisentraut]

On 03.07.24 17:20, Daniel Gustafsson wrote:

After fiddling a bit with the code and documentation I came up with the
attached version which also makes the testsuite use the list syntax in order to
test it.  It's essentially just polish and adding comments with the functional
changes that a) it parses the entire list of curves so all errors can be
reported instead of giving up at the first error; b) leaving the cipher suite
GUC blank will set the suites to the OpenSSL default vale.


It would be worth checking the discussion at 
 
about strtok()/strtok_r() issues.  First, for list parsing, it sometimes 
gives the wrong semantics, which I think might apply here.  Maybe it's 
worth comparing this with the semantics that OpenSSL provides natively. 
And second, strtok_r() is not available on Windows without the 
workaround provided in that thread.


I'm doubtful that it's worth replicating all this list parsing logic 
instead of just letting OpenSSL do it.  This is a very marginal feature 
after all.

Re: Add support to TLS 1.3 cipher suites and curves lists

[Daniel Gustafsson]

> On 11 Jul 2024, at 23:16, Peter Eisentraut  wrote:

> It would be worth checking the discussion at 
> 
>  about strtok()/strtok_r() issues.  First, for list parsing, it sometimes 
> gives the wrong semantics, which I think might apply here.  Maybe it's worth 
> comparing this with the semantics that OpenSSL provides natively. And second, 
> strtok_r() is not available on Windows without the workaround provided in 
> that thread.
> 
> I'm doubtful that it's worth replicating all this list parsing logic instead 
> of just letting OpenSSL do it.  This is a very marginal feature after all.

The original author added the string parsing in order to provide a good error
message in case of an error in the list, and since that seemed like a nice idea
I kept in my review revision.  With what you said above I agree it's not worth
the extra complexity it brings so the attached revision removes it.

--
Daniel Gustafsson



v4-0001-Support-multiple-ECDH-curves.patch
Description: Binary data


v4-0002-Support-TLSv1.3-cipher-suites.patch
Description: Binary data

Re: Add support to TLS 1.3 cipher suites and curves lists

[Jacob Champion]

On Fri, Jul 12, 2024 at 1:03 PM Daniel Gustafsson  wrote:
> The original author added the string parsing in order to provide a good error
> message in case of an error in the list, and since that seemed like a nice 
> idea
> I kept in my review revision.  With what you said above I agree it's not worth
> the extra complexity it brings so the attached revision removes it.

Misspelling a group now leads to the following error message for OpenSSL 3.0:

FATAL:  ECDH: failed to set curve names: no SSL error reported

Maybe a HINT would be nice here?:

HINT: Check that each group name is both spelled correctly and
supported by the installed version of OpenSSL.

or something.

> I don't have strong opinions on
> renaming ssl_ecdh_curve to reflect that it can take a list of multiple values,
> there is merit to having descriptive names but it would also be an invasive
> change for adding suffix 's'.

Can we just add an entry to map_old_guc_names to handle it? Something
like (untested)

 static const char *const map_old_guc_names[] = {
 "sort_mem", "work_mem",
 "vacuum_mem", "maintenance_work_mem",
+"ssl_ecdh_curve", "ssl_groups",
 NULL
 };

Re: Andres' concern about the ECDH part of the name, we could probably
keep the "dh" part, but I'd be wary of that changing underneath us
too. IANA changed the registry name to "TLS Supported Groups".

Thanks,
--Jacob

Re: Add support to TLS 1.3 cipher suites and curves lists

[Jacob Champion]

On Wed, Jul 3, 2024 at 9:20 AM Daniel Gustafsson  wrote:
> It's essentially just polish and adding comments with the functional
> changes that a) it parses the entire list of curves so all errors can be
> reported instead of giving up at the first error; b) leaving the cipher suite
> GUC blank will set the suites to the OpenSSL default vale.

Is there an advantage to setting it to a compile-time default, as
opposed to just leaving it alone and not setting it at all? With the
current patch, if you dropped in a more advanced OpenSSL 3.x that
changed up the defaults, you wouldn't see any benefit.

Thanks,
--Jacob

Re: Re: Add support to TLS 1.3 cipher suites and curves lists

[Michael Paquier]

On Fri, Jun 07, 2024 at 06:02:37PM +0800, Erica Zhang wrote:
> I see the https://commitfest.postgresql.org/48/ is still open, could
> it be possible to target for PG17? As I know PG17 is going to be
> release this year so that we can upgrade our instances to this new
> version accodingly.

Echoing with Peter, https://commitfest.postgresql.org/48/ is planned
to be the first commit fest of the development cycle for Postgres 18.
v17 is in feature freeze state and beta, where only bug fixes are
accepted, and not new features.
--
Michael


signature.asc
Description: PGP signature

Re: Re: Add support to TLS 1.3 cipher suites and curves lists

[Jacob Champion]

On Fri, Jun 7, 2024 at 3:02 AM Erica Zhang  wrote:
>
> For some security consideration, we prefer to use TLS1.3 cipher suites in our 
> product with some customization values instead of default value 
> "HIGH:MEDIUM:+3DES:!aNULL". Moreover we prefer to set a group of ecdh keys 
> instead of a single value.

+1 for the curve list feature, at least. No opinions on the 1.3
ciphersuites half, yet.

I've added this patch to my planned review for the v18 cycle. Some
initial notes:

- Could you separate the two features into two patches? That would
make it easier for reviewers. (They can still share the same thread
and CF entry.)
- The "curve" APIs have been renamed "group" in newer OpenSSLs for a
while now, and we should probably use those if possible.
- I think parsing apart the groups list to check NIDs manually could
lead to false negatives. From a docs skim, 3.0 allows providers to add
their own group names, and 3.3 now supports question marks in the
string to allow graceful fallbacks.
- I originally thought it'd be better to just stop calling
SSL_set_tmp_ecdh() entirely by default, so we could use OpenSSL's
builtin list of groups. But that may have denial-of-service concerns
[1]?
- We should maybe look into SSL_CTX_config(), if we haven't discussed
that already on the list, but that's probably a bigger tangent and
doesn't need to be part of this patch.

Thanks,
--Jacob

[1] 
https://www.openssl.org/blog/blog/2022/10/21/tls-groups-configuration/index.html

Re:Re: Re: Add support to TLS 1.3 cipher suites and curves lists

[Erica Zhang]

Hi Michael and Peter,
Thanks a lot for the elaboration of the patch process for PG17.  It's 
really unfortunate missing the the development cycle of PG17.
Just some context of why we hurry to try to catch up with PG17. 



There are certain government, financial and other enterprise organizations that 
have very strict requirements about the encrypted communication and more 
specifically about fine grained params like the TLS ciphers and curves that 
they use. The default ones for those customers are not acceptable. Any products 
that integrate Postgres and requires encrypted communication with the Postgres 
would have to fulfil those requirements.


So if we can have this patch in the upcoming new major version, that means 
Postgres users who have similar requirements can upgrade to PG17.


Thanks!



   
Original Email
   
 

Sender:"Michael Paquier"< mich...@paquier.xyz >;

Sent Time:2024/6/7 18:46

To:"Erica Zhang"< ericazhangy2...@qq.com >;

Cc recipient:"Peter Eisentraut"< pe...@eisentraut.org >;"pgsql-hackers"< 
pgsql-hackers@lists.postgresql.org >;

Subject:Re: Re: Add support to TLS 1.3 cipher suites and curves lists


On Fri, Jun 07, 2024 at 06:02:37PM +0800, Erica Zhang wrote:
> I see the https://commitfest.postgresql.org/48/ is still open, could
> it be possible to target for PG17? As I know PG17 is going to be
> release this year so that we can upgrade our instances to this new
> version accodingly.

Echoing with Peter, https://commitfest.postgresql.org/48/ is planned
to be the first commit fest of the development cycle for Postgres 18.
v17 is in feature freeze state and beta, where only bug fixes are
accepted, and not new features.
--
Michael

Re: Re: Re: Add support to TLS 1.3 cipher suites and curves lists

[Jelte Fennema-Nio]

On Wed, 12 Jun 2024 at 04:32, Erica Zhang  wrote:
> There are certain government, financial and other enterprise organizations 
> that have very strict requirements about the encrypted communication and more 
> specifically about fine grained params like the TLS ciphers and curves that 
> they use. The default ones for those customers are not acceptable. Any 
> products that integrate Postgres and requires encrypted communication with 
> the Postgres would have to fulfil those requirements.

Yeah, I ran into such requirements before too. So I do think it makes
sense to have such a feature in Postgres.

> So if we can have this patch in the upcoming new major version, that means 
> Postgres users who have similar requirements can upgrade to PG17.

As Daniel mentioned you can already achieve the same using the
"Ciphersuites" directive in openssl.conf. Also you could of course
always disable TLSv1.3 support.

Re:Re: Re: Re: Add support to TLS 1.3 cipher suites and curves lists

[Erica Zhang]

Hi Jelte and Daniel,


Based on my understanding currently there is no setting that controls the 
cipher choices used by TLS version 1.3 connections but the default 
value(HIGH:MEDIUM:+3DES:!aNULL) is used. So if I want to connect to Postgres 
(eg. Postgres 14) with different TLS versions of customized ciphers instead of 
default one like below:


eg. 

TLS1.2 of ciphers 
ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-GCM-SHA256:ECDHE-RSA-AES256-SHA:ECDHE-RSA-AES128-SHA:AES256-SHA:AES128-SHA

TLS1.3 of ciphers
TLS_AES_256_GCM_SHA384:TLS_AES_128_GCM_SHA256

For TLS1.2 connection, we can set the configuration in postgresql.conf as:
ssl_ciphers = 
'ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-GCM-SHA256:ECDHE-RSA-AES256-SHA:ECDHE-RSA-AES128-SHA:AES256-SHA:AES128-SHA'
How can I achieve the value for TLS1.3? Do you mean I can set the Ciphersuites 
in openssl.conf, then Postgres will pick up and use this value accordingly?

eg. I can run below command to set ciphersuites of TLS1.3 on my appliance:
openssl ciphers -ciphersuites TLS_AES_256_GCM_SHA384:TLS_AES_128_GCM_SHA256

then Postgres will use 'TLS_AES_256_GCM_SHA384:TLS_AES_128_GCM_SHA256" as 
ciphers for TLS1.3 connection?
Thanks,
Erica Zhang






   
Original Email
   
 

Sender:"Jelte Fennema-Nio"< postg...@jeltef.nl >;

Sent Time:2024/6/12 16:51

To:"Erica Zhang"< ericazhangy2...@qq.com >;

Cc recipient:"Michael Paquier"< mich...@paquier.xyz >;"Peter Eisentraut"< 
pe...@eisentraut.org >;"pgsql-hackers"< pgsql-hackers@lists.postgresql.org 
>;

Subject:Re: Re: Re: Add support to TLS 1.3 cipher suites and curves lists


On Wed, 12 Jun 2024 at 04:32, Erica Zhang  wrote:
> There are certain government, financial and other enterprise organizations 
that have very strict requirements about the encrypted communication and more 
specifically about fine grained params like the TLS ciphers and curves that 
they use. The default ones for those customers are not acceptable. Any products 
that integrate Postgres and requires encrypted communication with the Postgres 
would have to fulfil those requirements.

Yeah, I ran into such requirements before too. So I do think it makes
sense to have such a feature in Postgres.

> So if we can have this patch in the upcoming new major version, that means 
Postgres users who have similar requirements can upgrade to PG17.

As Daniel mentioned you can already achieve the same using the
"Ciphersuites" directive in openssl.conf. Also you could of course
always disable TLSv1.3 support.