Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RBAC checking via custom authHandler function + SAF query interpretation #281

Open
wants to merge 82 commits into
base: v1.x/staging
Choose a base branch
from
Open
Changes from 3 commits
Commits
Show all changes
82 commits
Select commit Hold shift + click to select a range
3a8e00b
Initial commit for SAF query interpretation methods
DivergentEuropeans Apr 13, 2021
94742a0
Increase buffer size for subUrl
DivergentEuropeans Apr 13, 2021
278dc2b
Changed buffer size for some other strings
DivergentEuropeans Apr 13, 2021
35fe4be
Code review comments regarding NULL & buffer size
DivergentEuropeans Apr 14, 2021
f4fb448
Added safeMalloc
DivergentEuropeans Apr 14, 2021
9a191ad
strcpy -> snprintf to avoid possible overflow
DivergentEuropeans Apr 14, 2021
2b271d7
Increased buffer for some variables
DivergentEuropeans Apr 14, 2021
40c0770
sprintf --> snprintf to avoid overflow
DivergentEuropeans Apr 14, 2021
7df8380
Small typo
DivergentEuropeans Apr 14, 2021
21eb96d
Changed snprintf method usage
DivergentEuropeans Apr 15, 2021
038b915
Revert
DivergentEuropeans Apr 15, 2021
0d154ad
Addressed more code review like NULL checks
DivergentEuropeans Apr 25, 2021
c91b2dc
Some prototype RBAC checking + unfinished comments
DivergentEuropeans May 24, 2021
54a35c5
Merge branch 'staging' of github.com:zowe/zss into RBAC-support
DivergentEuropeans May 24, 2021
a618f31
Annoying char change
DivergentEuropeans May 24, 2021
a6ef66e
Updated dep
DivergentEuropeans May 24, 2021
245cdbc
Added check for rbac: true | false
DivergentEuropeans May 25, 2021
98cd911
Merge github.com:zowe/zss into RBAC-support
DivergentEuropeans May 25, 2021
a7a4e1e
Updated zowe-common-c
DivergentEuropeans May 31, 2021
b0f8914
Moved core RBAC auth logic from zowe-common-c into ZSS
DivergentEuropeans Jun 3, 2021
2b55af4
Merge branch 'staging' of https://github.com/zowe/zss into RBAC-support
DivergentEuropeans Jun 3, 2021
63ce32f
Fixed merge conflicts
DivergentEuropeans Jun 5, 2021
a132c8b
Updated zowe-common-c
DivergentEuropeans Jun 7, 2021
0c1b0f5
Changed authType to string
DivergentEuropeans Jun 7, 2021
4c50186
Missed a few things
DivergentEuropeans Jun 7, 2021
8da7246
Removed comments
DivergentEuropeans Jun 7, 2021
f11219f
Updated zowe-common-c
DivergentEuropeans Jun 7, 2021
efde2a2
Removed leftover comments
DivergentEuropeans Jun 7, 2021
0add3ec
Code review adjustments from Irek
DivergentEuropeans Jun 11, 2021
3eaf7a0
Don't use URL query params in SAF query
DivergentEuropeans Jun 11, 2021
f35aa89
Changed #define value name to be more relevant
DivergentEuropeans Jun 11, 2021
357f0d0
Rename
DivergentEuropeans Jun 11, 2021
06a16ef
Removed regEx to use parsedFile instead
DivergentEuropeans Jun 16, 2021
86ee872
Re-added bypass services for NO RBAC
DivergentEuropeans Jun 18, 2021
21b1496
Removed comments + clean up unused variables
DivergentEuropeans Jun 18, 2021
970cd23
Some code cleanup
Jun 20, 2021
8a7ad43
Address code review
Jun 22, 2021
60f1e4c
More code cleanup
Jun 23, 2021
4beeca2
Respect env variable for RBAC too
DivergentEuropeans Jun 25, 2021
c537201
Added some useful debug logging
DivergentEuropeans Jun 25, 2021
371319a
Merge branch 'staging' of https://github.com/zowe/zss into RBAC-support
DivergentEuropeans Jun 25, 2021
a9f1ed1
Remove mistakenly added file
Jul 1, 2021
7840318
Update pointer to zowe-common-c
Jul 1, 2021
954f921
Remove SERVICE_AUTH_NATIVE_WITH_SESSION_TOKEN_NO_RBAC
Jul 1, 2021
11ff99f
Refactor authorization code
Jul 1, 2021
9590ce1
Register RBAC handler only if RBAC enabled
Jul 1, 2021
ac967f2
Refactoring + use rbacAuthorization only when username is on request
Jul 2, 2021
5ade5c9
Detect ZOWE_INSTANCE
Jul 2, 2021
ef8b441
Fix username check
Jul 2, 2021
ef78692
Rafactor SAF profile check
Jul 2, 2021
894bc65
ServerStatusServer has respect ZWED_dataserviceAuthentication_rbac en…
Jul 2, 2021
f0d839a
Minor fixes
Jul 5, 2021
5f4f553
Update pointer to zowe-common-c
Jul 5, 2021
f9e437d
Add const for getProfileNameFromRequest args
Jul 7, 2021
1a63674
Add profileNameBufSize arg
Jul 7, 2021
961b7a7
Add const for arguments
Jul 7, 2021
f774e80
Merge pull request #310 from lchudinov/rbac-code-cleanup
DivergentEuropeans Aug 19, 2021
a4ad82a
Merge branch 'staging' of github.com:zowe/zss into RBAC-support
DivergentEuropeans Aug 19, 2021
f3b8d91
Updated with more of Irek's suggested code changes
DivergentEuropeans Aug 23, 2021
ddf0408
Merge branch 'refactor-rbac-code' of https://github.com/lchudinov/zss…
DivergentEuropeans Aug 26, 2021
e2ed56e
Removed 'class' as an arg
DivergentEuropeans Aug 26, 2021
47a8af9
Merge branch 'staging' of github.com:zowe/zss into RBAC-support
DivergentEuropeans Aug 29, 2021
4fc145c
PassTicket REST API skeleton
Aug 6, 2021
542f458
Implement PassTicket generation
Aug 9, 2021
414466c
Refactoring for PassTicket
Aug 11, 2021
ad39d3b
Add passTicketService.c to build_zss.sh
Aug 11, 2021
3b10697
Update Changelog
Aug 11, 2021
918bf99
Cleanup includes
Aug 12, 2021
6b1f6b4
Merge branch 'feature/passticket-service' of https://github.com/lchud…
DivergentEuropeans Aug 30, 2021
b83e85e
Disable RBAC authorization for saf-auth service
Aug 31, 2021
42acd8e
Update pointer to zowe-common-c
Aug 31, 2021
a4b56df
Update pointer to zowe-common-c
Aug 31, 2021
909ecb2
Merge pull request #331 from lchudinov/feature/disable-rbac-authoriza…
lchudinov Aug 31, 2021
0899ef7
Fix buffer size
Aug 31, 2021
7c6cd42
Merge pull request #332 from lchudinov/bugfix/fix-buffer-size
lchudinov Aug 31, 2021
acf074c
Updated zowe-common-c
DivergentEuropeans Sep 24, 2021
46e527d
Merge branch 'RBAC-support' of github.com:zowe/zss into RBAC-support
DivergentEuropeans Sep 24, 2021
ea23113
Merge branch 'staging' of github.com:zowe/zss into RBAC-support
DivergentEuropeans Sep 24, 2021
65e909e
Updated zowe-common-c
DivergentEuropeans Sep 24, 2021
6a66374
Small changelog typo
DivergentEuropeans Sep 24, 2021
47b6665
Merge remote-tracking branch 'origin/staging' into RBAC-support
Nov 29, 2021
8160346
Adderss code review
Nov 29, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
211 changes: 209 additions & 2 deletions c/authService.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <iconv.h>
#include <dirent.h>
#include <pthread.h>
#include <regex.h>

