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

IBM MQ keystore and truststore configuration is set globally, causing services that rely on cacerts to fail #94

Closed
TheConen opened this issue Jul 5, 2023 · 4 comments

Comments

@TheConen
Copy link

TheConen commented Jul 5, 2023

ibmmq-jms-spring version(s) that are affected by this issue
3.1.1 (and presumable every version before that)

Java version (including vendor and platform).
OpenJDK 17.0.3, Windows/Linux

A small code sample that demonstrates the issue.
Given:
You connect to a secured MQ instance with something like the following configuration in application.yaml

ibm:
  mq:
    jks:
      key-store: src/main/resources/mq/keystore.jks
      key-store-password: changeit
      trust-store: src/main/resources/mq/truststore.jks
      trust-store-password: changeit

When:
Upon receiving a MQ message, you use some HTTP client (e.g. Spring Cloud Open Feign) to send a request to a HTTPS secured endpoint that is secured with a certificate that can be validated with the standard cacerts truststore.

Then:
The request will fail because the certificate chain could not be validated.

javax.net.ssl.SSLHandshakeException: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target

Possible reason:
com.ibm.mq.spring.boot.MQConnectionFactoryFactory#configureTLSStores sets the System properties javax.net.ssl.keystore and javax.net.ssl.truststore, causing everything that normally would use the default cacerts truststore to use the MQ truststore.

Possible solution:
IBM MQ should use its own, isolated SslContext, not impacting other services that rely on cacerts. This could e.g. be achieved with Spring's SslBundles.
https://docs.spring.io/spring-boot/docs/current/reference/html/features.html#features.ssl
https://spring.io/blog/2023/06/07/securing-spring-boot-applications-with-ssl

@ibmmqmet
Copy link
Collaborator

ibmmqmet commented Jul 7, 2023

The underlying MQ classes use by default the standard SSL mechanisms, which use those properties to deal with the environment. Anything else requires more complex manual setup of stores and certificates.

The SslBundles looks interesting, though as it's only available from Boot 3.1, it wouldn't fit neatly into a solution that's intended to keep running with Spring 2/JMS2 as well. It could be done, but it would be nicer if Java had #ifdef preprocessing operations to deal with different environments. Writing generic code for creating SslSocketFactory/SslContext objects is not easy, with different JRE/JSSE modules potentially having different ways of doing it - at least the SslBundles devolves that problem to Spring. I'll need to look further at options.

In the meantime, one solution is that you can write your own customize methods that can be applied to the ConnectionFactory, and which know more about your specific environment/JRE.

@TheConen
Copy link
Author

TheConen commented Jul 8, 2023

Could you elaborate on the usage of the customize method? Is that something that we could do in a @Configuration class?

May I suggest the following approach?
Keep the existing properties as they are with the existing behavior, but mark them as @Deprecated in future 3.1.x versions.
Following the same pattern as the other Spring projects (Cassandra, MongoDb, Redis etc),. add new properties ibm.mq.ssl.enabled and ibm.mq.ssl.bundle that allow configuring SSL via a SslBundle. If SSL is configured via a SSL bundle, IBM MQ should use its own SslContext instead of setting it globally. If one were to provide both the old and new properties, either the new ones should take priority or the application should fail starting with an appropriate error message.

That way the existing behavior is kept for backwards-compatibility, but when using Spring Boot >=3.1, SslBundles can be used for configuration.

I haven't used it myself yet, but the Manifold compiler plugin provides preprocessing capabilities to the Java compiler.
https://github.com/manifold-systems/manifold
https://github.com/manifold-systems/manifold/tree/master/manifold-deps-parent/manifold-preprocessor#setup

I also noticed that the existing properties and their behavior are not documented in the "Configuration Options" section of the README.

@ibmmqmet
Copy link
Collaborator

I'm talking about the common source code for the starter itself, which I am trying to keep identical between the JMS2 and Jakarta variants with only the necessary package name changes applied automatically via a build-time script. That can't be done quite as simply when the Spring classes are only available in the newer version. It can require a separate source file that cannot be compiled against the older versions of Spring. Iif #ifdef equivalents were standard and recognised in an IDE it would be easier to deal with. It can be done, but it's awkward. Especially when you want to avoid doing things with reflection operations - those can give common code but at the cost of being horrible.

Dealing with the property precedence/compatibility itself is simpler.

Customizer functions are beans you can define in the application. And in this case, you can then set/override any property on the CF. For example,

  @Bean
  public MQConnectionFactoryCustomizer myCustomizer() {
    MQConnectionFactoryCustomizer c = new MQConnectionFactoryCustomizer() {
      @Override
      public void customize(MQConnectionFactory factory) {
        System.out.println(">> In a customizer method that can modify class " + factory.getClass().getName()); 
      }
    };
    return c;
 }

ibmmqmet added a commit that referenced this issue Jul 21, 2023
Add SSLBundle configuration option for Boot 3.1 apps (#94)
@PascalSchumacher
Copy link

I guess this issue can be closed after 06a6782 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants