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

API ML enablers fail when some SSL configuration has keyring without keyAlias #2034

Closed
recaph opened this issue Jan 24, 2022 · 9 comments
Closed
Assignees
Labels
bug Verified defect in functionality clarification Issue is being clarified in the discussion with the creator of the issue

Comments

@recaph
Copy link

recaph commented Jan 24, 2022

Describe the bug
The API ML enablers fail when some SSL properties misses keyAlias even if it is not necessary for creation of keyring. The same keyring works with other applications or same application with apiml discovery disabled.

keyAlias is should only be mandatory if multiple certificates are defined in the keyring i.e. there are multiple aliases in the keyring.

Steps to Reproduce

  1. Use API ML enabler in an API service and specify for example these settings:
    server:
    ssl:
    keyAlias:
    keyStore: safkeyring:////SDKSERV/SDKRING
    keyStoreType: JCERACFKS
    trustStore: safkeyring:////SDKSERV/SDKRING
    trustStoreType: JCERACFKS
    verifySslCertificatesOfServices: true
    protocol: TLSv1.2

  2. Start the service

  3. The message is displayed: SSL parameters are missing or were not replaced by the system properties.

Expected behavior
Should only through this error if there are multiple keyAlias defined in the keyring or choose the only entry defined in the keyring by default.

Also, validations should match the required and optional behaviors of both certificates and keyrings.

Details

Version and build number: 1.27.5
Test environment: 60004

@recaph recaph added bug Verified defect in functionality new New issue that has not been worked on yet labels Jan 24, 2022
@JirkaAichler
Copy link
Contributor

JirkaAichler commented Jan 27, 2022

The keyring can have one default certificate that should be used when no alias is specified. However, this is quite tricky to implement and requires calling a native service.

https://www.ibm.com/docs/en/zos/2.5.0?topic=services-r-datalib-irrsdl00-irrsdl64-callable-service

@achmelo achmelo added clarification Issue is being clarified in the discussion with the creator of the issue and removed new New issue that has not been worked on yet labels Feb 2, 2022
@achmelo achmelo self-assigned this Feb 2, 2022
@recaph
Copy link
Author

recaph commented Feb 2, 2022

@JirkaAichler yes default key should be chosen when keyAlias is not specified.

I believe it will be great to have functionality built into a common infrastructure like apiml than requiring keyAlias from every onboarding api. One possible workaround could be to request security admin to enlist default label(s) into apiml configurations; that way keyAlias would not be needed for many.

Also it would be great to see // instead of //// being allowed as part of keyring url..

@JirkaAichler
Copy link
Contributor

It is not just a simpler configuration. It also allows the simpler renewal of certificates that are going to expire.

Securite admin adds a new certificate into a keyring making it default. Once the service is recycled it automatically uses the new certificate without any need to touch the configuration.

@achmelo
Copy link
Member

achmelo commented Feb 16, 2022

Currently, there is no code in api-layer repo that is responsible for loading private key by keyAlias. All is done by tomcat configured from application.yaml

server:
	ssl:
		keyAlias: localhost

Java API used for accessing keyrings does not provide any information about which private key is 'default' . This can be done by implementing native callable service as Jirka described in his comment.

The keyring can have one default certificate that should be used when no alias is specified. However, this is quite tricky to implement and requires calling a native service.

https://www.ibm.com/docs/en/zos/2.5.0?topic=services-r-datalib-irrsdl00-irrsdl64-callable-service 

Possible next steps:

  1. remove the exception from validation of keyAlias parameter from EurekaInstanceConfigValidator and replace it with WARN log message. This would result in the enabler using any private key available(probably the default one, this should be verified).
  2. Implement the logic of choosing the correct default key. It will make a new native dependency that can go inside the java-common repository.

From the point of view of renewal, there is no difference, just remove the expired certificate as there is no reason for it to be in the keyring.

@achmelo achmelo removed the clarification Issue is being clarified in the discussion with the creator of the issue label Feb 16, 2022
@achmelo
Copy link
Member

achmelo commented Feb 16, 2022

@recaph What is your opinion? Would it help if we remove the validation for now and just use the certificate that is available in keyring?

@recaph
Copy link
Author

recaph commented Feb 16, 2022

@achmelo it would definitely help to remove validation. I agree that we could have WARN log message to indicate that recommended keyAlias was not supplied.

Will the changes also allow just the '//' in the safkeyring string instead of needing '////'.

On topic of native dependency, I guess it would be something internal to apiml enablers / apiml and will not require additional requirements on the apis calling enablers.

@anton-brezina anton-brezina added the clarification Issue is being clarified in the discussion with the creator of the issue label Feb 17, 2022
@achmelo
Copy link
Member

achmelo commented Feb 25, 2022

@recaph code updates were merged, they are included in 1.27.6-SNAPSHOT . Requirements for 4 slashes were removed, however, I was not able to confirm that 2 slashes would work within java application. Please verify and reopen if needed.

@achmelo achmelo closed this as completed Feb 25, 2022
@recaph
Copy link
Author

recaph commented Feb 28, 2022

@achmelo I tried the changes. Its passed the key store keyAlias with below warning as expected

2022-02-28 11:20:39.611 WARN  (main) o.z.a.e.c.u.EurekaInstanceConfigValidator - Key alias is missing in the SSL configuration

but now its stucks here

2022-02-28 11:20:39.655 DEBUG (main) o.z.a.s.HttpsFactory - Protocol: TLSv1.2                                               
2022-02-28 11:20:39.661 INFO  (main) o.z.a.s.HttpsFactory - Loading trust store key ring: safkeyring://SDKSERV/SDKRING    
2022-02-28 11:20:39.662 ERROR (main) o.z.a.s.HttpsFactory - error                                                           
java.net.MalformedURLException: unknown protocol: safkeyring                                                                
.at java.net.URL.<init>(URL.java:629) ~.?:1.8.0.                                                                            
.at java.net.URL.<init>(URL.java:519) ~.?:1.8.0.                                                                            
.at java.net.URL.<init>(URL.java:468) ~.?:1.8.0.   
......
......
2022-02-28 11:20:39.665 ERROR (main) o.z.a.m.l.ApimlLogger - ZWEAM400E Error initializing SSL Context: 'unknown protocol: safkeyring'

@recaph
Copy link
Author

recaph commented Feb 28, 2022

public static URL keyRingUrl(String uri) throws MalformedURLException {
if (!uri.startsWith(SAFKEYRING + ":////")) {
throw new StoresNotInitializeException("Incorrect key ring format: " + uri
+ ". Make sure you use format safkeyring:////userId/keyRing");
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified defect in functionality clarification Issue is being clarified in the discussion with the creator of the issue
Projects
None yet
Development

No branches or pull requests

4 participants