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

Make SSL certificate validation respect wildcards #836

Merged
merged 38 commits into from
Jan 2, 2019
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
d260850
respect wildcard
peterbae Oct 10, 2018
9023466
update logic to reflect new findings.
peterbae Oct 18, 2018
e4225ae
improved logic + test cases
peterbae Oct 19, 2018
725fc70
apply formatter
peterbae Oct 19, 2018
67ace3d
run with junit
peterbae Oct 19, 2018
a230a82
do not allow wildcards past the first period, and update the test acc…
peterbae Oct 19, 2018
3d8e6c3
make some improvements with speed of logic
peterbae Oct 22, 2018
5bde5b7
Fix test comments
peterbae Oct 29, 2018
1d36666
make changes to handle more negative cases, and add testing for it
peterbae Oct 31, 2018
2c7271a
comment changes
peterbae Nov 23, 2018
70040a8
update parsing logic
peterbae Nov 28, 2018
e949547
clean up logic
peterbae Nov 28, 2018
20e86e3
log more
peterbae Nov 28, 2018
6de6bbb
modify logic + add comment
peterbae Nov 28, 2018
9be9d11
Merge branch 'dev' of https://github.com/Microsoft/mssql-jdbc into gi…
peterbae Nov 28, 2018
e31a51e
add license header
peterbae Nov 28, 2018
0c3255c
strenghen logic
peterbae Nov 28, 2018
67c553b
add test for latest logic
peterbae Nov 29, 2018
5fba39f
fix typo
peterbae Nov 29, 2018
4f2fbeb
remove unnecessary exception
peterbae Nov 29, 2018
6daf7e9
thank you rene
peterbae Nov 29, 2018
ed695ce
Merge branch 'dev' of https://github.com/Microsoft/mssql-jdbc into gi…
peterbae Nov 29, 2018
4105f34
Merge branch 'github-816' of https://github.com/peterbae/mssql-jdbc i…
peterbae Nov 29, 2018
d14af0a
use original logic
peterbae Nov 29, 2018
54bf5be
remove unnecessary exception
peterbae Dec 12, 2018
0da2160
more tests
peterbae Dec 12, 2018
fb4de5b
Merge remote-tracking branch 'upstream/dev' into PR836
ulvii Dec 20, 2018
b1905b7
Add | Adding a different way of wildcard certificate validation
ulvii Dec 21, 2018
b9bcc21
Merge branch 'ms-dev' into github-816
cheenamalhotra Dec 21, 2018
cd6d8f9
Merge branch 'github-816' of https://github.com/peterbae/mssql-jdbc i…
ulvii Dec 21, 2018
7beb2c4
Fix | Fix the issue where sub-domain contains wildcard at the end and…
ulvii Dec 21, 2018
ad7e3f0
Fix | Fix comment
ulvii Dec 21, 2018
7c07349
Fix | Add new line to the end of the file
ulvii Dec 21, 2018
cb33eea
Merge branch 'dev' of https://github.com/Microsoft/mssql-jdbc into gi…
peterbae Jan 2, 2019
9602a75
Merge pull request #7 from ulvii/PR836
peterbae Jan 2, 2019
a09ecb7
reflect comment
peterbae Jan 2, 2019
abd362d
reflect ulvi's changes
peterbae Jan 2, 2019
bfa48c6
change comment style
peterbae Jan 2, 2019
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
61 changes: 50 additions & 11 deletions src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java
Original file line number Diff line number Diff line change
Expand Up @@ -1400,27 +1400,66 @@ private String parseCommonName(String distinguishedName) {
return commonName;
}

