From 35ca19e0c8ac3ddf0f43f97efe2bced3c0d7dba8 Mon Sep 17 00:00:00 2001 From: Dan Fandrich Date: Mon, 20 Feb 2023 14:15:22 -0800 Subject: [PATCH 1/2] Fix AES-GCM decryption of block spanning frames The authentication tag was erroneously checked too soon in this case. --- src/transport.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/transport.c b/src/transport.c index 441a7016c5..aff323ec47 100644 --- a/src/transport.c +++ b/src/transport.c @@ -150,13 +150,14 @@ decrypt(LIBSSH2_SESSION * session, unsigned char *source, int decryptlen = MIN(blocksize, len); /* The first block is special (since it needs to be decoded to get the length of the remainder of the block) and takes priority. When the - length finally gets to the last blocksize bytes, it's the end. */ + length finally gets to the last blocksize bytes, and there's no + more data to come, it's the end. */ int lowerfirstlast = IS_FIRST(firstlast) ? FIRST_BLOCK : - ((len <= blocksize) ? LAST_BLOCK : MIDDLE_BLOCK); + ((len <= blocksize) ? firstlast : MIDDLE_BLOCK); /* If the last block would be less than a whole blocksize, combine it with the previous block to make it larger. This ensures that the whole MAC is included in a single decrypt call. */ - if(CRYPT_FLAG_L(session, PKTLEN_AAD) && !IS_FIRST(firstlast) + if(CRYPT_FLAG_L(session, PKTLEN_AAD) && IS_LAST(firstlast) && (len < blocksize*2)) { decryptlen = len; lowerfirstlast = LAST_BLOCK; From 4ac055f3ce8fdb5debb34df1370f4e87f7d8a179 Mon Sep 17 00:00:00 2001 From: Dan Fandrich Date: Tue, 21 Feb 2023 10:55:58 -0800 Subject: [PATCH 2/2] Fix AES-GCM when the last few bytes aren't received --- src/openssl.c | 1 + src/transport.c | 14 +++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/openssl.c b/src/openssl.c index cec73c072c..925efa8564 100644 --- a/src/openssl.c +++ b/src/openssl.c @@ -492,6 +492,7 @@ _libssh2_cipher_crypt(_libssh2_cipher_ctx * ctx, (void) algo; assert(blocksize <= sizeof(buf)); + assert(cryptlen >= 0); /* First block */ if(IS_FIRST(firstlast)) { diff --git a/src/transport.c b/src/transport.c index aff323ec47..018405a6b5 100644 --- a/src/transport.c +++ b/src/transport.c @@ -51,6 +51,9 @@ #include "transport.h" #include "mac.h" +#ifndef MAX +#define MAX(x,y) ((x)>(y)?(x):(y)) +#endif #ifndef MIN #define MIN(x,y) ((x)<(y)?(x):(y)) #endif @@ -638,7 +641,8 @@ int _libssh2_transport_read(LIBSSH2_SESSION * session) amount to decrypt even more */ if((p->data_num + numbytes) >= (p->total_num - skip)) { /* decrypt the entire rest of the package */ - numdecrypt = (p->total_num - skip) - p->data_num; + numdecrypt = MAX(0, + (int)(p->total_num - skip) - (int)p->data_num); firstlast = LAST_BLOCK; } else { @@ -653,6 +657,13 @@ int _libssh2_transport_read(LIBSSH2_SESSION * session) after it */ numbytes = 0; } + if(CRYPT_FLAG_R(session, INTEGRATED_MAC)) { + /* Make sure that we save enough bytes to make the last + * block large enough to hold the entire integrated MAC */ + numdecrypt = MIN(numdecrypt, (int) + (p->total_num - skip - blocksize - p->data_num)); + numbytes = 0; + } firstlast = MIDDLE_BLOCK; } @@ -661,6 +672,7 @@ int _libssh2_transport_read(LIBSSH2_SESSION * session) /* unencrypted data should not be decrypted at all */ numdecrypt = 0; } + assert(numdecrypt >= 0); /* if there are bytes to decrypt, do that */ if(numdecrypt > 0) {