Fix SSLHandshakeException while closing HTTPConduit #25148

sawadood · 2023-05-01T14:01:04Z

sawadood · 2023-05-01T14:28:54Z

#spawn.fullfat.buckets=com.ibm.ws.jaxws.2.2.webcontainer_fat,com.ibm.ws.jaxws.2.2.webcontainer_fat_extended,com.ibm.ws.jaxws.cdi_fat,com.ibm.ws.jaxws.clientcontainer_fat,com.ibm.ws.jaxws.ejb_fat,com.ibm.ws.jaxws.X.wsat_fat,io.openliberty.xmlws.4.0.internal_fat,io.openliberty.ws.jaxws.global.handler.internal_fat,io.openliberty.jaxws.security_fat.ssl,io.openliberty.jaxws.security_fat.1,io.openliberty.jaxws.security_fat,com.ibm.ws.wssecurity_fat.wsscxf.1,com.ibm.ws.wssecurity_fat.wsscxf.2,com.ibm.ws.wssecurity_fat.wsscxf.3, com.ibm.ws.wssecurity_fat.wsscxf.4,com.ibm.ws.wssecurity_fat.wsscxf.saml.1,com.ibm.ws.wssecurity_fat.wsscxf.saml.2,com.ibm.ws.wssecurity_fat.wsscxf.saml.3,com.ibm.ws.wssecurity_fat.wsscxf.saml.4,com.ibm.ws.wssecurity_fat.wsscxf.saml.5,com.ibm.ws.wssecurity_fat.wsscxf.saml.6

sawadood · 2023-05-01T15:12:47Z

#build
#spawn.fullfat.buckets=com.ibm.ws.jaxws.2.2.webcontainer_fat,com.ibm.ws.jaxws.2.2.webcontainer_fat_extended,com.ibm.ws.jaxws.cdi_fat,com.ibm.ws.jaxws.clientcontainer_fat,com.ibm.ws.jaxws.ejb_fat,com.ibm.ws.jaxws.X.wsat_fat,io.openliberty.xmlws.4.0.internal_fat,io.openliberty.ws.jaxws.global.handler.internal_fat,io.openliberty.jaxws.security_fat.ssl,io.openliberty.jaxws.security_fat.1,io.openliberty.jaxws.security_fat,com.ibm.ws.wssecurity_fat.wsscxf.1,com.ibm.ws.wssecurity_fat.wsscxf.2,com.ibm.ws.wssecurity_fat.wsscxf.3, com.ibm.ws.wssecurity_fat.wsscxf.4,com.ibm.ws.wssecurity_fat.wsscxf.saml.1,com.ibm.ws.wssecurity_fat.wsscxf.saml.2,com.ibm.ws.wssecurity_fat.wsscxf.saml.3,com.ibm.ws.wssecurity_fat.wsscxf.saml.4,com.ibm.ws.wssecurity_fat.wsscxf.saml.5,com.ibm.ws.wssecurity_fat.wsscxf.saml.6,com.ibm.ws.wsat_fat.2,com.ibm.ws.wsat_fat.3,com.ibm.ws.wsat_fat.4,com.ibm.ws.wsat_fat.5,com.ibm.ws.wsat_fat.6,com.ibm.ws.wsat_fat.7,com.ibm.ws.wsat_fat.db,com.ibm.ws.wsat.common_fat,com.ibm.ws.wsat.concurrent_fat,com.ibm.ws.wsat.migration_fat.1,com.ibm.ws.wsat.migration_fat.2,com.ibm.ws.wsat.migration_fat.3,com.ibm.ws.wsat.migration_fat.4,com.ibm.ws.wsat.migration_fat.5,com.ibm.ws.wsat.recovery_fat.lps,com.ibm.ws.wsat.recovery_fat.multi.1,com.ibm.ws.wsat.recovery_fat.multii.2,com.ibm.ws.wsat.recovery_fat.multii.3,com.ibm.ws.wsat.recovery_fat.multii.4,com.ibm.ws.wsat.recovery_fat.single.1,com.ibm.ws.wsat.recovery_fat.single.2,com.ibm.ws.jaxws.2.2.webcontainer_fat,com.ibm.ws.jaxws.2.2.webcontainer_fat_extended,com.ibm.ws.jaxws.cdi_fat,com.ibm.ws.jaxws.clientcontainer_fat,com.ibm.ws.jaxws.ejb_fat,com.ibm.ws.jaxws.X.wsat_fat,com.ibm.ws.javaee.dd.model_fat,com.ibm.ws.microprofile.rest.client.FT_fat,com.ibm.ws.microprofile.rest.client11_fat_tck,com.ibm.ws.microprofile.rest.client12_fat_tck,com.ibm.ws.microprofile.rest.client13_fat_tck,com.ibm.ws.microprofile.rest.client14_fat_tck,com.ibm.ws.microprofile.rest.client_fat,com.ibm.ws.microprofile.rest.client_fat_tck

