[MGNLDIFF-79] Regression: EasySslProtocolSocketFactory isn't used correctly Created: 23/Apr/14  Updated: 02/Oct/14  Resolved: 02/Oct/14

Status: Closed
Project: Magnolia Diff Module
Component/s: None
Affects Version/s: 1.2.1
Fix Version/s: 1.2.3, 1.5.4, 1.6.1

Type: Bug Priority: Blocker
Reporter: Marcel Stör Assignee: Philip Mundt
Resolution: Fixed Votes: 1
Labels: quickwin
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File VersionDiffHtmlServlet-setHostConfiguration.png    
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Visible to:
Zarko Ivanoski

 Description   

The implementation of MGNLDIFF-68 caused a serious regression: self-signed SSL certs are not accepted anymore.

The bug is caused because EasySSLProtocolSocketFactory isn't used correctly anymore. The Javadoc class comment explains that one must either use relative URLs or call

Protocol.registerProtocol("https", easyhttps);

In 1.1.5 Magnolia used relative URLs. The variable in VersionDiffHtmlServlet.setHostConfiguration() is even called relUrl to make that obvious. In 1.2.1 an absolute URL is assigned to it. See VersionDiffHtmlServlet-setHostConfiguration.png

The problem sounded somewhat familiar because I blogged about this in the past.



 Comments   
Comment by Philip Mundt [ 25/Apr/14 ]

To be on the safe side: this ticket describes the fact that requesting a diff from an instance with self signed certifcates doesn't work? Could you please be more precise about your setup? I have successfully tested this implementation with a self signed cert (configured in Apache using mod_proxy). As far as I can tell, there's one scenario where the current implementation doesn't work: when requesting the diff from a Magnolia instance with a given (non-ssl) port, i.e. 8080 and ssl set to true in the module's config.

The naming of the variable relUrl is misleading I have to admit. We will adjust that.

Comment by Marcel Stör [ 25/Apr/14 ]

requesting a diff from an instance with self signed certifcates doesn't work

Yes, that's correct. After upgrading to 1.2.1 we get the dreaded

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

I (remoted-)debugged the module. Once I realized what was going on it was easy to fix. Right after the Protocol easyHttps = initialization I call

Protocol.registerProtocol("https", easyHttps);

and all is good. With the EasySSLProtocolSocketFactory you either do this or use relative URLs, no way around that.

As for our setup I quote from SUPPORT-2971:

In our case the system chain is as follows: firewall, Apache, Tomcat. We need to use HTTPS, configured on Apache, to connect to the author. Apache/Tomcat are not allowed to connect to the outside world which means the diff viewer cannot use the external URL for the socket connection.
So, the URL to the diff Servlet is like https://vzd-fw.netcetera.ch:7444/.magnolia/versionDiff . The firewall does port forwarding to Apache. If I configure the diff viewer to use localhost:<Tomcat-port> I get the above exception because SSL is not available on Tomcat but only on Apache. Magnolia seems to just use the URI scheme of the original request (i.e. from the external URL). I need to configure localhost:<Apache-port>.

You mention

As far as I can tell, there's one scenario where the current implementation doesn't work: when requesting the diff from a Magnolia instance with a given (non-ssl) port, i.e. 8080 and ssl set to true in the module's config.

That's why I reopened MGNLDIFF-68

I patched the diff module for these two bugs and deployed it. So, temporarily our problem is solved but of course we'd like to get rid off our own patched version ASAP.

Comment by Philip Mundt [ 25/Apr/14 ]

That error

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

should be easily solvable by adding the self-signed certificate to the java keystore.

Re MGNLDIFF-68:

So in your setup scenario you don't want to use SSL for the diff even though Magnolia is accessed by HTTPS. That's why you would like to have the AND connection of the conditions!?

Comment by Marcel Stör [ 25/Apr/14 ]

should be easily solvable by adding the self-signed certificate to the java keystore

Sure...but then why did you integrate EasySSLProtocolSocketFactory in the first place? You apparently wanted to support self-signed certs transparently on purpose I guess. Hence, if that stops working due to a regression bug I expect it to be fixed.
If you don't want to support self-signed certs transparently any longer that'd be also ok. However, then you'd have to pull out EasySSLProtocolSocketFactory and report that in the release notes.

Comment by Philip Mundt [ 25/Apr/14 ]

I'm sorry, this was quite possibly a misunderstanding from my side.
I will have to consult with our support team.

Comment by Philip Mundt [ 25/Apr/14 ]

A quick test reveals that the fix works (I had my certs in the keystore of course).
Thank you Mr. Stör for supplying it.
As it's an easy fix I would vote for tackling it asap too.

Generated at Mon Feb 12 05:21:10 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.