#include "authService.h"
#include "zowetypes.h"
Expand Down Expand Up @@ -66,6 +67,19 @@

static int serveAuthCheck(HttpService *service, HttpResponse *response);

const char* getProfileNameFromRequest(char *url, const char *method, int instanceID);

const char* makeProfileName(
char *type,
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
char *productCode,
int instanceID,
char *pluginID,
char *rootServiceName,
char *serviceName,
char *method,
char *scope,
char subUrl[15][50]);

int installAuthCheckService(HttpServer *server) {
// zowelog(NULL, 0, ZOWE_LOG_DEBUG2, "begin %s\n",
// __FUNCTION__);
Expand All @@ -74,6 +88,7 @@ int installAuthCheckService(HttpServer *server) {
httpService->authType = SERVICE_AUTH_NATIVE_WITH_SESSION_TOKEN;
httpService->serviceFunction = &serveAuthCheck;
httpService->runInSubtask = FALSE;
// Test SAF query: getProfileNameFromRequest("/plugins", "GET", -1);
registerHttpService(server, httpService);
// zowelog(NULL, 0, ZOWE_LOG_DEBUG2, "end %s\n",
// __FUNCTION__);
Expand Down Expand Up @@ -173,16 +188,208 @@ static int serveAuthCheck(HttpService *service, HttpResponse *res) {
respondWithError(res, HTTP_STATUS_BAD_REQUEST, "Unexpected access level");
return 0;
}
/* printf("query: user %s, class %s, entity %s, access %d\n", userName, class,
entity, access); */
// printf("\n\nquery: user %s, class %s, entity %s, access %d accessStr %s\n", userName, class,
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
// entity, access, accessStr);
privilegedServerName = getConfiguredProperty(service->server,
HTTP_SERVER_PRIVILEGED_SERVER_PROPERTY);
rc = zisCheckEntity(privilegedServerName, userName, class, entity, access,
&reqStatus);
printf("\n\nprivileged server name: %s", privilegedServerName);
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
respond(res, rc, &reqStatus);
return 0;
}

const char* getProfileNameFromRequest(char *url, char *method, int instanceID) {
char type[8] = "NULL"; // core || config || service
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
char productCode[50] = "NULL";
char rootServiceName[50] = "NULL";
char subUrl[15][50];
char profileName[250];
char scope[50];
char _p[50] = "NULL", pluginID[50] = "NULL", _s[50] = "NULL", serviceName[50] = "NULL", _v[50] = "NULL";
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
char regexStr[] = "^/[A-Za-z0-9]*/plugins/";

regex_t regex;
int value;
value = regcomp(&regex, regexStr, REG_EXTENDED);

if (value != 0) {
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_DEBUG2,
"RegEx compiled successfully.");
} else {
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_DEBUG2,
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
"RegEx compilation error %s.", regexStr);
}
value = regexec(&regex, url, 0, NULL, 0);
char urlCpy[250];
strcpy(urlCpy, url);
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
int index = 0;
while (urlCpy[index]) { // Capitalize query
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
urlCpy[index] = toupper(urlCpy[index]);
index++;
}
if (instanceID < 0) { // Set instanceID
instanceID = 0;
}
if (value == REG_NOMATCH) {
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_DEBUG2,
"RegEx didn't match.");
char * token = strtok(urlCpy, "/");
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
int subUrlIndex = -1;
while( token != NULL ) {
if (strcmp(rootServiceName, "NULL") == 0)
{
strcpy(rootServiceName, token);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buffer overflow is possible.

} else {
strcpy(subUrl[subUrlIndex], token);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buffer overflow is possible.

}
subUrlIndex++;
token = strtok(NULL, "/");
}
strcpy(productCode, "ZLUX");
strcpy(type, "core");
}
else if (!value) {
char * token = strtok(urlCpy, "/");
int subUrlIndex;
subUrlIndex = 0;
while( token != NULL ) {
switch(subUrlIndex) {
case 0:
strcpy(productCode, token);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buffer overflow is possible.

break;
case 1:
strcpy(_p, token);
break;
case 2:
strcpy(pluginID, token);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buffer overflow is possible.

break;
case 3:
strcpy(_s, token);
break;
case 4:
strcpy(serviceName, token);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buffer overflow is possible.

break;
case 5:
strcpy(_v, token);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buffer overflow is possible.

break;
default:
strcpy(subUrl[subUrlIndex-6], token); // subtract 6 from maximum index to begin init subUrl array at 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buffer overflow is possible.

}

subUrlIndex++;
token = strtok(NULL, "/");
}
if ((strcmp(pluginID, "ORG.ZOWE.CONFIGJS") == 0) && (strcmp(serviceName, "DATA") == 0))
{
strcpy(type, "config");
strcpy(pluginID, subUrl[0]);
strcpy(scope, subUrl[1]);

} else {
strcpy(type, "service");
}
char* ch;
char* chReplace;
ch = ".";
chReplace = "_";
for (index = 0; index <= strlen(pluginID); index++)
{
if (pluginID[index] == *ch)
{
pluginID[index] = *chReplace;
}
}
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_DEBUG2,
"RegEx match OK.");
}
else {
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING,
"RegEx match failed.");
}
strcpy(profileName, makeProfileName(type,
productCode,
instanceID,
pluginID,
rootServiceName,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rootServiceName may be uninitialized.

serviceName,
method,
scope,
subUrl));
// printf("\n\nFinal query profileName & URL %s - %s\n\n", profileName, url);

