feat: Use networking standard config #3637

1000TurquoisePogs · 2024-07-09T10:28:17Z

Description

This PR attempts to implement configuring TLS according to the standard seen in this documentation https://github.com/zowe/docs-site/pull/3685/files#diff-411b1247ea1c2f1ca7f9db385604ed4a570f310b98aa375aed11184efe36e444R1

The standard has already been implemented by zss and app-server since v2.13, but I had uncertainty about how to configure the same things in APIML.

Discussion within #3601 revealed a solution which I implemented here.

Testing:

In firefox about:config, you can change max tls. When I changed it below what the server supports, I was unable to connect as expected.
I added and removed server ciphers to see which one would be selected, to confirm the list was being honored.
I set client ciphers in the same way to see how APIML would connect to zosmf when debug is enabled
I have NOT been able to test the "apiml.httpclient.ssl.enabled-protocols" version. It seems like when contacting zosmf, the value of "server.ssl.protocol" is used, so I'm not sure what the other variable is used for.

Linked to #3569

Type of change

feat: New feature (non-breaking change which adds functionality)

Checklist:

My code follows the style guidelines of this project
PR title conforms to commit message guideline ## Commit Message Structure Guideline
I have commented my code, particularly in hard-to-understand areas. In JS I did provide JSDoc
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
The java tests in the area I was working on leverage @nested annotations
Any dependent changes have been merged and published in downstream modules

…andardized way Signed-off-by: 1000TurquoisePogs <[email protected]>

Signed-off-by: 1000TurquoisePogs <[email protected]>

…at way Signed-off-by: 1000TurquoisePogs <[email protected]>

achmelo · 2024-07-09T14:58:41Z

api-catalog-package/src/main/resources/bin/start.sh

    -Dserver.ssl.enabled=${ZWE_configs_server_ssl_enabled:-true}  \
-    -Dserver.ssl.protocol=${ZWE_configs_server_ssl_protocol:-"TLSv1.2"}  \
+    -Dserver.ssl.protocol=${server_tls} \
+    -Dserver.ssl.ciphers=${ZWE_configs_zowe_network_server_tls_ciphers:-${ZWE_zowe_network_server_tls_ciphers:-}} \


does it make sense to have a default list of protocols?

do you mean protocols or ciphers?
for ciphers, it seems the default comes from within the server's code.
putting it here could be nice though - it would be more visible to users what the defaults are.

I am sorry, I meant ciphers. This property is setting ciphers for Tomcat and we don't have direct access to that part of the code. I think we can define them on the top of the script so it is better visible.

This line allows it to be possible to define such a standard cipher list within "defaults.yaml" here https://github.com/zowe/zowe-install-packaging/blob/v2.x/staging/files/defaults.yaml

An advantage of this is so that the default ciphers are visible in 1 place.
Otherwise you'd have defaults for APIML, app-server, and ZSS all in separate places that people don't know about (which is the current state)

Hi Sean, so if I understood correctly, there will be a new property zowe_network_server_tls_ciphers in this default.yaml, that's what you're proposing?

@1000TurquoisePogs , I would like to mention that there is a PR that is clashing with your one - #3689.

I understand you want to use the global configuration (I welcome it), but I'm not sure about a few things.

Why is the change not using the min tls? I assume we should construct 2 different values: protocol and enabledProtocols (see protocol=TLSv1.3, enabledProtocols=TLSv1.2,TLSv1.3). Then you are reusing the same structure as in the global configuration for the GW (and other components), even if we have a different structure of SSL configuration. Do you want to ensure that all components will use the same structure?

In all cases, we should merge your and min PR in the one new because they are modifying the same properties.

After a discussion with @taban03 we have decided to make changes in PR #3689. @1000TurquoisePogs , could you please check if it could be ok from zour point of view? It is just an example for GW now.

caching-service-package/src/main/resources/bin/start.sh

1000TurquoisePogs · 2024-07-10T12:26:11Z

