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

Updated to re-use original socket connection when enabling SSL + TDS 8 #1817

Merged
merged 4 commits into from
Apr 27, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ private ExtendedSocketOptions() {}

final class TDS {
// TDS protocol versions
static final String VER_TDS80 = "tds/8.0"; // TLS-first connections

static final int VER_DENALI = 0x74000004; // TDS 7.4
static final int VER_KATMAI = 0x730B0003; // TDS 7.3B(includes null bit compression)
static final int VER_YUKON = 0x72090002; // TDS 7.2
Expand Down Expand Up @@ -1563,7 +1565,7 @@ enum SSLHandhsakeState {
* @throws SQLServerException
*/
void enableSSL(String host, int port, String clientCertificate, String clientKey, String clientKeyPassword,
boolean isTDSS) throws SQLServerException {
boolean isTDS8) throws SQLServerException {
// If enabling SSL fails, which it can for a number of reasons, the following items
// are used in logging information to the TDS channel logger to help diagnose the problem.
Provider tmfProvider = null; // TrustManagerFactory provider
Expand Down Expand Up @@ -1615,7 +1617,7 @@ void enableSSL(String host, int port, String clientCertificate, String clientKey
assert TDS.ENCRYPT_OFF == requestedEncryptLevel || // Login only SSL
TDS.ENCRYPT_ON == requestedEncryptLevel || // Full SSL
TDS.ENCRYPT_REQ == requestedEncryptLevel || // Full SSL
(isTDSS && TDS.ENCRYPT_NOT_SUP == requestedEncryptLevel); // TDSS
(isTDS8 && TDS.ENCRYPT_NOT_SUP == requestedEncryptLevel); // TDS 8

// If encryption wasn't negotiated or trust server certificate is specified,
// then we'll "validate" the server certificate using a naive TrustManager that trusts
Expand All @@ -1637,9 +1639,9 @@ else if (con.getTrustManagerClass() != null) {
// Otherwise, we'll validate the certificate using a real TrustManager obtained
// from the a security provider that is capable of validating X.509 certificates.
else {
if (isTDSS) {
if (isTDS8) {
if (logger.isLoggable(Level.FINEST))
logger.finest(toString() + " Verify server certificate for TDSS");
logger.finest(toString() + " Verify server certificate for TDS 8");

if (null != hostNameInCertificate) {
tm = new TrustManager[] {
Expand All @@ -1656,7 +1658,7 @@ else if (con.getTrustManagerClass() != null) {
// If we are using the system default trustStore and trustStorePassword
// then we can skip all of the KeyStore loading logic below.
// The security provider's implementation takes care of everything for us.
if (null == trustStoreFileName && null == trustStorePassword && !isTDSS) {
if (null == trustStoreFileName && null == trustStorePassword && !isTDS8) {
if (logger.isLoggable(Level.FINER)) {
logger.finer(toString() + " Using system default trust store and password");
}
Expand Down Expand Up @@ -1762,12 +1764,12 @@ else if (con.getTrustManagerClass() != null) {

proxySocket = new ProxySocket(this);

if (isTDSS) {
sslSocket = (SSLSocket) sslContext.getSocketFactory().createSocket(host, port);
if (isTDS8) {
sslSocket = (SSLSocket) sslContext.getSocketFactory().createSocket(channelSocket, host, port, true);

// set ALPN values
SSLParameters sslParam = sslSocket.getSSLParameters();
sslParam.setApplicationProtocols(new String[] {"tds/8.0"});
sslParam.setApplicationProtocols(new String[] {TDS.VER_TDS80});
sslSocket.setSSLParameters(sslParam);
} else {
// don't close proxy when SSL socket is closed
Expand All @@ -1783,14 +1785,14 @@ else if (con.getTrustManagerClass() != null) {
sslSocket.startHandshake();
handshakeState = SSLHandhsakeState.SSL_HANDHSAKE_COMPLETE;

if (isTDSS) {
if (isTDS8) {
if (logger.isLoggable(Level.FINEST)) {
String negotiatedProtocol = sslSocket.getApplicationProtocol();
logger.finest(toString() + " Application Protocol negotiated: "
+ ((negotiatedProtocol == null) ? "null" : negotiatedProtocol));
}
}

// After SSL handshake is complete, re-wire proxy socket to use raw TCP/IP streams ...
if (logger.isLoggable(Level.FINEST))
logger.finest(toString() + " Rewiring proxy streams after handshake");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ public class SQLServerConnection implements ISQLServerConnection, java.io.Serial
private int connectRetryInterval = 0;

/** flag indicating whether prelogin TLS handshake is required */
private boolean isTDSS = false;
private boolean isTDS8 = false;

String encryptedTrustStorePassword = null;

Expand Down Expand Up @@ -768,7 +768,7 @@ final String getServerCertificate() {
private byte negotiatedEncryptionLevel = TDS.ENCRYPT_INVALID;

final byte getNegotiatedEncryptionLevel() {
assert (!isTDSS ? TDS.ENCRYPT_INVALID != negotiatedEncryptionLevel : true);
assert (!isTDS8 ? TDS.ENCRYPT_INVALID != negotiatedEncryptionLevel : true);
return negotiatedEncryptionLevel;
}

Expand Down Expand Up @@ -2148,7 +2148,7 @@ Connection connectInternal(Properties propsIn,
.getProperty(SQLServerDriverStringProperty.SERVER_CERTIFICATE.toString());

// prelogin TLS handshake is required
isTDSS = true;
isTDS8 = true;
} else {
MessageFormat form = new MessageFormat(
SQLServerException.getErrString("R_InvalidConnectionSetting"));
Expand Down Expand Up @@ -3289,16 +3289,16 @@ private InetSocketAddress connectHelper(ServerPortPlaceHolder serverInfo, int ti
}
assert null != clientConnectionId;

if (isTDSS) {
if (isTDS8) {
tdsChannel.enableSSL(serverInfo.getParsedServerName(), serverInfo.getPortNumber(), clientCertificate,
clientKey, clientKeyPassword, isTDSS);
clientKey, clientKeyPassword, isTDS8);
clientKeyPassword = "";
}

prelogin(serverInfo.getServerName(), serverInfo.getPortNumber());

// If not enabled already and prelogin negotiated SSL encryption then, enable it on the TDS channel.
if (!isTDSS && TDS.ENCRYPT_NOT_SUP != negotiatedEncryptionLevel) {
if (!isTDS8 && TDS.ENCRYPT_NOT_SUP != negotiatedEncryptionLevel) {
tdsChannel.enableSSL(serverInfo.getParsedServerName(), serverInfo.getPortNumber(), clientCertificate,
clientKey, clientKeyPassword, false);
clientKeyPassword = "";
Expand Down Expand Up @@ -3408,7 +3408,7 @@ void prelogin(String serverName, int portNumber) throws SQLServerException {
(byte) (SQLJdbcVersion.build & 0xff), (byte) ((SQLJdbcVersion.build & 0xff00) >> 8),

// Encryption
// turn encryption off for TDSS since it's already enabled
// turn encryption off for TDS 8 since it's already enabled
(null == clientCertificate) ? requestedEncryptionLevel
: (byte) (requestedEncryptionLevel | TDS.ENCRYPT_CLIENT_CERT),

Expand Down Expand Up @@ -3691,7 +3691,7 @@ void prelogin(String serverName, int portNumber) throws SQLServerException {
// If we say we don't support SSL and the server doesn't accept unencrypted connections,
// then terminate the connection.
if (TDS.ENCRYPT_NOT_SUP == requestedEncryptionLevel
&& TDS.ENCRYPT_NOT_SUP != negotiatedEncryptionLevel && !isTDSS) {
&& TDS.ENCRYPT_NOT_SUP != negotiatedEncryptionLevel && !isTDS8) {
// If the server required an encrypted connection then terminate with an appropriate error.
if (TDS.ENCRYPT_REQ == negotiatedEncryptionLevel)
terminate(SQLServerException.DRIVER_ERROR_SSL_FAILED,
Expand Down Expand Up @@ -4554,7 +4554,7 @@ int writeAEFeatureRequest(boolean write, /* if false just calculates the length
if (write) {
tdsWriter.writeByte(TDS.TDS_FEATURE_EXT_AE); // FEATUREEXT_TC
tdsWriter.writeInt(1); // length of version

// For protocol = HGS,AAS, at this point it can only have a valid URL, therefore is V2
// For protocol = NONE, it is V2 regardless
// For protocol = null, we always want V1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,7 @@ public void testBadServerCert() throws SQLException {
try (Connection con = ds.getConnection()) {
fail(TestResource.getResource("R_expectedFailPassed"));
} catch (SQLException e) {
// TODO: servers which do not support TDSS will return SSL failed error, test should be updated once server
// TODO: servers which do not support TDS 8 will return SSL failed error, test should be updated once server
// available
assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_serverCertError"))
|| e.getMessage().matches(TestUtils.formatErrorMsg("R_sslFailed")), e.getMessage());
Expand All @@ -963,7 +963,7 @@ public void testBadServerCert() throws SQLException {
connectionString + ";encrypt=strict;trustServerCertificate=false;serverCertificate=badCert")) {
fail(TestResource.getResource("R_expectedFailPassed"));
} catch (SQLException e) {
// TODO: servers which do not support TDSS will return SSL failed error, test should be updated once server
// TODO: servers which do not support TDS 8 will return SSL failed error, test should be updated once server
// available
assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_serverCertError"))
|| e.getMessage().matches(TestUtils.formatErrorMsg("R_sslFailed")), e.getMessage());
Expand Down