MIME-Version: 1.0
References: <tencent_F551A28C9CFA3CE8E278FD23E22F261EE607@qq.com>
In-Reply-To: <tencent_F551A28C9CFA3CE8E278FD23E22F261EE607@qq.com>
From: Jacob Champion <jacob.champion@enterprisedb.com>
Date: Fri, 7 Jun 2024 10:14:50 -0700
Message-ID: <CAOYmi+k2qaJio+JkfKC2J+Shx8Fym-ifERhFLCR6mz7UXKxLVw@mail.gmail.com>
Subject: Re: Re: Add support to TLS 1.3 cipher suites and curves lists
To: Erica Zhang <ericazhangy2021@qq.com>
Cc: Peter Eisentraut <peter@eisentraut.org>, 
	pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAOYmi%2Bk2qaJio%2BJkfKC2J%2BShx8Fym-ifERhFLCR6mz7UXKxLVw%40mail.gmail.com>
Precedence: bulk

On Fri, Jun 7, 2024 at 3:02=E2=80=AFAM Erica Zhang <ericazhangy2021@qq.com>=
 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 instea=
d 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/i=
ndex.html