Hi Jiewen,
1) {0x13, *} is defined in TLS1.3
Due to TlsLib in edk2 is not fully support TLS1.3, I think I need to remove 1.3 cipher suite to avoid confusion.
2) 3) Although it is not absolutely required, I highly recommend to add specific value to TLS_HASH_ALGO, to align with definition in RFC.
I think we can add the version value to the enum name to identify it is TLS1.2 specific.
> + Tls12HashAlgoSha512 = 6,
> +} TLS1_2_HASH_ALGO;
4) RFC4492 is obsoleted by RFC8422 -
https://datatracker.ietf.org/doc/html/rfc8422#section-5.1.1
Agree with it, should remove deprecated algo.
5) 6) "signature_algorithms" is changed in TLS 1.3 -
https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.3.
Similar to 1), if we align with tls1.2, we need to continue to use
Hash&Signature Pair instead of Scheme,
even if some algorithms are deprecated in tls1.3. I prefer to keep using this
Pair.
Since this is a backwards compatible extension field, we of course could use the
Signature Scheme to deprecate MD5 SHA1 and others,
but this would be a bit confusing due to the sudden appearance of a tls1.3 structure.
7) Last but not least, I hope to see how those new definition is used.
This commit show how we use Hash&Signature Pair to set SignatureAlgoList:
Consumer can call TlsSetSignatureAlgoList() with a Hash&Signature Pair array,
Then form a parameter list according to the name map TlsSignatureAlgoToName[] and TlsHashAlgoToName[],
Finally call the Openssl function like: SSL_set1_sigalgs_list(ssl,"DSA+SHA512:RSA+SHA512:ECDSA+SHA512 ");
The TLS code is in final testing, it would be very helpful if you could give some advice.
Thank you,
Yi
-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Wednesday, May 4, 2022 6:13 PM
To: devel@edk2.groups.io; Li, Yi1 <yi1.li@intel.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg: Add WPA3 related TLS configure macro
Thanks Yi.
Some feedback:
1) {0x13, *} is defined in TLS1.3 -
https://datatracker.ietf.org/doc/html/rfc8446#appendix-B.4
The comment "> /// TLS Cipher Suite, refers to A.5 of rfc-2246, rfc-4346 and rfc-5246." should be updated to include 8446 as well.
2) Although it is not absolutely required, I highly recommend to add specific value to TLS_HASH_ALGO, to align with definition in RFC.
> + TlsHashAlgoNone = 0,
> + TlsHashAlgoMd5 = 1,
> + TlsHashAlgoSha1 = 2,
> + TlsHashAlgoSha224 = 3,
> + TlsHashAlgoSha256 = 4,
> + TlsHashAlgoSha384 = 5,
> + TlsHashAlgoSha512 = 6,
> +} TLS_HASH_ALGO;
3) Ditto, for TLS_SIGNATURE_ALGO.
> + TlsSignatureAlgoAnonymous = 0,
> + TlsSignatureAlgoRsa = 1,
> + TlsSignatureAlgoDsa = 2,
> + TlsSignatureAlgoEcdsa = 3,
> +} TLS_SIGNATURE_ALGO;
The value is assigned in the spec. It cannot be changed.
4) RFC4492 is obsoleted by RFC8422 -
https://datatracker.ietf.org/doc/html/rfc8422#section-5.1.1
=================
RFC 4492 defined 25 different curves in the NamedCurve registry (now
renamed the "TLS Supported Groups" registry, although the enumeration
below is still named NamedCurve) for use in TLS. Only three have
seen much use. This specification is deprecating the rest (with
numbers 1-22).
=================
I don’t see a reason to define so many deprecated algorithms.
Would you please align with section 5.1.1 in RFC8422? You may consider to add x25519 and x448 as well.
============
enum {
deprecated(1..22),
secp256r1 (23), secp384r1 (24), secp521r1 (25),
x25519(29), x448(30),
reserved (0xFE00..0xFEFF),
deprecated(0xFF01..0xFF02),
(0xFFFF)
} NamedCurve;
============
5) Since you added TLS 1.3 cipher suit, I assume you also want to add definition for TLS 1.3.
Please aware that "signature_algorithms" is changed in TLS 1.3 -
https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.3.
I am not sure if you need define that as well.
============
enum {
/* RSASSA-PKCS1-v1_5 algorithms */
rsa_pkcs1_sha256(0x0401),
rsa_pkcs1_sha384(0x0501),
rsa_pkcs1_sha512(0x0601),
/* ECDSA algorithms */
ecdsa_secp256r1_sha256(0x0403),
ecdsa_secp384r1_sha384(0x0503),
ecdsa_secp521r1_sha512(0x0603),
/* RSASSA-PSS algorithms with public key OID rsaEncryption */
rsa_pss_rsae_sha256(0x0804),
rsa_pss_rsae_sha384(0x0805),
rsa_pss_rsae_sha512(0x0806),
/* EdDSA algorithms */
ed25519(0x0807),
ed448(0x0808),
/* RSASSA-PSS algorithms with public key OID RSASSA-PSS */
rsa_pss_pss_sha256(0x0809),
rsa_pss_pss_sha384(0x080a),
rsa_pss_pss_sha512(0x080b),
...
} SignatureScheme;
============
6) Ditto. Please aware that "NamedCurve" is changed in TLS 1.3 -
https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.7
I am not sure if you need define that as well.
============
enum {
/* Elliptic Curve Groups (ECDHE) */
secp256r1(0x0017), secp384r1(0x0018), secp521r1(0x0019),
x25519(0x001D), x448(0x001E),
...
} NamedGroup;
============
7) Last but not least, I hope to see how those new definition is used.
Without consumer, it is hard for me to understand why they are needed, or if we miss something else.
Thank you
Yao, Jiewen
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io>
On Behalf Of yi1 li
> Sent: Wednesday, May 4, 2022 5:31 PM
> To: devel@edk2.groups.io
> Cc: Li, Yi1 <yi1.li@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel] [PATCH 1/1] MdePkg: Add WPA3 related TLS
> configure macro
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3892
>
> Which are needed for SUITE-B and SUITE-B-192.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: yi1 li <yi1.li@intel.com>
> ---
> MdePkg/Include/IndustryStandard/Tls1.h | 133
> ++++++++++++++++++-------
> 1 file changed, 97 insertions(+), 36 deletions(-)
>
> diff --git a/MdePkg/Include/IndustryStandard/Tls1.h
> b/MdePkg/Include/IndustryStandard/Tls1.h
> index cf67428b1129..6519afe15e78 100644
> --- a/MdePkg/Include/IndustryStandard/Tls1.h
> +++ b/MdePkg/Include/IndustryStandard/Tls1.h
> @@ -15,42 +15,49 @@
> ///
> /// TLS Cipher Suite, refers to A.5 of rfc-2246, rfc-4346 and rfc-5246.
> ///
> -#define TLS_RSA_WITH_NULL_MD5 {0x00, 0x01}
> -#define TLS_RSA_WITH_NULL_SHA {0x00, 0x02}
> -#define TLS_RSA_WITH_RC4_128_MD5 {0x00, 0x04}
> -#define TLS_RSA_WITH_RC4_128_SHA {0x00, 0x05}
> -#define TLS_RSA_WITH_IDEA_CBC_SHA {0x00, 0x07}
> -#define TLS_RSA_WITH_DES_CBC_SHA {0x00, 0x09}
> -#define TLS_RSA_WITH_3DES_EDE_CBC_SHA {0x00, 0x0A}
> -#define TLS_DH_DSS_WITH_DES_CBC_SHA {0x00, 0x0C}
> -#define TLS_DH_DSS_WITH_3DES_EDE_CBC_SHA {0x00, 0x0D}
> -#define TLS_DH_RSA_WITH_DES_CBC_SHA {0x00, 0x0F}
> -#define TLS_DH_RSA_WITH_3DES_EDE_CBC_SHA {0x00, 0x10}
> -#define TLS_DHE_DSS_WITH_DES_CBC_SHA {0x00, 0x12}
> -#define TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA {0x00, 0x13}
> -#define TLS_DHE_RSA_WITH_DES_CBC_SHA {0x00, 0x15}
> -#define TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA {0x00, 0x16}
> -#define TLS_RSA_WITH_AES_128_CBC_SHA {0x00, 0x2F}
> -#define TLS_DH_DSS_WITH_AES_128_CBC_SHA {0x00, 0x30}
> -#define TLS_DH_RSA_WITH_AES_128_CBC_SHA {0x00, 0x31}
> -#define TLS_DHE_DSS_WITH_AES_128_CBC_SHA {0x00, 0x32}
> -#define TLS_DHE_RSA_WITH_AES_128_CBC_SHA {0x00, 0x33}
> -#define TLS_RSA_WITH_AES_256_CBC_SHA {0x00, 0x35}
> -#define TLS_DH_DSS_WITH_AES_256_CBC_SHA {0x00, 0x36}
> -#define TLS_DH_RSA_WITH_AES_256_CBC_SHA {0x00, 0x37}
> -#define TLS_DHE_DSS_WITH_AES_256_CBC_SHA {0x00, 0x38}
> -#define TLS_DHE_RSA_WITH_AES_256_CBC_SHA {0x00, 0x39}
> -#define TLS_RSA_WITH_NULL_SHA256 {0x00, 0x3B}
> -#define TLS_RSA_WITH_AES_128_CBC_SHA256 {0x00, 0x3C}
> -#define TLS_RSA_WITH_AES_256_CBC_SHA256 {0x00, 0x3D}
> -#define TLS_DH_DSS_WITH_AES_128_CBC_SHA256 {0x00, 0x3E}
> -#define TLS_DH_RSA_WITH_AES_128_CBC_SHA256 {0x00, 0x3F}
> -#define TLS_DHE_DSS_WITH_AES_128_CBC_SHA256 {0x00, 0x40} -#define
> TLS_DHE_RSA_WITH_AES_128_CBC_SHA256 {0x00, 0x67}
> -#define TLS_DH_DSS_WITH_AES_256_CBC_SHA256 {0x00, 0x68}
> -#define TLS_DH_RSA_WITH_AES_256_CBC_SHA256 {0x00, 0x69}
> -#define TLS_DHE_DSS_WITH_AES_256_CBC_SHA256 {0x00, 0x6A} -#define
> TLS_DHE_RSA_WITH_AES_256_CBC_SHA256 {0x00, 0x6B}
> +#define TLS_RSA_WITH_NULL_MD5 {0x00, 0x01}
> +#define TLS_RSA_WITH_NULL_SHA {0x00, 0x02}
> +#define TLS_RSA_WITH_RC4_128_MD5 {0x00, 0x04}
> +#define TLS_RSA_WITH_RC4_128_SHA {0x00, 0x05}
> +#define TLS_RSA_WITH_IDEA_CBC_SHA {0x00, 0x07}
> +#define TLS_RSA_WITH_DES_CBC_SHA {0x00, 0x09}
> +#define TLS_RSA_WITH_3DES_EDE_CBC_SHA {0x00, 0x0A}
> +#define TLS_DH_DSS_WITH_DES_CBC_SHA {0x00, 0x0C}
> +#define TLS_DH_DSS_WITH_3DES_EDE_CBC_SHA {0x00, 0x0D}
> +#define TLS_DH_RSA_WITH_DES_CBC_SHA {0x00, 0x0F}
> +#define TLS_DH_RSA_WITH_3DES_EDE_CBC_SHA {0x00, 0x10}
> +#define TLS_DHE_DSS_WITH_DES_CBC_SHA {0x00, 0x12}
> +#define TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA {0x00, 0x13}
> +#define TLS_DHE_RSA_WITH_DES_CBC_SHA {0x00, 0x15}
> +#define TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA {0x00, 0x16}
> +#define TLS_RSA_WITH_AES_128_CBC_SHA {0x00, 0x2F}
> +#define TLS_DH_DSS_WITH_AES_128_CBC_SHA {0x00, 0x30}
> +#define TLS_DH_RSA_WITH_AES_128_CBC_SHA {0x00, 0x31}
> +#define TLS_DHE_DSS_WITH_AES_128_CBC_SHA {0x00, 0x32}
> +#define TLS_DHE_RSA_WITH_AES_128_CBC_SHA {0x00, 0x33}
> +#define TLS_RSA_WITH_AES_256_CBC_SHA {0x00, 0x35}
> +#define TLS_DH_DSS_WITH_AES_256_CBC_SHA {0x00, 0x36}
> +#define TLS_DH_RSA_WITH_AES_256_CBC_SHA {0x00, 0x37}
> +#define TLS_DHE_DSS_WITH_AES_256_CBC_SHA {0x00, 0x38}
> +#define TLS_DHE_RSA_WITH_AES_256_CBC_SHA {0x00, 0x39}
> +#define TLS_RSA_WITH_NULL_SHA256 {0x00, 0x3B}
> +#define TLS_RSA_WITH_AES_128_CBC_SHA256 {0x00, 0x3C}
> +#define TLS_RSA_WITH_AES_256_CBC_SHA256 {0x00, 0x3D}
> +#define TLS_DH_DSS_WITH_AES_128_CBC_SHA256 {0x00, 0x3E}
> +#define TLS_DH_RSA_WITH_AES_128_CBC_SHA256 {0x00, 0x3F}
> +#define TLS_DHE_DSS_WITH_AES_128_CBC_SHA256 {0x00, 0x40}
> +#define TLS_DHE_RSA_WITH_AES_128_CBC_SHA256 {0x00, 0x67}
> +#define TLS_DH_DSS_WITH_AES_256_CBC_SHA256 {0x00, 0x68}
> +#define TLS_DH_RSA_WITH_AES_256_CBC_SHA256 {0x00, 0x69}
> +#define TLS_DHE_DSS_WITH_AES_256_CBC_SHA256 {0x00, 0x6A}
> +#define TLS_DHE_RSA_WITH_AES_256_CBC_SHA256 {0x00, 0x6B}
> +#define TLS_DHE_RSA_WITH_AES_256_GCM_SHA384 {0x00, 0x9F}
> +#define TLS_AES_128_GCM_SHA256 {0x13, 0x01}
> +#define TLS_AES_256_GCM_SHA384 {0x13, 0x02}
> +#define TLS_CHACHA20_POLY1305_SHA256 {0x13, 0x03}
> +#define TLS_ECDHE_ECDSA_AES128_GCM_SHA256 {0xC0, 0x2B}
> +#define TLS_ECDHE_ECDSA_AES256_GCM_SHA384 {0xC0, 0x2C}
> +#define TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 {0xC0, 0x30}
>
> ///
> /// TLS Version, refers to A.1 of rfc-2246, rfc-4346 and rfc-5246.
> @@ -95,6 +102,60 @@ typedef struct {
> //
> #define TLS_CIPHERTEXT_RECORD_MAX_PAYLOAD_LENGTH 18432
>
> +///
> +/// TLS Hash algorithm, refers to section 7.4.1.4.1. of rfc-5246.
> +///
> +typedef enum {
> + TlsHashAlgoNone = 0,
> + TlsHashAlgoMd5,
> + TlsHashAlgoSha1,
> + TlsHashAlgoSha224,
> + TlsHashAlgoSha256,
> + TlsHashAlgoSha384,
> + TlsHashAlgoSha512,
> +} TLS_HASH_ALGO;
> +
> +///
> +/// TLS Signature algorithm, refers to section 7.4.1.4.1. of rfc-5246.
> +///
> +typedef enum {
> + TlsSignatureAlgoAnonymous = 0,
> + TlsSignatureAlgoRsa,
> + TlsSignatureAlgoDsa,
> + TlsSignatureAlgoEcdsa,
> +} TLS_SIGNATURE_ALGO;
> +
> +///
> +/// TLS Supported Elliptic Curves Extensions, refers to section 5.1.1
> +of rfc-4492 /// typedef enum {
> + TlsEcNamedCurve_sect163k1 = 1,
> + TlsEcNamedCurve_sect163r1, // 2,
> + TlsEcNamedCurve_sect163r2, // 3,
> + TlsEcNamedCurve_sect193r1, // 4,
> + TlsEcNamedCurve_sect193r2, // 5,
> + TlsEcNamedCurve_sect233k1, // 6,
> + TlsEcNamedCurve_sect233r1, // 7,
> + TlsEcNamedCurve_sect239k1, // 8,
> + TlsEcNamedCurve_sect283k1, // 9,
> + TlsEcNamedCurve_sect283r1, // 10,
> + TlsEcNamedCurve_sect409k1, // 11,
> + TlsEcNamedCurve_sect409r1, // 12,
> + TlsEcNamedCurve_sect571k1, // 13,
> + TlsEcNamedCurve_sect571r1, // 14,
> + TlsEcNamedCurve_secp160k1, // 15,
> + TlsEcNamedCurve_secp160r1, // 16,
> + TlsEcNamedCurve_secp160r2, // 17,
> + TlsEcNamedCurve_secp192k1, // 18,
> + TlsEcNamedCurve_secp192r1, // 19,
> + TlsEcNamedCurve_secp224k1, // 20,
> + TlsEcNamedCurve_secp224r1, // 21,
> + TlsEcNamedCurve_secp256k1, // 22,
> + TlsEcNamedCurve_secp256r1, // 23,
> + TlsEcNamedCurve_secp384r1, // 24,
> + TlsEcNamedCurve_secp521r1, // 25,
> +} TLS_EC_NAMED_CUREVE;
> +
> #pragma pack()
>
> #endif
> --
> 2.31.1.windows.1
>
>
>
>
>