-
Notifications
You must be signed in to change notification settings - Fork 45
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
base: v1.x/staging
Are you sure you want to change the base?
Changes from all commits
3a8e00b
94742a0
278dc2b
35fe4be
f4fb448
9a191ad
2b271d7
40c0770
7df8380
21eb96d
038b915
0d154ad
c91b2dc
54a35c5
a618f31
a6ef66e
245cdbc
98cd911
a7a4e1e
b0f8914
2b55af4
63ce32f
a132c8b
0c1b0f5
4c50186
8da7246
f11219f
efde2a2
0add3ec
3eaf7a0
f35aa89
357f0d0
06a16ef
86ee872
21b1496
970cd23
8a7ad43
60f1e4c
4beeca2
c537201
371319a
a9f1ed1
7840318
954f921
11ff99f
9590ce1
ac967f2
5ade5c9
ef8b441
ef78692
894bc65
f0d839a
5f4f553
f9e437d
1a63674
961b7a7
f774e80
a4ad82a
f3b8d91
ddf0408
e2ed56e
47a8af9
4fc145c
542f458
414466c
ad39d3b
3b10697
918bf99
6b1f6b4
b83e85e
42acd8e
a4b56df
909ecb2
0899ef7
7c6cd42
acf074c
46e527d
ea23113
65e909e
6a66374
47b6665
8160346
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,8 +35,9 @@ | |
#include "httpserver.h" | ||
#include "zssLogging.h" | ||
|
||
#define SAF_CLASS "ZOWE" | ||
#define JSON_ERROR_BUFFER_SIZE 1024 | ||
#define JSON_ERROR_BUFFER_SIZE 1024 | ||
#define STRING_BUFFER_SIZE 1024 | ||
#define SAF_SUB_URL_SIZE 32 | ||
|
||
#define SAF_PASSWORD_RESET_RC_OK 0 | ||
#define SAF_PASSWORD_RESET_RC_WRONG_PASSWORD 111 | ||
|
@@ -45,7 +46,10 @@ | |
#define SAF_PASSWORD_RESET_RC_NO_NEW_PASSSWORD 168 | ||
#define SAF_PASSWORD_RESET_RC_WRONG_FORMAT 169 | ||
|
||
#define RESPONSE_MESSAGE_LENGTH 100 | ||
#define RESPONSE_MESSAGE_LENGTH 100 | ||
|
||
#define SAF_PLUGIN_ID "ORG.ZOWE.CONFIGJS" | ||
#define SAF_SERVICE_NAME "DATA" | ||
|
||
/* | ||
* A handler performing the SAF_AUTH check: checks if the user has the | ||
|
@@ -66,6 +70,26 @@ | |
|
||
static int serveAuthCheck(HttpService *service, HttpResponse *response); | ||
|
||
static int makeProfileName( | ||
char *profileName, | ||
int profileNameBufSize, | ||
const char *type, | ||
const char *productCode, | ||
int instanceID, | ||
const char *pluginID, | ||
const char *rootServiceName, | ||
const char *serviceName, | ||
const char *method, | ||
const char *scope, | ||
char subUrl[SAF_SUB_URL_SIZE][STRING_BUFFER_SIZE]); | ||
|
||
static void setProfileNameAttribs( | ||
char *pluginID, | ||
char *serviceName, | ||
char *type, | ||
char *scope, | ||
char subUrl[SAF_SUB_URL_SIZE][STRING_BUFFER_SIZE]); | ||
|
||
int installAuthCheckService(HttpServer *server) { | ||
// zowelog(NULL, 0, ZOWE_LOG_DEBUG2, "begin %s\n", | ||
// __FUNCTION__); | ||
|
@@ -74,6 +98,7 @@ int installAuthCheckService(HttpServer *server) { | |
httpService->authType = SERVICE_AUTH_NATIVE_WITH_SESSION_TOKEN; | ||
httpService->serviceFunction = &serveAuthCheck; | ||
httpService->runInSubtask = FALSE; | ||
httpService->authorizationType = SERVICE_AUTHORIZATION_TYPE_NONE; | ||
registerHttpService(server, httpService); | ||
// zowelog(NULL, 0, ZOWE_LOG_DEBUG2, "end %s\n", | ||
// __FUNCTION__); | ||
|
@@ -162,7 +187,7 @@ static int serveAuthCheck(HttpService *service, HttpResponse *res) { | |
int rc = 0, rsn = 0, safStatus = 0; | ||
ZISAuthServiceStatus reqStatus = {0}; | ||
CrossMemoryServerName *privilegedServerName; | ||
const char *userName = req->username, *class = SAF_CLASS; | ||
const char *userName = req->username, *class = ZOWE_SAF_CLASS; | ||
rc = extractQuery(req->parsedFile, &entity, &accessStr); | ||
if (rc != 0) { | ||
respondWithError(res, HTTP_STATUS_BAD_REQUEST, "Broken auth query"); | ||
|
@@ -173,16 +198,218 @@ 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); */ | ||
|
||
privilegedServerName = getConfiguredProperty(service->server, | ||
HTTP_SERVER_PRIVILEGED_SERVER_PROPERTY); | ||
rc = zisCheckEntity(privilegedServerName, userName, class, entity, access, | ||
&reqStatus); | ||
|
||
respond(res, rc, &reqStatus); | ||
return 0; | ||
} | ||
|
||
int verifyAccessToSafProfile(HttpServer *server, const char *userName, const char *entity, const int access) { | ||
CrossMemoryServerName *privilegedServerName = getConfiguredProperty(server, HTTP_SERVER_PRIVILEGED_SERVER_PROPERTY); | ||
ZISAuthServiceStatus reqStatus = {0}; | ||
const char *class = ZOWE_SAF_CLASS; | ||
|
||
int rc = zisCheckEntity(privilegedServerName, userName, class, entity, access, &reqStatus); | ||
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_DEBUG2, | ||
"verifyAccessToSafProfile entity '%s' class '%s' access '%d' , rc: %d\n", entity, class, access, rc); | ||
|
||
return (rc != RC_ZIS_SRVC_OK) ? -1 : 0; | ||
} | ||
|
||
int getProfileNameFromRequest(char *profileName, const int profileNameBufSize, StringList *parsedFile, const char *method, int instanceID) { | ||
char type[STRING_BUFFER_SIZE] = {0}; // core || config || service | ||
char productCode[STRING_BUFFER_SIZE] = {0}; | ||
char rootServiceName[STRING_BUFFER_SIZE] = {0}; | ||
char subUrl[SAF_SUB_URL_SIZE][STRING_BUFFER_SIZE] = {0}; | ||
char scope[STRING_BUFFER_SIZE] = {0}; | ||
char pluginID[STRING_BUFFER_SIZE] = {0}; | ||
char serviceName[STRING_BUFFER_SIZE] = {0}; | ||
char urlSegment[STRING_BUFFER_SIZE] = {0}; | ||
int subUrlIndex = 0; | ||
bool isRootServiceNameInited = false; | ||
|
||
snprintf(urlSegment, sizeof(urlSegment), "%s", stringListPrint(parsedFile, 1, 1, "/", 0)); | ||
StringListElt *pathSegment = firstStringListElt(parsedFile); | ||
|
||
strupcase(urlSegment); | ||
DivergentEuropeans marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (instanceID < 0) { // Set instanceID | ||
instanceID = 0; | ||
} | ||
if (strcmp(urlSegment, "PLUGINS") != 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please clarify the logic? Also, examples of different cases would be really helpful. |
||
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_DEBUG2, | ||
"parsedFile urlSegment check didn't match.\n"); | ||
subUrlIndex = -1; | ||
while (pathSegment != NULL) { | ||
snprintf(urlSegment, sizeof(urlSegment), "%s", pathSegment->string); | ||
strupcase(urlSegment); | ||
if (!isRootServiceNameInited) { | ||
snprintf(rootServiceName, sizeof(rootServiceName), urlSegment); | ||
isRootServiceNameInited = true; | ||
} else { //If URL subsections > SAF_SUB_URL_SIZE, we trim them from profile name (by not appending them) | ||
if (subUrlIndex < SAF_SUB_URL_SIZE) { | ||
DivergentEuropeans marked this conversation as resolved.
Show resolved
Hide resolved
|
||
snprintf(subUrl[subUrlIndex], sizeof(subUrl), urlSegment); | ||
} | ||
} | ||
subUrlIndex++; | ||
pathSegment = pathSegment->next; | ||
} | ||
snprintf(productCode, sizeof(productCode), "ZLUX"); | ||
snprintf(type, sizeof(type), "core"); | ||
} else { | ||
subUrlIndex = 0; | ||
|
||
while (pathSegment != NULL) { | ||
snprintf(urlSegment, sizeof(urlSegment), "%s", pathSegment->string); | ||
strupcase(urlSegment); | ||
switch(subUrlIndex) { | ||
case 0: | ||
snprintf(productCode, sizeof(productCode), urlSegment); | ||
break; | ||
case 1: | ||
break; | ||
case 2: | ||
snprintf(pluginID, sizeof(pluginID), urlSegment); | ||
break; | ||
case 3: | ||
break; | ||
case 4: | ||
snprintf(serviceName, sizeof(serviceName), urlSegment); | ||
break; | ||
Comment on lines
+269
to
+281
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If these cases are never entered, these buffers will stay uninitialized, this will cause issues in |
||
case 5: | ||
break; | ||
default: { | ||
int adjustedSubUrlIndex = subUrlIndex - 6; // subtract 6 from maximum index to begin init subUrl array at 0 | ||
if (adjustedSubUrlIndex < SAF_SUB_URL_SIZE) { | ||
snprintf(subUrl[adjustedSubUrlIndex], sizeof(subUrl), urlSegment); | ||
} | ||
} | ||
} | ||
subUrlIndex++; | ||
pathSegment = pathSegment->next; | ||
} | ||
|
||
setProfileNameAttribs(pluginID, serviceName, type, scope, subUrl); | ||
int pluginIDLen = strlen(pluginID); | ||
for (int index = 0; index < pluginIDLen; index++) { | ||
if (pluginID[index] == '.') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
pluginID[index] = '_'; | ||
} | ||
} | ||
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_DEBUG2, | ||
"parsedFile urlSegment check OK.\n"); | ||
} | ||
return makeProfileName(profileName, profileNameBufSize, | ||
type, | ||
productCode, | ||
instanceID, | ||
pluginID, | ||
rootServiceName, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
serviceName, | ||
method, | ||
scope, | ||
subUrl); | ||
} | ||
|
||
static void setProfileNameAttribs( | ||
char *pluginID, | ||
const char *serviceName, | ||
char *type, | ||
char *scope, | ||
char subUrl[SAF_SUB_URL_SIZE][STRING_BUFFER_SIZE]) { | ||
if ((strcmp(pluginID, SAF_PLUGIN_ID) == 0) && (strcmp(serviceName, SAF_SERVICE_NAME) == 0)) | ||
{ | ||
snprintf(type, STRING_BUFFER_SIZE, "config"); | ||
snprintf(pluginID, STRING_BUFFER_SIZE, subUrl[0]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we overwrite the |
||
snprintf(scope, STRING_BUFFER_SIZE, subUrl[1]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why |
||
|
||
} else { | ||
snprintf(type, STRING_BUFFER_SIZE, "service"); | ||
} | ||
} | ||
|
||
static int makeProfileName( | ||
char *profileName, | ||
DivergentEuropeans marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const int profileNameBufSize, | ||
const char *type, | ||
const char *productCode, | ||
int instanceID, | ||
const char *pluginID, | ||
const char *rootServiceName, | ||
const char *serviceName, | ||
const char *method, | ||
const char *scope, | ||
char subUrl[SAF_SUB_URL_SIZE][STRING_BUFFER_SIZE]) { | ||
if (instanceID == -1) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it ever if (instanceID < 0) { // Set instanceID
instanceID = 0;
} |
||
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING, | ||
"Broken SAF query. Missing instance ID.\n"); | ||
return -1; | ||
} | ||
if (method == NULL) { | ||
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING, | ||
"Broken SAF query. Missing method.\n"); | ||
return -1; | ||
} | ||
int pos = 0; | ||
if (strcmp(type, "service") == 0) { | ||
if (pluginID == NULL) { | ||
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.\n"); | ||
return -1; | ||
} | ||
if (serviceName == NULL) { | ||
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING, | ||
"Broken SAF query. Missing service name.\n"); | ||
return -1; | ||
} | ||
pos = snprintf(profileName, profileNameBufSize, "%s.%d.SVC.%s.%s.%s", productCode, instanceID, pluginID, serviceName, method); | ||
} else if (strcmp(type, "config") == 0) { | ||
if (pluginID == NULL) { | ||
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING, | ||
"Broken SAF query. Missing plugin ID.\n"); | ||
return -1; | ||
} | ||
if (scope == NULL) { | ||
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING, | ||
"Broken SAF query. Missing scope.\n"); | ||
return -1; | ||
} | ||
pos = snprintf(profileName, profileNameBufSize, "%s.%d.CFG.%s.%s.%s", productCode, instanceID, pluginID, method, scope); | ||
} else if (strcmp(type, "core") == 0) { | ||
if (rootServiceName == NULL) { | ||
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING, | ||
"Broken SAF query. Missing root service name.\n"); | ||
return -1; | ||
} | ||
pos = snprintf(profileName, profileNameBufSize, "%s.%d.COR.%s.%s", productCode, instanceID, method, rootServiceName); | ||
} else if (pos < 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be just a standalone if-statement? |
||
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING, | ||
"Internal string encoding error.\n"); | ||
return -1; | ||
} | ||
// Child endpoints housed via subUrl | ||
int index = 0; | ||
while (index < SAF_SUB_URL_SIZE && strcmp(subUrl[index], "") != 0) { | ||
if (pos >= profileNameBufSize) { | ||
break; | ||
} | ||
pos += snprintf(profileName + pos, profileNameBufSize - pos, ".%s", subUrl[index]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to also check if the result is < 0 since we're doing that for the other calls? |
||
index++; | ||
} | ||
if (pos >= profileNameBufSize) { | ||
char errMsg[256]; | ||
snprintf(errMsg, sizeof(errMsg), "Generated SAF query longer than %d\n", profileNameBufSize - 1); | ||
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING, errMsg); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you use the following instead of writing to zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_WARNING,
"Generated SAF query longer than %d\n", profileNameBufSize - 1); |
||
return -1; | ||
} | ||
zowelog(NULL, LOG_COMP_ID_SECURITY, ZOWE_LOG_DEBUG2, | ||
"Finished generating profileName: %s\n", profileName); | ||
return 0; | ||
} | ||
|
||
void respondWithJsonStatus(HttpResponse *response, const char *status, int statusCode, const char *statusMessage) { | ||
jsonPrinter *out = respondWithJsonPrinter(response); | ||
setResponseStatus(response,statusCode,(char *)statusMessage); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -188,6 +188,7 @@ void installDatasetContentsService(HttpServer *server) { | |
|
||
HttpService *httpService = makeGeneratedService("datasetContents", "/datasetContents/**"); | ||
httpService->authType = SERVICE_AUTH_NATIVE_WITH_SESSION_TOKEN; | ||
httpService->authorizationType = SERVICE_AUTHORIZATION_TYPE_NONE; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do 3rd party plugins have to do this as well and then rebuild the binaries, or will everything work without recompilation? |
||
httpService->runInSubtask = TRUE; | ||
httpService->doImpersonation = TRUE; | ||
httpService->serviceFunction = serveDatasetContents; | ||
|
@@ -201,6 +202,7 @@ void installVSAMDatasetContentsService(HttpServer *server) { | |
|
||
HttpService *httpService = makeGeneratedService("VSAMdatasetContents", "/VSAMdatasetContents/**"); | ||
httpService->authType = SERVICE_AUTH_NATIVE_WITH_SESSION_TOKEN; | ||
httpService->authorizationType = SERVICE_AUTHORIZATION_TYPE_NONE; | ||
httpService->runInSubtask = TRUE; | ||
httpService->doImpersonation = TRUE; | ||
httpService->serviceFunction = serveVSAMDatasetContents; | ||
|
@@ -219,6 +221,7 @@ void installDatasetMetadataService(HttpServer *server) { | |
|
||
HttpService *httpService = makeGeneratedService("datasetMetadata", "/datasetMetadata/**"); | ||
httpService->authType = SERVICE_AUTH_NATIVE_WITH_SESSION_TOKEN; | ||
httpService->authorizationType = SERVICE_AUTHORIZATION_TYPE_NONE; | ||
httpService->runInSubtask = TRUE; | ||
httpService->doImpersonation = TRUE; | ||
httpService->serviceFunction = serveDatasetMetadata; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add
zowedump
forreqStatus
so you can debug what's happening.