/* Free memory allocated to the pattern buffer by regcomp() */
regfree(&regex);
return profileName;
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
}

const char* makeProfileName(
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
char *type,
char *productCode,
int instanceID,
char *pluginID,
char *rootServiceName,
char *serviceName,
char *method,
char *scope,
char subUrl[250][50]) {
char profileName[250] = "";
if (strcmp(productCode, "NULL") == 0) {
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING,
"Broken SAF query. Missing product code.");
return "NULL";
}
if (instanceID == -1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ever -1? getProfileNameFromRequest does the following:

  if (instanceID < 0) { // Set instanceID
    instanceID = 0;
  }

zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING,
"Broken SAF query. Missing instance ID.");
return "NULL";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't NULL pointer work here?

}
if (strcmp(method, "NULL") == 0) {
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING,
"Broken SAF query. Missing method.");
return "NULL";
}
// char someString[50] = { strcpy(*someString, type) };
if (strcmp(type, "service") == 0) {
if (strcmp(pluginID, "NULL") == 0) {
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that someone malicious will send multiple malformed requests flooding the log?

"Broken SAF query. Missing plugin ID.");
return "NULL";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

}
if (strcmp(serviceName, "NULL") == 0) {
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING,
"Broken SAF query. Missing service name.");
return "NULL";
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
}
snprintf(profileName, 1024, "%s.%d.SVC.%s.%s.%s", productCode, instanceID, pluginID, serviceName, method);
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
return profileName;

} else if (strcmp(type, "config") == 0) {
if (strcmp(pluginID, "NULL") == 0) {
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING,
"Broken SAF query. Missing plugin ID.");
return "NULL";
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
}
if (strcmp(scope, "NULL") == 0) {
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING,
"Broken SAF query. Missing scope.");
return "NULL";
}
snprintf(profileName, 1024, "%s.%d.CFG.%s.%s.%s", productCode, instanceID, pluginID, method, scope);
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
return profileName;
} else if (strcmp(type, "core") == 0) {
if (strcmp(rootServiceName, "NULL") == 0) {
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING,
"Broken SAF query. Missing root service name.");
return "NULL";
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
}
snprintf(profileName, 1024, "%s.%d.COR.%s.%s", productCode, instanceID, method, rootServiceName);
return profileName;
DivergentEuropeans marked this conversation as resolved.
Show resolved Hide resolved
}
}

/* Method goes here to do the same thing serveAuthCheck is doing except w/o input HttpService */

void respondWithJsonStatus(HttpResponse *response, const char *status, int statusCode, const char *statusMessage) {
jsonPrinter *out = respondWithJsonPrinter(response);
setResponseStatus(response,statusCode,(char *)statusMessage);
Expand Down