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

fix: allow icsf keys when using at-tls #3612

Merged
merged 2 commits into from
Jun 26, 2024
Merged

Conversation

1000TurquoisePogs
Copy link
Member

As noted in slack https://openmainframeproject.slack.com/archives/CC5UUL005/p1717504267648599
APIML cannot seem to read ICSF keys,

I have an ICSF key.
It looks like APIML cannot read it with java 8, so I am trying to use AT-TLS instead.
When I dont use AT-TLS, type=JCERACFKS does not work.
You get ZWEAM510E Invalid key alias 'site default icsf'
When type=JCECCARACFKS is used instead, you get
java.security.KeyStoreException: JCECCARACFKS not found
When I turn on AT-TLS, gateway still tries to read whatever is in yaml entry zowe.certificate, so even though it should work, the above errors still occur.

 certificate:
    keystore:
      type: "JCECCARACFKS"
      file: "safkeyring://username/ringname"
      alias: "site default icsf"
      password: "password"
    truststore:
      type: "JCECCARACFKS"
      file: "safkeyring://username/ringname"
      password: "password"
gateway:
    server:
      ssl:
        enabled: false
    spring:
      profiles:
        active: attls

The only combination that seems to work is preventing APIML from reading the zowe yaml keystore content.

Reading the truststore appears fine; I dont believe ICSF changes CA content type, so APIML should still be able to validate keys that are incoming.

This probably doesnt work for situations in which APIML needs a key for its own use - Perhaps client cert still works because the cert comes from ATTLS, but situations like provider=saf wont work, because the key wouldnt be visible to gateway at startup.

So, this is a fix, but also a tradeoff.

If you would like to put this under some variable we can do so, since it is a tradeoff. I just could not think of the right name so I leave it up to the squad.

Type of change

  • fix: Bug fix (non-breaking change which fixes an issue)

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

…Keystore reading of ICSF keys is failing, but ICSF keys can be used with ATTLS.

Signed-off-by: 1000TurquoisePogs <[email protected]>
@1000TurquoisePogs
Copy link
Member Author

Notes:

  • Lot of copy-paste for networking stuff in the start scripts. Better to move to a common script that is packaged with all of them?
  • metrics service script doesnt have the java 17 logic I saw in the other scripts

@balhar-jakub
Copy link
Member

As for the metrics-service. It's technical preview and extension and the current plan is to stop the support altogether, as such we aren't really looking into validating the service on Java 17.

Copy link
Contributor

@pablocarle pablocarle left a comment

Choose a reason for hiding this comment

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

LGTM, reusing parts of the scripts could be another issue / PR

Copy link

@pablocarle pablocarle merged commit bd4e738 into v2.x.x Jun 26, 2024
31 checks passed
@pablocarle pablocarle mentioned this pull request Jul 3, 2024
10 tasks
@arxioly arxioly deleted the fix/v2/icsf-attls-keys branch August 9, 2024 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

3 participants