From a14b26d049a304528512ef2e60aea5ab27e1bc0f Mon Sep 17 00:00:00 2001 From: samuel40791765 Date: Tue, 30 Apr 2024 21:58:18 +0000 Subject: [PATCH 1/7] Update MySQL CI to 8.4 --- .../integration/mysql_patch/pending_size_t.patch | 16 ++++++++++++++++ .../mysql_patch/test_wl13075-off.patch | 14 +++++++------- tests/ci/integration/run_mysql_integration.sh | 11 +++++------ 3 files changed, 28 insertions(+), 13 deletions(-) create mode 100644 tests/ci/integration/mysql_patch/pending_size_t.patch diff --git a/tests/ci/integration/mysql_patch/pending_size_t.patch b/tests/ci/integration/mysql_patch/pending_size_t.patch new file mode 100644 index 0000000000..9fa07d2e3e --- /dev/null +++ b/tests/ci/integration/mysql_patch/pending_size_t.patch @@ -0,0 +1,16 @@ +diff --git a/router/src/openssl/include/tls/details/ssl_operation.h b/router/src/openssl/include/tls/details/ssl_operation.h +index 44b980d3e4..bdf79319f2 100644 +--- a/router/src/openssl/include/tls/details/ssl_operation.h ++++ b/router/src/openssl/include/tls/details/ssl_operation.h +@@ -91,7 +91,11 @@ class Operation { + + BIO *bio_; + SSL *ssl_; ++#if defined (OPENSSL_IS_AWSLC) ++ size_t pending_; ++#else + int pending_; ++#endif + }; + }; + diff --git a/tests/ci/integration/mysql_patch/test_wl13075-off.patch b/tests/ci/integration/mysql_patch/test_wl13075-off.patch index e99a44eb63..338e584fd2 100644 --- a/tests/ci/integration/mysql_patch/test_wl13075-off.patch +++ b/tests/ci/integration/mysql_patch/test_wl13075-off.patch @@ -1,8 +1,8 @@ diff --git a/testclients/mysql_client_test.cc b/testclients/mysql_client_test.cc -index f1e6744b..26021419 100644 +index 8bc55eda26..a37134221b 100644 --- a/testclients/mysql_client_test.cc +++ b/testclients/mysql_client_test.cc -@@ -23050,6 +23050,9 @@ static void test_bug32915973() { +@@ -22324,6 +22324,9 @@ static void test_bug32915973() { mysql_stmt_close(stmt); } @@ -12,21 +12,21 @@ index f1e6744b..26021419 100644 static void test_wl13075() { int rc; myheader("test_wl13075"); -@@ -23182,6 +23185,7 @@ static void test_wl13075() { +@@ -22456,6 +22459,7 @@ static void test_wl13075() { DIE_UNLESS(ret_ses_data == nullptr); } } +#endif - static void finish_with_error(MYSQL *con) { - fprintf(stderr, "[%i] %s\n", mysql_errno(con), mysql_error(con)); -@@ -23841,7 +23845,9 @@ static struct my_tests_st my_tests[] = { + static void test_bug33535746() { + DBUG_TRACE; +@@ -23770,7 +23774,9 @@ static struct my_tests_st my_tests[] = { {"test_bug32892045", test_bug32892045}, {"test_bug33164347", test_bug33164347}, {"test_bug32915973", test_bug32915973}, +#if !defined (OPENSSL_IS_AWSLC) {"test_wl13075", test_wl13075}, +#endif - {"test_bug34007830", test_bug34007830}, {"test_bug33535746", test_bug33535746}, {"test_server_telemetry_traces", test_server_telemetry_traces}, + {"test_wl13128", test_wl13128}, diff --git a/tests/ci/integration/run_mysql_integration.sh b/tests/ci/integration/run_mysql_integration.sh index 012083aee8..6b1e1753f0 100755 --- a/tests/ci/integration/run_mysql_integration.sh +++ b/tests/ci/integration/run_mysql_integration.sh @@ -6,7 +6,7 @@ set -exu source tests/ci/common_posix_setup.sh -MYSQL_VERSION_TAG="mysql-8.3.0" +MYSQL_VERSION_TAG="mysql-cluster-8.4.0" # This directory is specific to the docker image used. Use -DDOWNLOAD_BOOST=1 -DWITH_BOOST= # with mySQL to download a compatible boost version locally. BOOST_INSTALL_FOLDER=/home/dependencies/boost @@ -39,7 +39,7 @@ cd ${SCRATCH_FOLDER} function mysql_patch_reminder() { # Check latest MySQL version. MySQL often updates with large changes depending on OpenSSL all at once, so we pin to a specific version. - LATEST_MYSQL_VERSION_TAG=`git describe --tags --abbrev=0` + LATEST_MYSQL_VERSION_TAG=`git tag --sort=-taggerdate | head -n 1` if [[ "${LATEST_MYSQL_VERSION_TAG}" != "${MYSQL_VERSION_TAG}" ]]; then aws cloudwatch put-metric-data --namespace AWS-LC --metric-name MySQLVersionMismatch --value 1 else @@ -61,16 +61,15 @@ function mysql_run_tests() { # to testing AWS-LC functionality. # Tests marked with Bug#0001 use stateful session resumption, otherwise known as session caching. It is known that AWS-LC does not # currently support this with TLS 1.3. - echo "main.mysqlpump_bugs : Bug#0000 Can't create/open a file ~/dump.sql' -main.restart_server : Bug#0000 mysqld is not managed by supervisor process + echo "main.restart_server : Bug#0000 mysqld is not managed by supervisor process main.udf_bug35242734 : Bug#0000 mysqld is not managed by supervisor process main.file_contents : Bug#0000 Cannot open 'INFO_SRC' in '' main.resource_group_thr_prio_unsupported : Bug#0000 Invalid thread priority value -5 -main.dd_upgrade_error : Bug#0000 running mysqld as root main.dd_upgrade_error_cs : Bug#0000 running mysqld as root main.basedir : Bug#0000 running mysqld as root main.lowercase_fs_off : Bug#0000 running mysqld as root main.upgrade : Bug#0000 running mysqld as root +main.partition_prefixkey_upgrade : Bug#0000 running mysqld as root main.mysqld_cmdline_warnings : Bug#0000 running mysqld as root main.mysqld_daemon : Bug#0000 failed, error: 256, status: 1, errno: 2. main.mysqld_safe : Bug#0000 nonexistent: No such file or directory @@ -83,7 +82,7 @@ main.client_ssl_data_print : Bug#0001 AWS-LC does not support Stateful session main.ssl_cache : Bug#0001 AWS-LC does not support Stateful session resumption (Session Caching). main.ssl_cache_tls13 : Bug#0001 AWS-LC does not support Stateful session resumption (Session Caching). "> skiplist - ./mtr --suite=main --force --parallel=auto --skip-test-list=${MYSQL_BUILD_FOLDER}/mysql-test/skiplist --retry-failure=3 --retry=3 --report-unstable-tests + ./mtr --suite=main --force --parallel=auto --skip-test-list=${MYSQL_BUILD_FOLDER}/mysql-test/skiplist --retry-failure=5 --retry=5 --report-unstable-tests --max-test-fail=30 popd } From 899880e32efff4bf4c20dc276cedaee8a4ccadde Mon Sep 17 00:00:00 2001 From: samuel40791765 Date: Wed, 1 May 2024 00:13:09 +0000 Subject: [PATCH 2/7] add support for BIO_read/write_ex --- crypto/bio/bio.c | 42 ++++++++++++++++++++++++++++++++++++++++++ crypto/bio/bio_test.cc | 35 ++++++++++++++++++++++++++++++++++- include/openssl/bio.h | 13 ++++++++++++- 3 files changed, 88 insertions(+), 2 deletions(-) diff --git a/crypto/bio/bio.c b/crypto/bio/bio.c index 0e3a0d737d..17f2afdea3 100644 --- a/crypto/bio/bio.c +++ b/crypto/bio/bio.c @@ -186,6 +186,26 @@ int BIO_read(BIO *bio, void *buf, int len) { return ret; } +int BIO_read_ex(BIO *bio, void *data, size_t data_len, size_t *read_bytes) { + if (bio == NULL || read_bytes == NULL) { + OPENSSL_PUT_ERROR(BIO, BIO_R_NULL_PARAMETER); + return 0; + } + + int read_len = (int)data_len; + if (data_len > INT_MAX) { + read_len = INT_MAX; + } + + int ret = BIO_read(bio, data, read_len); + *read_bytes = ret; + if (ret > 0) { + return 1; + } else { + return 0; + } +} + int BIO_gets(BIO *bio, char *buf, int len) { if (bio == NULL || bio->method == NULL || bio->method->bgets == NULL) { OPENSSL_PUT_ERROR(BIO, BIO_R_UNSUPPORTED_METHOD); @@ -236,6 +256,28 @@ int BIO_write(BIO *bio, const void *in, int inl) { return ret; } +int BIO_write_ex(BIO *bio, const void *data, size_t data_len, size_t *written_bytes) { + if (bio == NULL) { + OPENSSL_PUT_ERROR(BIO, BIO_R_NULL_PARAMETER); + return 0; + } + + int write_len = (int)data_len; + if (data_len > INT_MAX) { + write_len = INT_MAX; + } + + int ret = BIO_write(bio, data, write_len); + if (written_bytes != NULL) { + *written_bytes = ret; + } + if (ret > 0) { + return 1; + } else { + return 0; + } +} + int BIO_write_all(BIO *bio, const void *data, size_t len) { const uint8_t *data_u8 = data; while (len > 0) { diff --git a/crypto/bio/bio_test.cc b/crypto/bio/bio_test.cc index e76f3eb32f..479d66c53e 100644 --- a/crypto/bio/bio_test.cc +++ b/crypto/bio/bio_test.cc @@ -89,7 +89,7 @@ class OwnedSocket { }; struct SockaddrStorage { - SockaddrStorage() : storage() , len(sizeof(storage)) {} + SockaddrStorage() : storage(), len(sizeof(storage)) {} int family() const { return storage.ss_family; } @@ -1063,3 +1063,36 @@ TEST(BIOTest, InvokeConnectCallback) { } // namespace INSTANTIATE_TEST_SUITE_P(All, BIOPairTest, testing::Values(false, true)); + +TEST(BIOTest, ReadWriteEx) { + bssl::UniquePtr bio(BIO_new(BIO_s_mem())); + ASSERT_TRUE(bio); + + size_t written = 0; + ASSERT_TRUE(BIO_write_ex(bio.get(), "abcdef", 6, &written)); + EXPECT_EQ(written, (size_t)6); + + char buf[32]; + size_t read = 0; + ASSERT_TRUE(BIO_read_ex(bio.get(), buf, sizeof(buf), &read)); + EXPECT_GT(read, (size_t)0); + EXPECT_EQ(Bytes(buf, read), Bytes("abcdef")); + + // Test NULL |written_bytes| behavior works. + read = 0; + ASSERT_TRUE(BIO_write_ex(bio.get(), "ghilmnop", 8, nullptr)); + ASSERT_TRUE(BIO_read_ex(bio.get(), buf, sizeof(buf), &read)); + EXPECT_GT(read, (size_t)0); + EXPECT_EQ(Bytes(buf, read), Bytes("ghilmnop")); + + // Test NULL |read_bytes| behavior fails. + ASSERT_TRUE(BIO_write_ex(bio.get(), "ghilmnop", 8, nullptr)); + ASSERT_FALSE(BIO_read_ex(bio.get(), buf, sizeof(buf), nullptr)); + + // Test that |BIO_write/read_ex| align with their non-ex counterparts, when + // encountering NULL data. + EXPECT_FALSE(BIO_write(bio.get(), nullptr, 0)); + EXPECT_FALSE(BIO_write_ex(bio.get(), nullptr, 0, &written)); + EXPECT_FALSE(BIO_read(bio.get(), nullptr, 0)); + EXPECT_FALSE(BIO_read_ex(bio.get(), nullptr, 0, &read)); +} diff --git a/include/openssl/bio.h b/include/openssl/bio.h index 262707f541..6ac91faf4a 100644 --- a/include/openssl/bio.h +++ b/include/openssl/bio.h @@ -110,6 +110,11 @@ OPENSSL_EXPORT int BIO_up_ref(BIO *bio); // of bytes read, zero on EOF, or a negative number on error. OPENSSL_EXPORT int BIO_read(BIO *bio, void *data, int len); +// BIO_read_ex calls |BIO_read| and stores the number of bytes read in +// |read_bytes|. It returns one on success and zero otherwise. +OPENSSL_EXPORT int BIO_read_ex(BIO *bio, void *data, size_t data_len, + size_t *read_bytes); + // BIO_gets reads a line from |bio| and writes at most |size| bytes into |buf|. // It returns the number of bytes read or a negative number on error. This // function's output always includes a trailing NUL byte, so it will read at @@ -127,6 +132,12 @@ OPENSSL_EXPORT int BIO_gets(BIO *bio, char *buf, int size); // number of bytes written, or a negative number on error. OPENSSL_EXPORT int BIO_write(BIO *bio, const void *data, int len); +// BIO_write_ex calls |BIO_write| and stores the number of bytes written in +// |written_bytes|, unless |written_bytes| is NULL. It returns one on success +// and zero otherwise. +OPENSSL_EXPORT int BIO_write_ex(BIO *bio, const void *data, size_t data_len, + size_t *written_bytes); + // BIO_write_all writes |len| bytes from |data| to |bio|, looping as necessary. // It returns one if all bytes were successfully written and zero on error. OPENSSL_EXPORT int BIO_write_all(BIO *bio, const void *data, size_t len); @@ -880,7 +891,7 @@ OPENSSL_EXPORT int (*BIO_meth_get_puts(const BIO_METHOD *method)) (BIO *, const // does not support secure heaps. OPENSSL_EXPORT OPENSSL_DEPRECATED const BIO_METHOD *BIO_s_secmem(void); - + // General No-op Functions [Deprecated]. // BIO_set_write_buffer_size returns zero. From 7ae79c0d655917d1a5ff767cc63b6ad711064df8 Mon Sep 17 00:00:00 2001 From: samuel40791765 Date: Wed, 1 May 2024 21:08:22 +0000 Subject: [PATCH 3/7] update read/written bytes only on success --- crypto/bio/bio.c | 12 ++++++++---- crypto/bio/bio_test.cc | 4 ++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/crypto/bio/bio.c b/crypto/bio/bio.c index 17f2afdea3..fc6178d61d 100644 --- a/crypto/bio/bio.c +++ b/crypto/bio/bio.c @@ -198,10 +198,11 @@ int BIO_read_ex(BIO *bio, void *data, size_t data_len, size_t *read_bytes) { } int ret = BIO_read(bio, data, read_len); - *read_bytes = ret; if (ret > 0) { + *read_bytes = ret; return 1; } else { + *read_bytes = 0; return 0; } } @@ -268,12 +269,15 @@ int BIO_write_ex(BIO *bio, const void *data, size_t data_len, size_t *written_by } int ret = BIO_write(bio, data, write_len); - if (written_bytes != NULL) { - *written_bytes = ret; - } if (ret > 0) { + if (written_bytes != NULL) { + *written_bytes = ret; + } return 1; } else { + if (written_bytes != NULL) { + *written_bytes = 0; + } return 0; } } diff --git a/crypto/bio/bio_test.cc b/crypto/bio/bio_test.cc index 479d66c53e..578fedc059 100644 --- a/crypto/bio/bio_test.cc +++ b/crypto/bio/bio_test.cc @@ -1091,8 +1091,12 @@ TEST(BIOTest, ReadWriteEx) { // Test that |BIO_write/read_ex| align with their non-ex counterparts, when // encountering NULL data. + written = 1; EXPECT_FALSE(BIO_write(bio.get(), nullptr, 0)); EXPECT_FALSE(BIO_write_ex(bio.get(), nullptr, 0, &written)); + EXPECT_EQ(written, (size_t)0); + read = 1; EXPECT_FALSE(BIO_read(bio.get(), nullptr, 0)); EXPECT_FALSE(BIO_read_ex(bio.get(), nullptr, 0, &read)); + EXPECT_EQ(read, (size_t)0); } From 6296b00878a00fbdf8e8a98a9ead3f90bf2d5f60 Mon Sep 17 00:00:00 2001 From: samuel40791765 Date: Thu, 2 May 2024 20:34:12 +0000 Subject: [PATCH 4/7] use nonroot for mysql; only build for arm; --- .../codebuild/github_ci_integration_omnibus.yaml | 4 ++-- tests/ci/integration/run_mysql_integration.sh | 13 ++++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/tests/ci/cdk/cdk/codebuild/github_ci_integration_omnibus.yaml b/tests/ci/cdk/cdk/codebuild/github_ci_integration_omnibus.yaml index ce8bbc583b..075c1eb183 100644 --- a/tests/ci/cdk/cdk/codebuild/github_ci_integration_omnibus.yaml +++ b/tests/ci/cdk/cdk/codebuild/github_ci_integration_omnibus.yaml @@ -102,7 +102,7 @@ batch: # MySQL build is bloated without any obvious build configurations we can use to speed up the build, so we use a larger instance here. - identifier: mysql_integration_x86_64 - buildspec: tests/ci/codebuild/common/run_simple_target.yml + buildspec: tests/ci/codebuild/common/run_nonroot_target.yml env: type: LINUX_CONTAINER privileged-mode: false @@ -112,7 +112,7 @@ batch: AWS_LC_CI_TARGET: "tests/ci/integration/run_mysql_integration.sh" - identifier: mysql_integration_aarch - buildspec: tests/ci/codebuild/common/run_simple_target.yml + buildspec: tests/ci/codebuild/common/run_nonroot_target.yml env: type: ARM_CONTAINER privileged-mode: false diff --git a/tests/ci/integration/run_mysql_integration.sh b/tests/ci/integration/run_mysql_integration.sh index 6b1e1753f0..bd5f9b7dd5 100755 --- a/tests/ci/integration/run_mysql_integration.sh +++ b/tests/ci/integration/run_mysql_integration.sh @@ -65,12 +65,6 @@ function mysql_run_tests() { main.udf_bug35242734 : Bug#0000 mysqld is not managed by supervisor process main.file_contents : Bug#0000 Cannot open 'INFO_SRC' in '' main.resource_group_thr_prio_unsupported : Bug#0000 Invalid thread priority value -5 -main.dd_upgrade_error_cs : Bug#0000 running mysqld as root -main.basedir : Bug#0000 running mysqld as root -main.lowercase_fs_off : Bug#0000 running mysqld as root -main.upgrade : Bug#0000 running mysqld as root -main.partition_prefixkey_upgrade : Bug#0000 running mysqld as root -main.mysqld_cmdline_warnings : Bug#0000 running mysqld as root main.mysqld_daemon : Bug#0000 failed, error: 256, status: 1, errno: 2. main.mysqld_safe : Bug#0000 nonexistent: No such file or directory main.grant_user_lock : Bug#0000 Access denied for user root at localhost @@ -127,7 +121,12 @@ mysql_patch_tests mysql_patch_error_strings mysql_build -mysql_run_tests +if [ $(uname -p) != "aarch64" ]; then + # MySQL's tests use extensive resources. They are slow on ARM and flaky race conditions occur. + # TODO: Enable ARM testing when Codebuild releases a larger ARM type (Current Type: 16vCPU, 32GB). + mysql_run_tests +fi + popd ldd "${MYSQL_BUILD_FOLDER}/lib/libmysqlclient.so" | grep "${AWS_LC_INSTALL_FOLDER}/lib/libcrypto.so" || exit 1 From 1762e41f6947cd37d7105c14904a09e981e142f4 Mon Sep 17 00:00:00 2001 From: samuel40791765 Date: Fri, 3 May 2024 19:28:52 +0000 Subject: [PATCH 5/7] PR comments; check equality --- crypto/bio/bio_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto/bio/bio_test.cc b/crypto/bio/bio_test.cc index 578fedc059..16d4509e56 100644 --- a/crypto/bio/bio_test.cc +++ b/crypto/bio/bio_test.cc @@ -1075,14 +1075,14 @@ TEST(BIOTest, ReadWriteEx) { char buf[32]; size_t read = 0; ASSERT_TRUE(BIO_read_ex(bio.get(), buf, sizeof(buf), &read)); - EXPECT_GT(read, (size_t)0); + EXPECT_EQ(read, (size_t)6); EXPECT_EQ(Bytes(buf, read), Bytes("abcdef")); // Test NULL |written_bytes| behavior works. read = 0; ASSERT_TRUE(BIO_write_ex(bio.get(), "ghilmnop", 8, nullptr)); ASSERT_TRUE(BIO_read_ex(bio.get(), buf, sizeof(buf), &read)); - EXPECT_GT(read, (size_t)0); + EXPECT_EQ(read, (size_t)8); EXPECT_EQ(Bytes(buf, read), Bytes("ghilmnop")); // Test NULL |read_bytes| behavior fails. From 7c7f0f818a45eb915afae2e04d45f187d12a5420 Mon Sep 17 00:00:00 2001 From: samuel40791765 Date: Fri, 3 May 2024 20:50:54 +0000 Subject: [PATCH 6/7] PR comments; return success on EOF for BIO_read_ex --- crypto/bio/bio.c | 2 +- crypto/bio/bio_test.cc | 24 ++++++++++++++---------- include/openssl/bio.h | 6 ++++++ 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/crypto/bio/bio.c b/crypto/bio/bio.c index fc6178d61d..9bf2dc1aee 100644 --- a/crypto/bio/bio.c +++ b/crypto/bio/bio.c @@ -198,7 +198,7 @@ int BIO_read_ex(BIO *bio, void *data, size_t data_len, size_t *read_bytes) { } int ret = BIO_read(bio, data, read_len); - if (ret > 0) { + if (ret >= 0) { *read_bytes = ret; return 1; } else { diff --git a/crypto/bio/bio_test.cc b/crypto/bio/bio_test.cc index 16d4509e56..eb51f37a0c 100644 --- a/crypto/bio/bio_test.cc +++ b/crypto/bio/bio_test.cc @@ -1068,18 +1068,23 @@ TEST(BIOTest, ReadWriteEx) { bssl::UniquePtr bio(BIO_new(BIO_s_mem())); ASSERT_TRUE(bio); - size_t written = 0; + // Reading from an initially empty bio should default to returning a error. + // Check that both |BIO_read| and |BIO_read_ex| fail. + char buf[32]; + size_t read = 1; + EXPECT_EQ(BIO_read(bio.get(), buf, sizeof(buf)), -1); + EXPECT_FALSE(BIO_read_ex(bio.get(), buf, sizeof(buf), &read)); + EXPECT_EQ(read, (size_t)0); + + // Write and read normally from buffer. + size_t written = 1; ASSERT_TRUE(BIO_write_ex(bio.get(), "abcdef", 6, &written)); EXPECT_EQ(written, (size_t)6); - - char buf[32]; - size_t read = 0; ASSERT_TRUE(BIO_read_ex(bio.get(), buf, sizeof(buf), &read)); EXPECT_EQ(read, (size_t)6); EXPECT_EQ(Bytes(buf, read), Bytes("abcdef")); // Test NULL |written_bytes| behavior works. - read = 0; ASSERT_TRUE(BIO_write_ex(bio.get(), "ghilmnop", 8, nullptr)); ASSERT_TRUE(BIO_read_ex(bio.get(), buf, sizeof(buf), &read)); EXPECT_EQ(read, (size_t)8); @@ -1090,13 +1095,12 @@ TEST(BIOTest, ReadWriteEx) { ASSERT_FALSE(BIO_read_ex(bio.get(), buf, sizeof(buf), nullptr)); // Test that |BIO_write/read_ex| align with their non-ex counterparts, when - // encountering NULL data. - written = 1; + // encountering NULL data. EOF in |BIO_read| is indicated by returning 0. + // In AWS-LC's |BIO_read_ex|, this should return success and set |read| to 0. EXPECT_FALSE(BIO_write(bio.get(), nullptr, 0)); EXPECT_FALSE(BIO_write_ex(bio.get(), nullptr, 0, &written)); EXPECT_EQ(written, (size_t)0); - read = 1; - EXPECT_FALSE(BIO_read(bio.get(), nullptr, 0)); - EXPECT_FALSE(BIO_read_ex(bio.get(), nullptr, 0, &read)); + EXPECT_EQ(BIO_read(bio.get(), nullptr, 0), 0); + ASSERT_TRUE(BIO_read_ex(bio.get(), nullptr, 0, &read)); EXPECT_EQ(read, (size_t)0); } diff --git a/include/openssl/bio.h b/include/openssl/bio.h index 6ac91faf4a..a2b63dcb9b 100644 --- a/include/openssl/bio.h +++ b/include/openssl/bio.h @@ -112,6 +112,12 @@ OPENSSL_EXPORT int BIO_read(BIO *bio, void *data, int len); // BIO_read_ex calls |BIO_read| and stores the number of bytes read in // |read_bytes|. It returns one on success and zero otherwise. +// +// Note: OpenSSL's |BIO_read_ex| returns zero on EOF. This disallows any +// mechanism to notify the user that an EOF has occurred and renders this API +// unusable. See openssl/openssl#8208. +// |BIO_read_ex| will return one for success and set |read_bytes| to 0 on EOF in +// AWS-LC. OPENSSL_EXPORT int BIO_read_ex(BIO *bio, void *data, size_t data_len, size_t *read_bytes); From 3edd87c1dd32ee088913f8de3ab9f6579078700e Mon Sep 17 00:00:00 2001 From: samuel40791765 Date: Wed, 8 May 2024 18:45:33 +0000 Subject: [PATCH 7/7] revert success on EOF and align with OpenSSL --- crypto/bio/bio.c | 2 +- crypto/bio/bio_test.cc | 4 ++-- include/openssl/bio.h | 8 +++----- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/crypto/bio/bio.c b/crypto/bio/bio.c index 9bf2dc1aee..fc6178d61d 100644 --- a/crypto/bio/bio.c +++ b/crypto/bio/bio.c @@ -198,7 +198,7 @@ int BIO_read_ex(BIO *bio, void *data, size_t data_len, size_t *read_bytes) { } int ret = BIO_read(bio, data, read_len); - if (ret >= 0) { + if (ret > 0) { *read_bytes = ret; return 1; } else { diff --git a/crypto/bio/bio_test.cc b/crypto/bio/bio_test.cc index eb51f37a0c..661ef0d4b7 100644 --- a/crypto/bio/bio_test.cc +++ b/crypto/bio/bio_test.cc @@ -1096,11 +1096,11 @@ TEST(BIOTest, ReadWriteEx) { // Test that |BIO_write/read_ex| align with their non-ex counterparts, when // encountering NULL data. EOF in |BIO_read| is indicated by returning 0. - // In AWS-LC's |BIO_read_ex|, this should return success and set |read| to 0. + // In |BIO_read_ex| however, EOF returns a failure and sets |read| to 0. EXPECT_FALSE(BIO_write(bio.get(), nullptr, 0)); EXPECT_FALSE(BIO_write_ex(bio.get(), nullptr, 0, &written)); EXPECT_EQ(written, (size_t)0); EXPECT_EQ(BIO_read(bio.get(), nullptr, 0), 0); - ASSERT_TRUE(BIO_read_ex(bio.get(), nullptr, 0, &read)); + EXPECT_FALSE(BIO_read_ex(bio.get(), nullptr, 0, &read)); EXPECT_EQ(read, (size_t)0); } diff --git a/include/openssl/bio.h b/include/openssl/bio.h index a2b63dcb9b..8b6b192a0f 100644 --- a/include/openssl/bio.h +++ b/include/openssl/bio.h @@ -113,11 +113,9 @@ OPENSSL_EXPORT int BIO_read(BIO *bio, void *data, int len); // BIO_read_ex calls |BIO_read| and stores the number of bytes read in // |read_bytes|. It returns one on success and zero otherwise. // -// Note: OpenSSL's |BIO_read_ex| returns zero on EOF. This disallows any -// mechanism to notify the user that an EOF has occurred and renders this API -// unusable. See openssl/openssl#8208. -// |BIO_read_ex| will return one for success and set |read_bytes| to 0 on EOF in -// AWS-LC. +// WARNING: Don't use this, use |BIO_read| instead. |BIO_read_ex| returns zero +// on EOF, which disallows any mechanism to notify the user that an EOF has +// occurred and renders this API unusable. See openssl/openssl#8208. OPENSSL_EXPORT int BIO_read_ex(BIO *bio, void *data, size_t data_len, size_t *read_bytes);