From 6acc28125f1c88e531256580d3ba70a6356b6f19 Mon Sep 17 00:00:00 2001 From: "arthur.barr@uk.ibm.com" Date: Tue, 2 Aug 2022 11:28:31 +0100 Subject: [PATCH] Use alternative string trimming in auth service Previous string trimming was changing the strings supplied by MQ to be null-terminated. MQ uses fixed-width strings, and the changes to the data could cause problems in the queue manager. --- authservice/mqhtpass/Makefile | 8 +- authservice/mqhtpass/src/log.c | 16 +++- authservice/mqhtpass/src/log.h | 9 ++- authservice/mqhtpass/src/log_test.c | 120 ++++++++++++++++++++++++++++ authservice/mqhtpass/src/mqhtpass.c | 58 ++++++-------- 5 files changed, 173 insertions(+), 38 deletions(-) create mode 100644 authservice/mqhtpass/src/log_test.c diff --git a/authservice/mqhtpass/Makefile b/authservice/mqhtpass/Makefile index 82f92ba..364fa90 100644 --- a/authservice/mqhtpass/Makefile +++ b/authservice/mqhtpass/Makefile @@ -33,12 +33,18 @@ CFLAGS += -std=gnu11 -fPIC -Wall ${CFLAGS.${ARCH}} LIB_APR = -L/usr/lib64 -lapr-1 -laprutil-1 LIB_MQ = -L/opt/mqm/lib64 -lmqm_r -all: $(BUILD_DIR)/mqhtpass.so $(BUILD_DIR)/htpass_test +all: $(BUILD_DIR)/mqhtpass.so $(BUILD_DIR)/htpass_test $(BUILD_DIR)/log_test $(BUILD_DIR)/log.o : $(SRC_DIR)/log.c $(SRC_DIR)/log.h mkdir -p ${dir $@} gcc $(CFLAGS) -c $(SRC_DIR)/log.c -o $@ +$(BUILD_DIR)/log_test : $(BUILD_DIR)/log.o + mkdir -p ${dir $@} + gcc $(CFLAGS) $(SRC_DIR)/log_test.c $^ -o $@ +# Run Logging tests, and print log if they fail + $@ || (cat log_test*.log && exit 1) + $(BUILD_DIR)/htpass.o : $(SRC_DIR)/htpass.c $(SRC_DIR)/htpass.h mkdir -p ${dir $@} gcc $(CFLAGS) -c $(SRC_DIR)/htpass.c -I /usr/include/apr-1 -o $@ diff --git a/authservice/mqhtpass/src/log.c b/authservice/mqhtpass/src/log.c index 70675c0..c422c90 100644 --- a/authservice/mqhtpass/src/log.c +++ b/authservice/mqhtpass/src/log.c @@ -1,5 +1,5 @@ /* -© Copyright IBM Corporation 2021 +© Copyright IBM Corporation 2021, 2022 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -124,7 +124,7 @@ void log_printf(const char *source_file, int source_line, const char *level, con if (strftime(date_buf, sizeof date_buf, "%FT%T", utc)) { // Round microseconds down to milliseconds, for consistency - cur += snprintf(cur, end-cur, ", \"ibm_datetime\":\"%s.%03ldZ\"", date_buf, now.tv_usec / 1000); + cur += snprintf(cur, end-cur, ", \"ibm_datetime\":\"%s.%03ldZ\"", date_buf, now.tv_usec / (long)1000); } cur += snprintf(cur, end-cur, ", \"ibm_processId\":\"%d\"", pid); cur += snprintf(cur, end-cur, ", \"host\":\"%s\"", hostname); @@ -146,7 +146,17 @@ void log_printf(const char *source_file, int source_line, const char *level, con // Important: Just do one file write, to prevent problems with multi-threading. // This only works if the log message is not too long for the buffer. - fprintf(fp, buf); + fprintf(fp, "%s", buf); } } +int trimmed_len(char *s, int max_len) +{ + int i; + for (i = max_len - 1; i >= 0; i--) + { + if (s[i] != ' ') + break; + } + return i+1; +} \ No newline at end of file diff --git a/authservice/mqhtpass/src/log.h b/authservice/mqhtpass/src/log.h index fa92135..1222157 100644 --- a/authservice/mqhtpass/src/log.h +++ b/authservice/mqhtpass/src/log.h @@ -1,5 +1,5 @@ /* -© Copyright IBM Corporation 2021 +© Copyright IBM Corporation 2021, 2022 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -59,5 +59,12 @@ void log_close(); */ #define log_debugf(format,...) log_printf(__FILE__, __LINE__, "DEBUG", format, ##__VA_ARGS__) +/** + * Return the length of the string when trimmed of trailing spaces. + * IBM MQ uses fixed length strings, so this function can be used to print + * a trimmed version of a string using the "%.*s" printf format string. + * For example, `log_printf("%.*s", trimmed_len(fw_str, 48), fw_str)` + */ +int trimmed_len(char *s, int); #endif \ No newline at end of file diff --git a/authservice/mqhtpass/src/log_test.c b/authservice/mqhtpass/src/log_test.c new file mode 100644 index 0000000..ad6cdf5 --- /dev/null +++ b/authservice/mqhtpass/src/log_test.c @@ -0,0 +1,120 @@ +/* +© Copyright IBM Corporation 2022 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include +#include +#include +#include +#include "log.h" + +// Headers for multi-threaded tests +#include + +// Start a test and log the function name +#define test_start() printf("=== RUN: %s\n", __func__) + +// Indicate test has passed +#define test_pass() printf("--- PASS: %s\n", __func__) + +// The length of strings used in the tests +#define STR_LEN 5 + +// Indicate test has failed +void test_fail(const char *test_name) +{ + printf("--- FAIL: %s\n", test_name); + exit(1); +} + +// Print a fixed-width string in hexadecimal +void print_hex(char fw_string[STR_LEN]) +{ + printf("["); + for (int i=0; iUserIdentifier), - trim(pApplicationContext->EffectiveUserID), - trim(pApplicationContext->ApplName), - trim(csp_user), + "User authentication failed due to invalid user. user=%.*s effuser=%.*s applname=%.*s csp_user=%s cc=%d reason=%d", + trimmed_len(pIdentityContext->UserIdentifier, MQ_USER_ID_LENGTH), + pIdentityContext->UserIdentifier, + trimmed_len(pApplicationContext->EffectiveUserID, MQ_USER_ID_LENGTH), + pApplicationContext->EffectiveUserID, + trimmed_len(pApplicationContext->ApplName, MQ_APPL_NAME_LENGTH), + pApplicationContext->ApplName, + csp_user, *pCompCode, *pReason); } @@ -192,11 +193,14 @@ static void MQENTRY mqhtpass_authenticate_user_csp( // Tell the queue manager to stop trying other authorization services. *pContinuation = MQZCI_STOP; log_debugf( - "User authentication failed due to invalid password. user=%s effuser=%s applname=%s csp_user=%s cc=%d reason=%d", - trim(pIdentityContext->UserIdentifier), - trim(pApplicationContext->EffectiveUserID), - trim(pApplicationContext->ApplName), - trim(csp_user), + "User authentication failed due to invalid password. user=%.*s effuser=%.*s applname=%.*s csp_user=%s cc=%d reason=%d", + trimmed_len(pIdentityContext->UserIdentifier, MQ_USER_ID_LENGTH), + pIdentityContext->UserIdentifier, + trimmed_len(pApplicationContext->EffectiveUserID, MQ_USER_ID_LENGTH), + pApplicationContext->EffectiveUserID, + trimmed_len(pApplicationContext->ApplName, MQ_APPL_NAME_LENGTH), + pApplicationContext->ApplName, + csp_user, *pCompCode, *pReason); } @@ -275,11 +279,14 @@ static void MQENTRY mqhtpass_authenticate_user( else { log_debugf( - "User authentication failed user=%s effuser=%s applname=%s cspuser=%s cc=%d reason=%d", - trim(pIdentityContext->UserIdentifier), - trim(pApplicationContext->EffectiveUserID), - trim(pApplicationContext->ApplName), - trim(spuser), + "User authentication failed user=%.*s effuser=%.*s applname=%.*s cspuser=%s cc=%d reason=%d", + trimmed_len(pIdentityContext->UserIdentifier, MQ_USER_ID_LENGTH), + pIdentityContext->UserIdentifier, + trimmed_len(pApplicationContext->EffectiveUserID, MQ_USER_ID_LENGTH), + pApplicationContext->EffectiveUserID, + trimmed_len(pApplicationContext->ApplName, MQ_APPL_NAME_LENGTH), + pApplicationContext->ApplName, + spuser, *pCompCode, *pReason); } @@ -333,18 +340,3 @@ static void MQENTRY mqhtpass_terminate( *pReason = MQRC_NONE; } -/** - * Remove trailing spaces from a string. - */ -static char *trim(char *s) -{ - int i; - for (i = strlen(s) - 1; i >= 0; i--) - { - if (s[i] == ' ') - s[i] = 0; - else - break; - } - return s; -}