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

Curve customization support: extract curve array from yaml and make a list of mapped number strings #721

Open
wants to merge 3 commits into
base: v2.x/staging
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ All notable changes to the ZSS package will be documented in this file.

## `2.18.0`
- Change log level for setting default value of 'httpRequestHeapMaxBlocks' to DEBUG instead of INFO.(#719)
- Enhancement: Curve customization support from array 'zowe.network.server.tls.curves' in zowe.yaml, only curves mentioned in https://www.ibm.com/docs/en/zos/3.1.0?topic=programming-cipher-suite-definitions#csdcwh__tttcsd are supported currently (#721).

## `2.17.0`
- Code to configure the SLH block size of the http server through 'httpRequestHeapMaxBlocks' in the yaml.(#701)
Expand Down
37 changes: 37 additions & 0 deletions c/zss.c
Original file line number Diff line number Diff line change
Expand Up @@ -1171,6 +1171,7 @@ static char* generateCookieNameV2(ConfigManager *configmgr, int port) {
#define ENV_AGENT_HTTPS_KEY(key) AGENT_HTTPS_PREFIX key

TLS_IANA_CIPHER_MAP(ianaCipherMap)
TLS_IANA_CURVE_MAP(ianaCurveMap)

static bool readAgentHttpsSettingsV2(ShortLivedHeap *slh,
ConfigManager *configmgr,
Expand Down Expand Up @@ -1226,6 +1227,42 @@ static bool readAgentHttpsSettingsV2(ShortLivedHeap *slh,

}

Json *tlsConfig = NULL;
int tlsGetStatus = cfgGetAnyC(configmgr,ZSS_CFGNAME,&tlsConfig, 4,"zowe","network","server","tls");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you in some cases use blanks between arguments and in some cases you don't?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing that out Irek, I will make correct this.

if (tlsGetStatus) {
zowelog(NULL, LOG_COMP_ID_MVD_SERVER, ZOWE_LOG_INFO, "TLS is NOT configured for this ZSS\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this have a message ID?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure on that because elsewhere is the code I can see its a similar message without any ID as such.
Thanks

} else {
JsonObject *tlsConfigObject = jsonAsObject(tlsConfig);
Json *curveJson = jsonObjectGetPropertyValue(tlsConfigObject, "curves");
char *curves = NULL;
if(jsonIsArray(curveJson)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if there is no curves in that JSON? I think you'll dereference a NULL pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes Irek that's true, thanks for pointing that one.

JsonArray *curveArray = jsonObjectGetArray(tlsConfigObject, "curves");
Gautham-coder marked this conversation as resolved.
Show resolved Hide resolved
int count = jsonArrayGetCount(curveArray);
int curveCharLength = 4;
Copy link
Contributor

Choose a reason for hiding this comment

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

Make this const to indicate intention.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure Irek.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it 4? Can you tie it to some struct's field size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We know that the length will always be 4. Its better not to put in the struct field because we are initializing the array of structs in zowe-common-c/h/tls.h , but we can make it as a constant as you recommended earlier.

curves = (char *)safeMalloc((sizeof(char) * curveCharLength * count)+1, "curve list");
Copy link
Contributor

Choose a reason for hiding this comment

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

NULL check missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Irek, I will correct it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you casting this to char *?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I think this is redundant, I will remove the typecast. Thanks

for (int i = 0; i < count; i++) {
char *ianaName = jsonArrayGetString(curveArray, i);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be NULL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is defined as a string so it can be NULL, I will add a check for this.
Thanks

zowelog(NULL, LOG_COMP_ID_MVD_SERVER, ZOWE_LOG_DEBUG, "curve request=%s\n", ianaName);
CurveMap *curve = (CurveMap *)ianaCurveMap;
bool found = false;
while (curve->groupId != NULL) {
if (!strcmp(ianaName, curve->name)) {
strcat(curves, curve->groupId);
Copy link
Contributor

Choose a reason for hiding this comment

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

How do you know groupId is exactly 4?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here: (https://www.ibm.com/docs/en/zos/3.1.0?topic=reference-gsk-attribute-set-buffer)
image

It says 1 or more 4-character decimal values.

zowelog(NULL, LOG_COMP_ID_MVD_SERVER, ZOWE_LOG_DEBUG, "Curve match=%s\n", curve->groupId);
found = true;
break;
}
++curve;
}
if (!found) {
zowelog(NULL, LOG_COMP_ID_MVD_SERVER, ZOWE_LOG_WARNING, ZSS_LOG_CURVE_INVALID_MSG, ianaName);
}
}
zowelog(NULL, LOG_COMP_ID_MVD_SERVER, ZOWE_LOG_DEBUG, "Curve array is %s\n", curves);
settings->curves = curves;
}
}

ECVT *ecvt = getECVT();
/*
2.3 (1020300) no tls 1.3
Expand Down
2 changes: 1 addition & 1 deletion deps/zowe-common-c
6 changes: 6 additions & 0 deletions h/zssLogging.h
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,12 @@ bool isLogLevelValid(int level);
#define ZSS_LOG_CIPHER_INVALID_MSG_TEXT "Requested cipher '%s' not available.\n"
#define ZSS_LOG_CIPHER_INVALID_MSG ZSS_LOG_CIPHER_INVALID_MSG_ID" "ZSS_LOG_CIPHER_INVALID_MSG_TEXT

#ifndef ZSS_LOG_CURVE_INVALID_MSG_ID
#define ZSS_LOG_CURVE_INVALID_MSG_ID ZSS_LOG_MSG_PRFX"1067W"
#endif
#define ZSS_LOG_CURVE_INVALID_MSG_TEXT "Requested curve '%s' not supported.\n"
#define ZSS_LOG_CURVE_INVALID_MSG ZSS_LOG_CURVE_INVALID_MSG_ID" "ZSS_LOG_CURVE_INVALID_MSG_TEXT


/* registerProduct */

Expand Down
Loading