Bug 55801 - Add ability to provide custom SSLContext for websocket client
Summary: Add ability to provide custom SSLContext for websocket client
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Catalina (show other bugs)
Version: trunk
Hardware: All All
: P2 enhancement with 1 vote (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-20 09:57 UTC by Maciej Lypik
Modified: 2013-12-07 20:20 UTC (History)
1 user (show)



Attachments
Proposed patch (1.81 KB, patch)
2013-11-20 09:57 UTC, Maciej Lypik
Details | Diff
Corrected patch (5.18 KB, patch)
2013-11-27 09:17 UTC, Maciej Lypik
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maciej Lypik 2013-11-20 09:57:24 UTC
Created attachment 31059 [details]
Proposed patch

Currently the only available customisation is setting trust store via SSL_TRUSTSTORE_PROPERTY. This is not enough in situations where custom trust manager has to be used. The most trivial example I can think of is accepting all certificates.

My proposition is to use new property to pass SSLContext using user properties of ClientEndpointConfig.

Attached is a proposed patch to add this capability.
Comment 1 Marek Jagielski 2013-11-21 14:39:17 UTC
Hello,
What is the chance to have this patch integrated into trunk? I want to avoid keeping my fork of tomcat that is only for client code.

This patch would be useful also for those who cope with self-signed certificates for test purposes.
Thanks,

Marek
Comment 2 Christopher Schultz 2013-11-21 23:07:07 UTC
(In reply to Marek Jagielski from comment #1)
> This patch would be useful also for those who cope with self-signed
> certificates for test purposes.

I'm just curious how this would help with self-signed certificates. What can you not accomplish with existing APIs/configuration?
Comment 3 Maciej Lypik 2013-11-22 08:53:07 UTC
(In reply to Christopher Schultz from comment #2)
> (In reply to Marek Jagielski from comment #1)
> > This patch would be useful also for those who cope with self-signed
> > certificates for test purposes.
> 
> I'm just curious how this would help with self-signed certificates. What can
> you not accomplish with existing APIs/configuration?

If certificate is known beforehand there is no problem - it can be simply added to the trust store.

My problem is that clients do not know anything about the server they are connecting to. I work in cloud environment and servers are instantiated and terminated as they are needed. Every time new server instance is created it gets new self-signed certificate. The only thing client knows about server is its IP address. Ability to temporarily ignore certificates would solve this problem for me.

Additionally, even if trust store solves problem for most cases, some people prefer to ignore certificates. I think it's fairly common practice in HTTPS clients. I remember doing so myself in Jersey Client API.
Comment 4 Maciej Lypik 2013-11-26 12:41:55 UTC
I'm wondering if I could get any feedback on this patch. I'd like for it to be integrated, as it is the only feasible solution to my problem.

I hate to push this myself, but with time it's becoming a blocking issue for me.
Comment 5 Konstantin Kolinko 2013-11-26 22:48:34 UTC
It is OK for me, but here are two minor glitches

1. Wasted work.

> SSLContext sslContext = SSLContext.getInstance("TLS");

Such getInstance() calls are usually slow. It'd be better to skip it if you are ignoring its result.

2. You have Javadoc (good), but these properties are also mentioned at the bottom of
\webapps\docs\web-socket-howto.xml

It would be good to mention somewhere that the value of a SSL_TRUSTSTORE_PROPERTY is ignored when using SSL_CONTEXT_PROPERTY.
Comment 6 Christopher Schultz 2013-11-26 23:35:25 UTC
I haven't looked at the Websocket client code, but if it's using HttpsURLConnection (which is a big IF) to negotiate the initial connection, it's trivial to re-configure it using the standard API.
Comment 7 Maciej Lypik 2013-11-27 09:17:28 UTC
Created attachment 31080 [details]
Corrected patch
Comment 8 Maciej Lypik 2013-12-02 16:53:40 UTC
Sorry for commenting so late. I've fixed the logic and added description to websocket-howto so it should be OK now.

I was also looking for a better solution to this problem.  I checked Tyrus and it also uses custom properties to configure SSLEngine. So I guess there is no perfect answer until client-side SSL configuration is defined in WebSocket spec.
Comment 9 Marek Jagielski 2013-12-04 15:04:51 UTC
Hi,
Can you give any feedback on this issue? I need to take a decision to move my project forward. I want to know if I will stay with tomcat implementation of websocket client or look for something else. I have already removed jwebsocket proprietary implementation to replace it with jsr356 one. 
I would like to use Tomcat implementation as I have a huge confidence to your project. However discussed ability will be crucial for me in the future. 
Thanks

Marek
Comment 10 Mark Thomas 2013-12-07 20:20:12 UTC
Thanks for the patch. This has been applied to 8.0.x and 7.0.x for 8.0.0-RC6 and 7.0.48 respectively.