api-catalog-package/src/main/resources/bin/start.sh

+client_max_tls=${ZWE_components_gateway_apiml_httpclient_ssl_enabled_protocols:-${ZWE_configs_zowe_network_client_tls_maxTls:-${ZWE_zowe_network_client_tls_maxTls:-${ZWE_configs_zowe_network_server_tls_maxTls:-${ZWE_zowe_network_server_tls_maxTls:-\
+"TLSv1.3"}}}}}
+client_min_tls=${ZWE_components_gateway_apiml_httpclient_ssl_enabled_protocols:-${ZWE_configs_zowe_network_client_tls_minTls:-${ZWE_zowe_network_client_tls_minTls:-${ZWE_configs_zowe_network_server_tls_minTls:-${ZWE_zowe_network_server_tls_minTls:-\
+"TLSv1.2"}}}}}
+
+server_max_tls=${ZWE_configs_server_ssl_protocol:-${ZWE_configs_zowe_network_server_tls_maxTls:-${ZWE_zowe_network_server_tls_maxTls:-"TLSv1.2,TLSv1.3"}}}
+server_min_tls=${ZWE_configs_server_ssl_protocol:-${ZWE_configs_zowe_network_server_tls_maxTls:-${ZWE_zowe_network_server_tls_maxTls:-"TLSv1.2,TLSv1.3"}}}


@achmelo defaults are here but... as you say I guess it cannot be a list?

yes, this would break the tomcat. we need to choose one version. I think that for Java it's OK to specify "TLS" only

Signed-off-by: 1000TurquoisePogs <[email protected]>

Signed-off-by: Pavel Jareš <[email protected]>

sonarcloud · 2024-09-03T07:26:33Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Signed-off-by: Pavel Jareš <[email protected]>

* draft Signed-off-by: Pavel Jareš <[email protected]> * fixes Signed-off-by: Pavel Jareš <[email protected]> * fix oidc Signed-off-by: Pavel Jareš <[email protected]> * corrections by zowe.yaml Signed-off-by: Pavel Jareš <[email protected]> * fixes Signed-off-by: Pavel Jareš <[email protected]> * fixes Signed-off-by: Pavel Jareš <[email protected]> * fixes Signed-off-by: Pavel Jareš <[email protected]> * fix Signed-off-by: Pavel Jareš <[email protected]> * fix ciphers Signed-off-by: Pavel Jareš <[email protected]> * support protocols and ciphers by #3637 Signed-off-by: Pavel Jareš <[email protected]> * add description of network configuration Signed-off-by: Pavel Jareš <[email protected]> * address comment Signed-off-by: Andrea Tabone <[email protected]> * address comment pt.2 Signed-off-by: Andrea Tabone <[email protected]> * add zowe config to other components schemas Signed-off-by: Andrea Tabone <[email protected]> * add ciphers and protocol setup logic to the other scripts Signed-off-by: Andrea Tabone <[email protected]> * revert back zowe configuration from the schema Signed-off-by: Andrea Tabone <[email protected]> * add fall back to gw config and add missing variables Signed-off-by: Andrea Tabone <[email protected]> * change the string comparison to use = for POSIX-compliant sh Signed-off-by: Andrea Tabone <[email protected]> * avoid using echo Signed-off-by: Andrea Tabone <[email protected]> * fix Signed-off-by: Andrea Tabone <[email protected]> * revert back Signed-off-by: Andrea Tabone <[email protected]> * comment tls version for dc Signed-off-by: Andrea Tabone <[email protected]> * fix Signed-off-by: Andrea Tabone <[email protected]> * use double quotes Signed-off-by: Andrea Tabone <[email protected]> * revert back Signed-off-by: Andrea Tabone <[email protected]> * remove unsupported protocols from schema property Signed-off-by: Andrea Tabone <[email protected]> * initialize variable Signed-off-by: Andrea Tabone <[email protected]> --------- Signed-off-by: Pavel Jareš <[email protected]> Signed-off-by: Andrea Tabone <[email protected]> Co-authored-by: Andrea Tabone <[email protected]>

