Merge pull request #7169 from julek-wolfssl/gh/7160

BIO_BIO: BIO_{write|read} on a BIO pair should wrap around ring buffer

Author: JacobBarthelmeh

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,35 +485,47 @@ 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
  */
 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;
-    }
-
-    sz = wolfSSL_BIO_nwrite(bio, &buf, len);
+    if (bio == NULL || data == NULL || len == 0)
+        return 0;
 
-    /* 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;
         }
 

tests/api.c

@@ -36801,19 +36801,22 @@ 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 */
 
     char  buff[300] = {0};
     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 */
 
 
@@ -69722,6 +69754,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),

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)