fix for BIO_get_mem_ptr() regression in 1.1.1b (#1691853)

This commit is contained in:
Tomas Mraz 2019-04-16 12:13:00 +02:00
parent 7a654fc69c
commit 1aaf4073e3
2 changed files with 331 additions and 1 deletions

View File

@ -0,0 +1,325 @@
diff --git a/crypto/bio/bss_mem.c b/crypto/bio/bss_mem.c
index 89c54b2d53..a7f2bfbae0 100644
--- a/crypto/bio/bss_mem.c
+++ b/crypto/bio/bss_mem.c
@@ -57,7 +57,12 @@ static const BIO_METHOD secmem_method = {
NULL, /* mem_callback_ctrl */
};
-/* BIO memory stores buffer and read pointer */
+/*
+ * BIO memory stores buffer and read pointer
+ * however the roles are different for read only BIOs.
+ * In that case the readp just stores the original state
+ * to be used for reset.
+ */
typedef struct bio_buf_mem_st {
struct buf_mem_st *buf; /* allocated buffer */
struct buf_mem_st *readp; /* read pointer */
@@ -192,11 +197,14 @@ static int mem_read(BIO *b, char *out, int outl)
BIO_BUF_MEM *bbm = (BIO_BUF_MEM *)b->ptr;
BUF_MEM *bm = bbm->readp;
+ if (b->flags & BIO_FLAGS_MEM_RDONLY)
+ bm = bbm->buf;
BIO_clear_retry_flags(b);
ret = (outl >= 0 && (size_t)outl > bm->length) ? (int)bm->length : outl;
if ((out != NULL) && (ret > 0)) {
memcpy(out, bm->data, ret);
bm->length -= ret;
+ bm->max -= ret;
bm->data += ret;
} else if (bm->length == 0) {
ret = b->num;
@@ -241,29 +249,36 @@ static long mem_ctrl(BIO *b, int cmd, long num, void *ptr)
BIO_BUF_MEM *bbm = (BIO_BUF_MEM *)b->ptr;
BUF_MEM *bm;
+ if (b->flags & BIO_FLAGS_MEM_RDONLY)
+ bm = bbm->buf;
+ else
+ bm = bbm->readp;
+
switch (cmd) {
case BIO_CTRL_RESET:
bm = bbm->buf;
if (bm->data != NULL) {
- /* For read only case reset to the start again */
- if ((b->flags & BIO_FLAGS_MEM_RDONLY) || (b->flags & BIO_FLAGS_NONCLEAR_RST)) {
- bm->length = bm->max;
+ if (!(b->flags & BIO_FLAGS_MEM_RDONLY)) {
+ if (b->flags & BIO_FLAGS_NONCLEAR_RST) {
+ bm->length = bm->max;
+ } else {
+ memset(bm->data, 0, bm->max);
+ bm->length = 0;
+ }
+ *bbm->readp = *bbm->buf;
} else {
- memset(bm->data, 0, bm->max);
- bm->length = 0;
+ /* For read only case just reset to the start again */
+ *bbm->buf = *bbm->readp;
}
- *bbm->readp = *bbm->buf;
}
break;
case BIO_CTRL_EOF:
- bm = bbm->readp;
ret = (long)(bm->length == 0);
break;
case BIO_C_SET_BUF_MEM_EOF_RETURN:
b->num = (int)num;
break;
case BIO_CTRL_INFO:
- bm = bbm->readp;
ret = (long)bm->length;
if (ptr != NULL) {
pptr = (char **)ptr;
@@ -278,8 +293,9 @@ static long mem_ctrl(BIO *b, int cmd, long num, void *ptr)
break;
case BIO_C_GET_BUF_MEM_PTR:
if (ptr != NULL) {
- mem_buf_sync(b);
- bm = bbm->readp;
+ if (!(b->flags & BIO_FLAGS_MEM_RDONLY))
+ mem_buf_sync(b);
+ bm = bbm->buf;
pptr = (char **)ptr;
*pptr = (char *)bm;
}
@@ -294,7 +310,6 @@ static long mem_ctrl(BIO *b, int cmd, long num, void *ptr)
ret = 0L;
break;
case BIO_CTRL_PENDING:
- bm = bbm->readp;
ret = (long)bm->length;
break;
case BIO_CTRL_DUP:
@@ -318,6 +333,8 @@ static int mem_gets(BIO *bp, char *buf, int size)
BIO_BUF_MEM *bbm = (BIO_BUF_MEM *)bp->ptr;
BUF_MEM *bm = bbm->readp;
+ if (bp->flags & BIO_FLAGS_MEM_RDONLY)
+ bm = bbm->buf;
BIO_clear_retry_flags(bp);
j = bm->length;
if ((size - 1) < j)
diff --git a/doc/man3/BIO_s_mem.pod b/doc/man3/BIO_s_mem.pod
index bd0824a080..6d9e747b25 100644
--- a/doc/man3/BIO_s_mem.pod
+++ b/doc/man3/BIO_s_mem.pod
@@ -88,6 +88,22 @@ a buffering BIO to the chain will speed up the process.
Calling BIO_set_mem_buf() on a BIO created with BIO_new_secmem() will
give undefined results, including perhaps a program crash.
+Switching the memory BIO from read write to read only is not supported and
+can give undefined results including a program crash. There are two notable
+exceptions to the rule. The first one is to assign a static memory buffer
+immediately after BIO creation and set the BIO as read only.
+
+The other supported sequence is to start with read write BIO then temporarily
+switch it to read only and call BIO_reset() on the read only BIO immediately
+before switching it back to read write. Before the BIO is freed it must be
+switched back to the read write mode.
+
+Calling BIO_get_mem_ptr() on read only BIO will return a BUF_MEM that
+contains only the remaining data to be read. If the close status of the
+BIO is set to BIO_NOCLOSE, before freeing the BUF_MEM the data pointer
+in it must be set to NULL as the data pointer does not point to an
+allocated memory.
+
=head1 BUGS
There should be an option to set the maximum size of a memory BIO.
diff --git a/test/bio_memleak_test.c b/test/bio_memleak_test.c
index 36680e30a8..fab5ce73cf 100644
--- a/test/bio_memleak_test.c
+++ b/test/bio_memleak_test.c
@@ -18,28 +18,170 @@ static int test_bio_memleak(void)
int ok = 0;
BIO *bio;
BUF_MEM bufmem;
- const char *str = "BIO test\n";
+ static const char str[] = "BIO test\n";
char buf[100];
bio = BIO_new(BIO_s_mem());
- if (bio == NULL)
+ if (!TEST_ptr(bio))
goto finish;
- bufmem.length = strlen(str) + 1;
+ bufmem.length = sizeof(str);
bufmem.data = (char *) str;
bufmem.max = bufmem.length;
BIO_set_mem_buf(bio, &bufmem, BIO_NOCLOSE);
BIO_set_flags(bio, BIO_FLAGS_MEM_RDONLY);
+ if (!TEST_int_eq(BIO_read(bio, buf, sizeof(buf)), sizeof(str)))
+ goto finish;
+ if (!TEST_mem_eq(buf, sizeof(str), str, sizeof(str)))
+ goto finish;
+ ok = 1;
- if (BIO_read(bio, buf, sizeof(buf)) <= 0)
- goto finish;
+finish:
+ BIO_free(bio);
+ return ok;
+}
- ok = strcmp(buf, str) == 0;
+static int test_bio_get_mem(void)
+{
+ int ok = 0;
+ BIO *bio = NULL;
+ BUF_MEM *bufmem = NULL;
+
+ bio = BIO_new(BIO_s_mem());
+ if (!TEST_ptr(bio))
+ goto finish;
+ if (!TEST_int_eq(BIO_puts(bio, "Hello World\n"), 12))
+ goto finish;
+ BIO_get_mem_ptr(bio, &bufmem);
+ if (!TEST_ptr(bufmem))
+ goto finish;
+ if (!TEST_int_gt(BIO_set_close(bio, BIO_NOCLOSE), 0))
+ goto finish;
+ BIO_free(bio);
+ bio = NULL;
+ if (!TEST_mem_eq(bufmem->data, bufmem->length, "Hello World\n", 12))
+ goto finish;
+ ok = 1;
finish:
BIO_free(bio);
+ BUF_MEM_free(bufmem);
return ok;
}
+static int test_bio_new_mem_buf(void)
+{
+ int ok = 0;
+ BIO *bio;
+ BUF_MEM *bufmem;
+ char data[16];
+
+ bio = BIO_new_mem_buf("Hello World\n", 12);
+ if (!TEST_ptr(bio))
+ goto finish;
+ if (!TEST_int_eq(BIO_read(bio, data, 5), 5))
+ goto finish;
+ if (!TEST_mem_eq(data, 5, "Hello", 5))
+ goto finish;
+ if (!TEST_int_gt(BIO_get_mem_ptr(bio, &bufmem), 0))
+ goto finish;
+ if (!TEST_int_lt(BIO_write(bio, "test", 4), 0))
+ goto finish;
+ if (!TEST_int_eq(BIO_read(bio, data, 16), 7))
+ goto finish;
+ if (!TEST_mem_eq(data, 7, " World\n", 7))
+ goto finish;
+ if (!TEST_int_gt(BIO_reset(bio), 0))
+ goto finish;
+ if (!TEST_int_eq(BIO_read(bio, data, 16), 12))
+ goto finish;
+ if (!TEST_mem_eq(data, 12, "Hello World\n", 12))
+ goto finish;
+ ok = 1;
+
+finish:
+ BIO_free(bio);
+ return ok;
+}
+
+static int test_bio_rdonly_mem_buf(void)
+{
+ int ok = 0;
+ BIO *bio, *bio2 = NULL;
+ BUF_MEM *bufmem;
+ char data[16];
+
+ bio = BIO_new_mem_buf("Hello World\n", 12);
+ if (!TEST_ptr(bio))
+ goto finish;
+ if (!TEST_int_eq(BIO_read(bio, data, 5), 5))
+ goto finish;
+ if (!TEST_mem_eq(data, 5, "Hello", 5))
+ goto finish;
+ if (!TEST_int_gt(BIO_get_mem_ptr(bio, &bufmem), 0))
+ goto finish;
+ (void)BIO_set_close(bio, BIO_NOCLOSE);
+
+ bio2 = BIO_new(BIO_s_mem());
+ if (!TEST_ptr(bio2))
+ goto finish;
+ BIO_set_mem_buf(bio2, bufmem, BIO_CLOSE);
+ BIO_set_flags(bio2, BIO_FLAGS_MEM_RDONLY);
+
+ if (!TEST_int_eq(BIO_read(bio2, data, 16), 7))
+ goto finish;
+ if (!TEST_mem_eq(data, 7, " World\n", 7))
+ goto finish;
+ if (!TEST_int_gt(BIO_reset(bio2), 0))
+ goto finish;
+ if (!TEST_int_eq(BIO_read(bio2, data, 16), 7))
+ goto finish;
+ if (!TEST_mem_eq(data, 7, " World\n", 7))
+ goto finish;
+ ok = 1;
+
+finish:
+ BIO_free(bio);
+ BIO_free(bio2);
+ return ok;
+}
+
+static int test_bio_rdwr_rdonly(void)
+{
+ int ok = 0;
+ BIO *bio = NULL;
+ char data[16];
+
+ bio = BIO_new(BIO_s_mem());
+ if (!TEST_ptr(bio))
+ goto finish;
+ if (!TEST_int_eq(BIO_puts(bio, "Hello World\n"), 12))
+ goto finish;
+
+ BIO_set_flags(bio, BIO_FLAGS_MEM_RDONLY);
+ if (!TEST_int_eq(BIO_read(bio, data, 16), 12))
+ goto finish;
+ if (!TEST_mem_eq(data, 12, "Hello World\n", 12))
+ goto finish;
+ if (!TEST_int_gt(BIO_reset(bio), 0))
+ goto finish;
+
+ BIO_clear_flags(bio, BIO_FLAGS_MEM_RDONLY);
+ if (!TEST_int_eq(BIO_puts(bio, "Hi!\n"), 4))
+ goto finish;
+ if (!TEST_int_eq(BIO_read(bio, data, 16), 16))
+ goto finish;
+
+ if (!TEST_mem_eq(data, 16, "Hello World\nHi!\n", 16))
+ goto finish;
+
+ ok = 1;
+
+finish:
+ BIO_free(bio);
+ return ok;
+}
+
+
int global_init(void)
{
CRYPTO_set_mem_debug(1);
@@ -50,5 +192,9 @@ int global_init(void)
int setup_tests(void)
{
ADD_TEST(test_bio_memleak);
+ ADD_TEST(test_bio_get_mem);
+ ADD_TEST(test_bio_new_mem_buf);
+ ADD_TEST(test_bio_rdonly_mem_buf);
+ ADD_TEST(test_bio_rdwr_rdonly);
return 1;
}

View File

@ -22,7 +22,7 @@
Summary: Utilities from the general purpose cryptography library with TLS implementation
Name: openssl
Version: 1.1.1b
Release: 4%{?dist}
Release: 5%{?dist}
Epoch: 1
# We have to remove certain patented algorithms from the openssl source
# tarball with the hobble-openssl script which is included below.
@ -62,6 +62,7 @@ Patch48: openssl-1.1.1-fips-post-rand.patch
Patch49: openssl-1.1.1-evp-kdf.patch
Patch50: openssl-1.1.1-ssh-kdf.patch
# Backported fixes including security fixes
Patch51: openssl-1.1.1-bio-mem-ptr.patch
License: OpenSSL
URL: http://www.openssl.org/
@ -158,6 +159,7 @@ cp %{SOURCE13} test/
%patch48 -p1 -b .fips-post-rand
%patch49 -p1 -b .evp-kdf
%patch50 -p1 -b .ssh-kdf
%patch51 -p1 -b .bio-mem-ptr
%build
@ -444,6 +446,9 @@ export LD_LIBRARY_PATH
%ldconfig_scriptlets libs
%changelog
* Tue Apr 16 2019 Tomáš Mráz <tmraz@redhat.com> 1.1.1b-5
- fix for BIO_get_mem_ptr() regression in 1.1.1b (#1691853)
* Wed Mar 27 2019 Tomáš Mráz <tmraz@redhat.com> 1.1.1b-4
- drop unused BuildRequires and Requires in the -devel subpackage