From a24258834ef69edc61d9c37f880052da793c6217 Mon Sep 17 00:00:00 2001 From: Arthur Barr Date: Thu, 7 Jan 2021 11:10:20 +0000 Subject: [PATCH] Better error handling in htpasswd CSP handling is now separate, and the MQ return codes are tidied up. Also added defaultIdentityTest to JMS tests and fixed copyright dates for htpasswd code --- Dockerfile-server | 2 +- Makefile | 2 +- authservice/mqhtpass/src/htpass.c | 14 +- authservice/mqhtpass/src/htpass.h | 9 +- authservice/mqhtpass/src/htpass_test.c | 51 +++--- authservice/mqhtpass/src/log.c | 2 +- authservice/mqhtpass/src/log.h | 2 +- authservice/mqhtpass/src/mqhtpass.c | 161 ++++++++++++------ cmd/runmqdevserver/main.go | 2 +- .../install-extra-packages.sh | 2 +- install-mq-server-prereqs.sh | 2 +- internal/htpasswd/htpasswd.go | 2 +- test/docker/devconfig_test.go | 1 + test/messaging/Dockerfile | 3 +- test/messaging/pom.xml | 8 +- .../com/ibm/mqcontainer/test/JMSTests.java | 84 +++++---- 16 files changed, 218 insertions(+), 129 deletions(-) diff --git a/Dockerfile-server b/Dockerfile-server index 10e2fbb..b8bf576 100644 --- a/Dockerfile-server +++ b/Dockerfile-server @@ -1,4 +1,4 @@ -# © Copyright IBM Corporation 2015, 2020 +# © Copyright IBM Corporation 2015, 2021 # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. diff --git a/Makefile b/Makefile index 136620d..2c65bc6 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -# © Copyright IBM Corporation 2017, 2020 +# © Copyright IBM Corporation 2017, 2021 # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. diff --git a/authservice/mqhtpass/src/htpass.c b/authservice/mqhtpass/src/htpass.c index c34ddd7..a4df731 100644 --- a/authservice/mqhtpass/src/htpass.c +++ b/authservice/mqhtpass/src/htpass.c @@ -1,5 +1,5 @@ /* -© Copyright IBM Corporation 2020 +© Copyright IBM Corporation 2021 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -20,6 +20,7 @@ limitations under the License. #include #include #include "log.h" +#include "htpass.h" #include #include #include @@ -51,9 +52,6 @@ bool htpass_valid_file(char *filename) valid = false; break; } - else { - - } } fclose(fp); if (line) @@ -106,12 +104,13 @@ char *find_hash(char *filename, char *user) return hash; } -bool htpass_authenticate_user(char *filename, char *user, char *password) +int htpass_authenticate_user(char *filename, char *user, char *password) { char *hash = find_hash(filename, user); - bool result = false; + int result = -1; if (hash == NULL) { + result = HTPASS_INVALID_USER; log_debugf("User does not exist. user=%s", user); } else @@ -122,11 +121,12 @@ bool htpass_authenticate_user(char *filename, char *user, char *password) // status is usually either APR_SUCCESS or APR_EMISMATCH if (status == APR_SUCCESS) { - result = true; + result = HTPASS_VALID; log_debugf("Correct password supplied. user=%s", user); } else { + result = HTPASS_INVALID_PASSWORD; log_debugf("Incorrect password supplied. user=%s", user); } } diff --git a/authservice/mqhtpass/src/htpass.h b/authservice/mqhtpass/src/htpass.h index a519702..305cd47 100644 --- a/authservice/mqhtpass/src/htpass.h +++ b/authservice/mqhtpass/src/htpass.h @@ -1,5 +1,5 @@ /* -© Copyright IBM Corporation 2020 +© Copyright IBM Corporation 2021 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -17,6 +17,10 @@ limitations under the License. #ifndef _HTPASS_H #define _HTPASS_H +#define HTPASS_VALID 0 +#define HTPASS_INVALID_USER 1 +#define HTPASS_INVALID_PASSWORD 2 + /** * Validate an HTPasswd file for use with IBM MQ. * @@ -30,8 +34,9 @@ _Bool htpass_valid_file(char *filename); * @param filename the HTPasswd file * @param user the user name to authenticate * @param password the password of the user + * @return HTPASS_VALID, HTPASS_INVALID_USER or HTPASS_INVALID_PASSWORD */ -_Bool htpass_authenticate_user(char *filename, char *user, char *password); +int htpass_authenticate_user(char *filename, char *user, char *password); /** * Validate that a user exists in the password file. diff --git a/authservice/mqhtpass/src/htpass_test.c b/authservice/mqhtpass/src/htpass_test.c index 8879783..30b611b 100644 --- a/authservice/mqhtpass/src/htpass_test.c +++ b/authservice/mqhtpass/src/htpass_test.c @@ -1,5 +1,5 @@ /* -© Copyright IBM Corporation 2020 +© Copyright IBM Corporation 2021 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -65,9 +65,9 @@ void test_htpass_valid_file_too_long() void test_htpass_authenticate_user_fred_valid() { test_start(); - int ok = htpass_authenticate_user("./src/htpass_test.htpasswd", "fred", "passw0rd"); - printf("%s: fred - %d\n", __func__, ok); - if (!ok) + int rc = htpass_authenticate_user("./src/htpass_test.htpasswd", "fred", "passw0rd"); + printf("%s: fred - %d\n", __func__, rc); + if (rc != HTPASS_VALID) test_fail(__func__); test_pass(); } @@ -75,9 +75,9 @@ void test_htpass_authenticate_user_fred_valid() void test_htpass_authenticate_user_fred_invalid1() { test_start(); - int ok = htpass_authenticate_user("./src/htpass_test.htpasswd", "fred", "passw0rd "); - printf("%s: fred - %d\n", __func__, ok); - if (ok) + int rc = htpass_authenticate_user("./src/htpass_test.htpasswd", "fred", "passw0rd "); + printf("%s: fred - %d\n", __func__, rc); + if (rc != HTPASS_INVALID_PASSWORD) test_fail(__func__); test_pass(); } @@ -85,9 +85,9 @@ void test_htpass_authenticate_user_fred_invalid1() void test_htpass_authenticate_user_fred_invalid2() { test_start(); - int ok = htpass_authenticate_user("./src/htpass_test.htpasswd", "fred", ""); - printf("%s: fred - %d\n", __func__, ok); - if (ok) + int rc = htpass_authenticate_user("./src/htpass_test.htpasswd", "fred", ""); + printf("%s: fred - %d\n", __func__, rc); + if (rc != HTPASS_INVALID_PASSWORD) test_fail(__func__); test_pass(); } @@ -95,9 +95,9 @@ void test_htpass_authenticate_user_fred_invalid2() void test_htpass_authenticate_user_fred_invalid3() { test_start(); - int ok = htpass_authenticate_user("./src/htpass_test.htpasswd", "fred", "clearlywrong"); - printf("%s: fred - %d\n", __func__, ok); - if (ok) + int rc = htpass_authenticate_user("./src/htpass_test.htpasswd", "fred", "clearlywrong"); + printf("%s: fred - %d\n", __func__, rc); + if (rc != HTPASS_INVALID_PASSWORD) test_fail(__func__); test_pass(); } @@ -105,9 +105,19 @@ void test_htpass_authenticate_user_fred_invalid3() void test_htpass_authenticate_user_barney_valid() { test_start(); - int ok = htpass_authenticate_user("./src/htpass_test.htpasswd", "barney", "s3cret"); - printf("%s: barney - %d\n", __func__, ok); - if (!ok) + int rc = htpass_authenticate_user("./src/htpass_test.htpasswd", "barney", "s3cret"); + printf("%s: barney - %d\n", __func__, rc); + if (rc != HTPASS_VALID) + test_fail(__func__); + test_pass(); +} + +void test_htpass_authenticate_user_unknown() +{ + test_start(); + int rc = htpass_authenticate_user("./src/htpass_test.htpasswd", "george", "s3cret"); + printf("%s: barney - %d\n", __func__, rc); + if (rc != HTPASS_INVALID_USER) test_fail(__func__); test_pass(); } @@ -127,11 +137,11 @@ void *authenticate_many_times(void *p) { for (int i = 0; i < NUM_TESTS_PER_THREAD; i++) { - int ok = htpass_authenticate_user("./src/htpass_test.htpasswd", "barney", "s3cret"); - if (!ok) + int rc = htpass_authenticate_user("./src/htpass_test.htpasswd", "barney", "s3cret"); + if (rc != HTPASS_VALID) test_fail(__func__); - ok = htpass_authenticate_user("./src/htpass_test.htpasswd", "fred", "passw0rd"); - if (!ok) + rc = htpass_authenticate_user("./src/htpass_test.htpasswd", "fred", "passw0rd"); + if (rc != HTPASS_VALID) test_fail(__func__); } pthread_exit(NULL); @@ -205,6 +215,7 @@ int main() test_htpass_authenticate_user_fred_invalid2(); test_htpass_authenticate_user_fred_invalid3(); test_htpass_authenticate_user_barney_valid(); + test_htpass_authenticate_user_unknown(); log_close(); // Call multi-threaded test last, because it re-initializes the log to use a file diff --git a/authservice/mqhtpass/src/log.c b/authservice/mqhtpass/src/log.c index 84a8205..70675c0 100644 --- a/authservice/mqhtpass/src/log.c +++ b/authservice/mqhtpass/src/log.c @@ -1,5 +1,5 @@ /* -© Copyright IBM Corporation 2020 +© Copyright IBM Corporation 2021 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/authservice/mqhtpass/src/log.h b/authservice/mqhtpass/src/log.h index a0527e8..fa92135 100644 --- a/authservice/mqhtpass/src/log.h +++ b/authservice/mqhtpass/src/log.h @@ -1,5 +1,5 @@ /* -© Copyright IBM Corporation 2020 +© Copyright IBM Corporation 2021 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/authservice/mqhtpass/src/mqhtpass.c b/authservice/mqhtpass/src/mqhtpass.c index 854d1f9..516e6ce 100644 --- a/authservice/mqhtpass/src/mqhtpass.c +++ b/authservice/mqhtpass/src/mqhtpass.c @@ -1,5 +1,5 @@ /* -© Copyright IBM Corporation 2020 +© Copyright IBM Corporation 2021 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -111,8 +111,109 @@ void MQENTRY MQStart( return; } +/** + * Called during the connection of any application which supplies an MQCSP (Connection Security Parameters). + * This is the usual case. + * See https://www.ibm.com/support/knowledgecenter/SSFKSJ_latest/com.ibm.mq.ref.dev.doc/q095610_.html + */ +static void MQENTRY mqhtpass_authenticate_user_csp( + PMQCHAR pQMgrName, + PMQCSP pSecurityParms, + PMQZAC pApplicationContext, + PMQZIC pIdentityContext, + PMQPTR pCorrelationPtr, + PMQBYTE pComponentData, + PMQLONG pContinuation, + PMQLONG pCompCode, + PMQLONG pReason) +{ + char *csp_user = NULL; + char *csp_pass = NULL; + + // Firstly, create null-terminated strings from the user credentials in the MQ CSP object + csp_user = malloc(pSecurityParms->CSPUserIdLength + 1); + if (!csp_user) + { + log_errorf("%s is unable to allocate memory for a user", NAME); + *pCompCode = MQCC_FAILED; + *pReason = MQRC_SERVICE_ERROR; + return; + } + strncpy(csp_user, pSecurityParms->CSPUserIdPtr, pSecurityParms->CSPUserIdLength); + csp_user[pSecurityParms->CSPUserIdLength] = 0; + csp_pass = malloc((pSecurityParms->CSPPasswordLength + 1)); + if (!csp_pass) + { + log_errorf("%s is unable to allocate memory for a password", NAME); + *pCompCode = MQCC_FAILED; + *pReason = MQRC_SERVICE_ERROR; + if (csp_user) + { + free(csp_user); + } + return; + } + strncpy(csp_pass, pSecurityParms->CSPPasswordPtr, pSecurityParms->CSPPasswordLength); + csp_pass[pSecurityParms->CSPPasswordLength] = 0; + log_debugf("%s with CSP user set. user=%s", __func__, csp_user); + int auth_result = htpass_authenticate_user(HTPASSWD_FILE, csp_user, csp_pass); + + if (auth_result == HTPASS_VALID) + { + // An OK completion code means MQ will accept this user is authenticated + *pCompCode = MQCC_OK; + *pReason = MQRC_NONE; + // Tell the queue manager to stop trying other authorization services. + *pContinuation = MQZCI_STOP; + memcpy(pIdentityContext->UserIdentifier, csp_user, sizeof(pIdentityContext->UserIdentifier)); + log_debugf("Authenticated user=%s", pIdentityContext->UserIdentifier); + } + // If the htpasswd file does not have an entry for this user + else if (auth_result == HTPASS_INVALID_USER) + { + *pCompCode = MQCC_WARNING; + *pReason = MQRC_NONE; + // Tell the queue manager to continue trying other authorization services, as they might have the user. + *pContinuation = MQZCI_CONTINUE; + log_debugf( + "User authentication failed due to invalid user. 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), + *pCompCode, + *pReason); + } + // If the htpasswd file has an entry for this user, but the password supplied is incorrect + else if (auth_result == HTPASS_INVALID_PASSWORD) + { + *pCompCode = MQCC_WARNING; + *pReason = MQRC_NOT_AUTHORIZED; + // 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), + *pCompCode, + *pReason); + } + if (csp_user) + { + free(csp_user); + } + if (csp_pass) + { + free(csp_pass); + } + return; +} + /** * Called during the connection of any application. + * For more information on the parameters, see https://www.ibm.com/support/knowledgecenter/en/SSFKSJ_latest/com.ibm.mq.ref.dev.doc/q110090_.html */ static void MQENTRY mqhtpass_authenticate_user( PMQCHAR pQMgrName, @@ -137,59 +238,7 @@ static void MQENTRY mqhtpass_authenticate_user( if ((pSecurityParms->AuthenticationType) == MQCSP_AUTH_USER_ID_AND_PWD) { - // Authenticating a user ID and password. - - // Firstly, create null-terminated strings from the user credentials in the MQ CSP object - spuser = malloc(pSecurityParms->CSPUserIdLength + 1); - if (!spuser) - { - log_errorf("%s is unable to allocate memory for a user", NAME); - return; - } - strncpy(spuser, pSecurityParms->CSPUserIdPtr, pSecurityParms->CSPUserIdLength); - spuser[pSecurityParms->CSPUserIdLength] = 0; - sppass = malloc((pSecurityParms->CSPPasswordLength + 1)); - if (!sppass) - { - log_errorf("%s is unable to allocate memory for a password", NAME); - if (spuser) - { - free(spuser); - } - return; - } - strncpy(sppass, pSecurityParms->CSPPasswordPtr, pSecurityParms->CSPPasswordLength); - sppass[pSecurityParms->CSPPasswordLength] = 0; - log_debugf("%s with CSP user set. user=%s", __func__, spuser); - bool authenticated = htpass_authenticate_user(HTPASSWD_FILE, spuser, sppass); - - if (authenticated) - { - *pCompCode = MQCC_OK; - *pReason = MQRC_NONE; - *pContinuation = MQZCI_CONTINUE; - memcpy(pIdentityContext->UserIdentifier, spuser, sizeof(pIdentityContext->UserIdentifier)); - log_debugf("Authenticated user=%s", pIdentityContext->UserIdentifier); - } - 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), - *pCompCode, - *pReason); - } - if (spuser) - { - free(spuser); - } - if (sppass) - { - free(sppass); - } + mqhtpass_authenticate_user_csp(pQMgrName, pSecurityParms, pApplicationContext, pIdentityContext, pCorrelationPtr, pComponentData, pContinuation, pCompCode, pReason); } else { @@ -198,6 +247,8 @@ static void MQENTRY mqhtpass_authenticate_user( if (!spuser) { log_errorf("%s is unable to allocate memory to check a user", NAME); + *pCompCode = MQCC_FAILED; + *pReason = MQRC_SERVICE_ERROR; return; } strncpy(spuser, pApplicationContext->EffectiveUserID, strlen(pApplicationContext->EffectiveUserID)); @@ -219,7 +270,7 @@ static void MQENTRY mqhtpass_authenticate_user( // An OK completion code means MQ will accept this user is authenticated *pCompCode = MQCC_OK; *pReason = MQRC_NONE; - *pContinuation = MQZCI_CONTINUE; + *pContinuation = MQZCI_STOP; memcpy(pIdentityContext->UserIdentifier, spuser, sizeof(pIdentityContext->UserIdentifier)); } else diff --git a/cmd/runmqdevserver/main.go b/cmd/runmqdevserver/main.go index d856e50..ff5a1a6 100644 --- a/cmd/runmqdevserver/main.go +++ b/cmd/runmqdevserver/main.go @@ -1,5 +1,5 @@ /* -© Copyright IBM Corporation 2018, 2020 +© Copyright IBM Corporation 2018, 2021 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/incubating/mqadvanced-server-dev/install-extra-packages.sh b/incubating/mqadvanced-server-dev/install-extra-packages.sh index d8518cf..9fd3414 100644 --- a/incubating/mqadvanced-server-dev/install-extra-packages.sh +++ b/incubating/mqadvanced-server-dev/install-extra-packages.sh @@ -1,6 +1,6 @@ #!/bin/bash # -*- mode: sh -*- -# © Copyright IBM Corporation 2019 +# © Copyright IBM Corporation 2019, 2021 # # # Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/install-mq-server-prereqs.sh b/install-mq-server-prereqs.sh index e5d4c99..bf96f7d 100644 --- a/install-mq-server-prereqs.sh +++ b/install-mq-server-prereqs.sh @@ -1,6 +1,6 @@ #!/bin/bash # -*- mode: sh -*- -# © Copyright IBM Corporation 2015, 2020 +# © Copyright IBM Corporation 2015, 2021 # # # Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/internal/htpasswd/htpasswd.go b/internal/htpasswd/htpasswd.go index e96fb84..8d8a917 100644 --- a/internal/htpasswd/htpasswd.go +++ b/internal/htpasswd/htpasswd.go @@ -1,5 +1,5 @@ /* -© Copyright IBM Corporation 2020 +© Copyright IBM Corporation 2020, 2021 Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/test/docker/devconfig_test.go b/test/docker/devconfig_test.go index 6634a71..5a53808 100644 --- a/test/docker/devconfig_test.go +++ b/test/docker/devconfig_test.go @@ -43,6 +43,7 @@ func TestDevGoldenPath(t *testing.T) { Env: []string{ "LICENSE=accept", "MQ_QMGR_NAME=" + qm, + "DEBUG=true", }, } id := runContainerWithPorts(t, cli, &containerConfig, []int{9443}) diff --git a/test/messaging/Dockerfile b/test/messaging/Dockerfile index 4dc87fc..92d78c1 100644 --- a/test/messaging/Dockerfile +++ b/test/messaging/Dockerfile @@ -1,4 +1,4 @@ -# © Copyright IBM Corporation 2018, 2019 +# © Copyright IBM Corporation 2018, 2021 # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -33,4 +33,5 @@ RUN find /usr/src/mymaven FROM docker.io/ibmjava:8-jre COPY --from=builder /usr/src/mymaven/target/*.jar /opt/app/ COPY --from=builder /usr/src/mymaven/target/lib/*.jar /opt/app/ +USER 1001 ENTRYPOINT ["java", "-classpath", "/opt/app/*", "org.junit.platform.console.ConsoleLauncher", "-p", "com.ibm.mqcontainer.test", "--details", "verbose"] diff --git a/test/messaging/pom.xml b/test/messaging/pom.xml index ee9a9be..55232f4 100644 --- a/test/messaging/pom.xml +++ b/test/messaging/pom.xml @@ -1,5 +1,5 @@