#130 Fix SSL connection issue 690

[Bob Loihl]

I work on a product that has SSL connection requirements and we need to use SSL against java 1.8. We filed defect [690][https://sourceforge.net/p/jtds/bugs/690/] and we're possibly encountering defect [708][https://sourceforge.net/p/jtds/bugs/708/]. We did some debugging and found that we were experiencing an error in the handshake related to the BEAST fix. We fixed and tested SSL connections off of HEAD in svn://svn.code.sf.net/p/jtds/code/branches/jTDS%201.2%20(stable) at revision 1289 which includes the patch for SSL defect [129][https://sourceforge.net/p/jtds/patches/129/]. I don't have a patch to apply against 2.0.

DETAILS BELOW:
This fix addresses the need to use -Djsse.enableCBCProtection=false for jtds to work with SQLServer in SSL mode.  This setting disables the Java SSL fix for the BEAST attack.  The actual code change was simply to change a “not equal” to be a “greater than” compare in an if statement.  However, more background is needed to understand the ‘why’ for this.

-The if block in question exists to deal with the fact that in Java 1.6, even when using TLS 1.0, the SSL Client Hello is sent using SSLv2 ‘framing’. Apparently, that was thought to be ‘a good thing’ as some point back when a client needed to work with SSLv2 servers as well as the new-fangled SSLv3/TLS 1.0 servers. Jtds needs to wrap the Client Hello in a TDS packet, so it needed to detect Client Hello when it is wrapped in TLS 1.0 framing and/or when it is wrapped in SSLv2 framing.  It chose to assume that any packet that violates TLS framing (as an SSLv2 Client Hello would) should be ‘wrapped’ in a TDS frame. The check was to see if the header in the write buffer matched the length of the write buffer. This actually worked… up until the Java 1.6.0_XX update that dealt with the BEAST attack.

-It turns out the way JSSE dealt with the BEAST attack was to ‘split’ application data writes into two SSL frames.  The first frame would have 1 byte of user data and the second would have the rest of the user data.  In order to avoid too much extra overhead on the network, JSSE buffered/packed these two SSL frames into one ‘write’ to the network.  And there was much rejoicing… except for jtds users.

-Now, for the first time, the jtds socket stream filter was presented with writes that contained not one, but two SSL frames.  Remember that ‘invalid TLS frame’ detection that was used to detect SSLv2 Client Hello.  It turns out the jtds filter never anticipated multiple TLS frames in one write.  And, of course, a write buffer with more than one valid TLS frame would fail the ‘header matches write size’ check.  So the jtds filter wrongly wrapped this write in a TDS frame.  And there was much wailing and gnashing of teeth.

-The fix I made was to simply change the test for invalid TLS frame from ‘size in header not equal write size’ to ‘size in header greater than write size’.  This was the minimal impact change… but it is only slightly less bad than what was there.  A more robust fix would involve: 1) an affirmative test for an SSLv2 frame header/size test and 2) proper handling of multiple frames in a single write buffer and, maybe, 3) only enabling any of this stuff during initial handshaking (but I’m not sure about how renegotiations are handled vs TDS, so maybe this last is not appropriate).

[Bob Loihl]

because onlu 1 file cna be uploaded at a time

[Anonymous]

Why not accept this?

[Mike Noordermeer]

I created a slightly different patch, that feels like less of a hack (but it is still suboptimal, IMHO the whole jTDS SSL handling could use a small cleanup, ditching all Java 6/SSLv2/SQL2k support).

Patch can be found here: https://gist.github.com/MikeN123/15ca4d45c50e7c3dfd57

[Brouno]

Hi Mike,

Can I ask you where is from the source code that you used in this patch ?

The last official source (here : http://sourceforge.net/projects/jtds/files/jtds/1.3.1/jtds-1.3.1-src.zip/download) doesn't match your patch.

in "TdsTlsOutputStream.java", for example the line from your patch reference an 'off' variable (offset I guess) :
int length = ( ( b[off + 3] & 0xFF ) << 8 ) | ( b[off + 4] & 0xFF );

but the official source code lack the 'off' variable :
int length = ((b[3] & 0xFF) << 8) | (b[4] & 0xFF);

Thx for your help ;-)
Brouno.

[Brouno]

I think I found it : sourceforge.net/p/jtds/patches/129/

Can you confirm ?

[Mike Noordermeer]

No, it's simply taken from the SVN tree (which contains that patch). https://sourceforge.net/p/jtds/code/HEAD/tree/branches/jTDS%201.3%20%28stable%29/

[Brouno]

OK , thx a lot :-)

[Anonymous]

Please apply this patch as soon as possible.

jTDS driver is now unusable in Java 8 environment

[Anonymous]

This is not the patch necessary for Java 8, that one has been merged already, so building master should give you a Java 8 compatible version.

[Anonymous]

You have right but latest maven artifact is from 8 June 2013 and It does not include this fix. IMHO after releasing patch artifacts should be updated

[kirthi]

[root@serverName java]# patch < /var/tmp/ssl-handshake-1.2-690.patch
can't find file to patch at input line 5
Perhaps you should have used the -p or --strip option?
The text leading up to this was:

