From 2b5c4ffa7f579ecb0703fc7663c34fa64cdfcc8c Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 13 Nov 2017 14:35:15 -0800 Subject: [PATCH 1/4] Enhancement to allow override of maximum sig/algos using new `WOLFSSL_MAX_SIGALGO` define (default is 32). --- src/internal.c | 6 +++--- wolfssl/internal.h | 9 +++++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/internal.c b/src/internal.c index 540376aa9..e71218679 100644 --- a/src/internal.c +++ b/src/internal.c @@ -22772,11 +22772,11 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, return BUFFER_ERROR; XMEMCPY(clSuites.hashSigAlgo, &input[i], - min(clSuites.hashSigAlgoSz, HELLO_EXT_SIGALGO_MAX)); + min(clSuites.hashSigAlgoSz, WOLFSSL_MAX_SIGALGO)); i += clSuites.hashSigAlgoSz; - if (clSuites.hashSigAlgoSz > HELLO_EXT_SIGALGO_MAX) - clSuites.hashSigAlgoSz = HELLO_EXT_SIGALGO_MAX; + if (clSuites.hashSigAlgoSz > WOLFSSL_MAX_SIGALGO) + clSuites.hashSigAlgoSz = WOLFSSL_MAX_SIGALGO; } #ifdef HAVE_EXTENDED_MASTER else if (extId == HELLO_EXT_EXTMS) diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 4dc29fe2c..cf40dce5e 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -1034,7 +1034,6 @@ enum Misc { HELLO_EXT_TYPE_SZ = 2, /* length of a hello extension type */ HELLO_EXT_SZ_SZ = 2, /* length of a hello extension size */ HELLO_EXT_SIGALGO_SZ = 2, /* length of number of items in sigalgo list */ - HELLO_EXT_SIGALGO_MAX = 32, /* number of items in the signature algo list */ DTLS_HANDSHAKE_HEADER_SZ = 12, /* normal + seq(2) + offset(3) + length(3) */ DTLS_RECORD_HEADER_SZ = 13, /* normal + epoch(2) + seq_num(6) */ @@ -1192,6 +1191,12 @@ enum Misc { /* 150 suites for now! */ #endif +/* number of items in the signature algo list */ +#ifndef WOLFSSL_MAX_SIGALGO + #define WOLFSSL_MAX_SIGALGO 32 +#endif + + /* set minimum ECC key size allowed */ #ifndef WOLFSSL_MIN_ECC_BITS #ifdef WOLFSSL_MAX_STRENGTH @@ -1527,7 +1532,7 @@ typedef struct Suites { word16 suiteSz; /* suite length in bytes */ word16 hashSigAlgoSz; /* SigAlgo extension length in bytes */ byte suites[WOLFSSL_MAX_SUITE_SZ]; - byte hashSigAlgo[HELLO_EXT_SIGALGO_MAX]; /* sig/algo to offer */ + byte hashSigAlgo[WOLFSSL_MAX_SIGALGO]; /* sig/algo to offer */ byte setSuites; /* user set suites from default */ byte hashAlgo; /* selected hash algorithm */ byte sigAlgo; /* selected sig algorithm */ From 9f7e40ad5c8097ff38d7caff4a9989db260981cc Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 13 Nov 2017 14:52:53 -0800 Subject: [PATCH 2/4] Fix to make sure provided sigalgo list doesn't overflow the buffer. --- src/tls.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/tls.c b/src/tls.c index e8495bd41..fe5f316e6 100644 --- a/src/tls.c +++ b/src/tls.c @@ -4906,8 +4906,11 @@ static int TLSX_SignatureAlgorithms_Parse(WOLFSSL *ssl, byte* input, if (length != OPAQUE16_LEN + len) return BUFFER_ERROR; - XMEMCPY(suites->hashSigAlgo, input, len); + /* truncate hashSigAlgo list if too long */ suites->hashSigAlgoSz = len; + if (suites->hashSigAlgoSz > WOLFSSL_MAX_SIGALGO) + suites->hashSigAlgoSz = WOLFSSL_MAX_SIGALGO; + XMEMCPY(suites->hashSigAlgo, input, suites->hashSigAlgoSz); return TLSX_SignatureAlgorithms_MapPss(ssl, input, len); } From b08a99057c70c111b58f21c203dc0bc14df5118c Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 13 Nov 2017 15:02:13 -0800 Subject: [PATCH 3/4] Cleanup of `hashSigAlgo` handling in `DoClientHello`. --- src/internal.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/internal.c b/src/internal.c index e71218679..427809d8a 100644 --- a/src/internal.c +++ b/src/internal.c @@ -13237,7 +13237,7 @@ int SendCertificateRequest(WOLFSSL* ssl) /* supported hash/sig */ if (IsAtLeastTLSv1_2(ssl)) { c16toa(ssl->suites->hashSigAlgoSz, &output[i]); - i += LENGTH_SZ; + i += OPAQUE16_LEN; XMEMCPY(&output[i], ssl->suites->hashSigAlgo, ssl->suites->hashSigAlgoSz); @@ -22765,18 +22765,22 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, return BUFFER_ERROR; if (extId == HELLO_EXT_SIG_ALGO) { - ato16(&input[i], &clSuites.hashSigAlgoSz); + word16 hashSigAlgoSz; + + ato16(&input[i], &hashSigAlgoSz); i += OPAQUE16_LEN; - if (OPAQUE16_LEN + clSuites.hashSigAlgoSz > extSz) + if (OPAQUE16_LEN + hashSigAlgoSz > extSz) return BUFFER_ERROR; - XMEMCPY(clSuites.hashSigAlgo, &input[i], - min(clSuites.hashSigAlgoSz, WOLFSSL_MAX_SIGALGO)); - i += clSuites.hashSigAlgoSz; - + clSuites.hashSigAlgoSz = hashSigAlgoSz; if (clSuites.hashSigAlgoSz > WOLFSSL_MAX_SIGALGO) clSuites.hashSigAlgoSz = WOLFSSL_MAX_SIGALGO; + + XMEMCPY(clSuites.hashSigAlgo, &input[i], + clSuites.hashSigAlgoSz); + + i += hashSigAlgoSz; } #ifdef HAVE_EXTENDED_MASTER else if (extId == HELLO_EXT_EXTMS) From 20f5c616755f43694eef8945e744489de23591bb Mon Sep 17 00:00:00 2001 From: David Garske Date: Tue, 14 Nov 2017 10:31:48 -0800 Subject: [PATCH 4/4] Added debug message when signature/algorithm list is truncated. --- src/internal.c | 5 ++++- src/tls.c | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/internal.c b/src/internal.c index 427809d8a..d62c24a56 100644 --- a/src/internal.c +++ b/src/internal.c @@ -22774,8 +22774,11 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, return BUFFER_ERROR; clSuites.hashSigAlgoSz = hashSigAlgoSz; - if (clSuites.hashSigAlgoSz > WOLFSSL_MAX_SIGALGO) + if (clSuites.hashSigAlgoSz > WOLFSSL_MAX_SIGALGO) { + WOLFSSL_MSG("ClientHello SigAlgo list exceeds max, " + "truncating"); clSuites.hashSigAlgoSz = WOLFSSL_MAX_SIGALGO; + } XMEMCPY(clSuites.hashSigAlgo, &input[i], clSuites.hashSigAlgoSz); diff --git a/src/tls.c b/src/tls.c index fe5f316e6..7c296b9bf 100644 --- a/src/tls.c +++ b/src/tls.c @@ -3771,7 +3771,7 @@ static int TLSX_SessionTicket_Parse(WOLFSSL* ssl, byte* input, word16 length, if (!isRequest) { if (TLSX_CheckUnsupportedExtension(ssl, TLSX_SESSION_TICKET)) return TLSX_HandleUnsupportedExtension(ssl); - + if (length != 0) return BUFFER_ERROR; @@ -4908,8 +4908,10 @@ static int TLSX_SignatureAlgorithms_Parse(WOLFSSL *ssl, byte* input, /* truncate hashSigAlgo list if too long */ suites->hashSigAlgoSz = len; - if (suites->hashSigAlgoSz > WOLFSSL_MAX_SIGALGO) + if (suites->hashSigAlgoSz > WOLFSSL_MAX_SIGALGO) { + WOLFSSL_MSG("TLSX SigAlgo list exceeds max, truncating"); suites->hashSigAlgoSz = WOLFSSL_MAX_SIGALGO; + } XMEMCPY(suites->hashSigAlgo, input, suites->hashSigAlgoSz); return TLSX_SignatureAlgorithms_MapPss(ssl, input, len);