LibbyBot · 2023-05-01T15:23:08Z

Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_sWlPYOgrEe2D8Kk7c5d7Ig

Target locations of links might be accessible only to IBM employees.

LibbyBot · 2023-05-02T08:25:29Z

The build sawadood-25148-20230501-0831
https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_sWlPYOgrEe2D8Kk7c5d7Ig
completed and has errors or failures.

For help analyzing your personal build, go to https://libh-proxy1.fyre.ibm.com/cognitive/buildAnalysis.html?uuid=_sWlPYOgrEe2D8Kk7c5d7Ig

sawadood · 2023-05-02T13:11:38Z

Personal build has 11 failures, all these failures have the same error ""invalid test bucket"" and not related to my fix.

BerksanAtes

It looks great! Especially, I believe added traces will helps us tremendously!!

sawadood · 2023-05-02T15:12:11Z

#run-libby-bot

sawadood · 2023-05-02T15:13:38Z

Tested this fix locally also and did not find any issues.

LibbyBot · 2023-05-02T15:58:36Z

Code analysis and actions

DO NOT DELETE THIS COMMENT.

4 product code files were changed.
Please describe in a separate comment how you tested your changes.

WhiteCat22 · 2023-05-02T16:39:56Z

...f.cxf.rt.transports.http.3.2/src/org/apache/cxf/transport/http/URLConnectionHTTPConduit.java

+
+        // Liberty start    
+	boolean isRestMessage =
+            PropertyUtils.isTrue(message.getExchange().get(org.apache.cxf.message.Message.REST_MESSAGE));


Can you confirm that org.apache.cxf.rest.message is present on MP Rest Client requests in addition to vanilla JAX-RS request please?

Yes, I have tested that org.apache.cxf.rest.message is present on MP REST client requests.

WhiteCat22 · 2023-05-02T16:43:50Z

...cxf.rt.transports.http.3.2/src/org/apache/cxf/transport/https/HttpsURLConnectionFactory.java

+	    }
+
+	    if (socketFactory != null) {
+               LOG.fine("SSL socketFactory: " +  socketFactory.getClass().getCanonicalName()); // Liberty Change


// Liberty Change End

WhiteCat22 · 2023-05-02T16:44:42Z

...cxf.rt.transports.http.3.2/src/org/apache/cxf/transport/https/HttpsURLConnectionFactory.java

+                   LOG.fine("No trustManagers set on tlsClientParameters, so use Liberty's DefaultSSLSocketFactory");
+                   socketFactory = HttpsURLConnection.getDefaultSSLSocketFactory();
+		}
+		// Liberty Change End


Move this // Liberty Change End to after line 178

WhiteCat22 · 2023-05-02T16:47:13Z

...s.org.apache.cxf.cxf.rt.transports.http.3.2/src/org/apache/cxf/transport/https/SSLUtils.java

            verifier = new AllowAllHostnameVerifier();
+	} else if (!performHostNameVerification) {  // Liberty Change Start
+	    LOG.fine("TS012061109: performHostNameVerification is false, setting verifier to AllowAllHostnameVerifier.");


Remove TS012061109

WhiteCat22 · 2023-05-02T16:47:25Z

...s.org.apache.cxf.cxf.rt.transports.http.3.2/src/org/apache/cxf/transport/https/SSLUtils.java

+        }
+        });
+        performHostNameVerification = b.booleanValue();
+        LOG.fine("TS012061109: Property com.ibm.ssl.performURLHostNameVerification is set to: " + performHostNameVerification);


remove TS012061109

WhiteCat22 · 2023-05-02T16:50:09Z

@sawadood I don't know why it's not allowing my to block the merge with "Request Changes", but I would like to see my comments addressed before you merge, thanks!

jim-krueger · 2023-05-02T17:32:47Z

...f.cxf.rt.transports.http.3.2/src/org/apache/cxf/transport/http/URLConnectionHTTPConduit.java

