From f0f286fd41387851ed1dd5cd7b15661a1270c63c Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Thu, 13 Dec 2018 15:55:51 -0700 Subject: [PATCH 1/3] check on mpint shared secret --- src/internal.c | 4 ++-- src/misc.c | 39 +++++++++++++++++++++++++++++++++++++++ wolfssh/misc.h | 1 + 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/internal.c b/src/internal.c index 8d896e4..25a220c 100644 --- a/src/internal.c +++ b/src/internal.c @@ -2266,7 +2266,7 @@ static int DoKexDhReply(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) word32 sigSz; word32 scratch; byte scratchLen[LENGTH_SZ]; - word32 kPad = 0; + byte kPad = 0; struct { byte useRsa; word32 keySz; @@ -2515,10 +2515,10 @@ static int DoKexDhReply(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) wc_ecc_free(&ssh->handshake->privKey.ecc); } } + CreateMpint(ssh->k, &ssh->kSz, &kPad); /* Hash in the shared secret K. */ if (ret == 0) { - kPad = (ssh->k[0] & 0x80) ? 1 : 0; c32toa(ssh->kSz + kPad, scratchLen); ret = wc_HashUpdate(&ssh->handshake->hash, ssh->handshake->hashId, scratchLen, LENGTH_SZ); diff --git a/src/misc.c b/src/misc.c index 2a336fe..75e55da 100644 --- a/src/misc.c +++ b/src/misc.c @@ -111,6 +111,45 @@ STATIC INLINE int ConstantCompare(const byte* a, const byte* b, } +/* create mpint type + * + * can decrease size of buf by 1 or more if leading bytes are 0's and not needed + * the input argument "sz" gets reset if that is the case. Buffer size is never + * increased. + * + * An example of this would be a buffer of 0053 changed to 53. + * If a padding value is needed then "pad" is set to 1 + * + */ +STATIC INLINE void CreateMpint(byte* buf, word32* sz, byte* pad) +{ + word32 i; + + if (buf == NULL || sz == NULL || pad == NULL) { + WLOG(WS_LOG_ERROR, "Internal argument error with CreateMpint"); + } + + /* check for leading 0's */ + for (i = 0; i < *sz; i++) { + if (buf[i] != 0x00) + break; + } + *pad = (buf[i] & 0x80) ? 1 : 0; + + /* if padding would be needed and have leading 0's already then do not add + * extra 0's */ + if (i > 0 && *pad == 1) { + i = i - 1; + *pad = 0; + } + + /* if i is still greater than 0 then the buffer needs shifted to remove + * leading 0's */ + if (i > 0) { + WMEMMOVE(buf, buf + i, *sz - i); + *sz = *sz - i; + } +} #undef STATIC diff --git a/wolfssh/misc.h b/wolfssh/misc.h index 491d639..01829e0 100644 --- a/wolfssh/misc.h +++ b/wolfssh/misc.h @@ -45,6 +45,7 @@ WOLFSSH_LOCAL void ato32(const byte*, word32*); WOLFSSH_LOCAL void c32toa(word32, byte*); WOLFSSH_LOCAL void ForceZero(const void*, word32); WOLFSSH_LOCAL int ConstantCompare(const byte*, const byte*, word32); +WOLFSSH_LOCAL void CreateMpint(byte*, wrod32*, byte*); #endif /* NO_INLINE */ From d34900b6ae584ff48a331f5f45fa6437117f263e Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Fri, 14 Dec 2018 14:01:20 -0700 Subject: [PATCH 2/3] refactor code and add more CreateMpint calls --- src/internal.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/internal.c b/src/internal.c index 25a220c..5703d7f 100644 --- a/src/internal.c +++ b/src/internal.c @@ -5324,10 +5324,10 @@ int SendKexDhReply(WOLFSSH* ssh) &sigKeyBlock.sk.rsa.nSz); if (ret == 0) { /* Add a pad byte if the mpint has the MSB set. */ - sigKeyBlock.sk.rsa.ePad = (sigKeyBlock.sk.rsa.e[0] & 0x80) ? - 1 : 0; - sigKeyBlock.sk.rsa.nPad = (sigKeyBlock.sk.rsa.n[0] & 0x80) ? - 1 : 0; + CreateMpint(sigKeyBlock.sk.rsa.e, &sigKeyBlock.sk.rsa.eSz, + &sigKeyBlock.sk.rsa.ePad); + CreateMpint(sigKeyBlock.sk.rsa.n, &sigKeyBlock.sk.rsa.nSz, + &sigKeyBlock.sk.rsa.nPad); sigKeyBlock.sz = (LENGTH_SZ * 3) + sigKeyBlock.nameSz + sigKeyBlock.sk.rsa.eSz + sigKeyBlock.sk.rsa.ePad + @@ -5496,8 +5496,7 @@ int SendKexDhReply(WOLFSSH* ssh) } /* Add a pad byte if the mpint has the MSB set. */ if (ret == 0) { - if (primeGroup[0] & 0x80) - primeGroupPad = 1; + CreateMpint((byte*)primeGroup, &primeGroupSz, &primeGroupPad); /* Hash in the length of the GEX prime group. */ c32toa(primeGroupSz + primeGroupPad, scratchLen); @@ -5521,8 +5520,7 @@ int SendKexDhReply(WOLFSSH* ssh) primeGroup, primeGroupSz); /* Add a pad byte if the mpint has the MSB set. */ if (ret == 0) { - if (generator[0] & 0x80) - generatorPad = 1; + CreateMpint((byte*)generator, &generatorSz, &generatorPad); /* Hash in the length of the GEX generator. */ c32toa(generatorSz + generatorPad, scratchLen); @@ -5614,7 +5612,7 @@ int SendKexDhReply(WOLFSSH* ssh) /* Hash in the server's DH f-value. */ if (ret == 0) { - fPad = (f[0] & 0x80) ? 1 : 0; + CreateMpint(f, &fSz, &fPad); c32toa(fSz + fPad, scratchLen); ret = wc_HashUpdate(&ssh->handshake->hash, ssh->handshake->hashId, scratchLen, LENGTH_SZ); @@ -5632,7 +5630,7 @@ int SendKexDhReply(WOLFSSH* ssh) /* Hash in the shared secret K. */ if (ret == 0) { - kPad = (ssh->k[0] & 0x80) ? 1 : 0; + CreateMpint(ssh->k, &ssh->kSz, &kPad); c32toa(ssh->kSz + kPad, scratchLen); ret = wc_HashUpdate(&ssh->handshake->hash, ssh->handshake->hashId, scratchLen, LENGTH_SZ); @@ -6101,8 +6099,8 @@ int SendKexDhInit(WOLFSSH* ssh) } if (ret == WS_SUCCESS) { - if (e[0] & 0x80) { - ePad = 1; + CreateMpint(e, &eSz, &ePad); + if (ePad == 1) { ssh->handshake->e[0] = 0; } WMEMCPY(ssh->handshake->e + ePad, e, eSz); From 5b917ef1c6a3336a552207ef4b0a01d877024d98 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Fri, 14 Dec 2018 14:54:38 -0700 Subject: [PATCH 3/3] add log.h include and fix typo --- src/misc.c | 1 + wolfssh/misc.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/misc.c b/src/misc.c index 75e55da..a1200f2 100644 --- a/src/misc.c +++ b/src/misc.c @@ -40,6 +40,7 @@ #include +#include #ifdef NO_INLINE diff --git a/wolfssh/misc.h b/wolfssh/misc.h index 01829e0..d79357d 100644 --- a/wolfssh/misc.h +++ b/wolfssh/misc.h @@ -45,7 +45,7 @@ WOLFSSH_LOCAL void ato32(const byte*, word32*); WOLFSSH_LOCAL void c32toa(word32, byte*); WOLFSSH_LOCAL void ForceZero(const void*, word32); WOLFSSH_LOCAL int ConstantCompare(const byte*, const byte*, word32); -WOLFSSH_LOCAL void CreateMpint(byte*, wrod32*, byte*); +WOLFSSH_LOCAL void CreateMpint(byte*, word32*, byte*); #endif /* NO_INLINE */