From 4f1d7770908ad716bb12368380cd17629f6b1499 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 24 Jan 2024 12:40:35 +0100 Subject: [PATCH] BIO_BIO: BIO_{write|read} on a BIO pair should wrap around ring buffer - BIO_nread0 should return 0 when no data to read and -2 when not initialized --- src/bio.c | 69 ++++++++++++++++++++++++++++++++++++---------------- tests/api.c | 41 ++++++++++++++++++++++++++++--- tests/unit.h | 3 ++- 3 files changed, 87 insertions(+), 26 deletions(-) diff --git a/src/bio.c b/src/bio.c index 85de16dd5..2dab43e67 100644 --- a/src/bio.c +++ b/src/bio.c @@ -70,16 +70,31 @@ static int wolfSSL_BIO_BASE64_read(WOLFSSL_BIO* bio, void* buf, int len) */ static int wolfSSL_BIO_BIO_read(WOLFSSL_BIO* bio, void* buf, int len) { - int sz; + int sz1; + int sz2; char* pt; - sz = wolfSSL_BIO_nread(bio, &pt, len); + if (buf == NULL || len == 0) + return 0; - if (sz > 0) { - XMEMCPY(buf, pt, sz); + sz1 = wolfSSL_BIO_nread(bio, &pt, len); + if (sz1 > 0) { + XMEMCPY(buf, pt, sz1); + buf = (char*)buf + sz1; + len -= sz1; + if (len > 0) { + /* try again to see if maybe we wrapped around the ring buffer */ + sz2 = wolfSSL_BIO_nread(bio, &pt, len); + if (sz2 > 0) { + XMEMCPY(buf, pt, sz2); + sz1 += sz2; + } + } } + if (sz1 == 0) + sz1 = -1; - return sz; + return sz1; } #ifndef WOLFSSL_BIO_RESIZE_THRESHOLD @@ -470,7 +485,6 @@ static int wolfSSL_BIO_SSL_write(WOLFSSL_BIO* bio, const void* data, } #endif /* WOLFCRYPT_ONLY */ - /* Writes to a WOLFSSL_BIO_BIO type. * * returns the amount written on success @@ -478,27 +492,40 @@ static int wolfSSL_BIO_SSL_write(WOLFSSL_BIO* bio, const void* data, static int wolfSSL_BIO_BIO_write(WOLFSSL_BIO* bio, const void* data, int len) { - int sz; + int sz1; + int sz2; char* buf; WOLFSSL_ENTER("wolfSSL_BIO_BIO_write"); /* adding in sanity checks for static analysis tools */ - if (bio == NULL || data == NULL) { - return BAD_FUNC_ARG; - } + if (bio == NULL || data == NULL || len == 0) + return 0; - sz = wolfSSL_BIO_nwrite(bio, &buf, len); - - /* test space for write */ - if (sz <= 0) { + sz1 = wolfSSL_BIO_nwrite(bio, &buf, len); + if (sz1 == 0) { WOLFSSL_MSG("No room left to write"); - return sz; + return WOLFSSL_BIO_ERROR; + } + if (sz1 < 0) { + WOLFSSL_MSG("Error in wolfSSL_BIO_nwrite"); + return sz1; + } + XMEMCPY(buf, data, sz1); + data = (char*)data + sz1; + len -= sz1; + + if (len > 0) { + /* try again to see if maybe we wrapped around the ring buffer */ + sz2 = wolfSSL_BIO_nwrite(bio, &buf, len); + if (sz2 > 0) { + XMEMCPY(buf, data, sz2); + sz1 += sz2; + } } - XMEMCPY(buf, data, sz); - return sz; + return sz1; } @@ -907,7 +934,7 @@ int wolfSSL_BIO_gets(WOLFSSL_BIO* bio, char* buf, int sz) char* c; int cSz; cSz = wolfSSL_BIO_nread0(bio, &c); - if (cSz == 0) { + if (cSz <= 0) { ret = 0; /* Nothing to read */ buf[0] = '\0'; break; @@ -1297,7 +1324,7 @@ int wolfSSL_BIO_nread0(WOLFSSL_BIO *bio, char **buf) if (bio == NULL || buf == NULL) { WOLFSSL_MSG("NULL argument passed in"); - return 0; + return -2; } /* if paired read from pair */ @@ -1314,7 +1341,7 @@ int wolfSSL_BIO_nread0(WOLFSSL_BIO *bio, char **buf) } } - return 0; + return -2; } @@ -1343,7 +1370,7 @@ int wolfSSL_BIO_nread(WOLFSSL_BIO *bio, char **buf, int num) /* get amount able to read and set buffer pointer */ sz = wolfSSL_BIO_nread0(bio, buf); - if (sz == 0) { + if (sz < 0) { return WOLFSSL_BIO_ERROR; } diff --git a/tests/api.c b/tests/api.c index ae58ab659..348b0089a 100644 --- a/tests/api.c +++ b/tests/api.c @@ -36801,12 +36801,14 @@ static int test_wolfSSL_Tls12_Key_Logging_test(void) /* clean up keylog file */ ExpectTrue((fp = XFOPEN("./MyKeyLog.txt", "w")) != XBADFILE); if (fp != XBADFILE) { + XFFLUSH(fp); XFCLOSE(fp); fp = XBADFILE; } ExpectIntEQ(test_wolfSSL_client_server_nofail_memio(&client_cbf, &server_cbf, NULL), TEST_SUCCESS); + XSLEEP_MS(100); /* check if the keylog file exists */ @@ -36814,6 +36816,7 @@ static int test_wolfSSL_Tls12_Key_Logging_test(void) int found = 0; ExpectTrue((fp = XFOPEN("./MyKeyLog.txt", "r")) != XBADFILE); + XFFLUSH(fp); /* Just to make sure any buffers get flushed */ while (EXPECT_SUCCESS() && XFGETS(buff, (int)sizeof(buff), fp) != NULL) { if (0 == strncmp(buff,"CLIENT_RANDOM ", sizeof("CLIENT_RANDOM ")-1)) { @@ -39231,12 +39234,12 @@ static int test_wolfSSL_BIO(void) for (i = 0; i < 20; i++) { ExpectIntEQ((int)bufPt[i], i); } - ExpectIntEQ(BIO_nread(bio2, &bufPt, 1), WOLFSSL_BIO_ERROR); + ExpectIntEQ(BIO_nread(bio2, &bufPt, 1), 0); ExpectIntEQ(BIO_nread(bio1, &bufPt, (int)BIO_ctrl_pending(bio1)), 8); for (i = 0; i < 8; i++) { ExpectIntEQ((int)bufPt[i], i); } - ExpectIntEQ(BIO_nread(bio1, &bufPt, 1), WOLFSSL_BIO_ERROR); + ExpectIntEQ(BIO_nread(bio1, &bufPt, 1), 0); ExpectIntEQ(BIO_ctrl_reset_read_request(bio1), 1); /* new pair */ @@ -39245,7 +39248,7 @@ static int test_wolfSSL_BIO(void) bio2 = NULL; ExpectIntEQ(BIO_make_bio_pair(bio1, bio3), WOLFSSL_SUCCESS); ExpectIntEQ((int)BIO_ctrl_pending(bio3), 0); - ExpectIntEQ(BIO_nread(bio3, &bufPt, 10), WOLFSSL_BIO_ERROR); + ExpectIntEQ(BIO_nread(bio3, &bufPt, 10), 0); /* test wrap around... */ ExpectIntEQ(BIO_reset(bio1), 0); @@ -39293,7 +39296,7 @@ static int test_wolfSSL_BIO(void) /* test reset on data in bio1 write buffer */ ExpectIntEQ(BIO_reset(bio1), 0); ExpectIntEQ((int)BIO_ctrl_pending(bio3), 0); - ExpectIntEQ(BIO_nread(bio3, &bufPt, 3), WOLFSSL_BIO_ERROR); + ExpectIntEQ(BIO_nread(bio3, &bufPt, 3), 0); ExpectIntEQ(BIO_nwrite(bio1, &bufPt, 20), 20); ExpectIntEQ((int)BIO_ctrl(bio1, BIO_CTRL_INFO, 0, &p), 20); ExpectNotNull(p); @@ -39408,6 +39411,35 @@ static int test_wolfSSL_BIO(void) return EXPECT_RESULT(); } +static int test_wolfSSL_BIO_BIO_ring_read(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_ALL) + BIO* bio1 = NULL; + BIO* bio2 = NULL; + byte data[50]; + byte tmp[50]; + + XMEMSET(data, 42, sizeof(data)); + + + ExpectIntEQ(BIO_new_bio_pair(&bio1, sizeof(data), &bio2, sizeof(data)), + SSL_SUCCESS); + + ExpectIntEQ(BIO_write(bio1, data, 40), 40); + ExpectIntEQ(BIO_read(bio1, tmp, 20), -1); + ExpectIntEQ(BIO_read(bio2, tmp, 20), 20); + ExpectBufEQ(tmp, data, 20); + ExpectIntEQ(BIO_write(bio1, data, 20), 20); + ExpectIntEQ(BIO_read(bio2, tmp, 40), 40); + ExpectBufEQ(tmp, data, 40); + + BIO_free(bio1); + BIO_free(bio2); +#endif + return EXPECT_RESULT(); +} + #endif /* !NO_BIO */ @@ -69716,6 +69748,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_wolfSSL_PEM_file_RSAPrivateKey), #ifndef NO_BIO TEST_DECL(test_wolfSSL_BIO), + TEST_DECL(test_wolfSSL_BIO_BIO_ring_read), TEST_DECL(test_wolfSSL_PEM_read_bio), TEST_DECL(test_wolfSSL_PEM_bio_RSAKey), TEST_DECL(test_wolfSSL_PEM_bio_DSAKey), diff --git a/tests/unit.h b/tests/unit.h index 185fc22df..061e84d83 100644 --- a/tests/unit.h +++ b/tests/unit.h @@ -217,7 +217,8 @@ int _z = (int)(z); \ int _w = ((_x) && (_y)) ? XMEMCMP(_x, _y, _z) : -1; \ Expect(_w op 0, ("%s " #op " %s for %s", #x, #y, #z), \ - ("\"%p\" " #er " \"%p\" for \"%d\"", _x, _y, _z));\ + ("\"%p\" " #er " \"%p\" for \"%d\"", \ + (const void *)_x, (const void *)_y, _z)); \ } \ } while(0)