@@ -393,6 +409,7 @@ protected InputStream getInputStream() throws IOException {
                        in = connection.getInputStream();
                    } catch (IOException ex) {
                        // ignore
+        	        LOG.fine("Ignoring unexpected exception in getInputStream(): " + ex); // Liberty Change


It seems to me that this code section might be incorrect (but would not cause a problem in most cases). They are attempting to get the "Error Stream". Shouldn't the if check here be if (in != null)?

Also, I'd suggest changing your log message to "Ignoring unexpected exception in getInputStream() when an error stream is present: "

jim-krueger

Looks good. Just a couple comments/suggestions.

jim-krueger · 2023-05-02T17:35:48Z

...cxf.rt.transports.http.3.2/src/org/apache/cxf/transport/https/HttpsURLConnectionFactory.java

+        if (HTTPS_URL_PROTOCOL_ID.equals(url.getProtocol())) {
+
+            if (tlsClientParameters == null) {
+		LOG.fine("tlsClientParameters is NULL, get new"); // Liberty Change


Minor: Change get to create

jim-krueger · 2023-05-02T17:42:48Z

...cxf.rt.transports.http.3.2/src/org/apache/cxf/transport/https/HttpsURLConnectionFactory.java

+
+	// Liberty Change Start
+	if (verifier != null) {
+           LOG.fine("Hostname verifier obtained from SSLUtils.getHostnameVerifier: " +  verifier.getClass().getCanonicalName());


Suggestion: Remove the if check and change the line to the following so you'll know if it is null or not more easily:
LOG.fine(""Hostname verifier obtained from SSLUtils.getHostnameVerifier: " + (verifier==null? "null": verifier.getClass().getCanonicalName()));

I think we should leave the if check, but only print a message if the verifier is null.

jim-krueger · 2023-05-02T17:43:24Z

...cxf.rt.transports.http.3.2/src/org/apache/cxf/transport/https/HttpsURLConnectionFactory.java

+	}
+
+	if (tlsClientParameters != null) {
+           LOG.fine("isDisableCNCheck value in tlsClientParameters: " +  tlsClientParameters.isDisableCNCheck());


same suggestion here as above.

jim-krueger · 2023-05-02T17:43:41Z

...cxf.rt.transports.http.3.2/src/org/apache/cxf/transport/https/HttpsURLConnectionFactory.java

+            HttpsURLConnection conn = (HttpsURLConnection) connection;
+	    // Liberty Change Start
+	    if (verifier != null) {
+               LOG.fine("Setting Hostname verifier to: " +  verifier.getClass().getCanonicalName());


same comment

Syed Wadood added 2 commits April 30, 2023 22:34

Original CXF 3.4.3 files for fixing issue 24986

ccc8d96

Fix SSLHandshakeException while closing HTTPConduit

00240fd

sawadood self-assigned this May 1, 2023

LibbyBot added the CLA Signed label May 1, 2023

sawadood requested a review from neuwerk May 1, 2023 14:29

sawadood requested a review from BerksanAtes May 1, 2023 19:26

neuwerk requested a review from WhiteCat22 May 2, 2023 11:37

BerksanAtes approved these changes May 2, 2023

View reviewed changes

sawadood merged commit 0e60434 into OpenLiberty:integration May 2, 2023

WhiteCat22 reviewed May 2, 2023

View reviewed changes

jim-krueger reviewed May 2, 2023

View reviewed changes

sawadood mentioned this pull request May 4, 2023

Address review comments from Pull Request 25148 #25201

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix SSLHandshakeException while closing HTTPConduit #25148

Fix SSLHandshakeException while closing HTTPConduit #25148

sawadood commented May 1, 2023

sawadood commented May 1, 2023

sawadood commented May 1, 2023

LibbyBot commented May 1, 2023

LibbyBot commented May 2, 2023

sawadood commented May 2, 2023

BerksanAtes left a comment

sawadood commented May 2, 2023

sawadood commented May 2, 2023

LibbyBot commented May 2, 2023

WhiteCat22 May 2, 2023

sawadood May 3, 2023

WhiteCat22 May 2, 2023

WhiteCat22 May 2, 2023

WhiteCat22 May 2, 2023

WhiteCat22 May 2, 2023

WhiteCat22 commented May 2, 2023

jim-krueger May 2, 2023

jim-krueger left a comment

jim-krueger May 2, 2023

jim-krueger May 2, 2023

WhiteCat22 May 2, 2023

jim-krueger May 2, 2023

jim-krueger May 2, 2023

Fix SSLHandshakeException while closing HTTPConduit #25148

Fix SSLHandshakeException while closing HTTPConduit #25148

Conversation

sawadood commented May 1, 2023

sawadood commented May 1, 2023

sawadood commented May 1, 2023

LibbyBot commented May 1, 2023

LibbyBot commented May 2, 2023

sawadood commented May 2, 2023

BerksanAtes left a comment

Choose a reason for hiding this comment

sawadood commented May 2, 2023

sawadood commented May 2, 2023

LibbyBot commented May 2, 2023

Code analysis and actions

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

WhiteCat22 commented May 2, 2023

Choose a reason for hiding this comment

jim-krueger left a 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