On 11/19/20 03:07, Liu, Zhiguang wrote:

Hi all,

I did some research about TLS v1.3 and here is my suggestion.

Related BZ:

https://bugzilla.tianocore.org/show_bug.cgi?id=2424

I want to first collect information if we should continue to support
TLS v1.1 or below.
I personally suggest stopping support TLS v1.1 or below, for this is a
trend.

The first suggestion is about TLS version configuration.
The supported TLS version should be a range not just a fixed value.
For example, a TLS version from 1.2 to 1.3 are both accepted.
However, there is not a direct UEFI API to set TLS version as a range.
SetSessionData method in EFI_TLS_PROTOCOL will set the maximum and
minimum TLS version as a same value.
I have two solutions about this issue and want to collect feedback.

Firstly, use a PCD to set the minimum TLS version in TlsDxe
drivers entry point. Like:
TlsCtxNew (PCD_Minimum_version_major, PCD_ Minimum_version_minor);
Then the accepted TLS version will be from PCD_Minimum_version to 1.3

Secondly, use loop to try different TLS versions, which can be
implemented in platform side. Like:
TLSVersionList = [1.3, 1.2, 1.1];
for (int i=0; i<len(TLSVersionList); i++) {
SetSessionData(EfiTlsVersion, TLSVersionList[i]);
// then, try to connect, if it works, break
}


The second comment is about how to set Cipher list.
We can now change the gEdkiiHttpTlsCipherListGuid variable to change
cipher list.
The same variable and API is used from TLS v1.0 to TLS v1.2
However, TLS v1.3 has a different API and totally different cipher
list parameters.
Below is the comparison:
SSL_set_cipher_list() sets the list of ciphers (TLSv1.2 and below)
The input cipher suite list must include at least one available cipher
suite for TLSv1.2 and below.
SSL_set_ciphersuites() is used to configure the available TLSv1.3
ciphersuites
The input cipher suite list must only include available cipher suite
for TLSv1.3.

I suggest introducing another table TlsV13CipherMappingTable to store
the cipher list suit for TLS v1.3.
Before calling SSL_set_ciphersuites(), use the mapping table to filter
the cipher list, only remain the available ones.
In this way, we can save all the cipher lists and cipher suites for
TLS 1.3 and TLS v1.2 or below in one same variable.
And before calling API to set cipher list, use the corresponding
mapping table to filter.

Also, for current TlsCipherMappingTable used for TLS v1.2 or below, I
think the below cipher list are all insecure for they use MD5 and SHA.
We should remove them.
MAP ( 0x0001, "NULL-MD5" ),
MAP ( 0x0002, "NULL-SHA" ),
MAP ( 0x0004, "RC4-MD5" ),
MAP ( 0x0005, "RC4-SHA" ),
MAP ( 0x000A, "DES-CBC3-SHA" ),
MAP ( 0x0016, "DHE-RSA-DES-CBC3-SHA" ),
MAP ( 0x002F, "AES128-SHA" ),
MAP ( 0x0030, "DH-DSS-AES128-SHA" ),
MAP ( 0x0031, "DH-RSA-AES128-SHA" ),
MAP ( 0x0033, "DHE-RSA-AES128-SHA" ),
MAP ( 0x0035, "AES256-SHA" ),
MAP ( 0x0036, "DH-DSS-AES256-SHA" ),
MAP ( 0x0037, "DH-RSA-AES256-SHA" ),
MAP ( 0x0039, "DHE-RSA-AES256-SHA" ),

And for TLS v1.3, I suggest we enable the following cipher suite.

1. TLS_AES_256_GCM_SHA384
2. TLS_AES_128_GCM_SHA256
3. TLS_AES_128_CCM_SHA256
4. TLS_CHACHA20_POLY1305_SHA256

Please correct me if I am wrong. Any comments are welcomed.

I'm not a crypto person, so please bear with me.

At Red Hat, we discussed TLSv1.3 in edk2 in the year 2018.

In particular, we discussed what UEFI (the spec) and edk2 (the reference
implementation) would need, for honoring what we call the "crypto
policy", with TLSv1.3.

After lots of discussions (which was basically a "learning from zero"
experience for me), I tried to summarize my then-understanding at:

https://bugzilla.redhat.com/show_bug.cgi?id=1559564#c17

Let me re-hash that here, in order to remove the OVMF-specific bits,
plus it's going to be easier for others to respond here in-line.

There are five aspects of TLSv1.3 that we would like to control. These
are:

(a) TLS protocol version.
(b) Acceptable ciphers.
(c) Acceptable groups (secp256r1/ffdhe etc).
(d) Acceptable signature algorithms (rsa-sha1, rsa-sha256, ...).
(e) Minimum acceptable DH parameters (for <tls1.2).

For each of these, it needs to be investigated whether the UEFI spec is
already flexible enough to express the particular configuration trait.
If so, then the question is whether edk2 implements that configuration
interface already.

