From b8fcd86fcf9740ab9f6dc0133b99a4a58e2de100 Mon Sep 17 00:00:00 2001 From: Vikash Prajapati Date: Fri, 15 Sep 2023 09:54:15 +0000 Subject: [PATCH 01/10] Adding Unit Tests For SSL Operations Signed-off-by: Vikash Prajapati --- .../src/backend/tds/tdssecure.c | 61 +++++- .../babelfishpg_tds/src/include/tds_secure.h | 6 + contrib/babelfishpg_unit/Makefile | 1 + contrib/babelfishpg_unit/babelfishpg_unit.c | 9 + contrib/babelfishpg_unit/babelfishpg_unit.h | 9 + contrib/babelfishpg_unit/test_ssl_read.c | 179 ++++++++++++++++++ contrib/babelfishpg_unit/test_ssl_write.c | 108 +++++++++++ 7 files changed, 368 insertions(+), 5 deletions(-) create mode 100644 contrib/babelfishpg_unit/test_ssl_read.c create mode 100644 contrib/babelfishpg_unit/test_ssl_write.c diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c b/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c index 6b3c0307f8..489907deee 100644 --- a/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c +++ b/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c @@ -40,6 +40,10 @@ int tds_ssl_min_protocol_version; int tds_ssl_max_protocol_version; + +static TdsSecureSocketApi tds_secure_raw_read = secure_raw_read; +static TdsSecureSocketApiConst tds_secure_raw_write = secure_raw_write; + #ifdef USE_SSL /* @@ -61,6 +65,8 @@ typedef union TDSPacketHeader { static int pkt_bytes_read = 0; static TDSPacketHeader pkt_data = {0}; +static bool unit_testing = false; + /* * SslRead - TDS secure read function, similar to my_sock_read */ @@ -71,7 +77,7 @@ SslRead(BIO * h, char *buf, int size) if (buf != NULL) { - res = secure_raw_read(((Port *) BIO_get_data(h)), buf, size); + res = tds_secure_raw_read(((Port *) BIO_get_data(h)), buf, size); BIO_clear_retry_flags(h); if (res <= 0) { @@ -126,7 +132,7 @@ SslHandShakeRead(BIO * h, char *buf, int size) } if (unlikely(pkt_data.header.pkt_type != TDS_PRELOGIN)) - ereport(FATAL, + ereport(unit_testing ? ERROR : FATAL, (errcode(ERRCODE_ADMIN_SHUTDOWN), errmsg("terminating connection due to unexpected ssl packet header"))); @@ -163,6 +169,29 @@ SslHandShakeRead(BIO * h, char *buf, int size) return res; } + +ssize_t +test_ssl_handshake_read(BIO * h, char *buf, int size, TdsSecureSocketApi mock_socket_read, int ReadPointer) +{ + + /* + * Accessing SslHandShakeRead() function directly from our testing functions created in the test_ssl_read.c file is not possible due to its static nature + * This intermediary function serves as a gateway, allowing us to invoke the SslHandShakeRead() function from our testing function + */ + + int res; + tds_secure_raw_read = mock_socket_read; + unit_testing = true; + pkt_bytes_read = ReadPointer; + + res = SslHandShakeRead(h, buf, size); + + unit_testing = false; + + return res; +} + + /* * SslWrite - Tds secure write function, similar to my_sock_write. */ @@ -171,7 +200,7 @@ SslWrite(BIO * h, const char *buf, int size) { int res = 0; - res = secure_raw_write(((Port *) BIO_get_data(h)), buf, size); + res = tds_secure_raw_write(((Port *) BIO_get_data(h)), buf, size); BIO_clear_retry_flags(h); if (res <= 0) { @@ -245,6 +274,28 @@ SslHandShakeWrite(BIO * h, const char *buf, int size) return (res - TDS_PACKET_HEADER_SIZE); } + +ssize_t +test_ssl_handshake_write(BIO * h, char *buf, int size, TdsSecureSocketApiConst mock_socket_write) +{ + + /* + * Accessing SslHandShakeWrite() function directly from our testing functions created in the test_ssl_write.c file is not possible due to its static nature + * This intermediary function serves as a gateway, allowing us to invoke the SslHandShakeWrite() function from our testing function + */ + + int res; + tds_secure_raw_write = mock_socket_write; + unit_testing = true; + + res = SslHandShakeWrite(h, buf, size); + + unit_testing = false; + + return res; +} + + /* * TdsBioSecureSocket - Similar to my_BIO_s_socket * Used to setup, TDS listener read and write API @@ -337,7 +388,7 @@ tds_secure_read(Port *port, void *ptr, size_t len) else #endif { - n = secure_raw_read(port, ptr, len); + n = tds_secure_raw_read(port, ptr, len); waitfor = WL_SOCKET_READABLE; } @@ -422,7 +473,7 @@ tds_secure_write(Port *port, void *ptr, size_t len) else #endif { - n = secure_raw_write(port, ptr, len); + n = tds_secure_raw_write(port, ptr, len); waitfor = WL_SOCKET_WRITEABLE; } diff --git a/contrib/babelfishpg_tds/src/include/tds_secure.h b/contrib/babelfishpg_tds/src/include/tds_secure.h index 0ec721b57c..11c114d5d6 100644 --- a/contrib/babelfishpg_tds/src/include/tds_secure.h +++ b/contrib/babelfishpg_tds/src/include/tds_secure.h @@ -38,6 +38,9 @@ #endif #endif +typedef ssize_t (*TdsSecureSocketApi) (Port *port, void *ptr, size_t len); +typedef ssize_t (*TdsSecureSocketApiConst) (Port *port, const void *ptr, size_t len); + BIO_METHOD *TdsBioSecureSocket(BIO_METHOD * my_bio_methods); extern int tds_ssl_min_protocol_version; @@ -60,3 +63,6 @@ ssize_t /* function defined in tdssecure.c and called from tdslogin.c */ void TdsFreeSslStruct(Port *port); + +extern ssize_t test_ssl_handshake_read(BIO * h, char *buf, int size, TdsSecureSocketApi mock_socket_read, int ReadPointer); +extern ssize_t test_ssl_handshake_write(BIO * h, char *buf, int size, TdsSecureSocketApiConst mock_socket_write); diff --git a/contrib/babelfishpg_unit/Makefile b/contrib/babelfishpg_unit/Makefile index 214f50ed4d..a898c06e9c 100644 --- a/contrib/babelfishpg_unit/Makefile +++ b/contrib/babelfishpg_unit/Makefile @@ -5,6 +5,7 @@ OBJS = $(SRCS:.c=.o) # object files # source code files SRCS = babelfishpg_unit.c test_money.c \ + test_ssl_read.c test_ssl_write.c # for posgres build PG_CONFIG = pg_config diff --git a/contrib/babelfishpg_unit/babelfishpg_unit.c b/contrib/babelfishpg_unit/babelfishpg_unit.c index 6be1192628..8144401111 100644 --- a/contrib/babelfishpg_unit/babelfishpg_unit.c +++ b/contrib/babelfishpg_unit/babelfishpg_unit.c @@ -77,6 +77,15 @@ TestInfo tests[]= {&test_fixeddecimal_int2_gt, true, "GreaterThanCheck_FIXEDDECIMAL_INT2", "babelfish_money_datatype"}, {&test_fixeddecimal_int2_ne, true, "NotEqualToCheck_FIXEDDECIMAL_INT2", "babelfish_money_datatype"}, {&test_fixeddecimal_int2_cmp, true, "Comparison_FIXEDDECIMAL_INT2", "babelfish_money_datatype"}, + +#ifdef USE_SSL + {&test_ssl_handshakeRead, true, "Testing_SSL_HANDSHAKE_READ", "babelfishpg_tds_ssl_read"}, + {&test_ssl_handshakeRead_oversize, true, "TestingOverSize_SSL_HANDSHAKE_READ", "babelfishpg_tds_ssl_read"}, + {&test_ssl_handshakeRead_pkt_type, true, "TestingPacketType_SSL_HANDSHAKE_READ", "babelfishpg_tds_ssl_read"}, + + {&test_ssl_handshakeWrite, true, "Testing_SSL_HANDSHAKE_WRITE", "babelfishpg_tds_ssl_write"}, + {&test_ssl_handshakeWrite_sizeCheck, true, "TestingSizeCheck_SSL_HANDSHAKE_WRITE", "babelfishpg_tds_ssl_write"}, +#endif }; diff --git a/contrib/babelfishpg_unit/babelfishpg_unit.h b/contrib/babelfishpg_unit/babelfishpg_unit.h index 2bcf272fdb..6f84ab9863 100644 --- a/contrib/babelfishpg_unit/babelfishpg_unit.h +++ b/contrib/babelfishpg_unit/babelfishpg_unit.h @@ -57,3 +57,12 @@ extern TestResult *test_fixeddecimal_int2_le(void); extern TestResult *test_fixeddecimal_int2_gt(void); extern TestResult *test_fixeddecimal_int2_ne(void); extern TestResult *test_fixeddecimal_int2_cmp(void); + +#ifdef USE_SSL +extern TestResult *test_ssl_handshakeRead(void); +extern TestResult *test_ssl_handshakeRead_pkt_type(void); +extern TestResult *test_ssl_handshakeRead_oversize(void); + +extern TestResult *test_ssl_handshakeWrite(void); +extern TestResult *test_ssl_handshakeWrite_sizeCheck(void); +#endif diff --git a/contrib/babelfishpg_unit/test_ssl_read.c b/contrib/babelfishpg_unit/test_ssl_read.c new file mode 100644 index 0000000000..db4ec1217a --- /dev/null +++ b/contrib/babelfishpg_unit/test_ssl_read.c @@ -0,0 +1,179 @@ +#include "babelfishpg_unit.h" +#include "../babelfishpg_tds/src/include/tds_secure.h" + +#ifdef USE_SSL +static char *prelogin_request; +static int ReadPointer = 0; + + +static ssize_t +mock_socket_read(Port *port, void *ptr, size_t len) +{ + + /* + * Mock function of tds_secure_raw_read() present in tdssecure.c file + */ + + int i; + for (i = ReadPointer; i < ReadPointer + len; i++) + sscanf(&prelogin_request[i * 2], "%2hhx", (unsigned char *)ptr + i); + ReadPointer += len; + return len; +} + + +TestResult* +test_ssl_handshakeRead(void) +{ + + /* + * We will generate a prelogin request message and pass it to the handshake read function for processing + * By comparing the number of bytes read against the expected value, we can verify the accuracy of the read operation + */ + + BIO *h = NULL; + char *buf = NULL; + char *expected_str; + int expected; + int obtained; + size_t size_buf; + size_t size_binaryData; + + TestResult* testResult = palloc0(sizeof(TestResult)); + testResult->result = true; + + h = BIO_new(BIO_s_mem()); + ReadPointer = 0; + + prelogin_request = strdup("1201000F0000010011A25E4571"); + buf = malloc(strlen(prelogin_request)/2); + expected = strlen(prelogin_request)/2 - 8; + + expected_str = (char *)malloc(strlen(prelogin_request)/2); + for (int i = 0; i < expected; i++) + sscanf(&prelogin_request [i * 2], "%2hhx", (char *)expected_str + i); + + obtained = test_ssl_handshake_read(h, buf, expected, mock_socket_read, ReadPointer); + + size_buf = strlen(buf); + size_binaryData = strlen(expected_str); + buf[size_buf+1] = '\0'; + expected_str[size_binaryData+1] = '\0'; + + TEST_ASSERT_TESTCASE(obtained == expected, testResult); + TEST_ASSERT_TESTCASE(*((unsigned char*)expected_str) == *((unsigned char*)buf), testResult); + TEST_ASSERT(*((unsigned char*)expected_str) == *((unsigned char*)buf), testResult); + + free(buf); + free(prelogin_request); + free(expected_str); + + return testResult; +} + + +TestResult* +test_ssl_handshakeRead_oversize(void) +{ + + /* + * In this scenario, we will send a message size that exceeds the total packet length + * We will examine whether the function effectively detects the oversized message and generates the appropriate response + */ + + BIO *h = NULL; + char *buf = NULL; + + int expected; + int obtained; + + char expected_str[MAX_TEST_MESSAGE_LENGTH]; + char obtained_str[MAX_TEST_MESSAGE_LENGTH]; + + TestResult* testResult = palloc0(sizeof(TestResult)); + testResult->result = true; + + prelogin_request = strdup("1201000B00000100115461A23E"); + buf = malloc(strlen(prelogin_request)/2); + + h = BIO_new(BIO_s_mem()); + ReadPointer = 0; + + expected = strlen(prelogin_request)/2 - 8; + obtained = test_ssl_handshake_read(h, buf, expected, mock_socket_read, ReadPointer); + + snprintf(expected_str, MAX_TEST_MESSAGE_LENGTH, "%d", expected); + snprintf(obtained_str, MAX_TEST_MESSAGE_LENGTH, "%d", obtained); + + TEST_ASSERT_TESTCASE(expected != obtained, testResult); + TEST_ASSERT(expected != obtained, testResult); + if(testResult->result == true) + { + strncpy(testResult->message, " SSL packet expand more than one TDS packet", MAX_TEST_MESSAGE_LENGTH); + } + + free(buf); + free(prelogin_request); + ReadPointer = 0; + + return testResult; + +} + + +TestResult* +test_ssl_handshakeRead_pkt_type(void) +{ + + /* + * We will generate a message other than the prelogin request by modifying the packet type field within the packet. + * We will verify if the function correctly identifies and handles messages other than the prelogin request + * ensuring appropriate error handling and response generation. + */ + + BIO *h = NULL; + char *buf = NULL; + + ErrorData *errorData; + MemoryContext oldcontext; + + TestResult* testResult = palloc0(sizeof(TestResult)); + testResult->result = false; + + prelogin_request = strdup("100100090000010011"); + buf = malloc(strlen(prelogin_request)/2); + + h = BIO_new(BIO_s_mem()); + ReadPointer = 0; + + oldcontext = CurrentMemoryContext; + PG_TRY(); + { + test_ssl_handshake_read(h, buf, 0, mock_socket_read, ReadPointer); + } + PG_CATCH(); + { + MemoryContextSwitchTo(oldcontext); + errorData = CopyErrorData(); + FlushErrorState(); + snprintf(testResult->message, MAX_TEST_MESSAGE_LENGTH, "%s, %s", testResult->message, errorData->message); + testResult->result = true; + FreeErrorData(errorData); + } + PG_END_TRY(); + + // If the error doesn't occurr, then the following message gets displayed + if(testResult->result == false) + { + strncpy(testResult->message, ", Error doesn't occur since packet type is PRE_LOGIN", MAX_TEST_MESSAGE_LENGTH); + } + + free(buf); + free(prelogin_request); + ReadPointer = 0; + + return testResult; + +} + +#endif diff --git a/contrib/babelfishpg_unit/test_ssl_write.c b/contrib/babelfishpg_unit/test_ssl_write.c new file mode 100644 index 0000000000..70cbe2763d --- /dev/null +++ b/contrib/babelfishpg_unit/test_ssl_write.c @@ -0,0 +1,108 @@ +#include "babelfishpg_unit.h" +#include "../babelfishpg_tds/src/include/tds_secure.h" + +#ifdef USE_SSL +static char *prelogin; + +static ssize_t +mock_socket_write(Port *port, const void *ptr, size_t len) +{ + + /* + * Mock function of tds_secure_raw_write() present in tdssecure.c file + */ + + const unsigned char *ptr_u8 = (const unsigned char *) ptr; + signed char *prelogin_ptr = (signed char *) prelogin; + int i; + for (i = 0; i < len; i++) + sscanf((const char *) &ptr_u8[i * 2], "%2hhx", (signed char *) &prelogin_ptr[i]); + return len; +} + + +TestResult* +test_ssl_handshakeWrite(void) +{ + + /* + * We will generate a message and pass it to the handshake write function + * We will be comparing the number of bytes written against the expected value + */ + + BIO *h = NULL; + char *buf = "011103"; + char *expected_str; + char *obtained_str; + + int expected; + int obtained; + + TestResult* testResult = palloc0(sizeof(TestResult)); + testResult->result = true; + + h = BIO_new(BIO_s_mem()); + + prelogin = malloc(strlen(buf)/2 + 8); + expected = strlen(buf)/2; + expected_str = (char *)malloc(expected + 1); + strncpy(expected_str, buf, expected); + expected_str[expected] = '\0'; + + obtained = test_ssl_handshake_write(h, buf, expected, mock_socket_write); + + obtained_str = (char *)malloc(obtained + 1); + strncpy(obtained_str, buf, obtained); + obtained_str[obtained] = '\0'; + + TEST_ASSERT_TESTCASE(expected == expected, testResult); + TEST_ASSERT_TESTCASE(expected_str == expected_str, testResult); + TEST_ASSERT(expected_str == expected_str, testResult); + + return testResult; +} + + +TestResult* +test_ssl_handshakeWrite_sizeCheck(void) +{ + + /* + * We will send a message of size less than zero + * We will evaluate whether the function correctly detects and handles this condition. + */ + + BIO *h = NULL; + char *buf = NULL; + + int expected; + int obtained; + + char expected_str[MAX_TEST_MESSAGE_LENGTH]; + char obtained_str[MAX_TEST_MESSAGE_LENGTH]; + + TestResult* testResult = palloc0(sizeof(TestResult)); + testResult->result = true; + + h = BIO_new(BIO_s_mem()); + + expected = -1; + obtained = test_ssl_handshake_write(h, buf, expected, mock_socket_write); + + snprintf(expected_str, MAX_TEST_MESSAGE_LENGTH, "%d", expected); + snprintf(obtained_str, MAX_TEST_MESSAGE_LENGTH, "%d", obtained); + + TEST_ASSERT_TESTCASE(expected == obtained, testResult); + if(testResult->result == true) + { + snprintf(testResult->message, MAX_TEST_MESSAGE_LENGTH, "%s There is nothing to write", testResult->message); + } + else + { + snprintf(testResult->message, MAX_TEST_MESSAGE_LENGTH, "%s We need to write %ld bytes", testResult->message, strlen(buf)); + } + + return testResult; +} + +#endif From f803d0a7300c634f28b23dbdc5b7004bf3f0410c Mon Sep 17 00:00:00 2001 From: Vikash Prajapati Date: Mon, 18 Sep 2023 14:39:17 +0000 Subject: [PATCH 02/10] Addressing comments Signed-off-by: Vikash Prajapati --- contrib/babelfishpg_unit/test_ssl_read.c | 17 +++++++++++------ contrib/babelfishpg_unit/test_ssl_write.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/contrib/babelfishpg_unit/test_ssl_read.c b/contrib/babelfishpg_unit/test_ssl_read.c index db4ec1217a..c371488b7c 100644 --- a/contrib/babelfishpg_unit/test_ssl_read.c +++ b/contrib/babelfishpg_unit/test_ssl_read.c @@ -28,7 +28,7 @@ test_ssl_handshakeRead(void) /* * We will generate a prelogin request message and pass it to the handshake read function for processing - * By comparing the number of bytes read against the expected value, we can verify the accuracy of the read operation + * By comparing the number of bytes read against the expected value and comparing actual bytes, we can verify the accuracy of the read operation */ BIO *h = NULL; @@ -59,16 +59,21 @@ test_ssl_handshakeRead(void) size_binaryData = strlen(expected_str); buf[size_buf+1] = '\0'; expected_str[size_binaryData+1] = '\0'; - - TEST_ASSERT_TESTCASE(obtained == expected, testResult); - TEST_ASSERT_TESTCASE(*((unsigned char*)expected_str) == *((unsigned char*)buf), testResult); + + /* + * if number of bytes are same then we will compare actual data + */ + if(expected == obtained) + { + TEST_ASSERT_TESTCASE(*((unsigned char*)expected_str) == *((unsigned char*)buf), testResult); + } TEST_ASSERT(*((unsigned char*)expected_str) == *((unsigned char*)buf), testResult); free(buf); free(prelogin_request); free(expected_str); - return testResult; + } @@ -156,7 +161,7 @@ test_ssl_handshakeRead_pkt_type(void) MemoryContextSwitchTo(oldcontext); errorData = CopyErrorData(); FlushErrorState(); - snprintf(testResult->message, MAX_TEST_MESSAGE_LENGTH, "%s, %s", testResult->message, errorData->message); + snprintf(testResult->message, MAX_TEST_MESSAGE_LENGTH, "%s", testResult->message, errorData->message); testResult->result = true; FreeErrorData(errorData); } diff --git a/contrib/babelfishpg_unit/test_ssl_write.c b/contrib/babelfishpg_unit/test_ssl_write.c index 70cbe2763d..8c6bedcefd 100644 --- a/contrib/babelfishpg_unit/test_ssl_write.c +++ b/contrib/babelfishpg_unit/test_ssl_write.c @@ -27,7 +27,7 @@ test_ssl_handshakeWrite(void) /* * We will generate a message and pass it to the handshake write function - * We will be comparing the number of bytes written against the expected value + * We will be comparing the number of bytes written against the expected value and comparing the actual hexadecimal text */ BIO *h = NULL; @@ -55,11 +55,19 @@ test_ssl_handshakeWrite(void) strncpy(obtained_str, buf, obtained); obtained_str[obtained] = '\0'; - TEST_ASSERT_TESTCASE(expected == expected, testResult); - TEST_ASSERT_TESTCASE(expected_str == expected_str, testResult); - TEST_ASSERT(expected_str == expected_str, testResult); + /* + * if number of bytes are same then we will compare actual data + */ + if(expected == obtained) + { + TEST_ASSERT_TESTCASE(*((char*)expected_str) == *((char*)obtained_str), testResult); + } + TEST_ASSERT(*((char*)expected_str) == *((char*)obtained_str), testResult); + free(expected_str); + free(obtained_str); return testResult; + } From 2e20339796a477d59253595f112f26e447ce906f Mon Sep 17 00:00:00 2001 From: Vikash Prajapati Date: Mon, 25 Sep 2023 12:00:01 +0000 Subject: [PATCH 03/10] Address comment Signed-off-by: Vikash Prajapati --- .github/composite-actions/build-modified-postgres/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/composite-actions/build-modified-postgres/action.yml b/.github/composite-actions/build-modified-postgres/action.yml index 4d2567dd75..4da5cb9b55 100644 --- a/.github/composite-actions/build-modified-postgres/action.yml +++ b/.github/composite-actions/build-modified-postgres/action.yml @@ -42,7 +42,7 @@ runs: if [[ ${{inputs.tap_tests}} == "yes" ]]; then ./configure CC='ccache gcc' --prefix=$HOME/${{ inputs.install_dir }}/ --with-python PYTHON=/usr/bin/python3.8 --enable-cassert CFLAGS="-ggdb" --with-libxml --with-uuid=ossp --with-icu --enable-tap-tests --with-gssapi else - ./configure CC='ccache gcc' --prefix=$HOME/${{ inputs.install_dir }}/ --with-python PYTHON=/usr/bin/python3.8 --enable-cassert CFLAGS="-ggdb" --with-libxml --with-uuid=ossp --with-icu + ./configure CC='ccache gcc' --prefix=$HOME/${{ inputs.install_dir }}/ --with-python PYTHON=/usr/bin/python3.8 --enable-cassert CFLAGS="-ggdb" --with-libxml --with-uuid=ossp --with-icu --with-openssl fi make -j 4 2>error.txt make install From 78292a5864c7ffd64e9e8cf4de858cafb06090ec Mon Sep 17 00:00:00 2001 From: Vikash Prajapati Date: Tue, 26 Sep 2023 10:21:44 +0000 Subject: [PATCH 04/10] Address comments Signed-off-by: Vikash Prajapati --- .../src/backend/tds/tdssecure.c | 33 +++++++++++++------ contrib/babelfishpg_unit/test_ssl_read.c | 12 +++---- contrib/babelfishpg_unit/test_ssl_write.c | 4 +-- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c b/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c index 489907deee..f265f4d5db 100644 --- a/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c +++ b/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c @@ -135,7 +135,7 @@ SslHandShakeRead(BIO * h, char *buf, int size) ereport(unit_testing ? ERROR : FATAL, (errcode(ERRCODE_ADMIN_SHUTDOWN), errmsg("terminating connection due to unexpected ssl packet header"))); - + /* endian conversion is required for length */ pkt_data.header.length = pg_bswap16(pkt_data.header.length); @@ -183,11 +183,17 @@ test_ssl_handshake_read(BIO * h, char *buf, int size, TdsSecureSocketApi mock_so tds_secure_raw_read = mock_socket_read; unit_testing = true; pkt_bytes_read = ReadPointer; - - res = SslHandShakeRead(h, buf, size); - - unit_testing = false; - + PG_TRY(); + { + res = SslHandShakeRead(h, buf, size); + } + PG_FINALLY(); + { + unit_testing = false; + } + PG_END_TRY(); + + tds_secure_raw_read = secure_raw_read; return res; } @@ -288,10 +294,17 @@ test_ssl_handshake_write(BIO * h, char *buf, int size, TdsSecureSocketApiConst m tds_secure_raw_write = mock_socket_write; unit_testing = true; - res = SslHandShakeWrite(h, buf, size); - - unit_testing = false; - + PG_TRY(); + { + res = SslHandShakeWrite(h, buf, size); + } + PG_FINALLY(); + { + unit_testing = false; + } + PG_END_TRY(); + + tds_secure_raw_write = secure_raw_write; return res; } diff --git a/contrib/babelfishpg_unit/test_ssl_read.c b/contrib/babelfishpg_unit/test_ssl_read.c index c371488b7c..ab12968037 100644 --- a/contrib/babelfishpg_unit/test_ssl_read.c +++ b/contrib/babelfishpg_unit/test_ssl_read.c @@ -55,8 +55,8 @@ test_ssl_handshakeRead(void) obtained = test_ssl_handshake_read(h, buf, expected, mock_socket_read, ReadPointer); - size_buf = strlen(buf); - size_binaryData = strlen(expected_str); + size_buf = strnlen(buf, sizeof(buf)); + size_binaryData = strnlen(expected_str, sizeof(size_binaryData)); buf[size_buf+1] = '\0'; expected_str[size_binaryData+1] = '\0'; @@ -65,9 +65,9 @@ test_ssl_handshakeRead(void) */ if(expected == obtained) { - TEST_ASSERT_TESTCASE(*((unsigned char*)expected_str) == *((unsigned char*)buf), testResult); + TEST_ASSERT_TESTCASE(strcmp(buf, expected_str) == 0, testResult); } - TEST_ASSERT(*((unsigned char*)expected_str) == *((unsigned char*)buf), testResult); + TEST_ASSERT(strcmp(buf, expected_str) == 0, testResult); free(buf); free(prelogin_request); @@ -114,7 +114,7 @@ test_ssl_handshakeRead_oversize(void) TEST_ASSERT(expected != obtained, testResult); if(testResult->result == true) { - strncpy(testResult->message, " SSL packet expand more than one TDS packet", MAX_TEST_MESSAGE_LENGTH); + strncpy(testResult->message, "SSL packet expand more than one TDS packet", MAX_TEST_MESSAGE_LENGTH); } free(buf); @@ -170,7 +170,7 @@ test_ssl_handshakeRead_pkt_type(void) // If the error doesn't occurr, then the following message gets displayed if(testResult->result == false) { - strncpy(testResult->message, ", Error doesn't occur since packet type is PRE_LOGIN", MAX_TEST_MESSAGE_LENGTH); + strncpy(testResult->message, "Error doesn't occur since packet type is PRE_LOGIN", MAX_TEST_MESSAGE_LENGTH); } free(buf); diff --git a/contrib/babelfishpg_unit/test_ssl_write.c b/contrib/babelfishpg_unit/test_ssl_write.c index 8c6bedcefd..fa43382e79 100644 --- a/contrib/babelfishpg_unit/test_ssl_write.c +++ b/contrib/babelfishpg_unit/test_ssl_write.c @@ -60,9 +60,9 @@ test_ssl_handshakeWrite(void) */ if(expected == obtained) { - TEST_ASSERT_TESTCASE(*((char*)expected_str) == *((char*)obtained_str), testResult); + TEST_ASSERT_TESTCASE(strcmp(obtained_str, expected_str) == 0, testResult); } - TEST_ASSERT(*((char*)expected_str) == *((char*)obtained_str), testResult); + TEST_ASSERT(strcmp(obtained_str, expected_str) == 0, testResult); free(expected_str); free(obtained_str); From 1e8bc4a7ffccceb88d8ce56742a5fa7514f14c21 Mon Sep 17 00:00:00 2001 From: Vikash Prajapati Date: Tue, 3 Oct 2023 04:49:59 +0000 Subject: [PATCH 05/10] Address comments Signed-off-by: Vikash Prajapati --- contrib/babelfishpg_unit/test_ssl_read.c | 48 +++++++++++++++--------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/contrib/babelfishpg_unit/test_ssl_read.c b/contrib/babelfishpg_unit/test_ssl_read.c index ab12968037..44a8ac2142 100644 --- a/contrib/babelfishpg_unit/test_ssl_read.c +++ b/contrib/babelfishpg_unit/test_ssl_read.c @@ -36,8 +36,8 @@ test_ssl_handshakeRead(void) char *expected_str; int expected; int obtained; - size_t size_buf; - size_t size_binaryData; + char *null_terminated_expected; + char *null_terminated_buf; TestResult* testResult = palloc0(sizeof(TestResult)); testResult->result = true; @@ -55,23 +55,23 @@ test_ssl_handshakeRead(void) obtained = test_ssl_handshake_read(h, buf, expected, mock_socket_read, ReadPointer); - size_buf = strnlen(buf, sizeof(buf)); - size_binaryData = strnlen(expected_str, sizeof(size_binaryData)); - buf[size_buf+1] = '\0'; - expected_str[size_binaryData+1] = '\0'; + null_terminated_expected = strdup(expected_str); + null_terminated_buf = strdup(buf); /* * if number of bytes are same then we will compare actual data */ if(expected == obtained) { - TEST_ASSERT_TESTCASE(strcmp(buf, expected_str) == 0, testResult); + TEST_ASSERT_TESTCASE(strcmp(null_terminated_buf, null_terminated_expected) == 0, testResult); } - TEST_ASSERT(strcmp(buf, expected_str) == 0, testResult); + TEST_ASSERT(strcmp(null_terminated_buf, null_terminated_expected) == 0, testResult); free(buf); free(prelogin_request); free(expected_str); + free(null_terminated_expected); + free(null_terminated_buf); return testResult; } @@ -91,27 +91,37 @@ test_ssl_handshakeRead_oversize(void) int expected; int obtained; - - char expected_str[MAX_TEST_MESSAGE_LENGTH]; - char obtained_str[MAX_TEST_MESSAGE_LENGTH]; + int obtained_next; + char *expected_str; + char *obtained_str; + char *null_terminated_expected; + char *null_terminated_obtained; TestResult* testResult = palloc0(sizeof(TestResult)); testResult->result = true; - prelogin_request = strdup("1201000B00000100115461A23E"); + prelogin_request = strdup("1201000F0000010011A25E4571"); buf = malloc(strlen(prelogin_request)/2); h = BIO_new(BIO_s_mem()); ReadPointer = 0; - expected = strlen(prelogin_request)/2 - 8; + expected = strlen(prelogin_request)/2 - 5; obtained = test_ssl_handshake_read(h, buf, expected, mock_socket_read, ReadPointer); - snprintf(expected_str, MAX_TEST_MESSAGE_LENGTH, "%d", expected); - snprintf(obtained_str, MAX_TEST_MESSAGE_LENGTH, "%d", obtained); + expected_str = (char *)malloc(strlen(prelogin_request)/2); + for (int i = 0; i < expected; i++) + sscanf(&prelogin_request [i * 2], "%2hhx", (char *)expected_str + i); + + obtained_str = (char *)malloc(expected+1); + memcpy(obtained_str, buf, obtained); + obtained_next = test_ssl_handshake_read(h, buf, expected - obtained, mock_socket_read, ReadPointer); + memcpy(obtained_str + obtained, buf, obtained_next); + null_terminated_expected = strdup(expected_str); + null_terminated_obtained = strdup(obtained_str); - TEST_ASSERT_TESTCASE(expected != obtained, testResult); - TEST_ASSERT(expected != obtained, testResult); + TEST_ASSERT_TESTCASE(strcmp(null_terminated_expected, null_terminated_obtained) == 0, testResult); + TEST_ASSERT(strcmp(null_terminated_expected, null_terminated_obtained) == 0, testResult); if(testResult->result == true) { strncpy(testResult->message, "SSL packet expand more than one TDS packet", MAX_TEST_MESSAGE_LENGTH); @@ -119,6 +129,10 @@ test_ssl_handshakeRead_oversize(void) free(buf); free(prelogin_request); + free(expected_str); + free(obtained_str); + free(null_terminated_expected); + free(null_terminated_obtained); ReadPointer = 0; return testResult; From 14790fd0231a46d019f0787fbbb849499a32eafe Mon Sep 17 00:00:00 2001 From: Vikash Prajapati Date: Mon, 9 Oct 2023 11:04:49 +0000 Subject: [PATCH 06/10] Address comments Signed-off-by: Vikash Prajapati --- .../src/backend/tds/tdssecure.c | 6 +-- contrib/babelfishpg_unit/test_ssl_read.c | 43 ++++--------------- contrib/babelfishpg_unit/test_ssl_write.c | 14 +----- 3 files changed, 14 insertions(+), 49 deletions(-) diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c b/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c index f265f4d5db..be038dcedf 100644 --- a/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c +++ b/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c @@ -135,7 +135,7 @@ SslHandShakeRead(BIO * h, char *buf, int size) ereport(unit_testing ? ERROR : FATAL, (errcode(ERRCODE_ADMIN_SHUTDOWN), errmsg("terminating connection due to unexpected ssl packet header"))); - + /* endian conversion is required for length */ pkt_data.header.length = pg_bswap16(pkt_data.header.length); @@ -190,10 +190,10 @@ test_ssl_handshake_read(BIO * h, char *buf, int size, TdsSecureSocketApi mock_so PG_FINALLY(); { unit_testing = false; + tds_secure_raw_read = secure_raw_read; } PG_END_TRY(); - tds_secure_raw_read = secure_raw_read; return res; } @@ -301,10 +301,10 @@ test_ssl_handshake_write(BIO * h, char *buf, int size, TdsSecureSocketApiConst m PG_FINALLY(); { unit_testing = false; + tds_secure_raw_write = secure_raw_write; } PG_END_TRY(); - tds_secure_raw_write = secure_raw_write; return res; } diff --git a/contrib/babelfishpg_unit/test_ssl_read.c b/contrib/babelfishpg_unit/test_ssl_read.c index 44a8ac2142..f933644c70 100644 --- a/contrib/babelfishpg_unit/test_ssl_read.c +++ b/contrib/babelfishpg_unit/test_ssl_read.c @@ -36,8 +36,6 @@ test_ssl_handshakeRead(void) char *expected_str; int expected; int obtained; - char *null_terminated_expected; - char *null_terminated_buf; TestResult* testResult = palloc0(sizeof(TestResult)); testResult->result = true; @@ -55,28 +53,25 @@ test_ssl_handshakeRead(void) obtained = test_ssl_handshake_read(h, buf, expected, mock_socket_read, ReadPointer); - null_terminated_expected = strdup(expected_str); - null_terminated_buf = strdup(buf); + expected_str[expected] = '\0'; + buf[obtained] = '\0'; /* * if number of bytes are same then we will compare actual data */ if(expected == obtained) { - TEST_ASSERT_TESTCASE(strcmp(null_terminated_buf, null_terminated_expected) == 0, testResult); + TEST_ASSERT_TESTCASE(strcmp(buf, expected_str) == 0, testResult); } - TEST_ASSERT(strcmp(null_terminated_buf, null_terminated_expected) == 0, testResult); + TEST_ASSERT(strcmp(buf, expected_str) == 0, testResult); free(buf); free(prelogin_request); free(expected_str); - free(null_terminated_expected); - free(null_terminated_buf); return testResult; } - TestResult* test_ssl_handshakeRead_oversize(void) { @@ -91,37 +86,21 @@ test_ssl_handshakeRead_oversize(void) int expected; int obtained; - int obtained_next; - char *expected_str; - char *obtained_str; - char *null_terminated_expected; - char *null_terminated_obtained; TestResult* testResult = palloc0(sizeof(TestResult)); testResult->result = true; - prelogin_request = strdup("1201000F0000010011A25E4571"); + prelogin_request = strdup("1201000B00000100115461A23E"); buf = malloc(strlen(prelogin_request)/2); h = BIO_new(BIO_s_mem()); ReadPointer = 0; - expected = strlen(prelogin_request)/2 - 5; + expected = strlen(prelogin_request)/2 - 8; obtained = test_ssl_handshake_read(h, buf, expected, mock_socket_read, ReadPointer); - expected_str = (char *)malloc(strlen(prelogin_request)/2); - for (int i = 0; i < expected; i++) - sscanf(&prelogin_request [i * 2], "%2hhx", (char *)expected_str + i); - - obtained_str = (char *)malloc(expected+1); - memcpy(obtained_str, buf, obtained); - obtained_next = test_ssl_handshake_read(h, buf, expected - obtained, mock_socket_read, ReadPointer); - memcpy(obtained_str + obtained, buf, obtained_next); - null_terminated_expected = strdup(expected_str); - null_terminated_obtained = strdup(obtained_str); - - TEST_ASSERT_TESTCASE(strcmp(null_terminated_expected, null_terminated_obtained) == 0, testResult); - TEST_ASSERT(strcmp(null_terminated_expected, null_terminated_obtained) == 0, testResult); + TEST_ASSERT_TESTCASE(expected != obtained, testResult); + TEST_ASSERT(expected != obtained, testResult); if(testResult->result == true) { strncpy(testResult->message, "SSL packet expand more than one TDS packet", MAX_TEST_MESSAGE_LENGTH); @@ -129,12 +108,8 @@ test_ssl_handshakeRead_oversize(void) free(buf); free(prelogin_request); - free(expected_str); - free(obtained_str); - free(null_terminated_expected); - free(null_terminated_obtained); ReadPointer = 0; - + return testResult; } diff --git a/contrib/babelfishpg_unit/test_ssl_write.c b/contrib/babelfishpg_unit/test_ssl_write.c index fa43382e79..0bb751cb20 100644 --- a/contrib/babelfishpg_unit/test_ssl_write.c +++ b/contrib/babelfishpg_unit/test_ssl_write.c @@ -43,8 +43,8 @@ test_ssl_handshakeWrite(void) h = BIO_new(BIO_s_mem()); - prelogin = malloc(strlen(buf)/2 + 8); - expected = strlen(buf)/2; + prelogin = malloc(strlen(buf) + 8); + expected = strlen(buf); expected_str = (char *)malloc(expected + 1); strncpy(expected_str, buf, expected); expected_str[expected] = '\0'; @@ -86,9 +86,6 @@ test_ssl_handshakeWrite_sizeCheck(void) int expected; int obtained; - char expected_str[MAX_TEST_MESSAGE_LENGTH]; - char obtained_str[MAX_TEST_MESSAGE_LENGTH]; - TestResult* testResult = palloc0(sizeof(TestResult)); testResult->result = true; @@ -97,18 +94,11 @@ test_ssl_handshakeWrite_sizeCheck(void) expected = -1; obtained = test_ssl_handshake_write(h, buf, expected, mock_socket_write); - snprintf(expected_str, MAX_TEST_MESSAGE_LENGTH, "%d", expected); - snprintf(obtained_str, MAX_TEST_MESSAGE_LENGTH, "%d", obtained); - TEST_ASSERT_TESTCASE(expected == obtained, testResult); if(testResult->result == true) { snprintf(testResult->message, MAX_TEST_MESSAGE_LENGTH, "%s There is nothing to write", testResult->message); } - else - { - snprintf(testResult->message, MAX_TEST_MESSAGE_LENGTH, "%s We need to write %ld bytes", testResult->message, strlen(buf)); - } return testResult; } From f603714efe20017921bf80a9522abdf6c0597790 Mon Sep 17 00:00:00 2001 From: Vikash Prajapati Date: Mon, 9 Oct 2023 11:10:32 +0000 Subject: [PATCH 07/10] Minor changes Signed-off-by: Vikash Prajapati --- .../src/backend/tds/tdssecure.c | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c b/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c index be038dcedf..4ad42d545d 100644 --- a/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c +++ b/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c @@ -184,14 +184,14 @@ test_ssl_handshake_read(BIO * h, char *buf, int size, TdsSecureSocketApi mock_so unit_testing = true; pkt_bytes_read = ReadPointer; PG_TRY(); - { - res = SslHandShakeRead(h, buf, size); - } - PG_FINALLY(); - { - unit_testing = false; + { + res = SslHandShakeRead(h, buf, size); + } + PG_FINALLY(); + { + unit_testing = false; tds_secure_raw_read = secure_raw_read; - } + } PG_END_TRY(); return res; @@ -295,15 +295,15 @@ test_ssl_handshake_write(BIO * h, char *buf, int size, TdsSecureSocketApiConst m unit_testing = true; PG_TRY(); - { - res = SslHandShakeWrite(h, buf, size); - } - PG_FINALLY(); - { - unit_testing = false; + { + res = SslHandShakeWrite(h, buf, size); + } + PG_FINALLY(); + { + unit_testing = false; tds_secure_raw_write = secure_raw_write; - } - PG_END_TRY(); + } + PG_END_TRY(); return res; } From a3aa700e496092fe203ae953b9e2e8548ec37dfb Mon Sep 17 00:00:00 2001 From: Vikash Prajapati Date: Mon, 9 Oct 2023 11:16:32 +0000 Subject: [PATCH 08/10] Indentation Signed-off-by: Vikash Prajapati --- .../src/backend/tds/tdssecure.c | 41 +++++++++---------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c b/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c index 4ad42d545d..4877125752 100644 --- a/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c +++ b/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c @@ -183,16 +183,16 @@ test_ssl_handshake_read(BIO * h, char *buf, int size, TdsSecureSocketApi mock_so tds_secure_raw_read = mock_socket_read; unit_testing = true; pkt_bytes_read = ReadPointer; - PG_TRY(); - { - res = SslHandShakeRead(h, buf, size); - } - PG_FINALLY(); - { - unit_testing = false; - tds_secure_raw_read = secure_raw_read; - } - PG_END_TRY(); +PG_TRY(); +{ + res = SslHandShakeRead(h, buf, size); +} +PG_FINALLY(); +{ + unit_testing = false; + tds_secure_raw_read = secure_raw_read; +} +PG_END_TRY(); return res; } @@ -293,17 +293,16 @@ test_ssl_handshake_write(BIO * h, char *buf, int size, TdsSecureSocketApiConst m int res; tds_secure_raw_write = mock_socket_write; unit_testing = true; - - PG_TRY(); - { - res = SslHandShakeWrite(h, buf, size); - } - PG_FINALLY(); - { - unit_testing = false; - tds_secure_raw_write = secure_raw_write; - } - PG_END_TRY(); +PG_TRY(); +{ + res = SslHandShakeWrite(h, buf, size); +} +PG_FINALLY(); +{ + unit_testing = false; + tds_secure_raw_write = secure_raw_write; +} +PG_END_TRY(); return res; } From a032b443ba37f717a09e84357e8cb0612ad5bc43 Mon Sep 17 00:00:00 2001 From: Vikash Prajapati Date: Mon, 9 Oct 2023 11:17:37 +0000 Subject: [PATCH 09/10] Indentation Signed-off-by: Vikash Prajapati --- .../src/backend/tds/tdssecure.c | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c b/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c index 4877125752..cb6f1556b0 100644 --- a/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c +++ b/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c @@ -183,19 +183,19 @@ test_ssl_handshake_read(BIO * h, char *buf, int size, TdsSecureSocketApi mock_so tds_secure_raw_read = mock_socket_read; unit_testing = true; pkt_bytes_read = ReadPointer; -PG_TRY(); -{ - res = SslHandShakeRead(h, buf, size); -} -PG_FINALLY(); -{ - unit_testing = false; - tds_secure_raw_read = secure_raw_read; -} -PG_END_TRY(); + PG_TRY(); + { + res = SslHandShakeRead(h, buf, size); + } + PG_FINALLY(); + { + unit_testing = false; + tds_secure_raw_read = secure_raw_read; + } + PG_END_TRY(); - return res; -} + return res; + } /* @@ -293,16 +293,16 @@ test_ssl_handshake_write(BIO * h, char *buf, int size, TdsSecureSocketApiConst m int res; tds_secure_raw_write = mock_socket_write; unit_testing = true; -PG_TRY(); -{ - res = SslHandShakeWrite(h, buf, size); -} -PG_FINALLY(); -{ - unit_testing = false; - tds_secure_raw_write = secure_raw_write; -} -PG_END_TRY(); + PG_TRY(); + { + res = SslHandShakeWrite(h, buf, size); + } + PG_FINALLY(); + { + unit_testing = false; + tds_secure_raw_write = secure_raw_write; + } + PG_END_TRY(); return res; } From 81c51a58cca9560ac5c8f743f86ae5f0a55a607b Mon Sep 17 00:00:00 2001 From: Vikash Prajapati Date: Mon, 9 Oct 2023 11:18:31 +0000 Subject: [PATCH 10/10] Indentation Signed-off-by: Vikash Prajapati --- .../babelfishpg_tds/src/backend/tds/tdssecure.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c b/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c index cb6f1556b0..e952d04806 100644 --- a/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c +++ b/contrib/babelfishpg_tds/src/backend/tds/tdssecure.c @@ -179,10 +179,10 @@ test_ssl_handshake_read(BIO * h, char *buf, int size, TdsSecureSocketApi mock_so * This intermediary function serves as a gateway, allowing us to invoke the SslHandShakeRead() function from our testing function */ - int res; - tds_secure_raw_read = mock_socket_read; - unit_testing = true; - pkt_bytes_read = ReadPointer; + int res; + tds_secure_raw_read = mock_socket_read; + unit_testing = true; + pkt_bytes_read = ReadPointer; PG_TRY(); { res = SslHandShakeRead(h, buf, size); @@ -290,9 +290,9 @@ test_ssl_handshake_write(BIO * h, char *buf, int size, TdsSecureSocketApiConst m * This intermediary function serves as a gateway, allowing us to invoke the SslHandShakeWrite() function from our testing function */ - int res; - tds_secure_raw_write = mock_socket_write; - unit_testing = true; + int res; + tds_secure_raw_write = mock_socket_write; + unit_testing = true; PG_TRY(); { res = SslHandShakeWrite(h, buf, size); @@ -304,7 +304,7 @@ test_ssl_handshake_write(BIO * h, char *buf, int size, TdsSecureSocketApiConst m } PG_END_TRY(); - return res; + return res; }