1000TurquoisePogs · 2024-10-01T15:53:31Z

I'm closing this since #3765 appears to accomplish the same goals, thanks all!

1000TurquoisePogs added 5 commits July 3, 2024 14:31

Allow users to set network settings globally or per-component in a st…

83d2d2d

…andardized way Signed-off-by: 1000TurquoisePogs <[email protected]>

Fix min/maxTls -> tls_min/maxTls

49f57e3

Signed-off-by: 1000TurquoisePogs <[email protected]>

switch ZWE_network to ZWE_zowe_network

9b40828

Signed-off-by: 1000TurquoisePogs <[email protected]>

Add cipher customization, and default tls to 1.2 and 1.3

d723932

Signed-off-by: 1000TurquoisePogs <[email protected]>

Fix brackets for vars, and enumerate tls 1.2 and 1.3 if configured th…

c23be9b

…at way Signed-off-by: 1000TurquoisePogs <[email protected]>

pull-request-size bot added the size/L label Jul 9, 2024

1000TurquoisePogs marked this pull request as draft July 9, 2024 14:53

achmelo reviewed Jul 9, 2024

View reviewed changes

1000TurquoisePogs commented Jul 10, 2024

View reviewed changes

achmelo and others added 4 commits July 25, 2024 14:09

Merge branch 'v2.x.x' into feat/use-networking-standard-config

3e2f27d

Merge branch 'v2.x.x' into feat/use-networking-standard-config

1d94c03

Merge branch 'v2.x.x' into feat/use-networking-standard-config

72118c7

Use only 1 version of TLS, and default to TLS as per review comments

2f15910

Signed-off-by: 1000TurquoisePogs <[email protected]>

pull-request-size bot added size/M and removed size/L labels Aug 15, 2024

Merge branch 'v2.x.x' into feat/use-networking-standard-config

984812e

Signed-off-by: 1000TurquoisePogs <[email protected]>

pj892031 added a commit that referenced this pull request Aug 23, 2024

support protocols and ciphers by #3637

c92bcdf

Signed-off-by: Pavel Jareš <[email protected]>

taban03 pushed a commit that referenced this pull request Aug 29, 2024

support protocols and ciphers by #3637

08cc938

Signed-off-by: Pavel Jareš <[email protected]>

Merge branch 'v2.x.x' into feat/use-networking-standard-config

807b8af

taban03 pushed a commit that referenced this pull request Sep 5, 2024

support protocols and ciphers by #3637

c42fc85

Signed-off-by: Pavel Jareš <[email protected]>

taban03 pushed a commit that referenced this pull request Sep 6, 2024

support protocols and ciphers by #3637

5f3fea9

Signed-off-by: Pavel Jareš <[email protected]>

arxioly mentioned this pull request Sep 12, 2024

feat: Use networking standard config (improved) (V3) #3765

Merged

10 tasks

1000TurquoisePogs closed this Oct 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Use networking standard config #3637

feat: Use networking standard config #3637

1000TurquoisePogs commented Jul 9, 2024

achmelo Jul 9, 2024

1000TurquoisePogs Jul 10, 2024

achmelo Jul 23, 2024

1000TurquoisePogs Aug 15, 2024

taban03 Aug 20, 2024

pj892031 Aug 23, 2024

pj892031 Aug 23, 2024

1000TurquoisePogs Jul 10, 2024

achmelo Jul 23, 2024

sonarcloud bot commented Sep 3, 2024

1000TurquoisePogs commented Oct 1, 2024

feat: Use networking standard config #3637

feat: Use networking standard config #3637

Conversation

1000TurquoisePogs commented Jul 9, 2024

Description

Type of change

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Sep 3, 2024

Quality Gate passed

1000TurquoisePogs commented Oct 1, 2024