private boolean validateServerName(String nameInCert) throws CertificateException {
private boolean validateServerName(String nameInCert) {
// Failed to get the common name from DN or empty CN
if (null == nameInCert) {
if (logger.isLoggable(Level.FINER))
if (logger.isLoggable(Level.FINER)) {
logger.finer(logContext + " Failed to parse the name from the certificate or name is empty.");
}
return false;
}

// We do not allow wildcards in IDNs (xn--).
if (!nameInCert.startsWith("xn--") && nameInCert.contains("*")) {
int hostIndex = 0, certIndex = 0, match = 0, startIndex = -1, periodCount = 0;
while (hostIndex < hostName.length()) {
if ('.' == hostName.charAt(hostIndex)) {
periodCount++;
}
if (certIndex < nameInCert.length() && hostName.charAt(hostIndex) == nameInCert.charAt(certIndex)) {
hostIndex++;
certIndex++;
} else if (certIndex < nameInCert.length() && '*' == nameInCert.charAt(certIndex)) {
startIndex = certIndex;
match = hostIndex;
certIndex++;
} else if (startIndex != -1 && 0 == periodCount) {
certIndex = startIndex + 1;
match++;
hostIndex = match;
} else {
logFailMessage(nameInCert);
return false;
}
}
if (nameInCert.length() == certIndex && periodCount > 1) {
logSuccessMessage(nameInCert);
return true;
} else {
logFailMessage(nameInCert);
return false;
}
}
// Verify that the name in certificate matches exactly with the host name
if (!nameInCert.equals(hostName)) {
if (logger.isLoggable(Level.FINER))
logger.finer(logContext + " The name in certificate " + nameInCert
+ " does not match with the server name " + hostName + ".");
logFailMessage(nameInCert);
return false;
}
logSuccessMessage(nameInCert);
return true;
}

if (logger.isLoggable(Level.FINER))
private void logFailMessage(String nameInCert) {
if (logger.isLoggable(Level.FINER)) {
logger.finer(logContext + " The name in certificate " + nameInCert
+ " does not match with the server name " + hostName + ".");
}
}

private void logSuccessMessage(String nameInCert) {
if (logger.isLoggable(Level.FINER)) {
logger.finer(logContext + " The name in certificate:" + nameInCert + " validated against server name "
+ hostName + ".");

return true;
}
}

public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
Expand Down Expand Up @@ -4591,8 +4630,8 @@ void writeTVPRows(TVP value) throws SQLServerException {
SQLServerError databaseError = new SQLServerError();
databaseError.setFromTDS(tdsReader);

SQLServerException.makeFromDatabaseError(con, null, databaseError.getErrorMessage(), databaseError,
false);
SQLServerException.makeFromDatabaseError(con, null, databaseError.getErrorMessage(),
databaseError, false);
}

command.setInterruptsEnabled(true);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
/*
* Microsoft JDBC Driver for SQL Server Copyright(c) Microsoft Corporation All rights reserved. This program is made
* available under the terms of the MIT License. See the LICENSE file in the project root for more information.
*/

package com.microsoft.sqlserver.jdbc;
ulvii marked this conversation as resolved.
Show resolved Hide resolved

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import java.lang.reflect.Constructor;
import java.lang.reflect.Method;

import org.junit.jupiter.api.Test;
import org.junit.platform.runner.JUnitPlatform;
import org.junit.runner.RunWith;


@RunWith(JUnitPlatform.class)
public class SSLCertificateValidation {

/**
* Tests our internal method, validateServerName() against different possible names in SSL certificate.
*
* @throws Exception
*/
@Test
public void testValidateServerName() throws Exception {

String serverName = "msjdbc.database.windows.net";
String serverName2 = "bbbbuuzzuzzzzzz.example.net";
String serverName3 = "xn--ms.database.windows.net";

// Set up the HostNameOverrideX509TrustManager object using reflection
TDSChannel tdsc = new TDSChannel(new SQLServerConnection("someConnectionProperty"));
Class<?> hsoClass = Class.forName("com.microsoft.sqlserver.jdbc.TDSChannel$HostNameOverrideX509TrustManager");
Constructor<?> constructor = hsoClass.getDeclaredConstructors()[0];
constructor.setAccessible(true);
Object hsoObject = constructor.newInstance(null, tdsc, null, serverName);
Method method = hsoObject.getClass().getDeclaredMethod("validateServerName", String.class);
method.setAccessible(true);

// Server Name = msjdbc.database.windows.net
// SAN = msjdbc.database.windows.net
// Expected result: true
assertTrue((boolean) method.invoke(hsoObject, "msjdbc.database.windows.net"));

// Server Name = msjdbc.database.windows.net
// SAN = msjdbc***.database.windows.net
// Expected result: true
assertTrue((boolean) method.invoke(hsoObject, "msjdbc***.database.windows.net"));

// Server Name = msjdbc.database.windows.net
// SAN = ms*bc.database.windows.net
// Expected result: true
assertTrue((boolean) method.invoke(hsoObject, "ms*bc.database.windows.net"));

// Server Name = msjdbc.database.windows.net
// SAN = *bc.database.windows.net
// Expected result: true
assertTrue((boolean) method.invoke(hsoObject, "*bc.database.windows.net"));

// Server Name = msjdbc.database.windows.net
// SAN = ms*.database.windows.net
// Expected result: true
assertTrue((boolean) method.invoke(hsoObject, "ms*.database.windows.net"));

// Server Name = msjdbc.database.windows.net
// SAN = *jd*.database.windows.net
// Expected result: true
assertTrue((boolean) method.invoke(hsoObject, "*jd*.database.windows.net"));

// Server Name = msjdbc.database.windows.net
// SAN = ms.*.net
// Expected result: false
assertFalse((boolean) method.invoke(hsoObject, "ms.*.net"));

// Server Name = msjdbc.database.windows.net
// SAN = msjdbc.asd*dsa.windows.net
// Expected result: false
assertFalse((boolean) method.invoke(hsoObject, "msjdbc.asd*dsa.windows.net"));

// Server Name = msjdbc.database.windows.net
// SAN = .*.windows.net
// Expected result: false
assertFalse((boolean) method.invoke(hsoObject, ".*.windows.net"));

// Server Name = msjdbc.database.windows.net
// SAN = msjdbc.*.windows.net
// Expected result: false
assertFalse((boolean) method.invoke(hsoObject, "msjdbc.*.windows.net"));

// Server Name = msjdbc.database.windows.net
// SAN = *.*.windows.net
// Expected result: false
// Note: multiple wildcards are not allowed, so this case shouldn't happen, but we still make sure to fail this.
assertFalse((boolean) method.invoke(hsoObject, "*.*.windows.net"));

// Server Name = msjdbc.database.windows.net
// SAN = *.com
// Expected result: false
// A cert with * plus a top-level domain is not allowed.
assertFalse((boolean) method.invoke(hsoObject, "*.com"));

// Server Name = msjdbc.database.windows.net
// SAN = xn--ms*.database.windows.net
// Expected result: fail
assertFalse((boolean) method.invoke(hsoObject, "xn--ms*.database.windows.net"));

// Server Name = msjdbc.database.windows.net
// SAN = *
// Expected result: fail
assertFalse((boolean) method.invoke(hsoObject, "*"));

// Server Name = msjdbc.database.windows.net
// SAN = ms*atabase.windows.net
// Expected result: fail
assertFalse((boolean) method.invoke(hsoObject, "ms*atabase.windows.net"));

hsoObject = constructor.newInstance(null, tdsc, null, serverName2);
method = hsoObject.getClass().getDeclaredMethod("validateServerName", String.class);
method.setAccessible(true);

// Server Name = bbbbuuzzuzzzzzz.example.net
// SAN = b*zzz.example.net
// Expected result: true
assertTrue((boolean) method.invoke(hsoObject, "b*zzz.example.net"));

hsoObject = constructor.newInstance(null, tdsc, null, serverName3);
method = hsoObject.getClass().getDeclaredMethod("validateServerName", String.class);
method.setAccessible(true);

// Server Name = xn--ms.database.windows.net
// SAN = xn--ms.database.windows.net
// Expected result: true
assertTrue((boolean) method.invoke(hsoObject, "xn--ms.database.windows.net"));
}
}