Skip to content

Commit

Permalink
Add support for BIO_read/write_ex; Update MySQL CI to 8.4; (#1568)
Browse files Browse the repository at this point in the history
MySQL 8.4 has been released, this updates our CI to run against the
latest version. This time we only need two missing APIs from OpenSSL.
* `BIO_read_ex`
* `BIO_write_ex`

### MySQL CI changes:
* Removed disabled tests were removed from mysql's test suite, so I
removed them from `mysql_run_tests`
* Original patch needed to be updated, new patch is needed for `int` to
`size_t` mismatch for `BIO_pending`. I considered switching our
`BIO_pending` to `int` instead, but our version has been using the
`size_t` signature for a decade now, so this doesn't seem easily
convertible.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.
  • Loading branch information
samuel40791765 authored May 8, 2024
1 parent 2dc5184 commit 8ae155b
Show file tree
Hide file tree
Showing 7 changed files with 139 additions and 23 deletions.
46 changes: 46 additions & 0 deletions crypto/bio/bio.c
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,27 @@ 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);
if (ret > 0) {
*read_bytes = ret;
return 1;
} else {
*read_bytes = 0;
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);
Expand Down Expand Up @@ -236,6 +257,31 @@ 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 (ret > 0) {
if (written_bytes != NULL) {
*written_bytes = ret;
}
return 1;
} else {
if (written_bytes != NULL) {
*written_bytes = 0;
}
return 0;
}
}

int BIO_write_all(BIO *bio, const void *data, size_t len) {
const uint8_t *data_u8 = data;
while (len > 0) {
Expand Down
43 changes: 42 additions & 1 deletion crypto/bio/bio_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down Expand Up @@ -1063,3 +1063,44 @@ TEST(BIOTest, InvokeConnectCallback) {
} // namespace

INSTANTIATE_TEST_SUITE_P(All, BIOPairTest, testing::Values(false, true));

TEST(BIOTest, ReadWriteEx) {
bssl::UniquePtr<BIO> bio(BIO_new(BIO_s_mem()));
ASSERT_TRUE(bio);

// 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);
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.
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);
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. EOF in |BIO_read| is indicated by returning 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);
EXPECT_FALSE(BIO_read_ex(bio.get(), nullptr, 0, &read));
EXPECT_EQ(read, (size_t)0);
}
17 changes: 16 additions & 1 deletion include/openssl/bio.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,15 @@ 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.
//
// 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);

// 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
Expand All @@ -127,6 +136,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);
Expand Down Expand Up @@ -880,7 +895,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.
Expand Down
4 changes: 2 additions & 2 deletions tests/ci/cdk/cdk/codebuild/github_ci_integration_omnibus.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
16 changes: 16 additions & 0 deletions tests/ci/integration/mysql_patch/pending_size_t.patch
Original file line number Diff line number Diff line change
@@ -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
};
};

14 changes: 7 additions & 7 deletions tests/ci/integration/mysql_patch/test_wl13075-off.patch
Original file line number Diff line number Diff line change
@@ -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);
}

Expand All @@ -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},
22 changes: 10 additions & 12 deletions tests/ci/integration/run_mysql_integration.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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=<directory>
# with mySQL to download a compatible boost version locally.
BOOST_INSTALL_FOLDER=/home/dependencies/boost
Expand Down Expand Up @@ -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
Expand All @@ -61,17 +61,10 @@ 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.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
Expand All @@ -83,7 +76,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
}

Expand Down Expand Up @@ -128,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
Expand Down

0 comments on commit 8ae155b

Please sign in to comment.