---

|Index: src/main/net/sourceforge/jtds/ssl/TdsTlsOutputStream.java
|===================================================================
|--- src/main/net/sourceforge/jtds/ssl/TdsTlsOutputStream.java (revision 1289)
|+++ src/main/net/sourceforge/jtds/ssl/TdsTlsOutputStream.java (working copy)

---

File to patch: /?/?/
Skip this patch? [y] y
Skipping patch.
1 out of 1 hunk ignored

I keep getting errors when applying the patch even with the “-p1” argument. What is the filename I need to be using when the patch asks what file to patch?

[JerryPan]

The patch does not seem to work for latest JRE (update 75, probably earlier versions as well, and IBM Java 7 SR 9), Has anyone seen the same?

[Mike Noordermeer]

My patch (see comment above) works for us on 7u80 and 8u51, did you try that one? What behaviour are you seeing?

[JerryPan]

Hi Mike, thanks for the pointer, I tried your patch, an exception is now thrown (so it was hang but now it is exception):

Exception in thread "main" java.sql.SQLException: I/O Error: DB server closed connection.
at net.sourceforge.jtds.jdbc.TdsCore.executeSQL(TdsCore.java:1103)
at net.sourceforge.jtds.jdbc.JtdsStatement.executeSQLQuery(JtdsStatement.java:528)
at net.sourceforge.jtds.jdbc.JtdsStatement.executeQuery(JtdsStatement.java:1460)
at net.sourceforge.jtds.jdbc.JtdsConnection.<init>(JtdsConnection.java:416)
at net.sourceforge.jtds.jdbc.Driver.connect(Driver.java:184)
at java.sql.DriverManager.getConnection(DriverManager.java:583)
at java.sql.DriverManager.getConnection(DriverManager.java:227)
at jerry.test.TestSSL.main(TestSSL.java:28)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:95)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:56)
at java.lang.reflect.Method.invoke(Method.java:620)
at com.intellij.rt.execution.application.AppMain.main(AppMain.java:140)
Caused by: java.io.IOException: DB server closed connection.
at net.sourceforge.jtds.jdbc.SharedSocket.readPacket(SharedSocket.java:873)
at net.sourceforge.jtds.jdbc.SharedSocket.getNetPacket(SharedSocket.java:730)
at net.sourceforge.jtds.jdbc.ResponseStream.getPacket(ResponseStream.java:477)
at net.sourceforge.jtds.jdbc.ResponseStream.read(ResponseStream.java:114)
at net.sourceforge.jtds.jdbc.ResponseStream.peek(ResponseStream.java:99)
at net.sourceforge.jtds.jdbc.TdsCore.wait(TdsCore.java:4162)
at net.sourceforge.jtds.jdbc.TdsCore.executeSQL(TdsCore.java:1096)
... 12 more

Any suggestions? Is my SQL server need patch?

[Mike Noordermeer]

Are you connecting with ssl=require in the connection string? If not, the DB server will close the connection as off is the default.

[JerryPan]

Yes, that is right, ss=require.

[JerryPan]

I meant "ssl=require"

[Mike Noordermeer]

No idea what's wrong then - maybe you can check the SQL Server logs? We haven't seen any issues with the patch (but I'm not 100% sure we haven't got more patches (I think it's just master with this patch). You can check out our build at https://maven.eveoh.nl/content/repositories/releases/net/sourceforge/jtds/jtds/1.3.1-eveoh2/

[JerryPan]

Mike, that exception trace might be realted MS SQL Server no patched, I have success with some instance of SQL servers and failures on others, it's likely the patch level and some fixes on the server side.

[Brouno]

Hi all,

I can confirm all of this
I just ran into this problem between a Tomcat java app & MS SQL 2008R2 SP3 on win2K8R2.

Activating SSL in connection result in resetting problems during JTDS<->SQL connection

After using the patched JTDS version of Mike Noordermeer (https://maven.eveoh.nl/content/repositories/releases/net/sourceforge/jtds/jtds/1.3.1-eveoh2/) all run fine. i.e. ssl connection between the tomcat app & SQL is OK.

The test was done in official production of one of my customer ;-)

I now need an official integration of this kind of correction into JTDS 1.3.1.

Momo, can you acknowledge this correction ? I guess a corrected official version would be great for everybody , as SSL connections to MS SQL server is currently ramping-up I guess ;-)

Thx.
Brouno

[Mike Noordermeer]

Hi Brouno, you asked in a (deleted?) post:

Can I ask you where is from the source code that you used in this patch ?

I think it's based on the SVN latest code for 1.3. Hope that helps.

[Brouno]

Yes, was better in the original patch post below ;-), Thx again.

B.

[Nida]

Can this fix me accepted? it works like a charm

[Chris Collins]

Certainly would love to see this fix get in if it covers all all cases. Having to set -Djsse.enableCBCProtection=false for a couple of years now certainly isn't ideal.

[Anonymous]

Please release a maven artifact as we need to consume a released artifact