(a) TLS protocol version:

- Currently, the UEFI spec allows for a single version setting.

- For accepting a version range (or set), spec changes don't look
necessary. The set of desired TLS version should be possible to
pass via the following interface:

- EFI_TLS_PROTOCOL.SetSessionData()

- DataType=EfiTlsExtensionData

- ExtensionType=43 ("supported_versions"):
https://tools.ietf.org/html/rfc8446#section-4.2.1

- Edk2 implementation: A PCD doesn't appear necessary. Edk2 does not
implement DataType=EfiTlsExtensionData though, at the moment
("Status = EFI_UNSUPPORTED").

Suggestion (in accordance with the spec interface):

#pragma pack (1)
typedef struct {
UINT16 ExtensionType; // this is where
// the protocol
// user would put
// value 43
UINT16 Length;
UINT8 Data[1]; // encode TLS
// ProtocolVersion
// (UINT16) values,
// likely as
// little-endian
} EFI_TLS_EXTENSION;
#pragma pack ()

Use of this interface would supersede / conflict with
EfiTlsVersion. In other words, using this facility after setting
EfiTlsVersion would fail, and vice versa. The return value could
be EFI_ALREADY_STARTED.

Multiple TCP connections should not be created, especially not
driven from the platform side -- OpenSSL should be pre-configured
with the acceptable version list, and negotiation should occur
internally to TLS, using just one TCP socket.


(b) Acceptable ciphers

- The UEFI spec allows for an array of cipher IDs (EFI_TLS_CIPHER),
which seems suitable wrt. TLSv1.3 as well.

- Edk2 implements the interface already.

- The edk2 implementation is not universally loved. There are (or have
been) mismatches between the set of ciphers we include in the
OpensslLib INF files and the "cipher suite filtering table" in the C
code. Also, some contributors either dislike the specific content of
the filter table, or even the fact that edk2 attempts to perform any
kind of filtering there -- if a protocol client requests a
particular cipher, there's an argument that the edk2 core should not
judge whether that's a reasonable request or not. Under this idea
the whole edk2-internal filtering should be removed (sane defaults
should not be removed though).


(c) Acceptable groups (secp256r1/ffdhe etc)

- Regarding the spec, this config property should map to the
following (no new interface needed):

- EFI_TLS_PROTOCOL.SetSessionData()

- DataType=EfiTlsExtensionData

- ExtensionType=10 ("supported_groups"):
https://tools.ietf.org/html/rfc8446#section-4.2.7

- Edk2 implementation: none currently; suggested:

#pragma pack (1)
typedef struct {
UINT16 ExtensionType; // this is where
// the protocol
// user would put
// value 10
UINT16 Length;
UINT8 Data[1]; // encode TLS
// NamedGroup (UINT16)
// values, likely as
// little-endian
} EFI_TLS_EXTENSION;
#pragma pack ()


(d) Acceptable signature algorithms (rsa-sha1, rsa-sha256, ...)

Same status as seen under (a) and (c), only with ExtensionType=13/50
("signature_algorithms" / "signature_algorithms_cert"):
https://tools.ietf.org/html/rfc8446#section-4.2.3

Suggested implementation (in accordance with the spec interface):

#pragma pack (1)
typedef struct {
UINT16 ExtensionType; // this is where
// the protocol
// user would put
// value 10 or 50
UINT16 Length;
UINT8 Data[1]; // encode TLS
// SignatureScheme
// (UINT16) values,
// likely as
// little-endian
} EFI_TLS_EXTENSION;
#pragma pack ()


(e) Minimum acceptable DH parameters (for <tls1.2)

- UEFI spec interface:

- key length is inexpressible

- permitted groups map to (c)

- no changes required because (at least from my notes from the
last discussion at RH) expressing the permitted groups through
(c) is sufficient

- Edk2 implementation: none. However, a TLS protocol client can get
around this limitation by forbidding such TLS cipher suites
altogether that use DH key exchange (so DH parameters won't
matter), via (b) / (c).


So, if I remember correctly our perspective at Red Hat,

- we'd need no spec changes,

- we'd need support for DataType=EfiTlsExtensionData in
EFI_TLS_PROTOCOL.SetSessionData(), wherein ExtensionType values 43,
10, and 13/50 should be propagated to OpenSSL, with their
corresponding UINT16 (ProtocolVersion / NamedGroup / SignatureScheme)
arrays encoded in EFI_TLS_EXTENSION.Length and EFI_TLS_EXTENSION.Data,

- the cipher suite filtering that's internal to edk2 remains dubious --
minimally the INF file contents (OpenSSL source file list) should be
synchronized with the filtering table(s).


... I'm fully aware this is not overly helpful, I'm just trying to
bridge the gap between our crypto experts (who haven't delved too much
into UEFI/edk2) and UEFI/edk2, with myself not being a crypto guy at
all.

Thanks
Laszlo