Removed illegal reflection access in kerberos authentication (#839)

* tests cleanup * resolved conflict * fixed typo * fixes * fixes * fixed * review comments * review updates * review update * more review changes * more try-with-resources changes * more * spacing change only * use shared statement in Abstract test again, this will be removed in another PR * try-with-resources for Stream types * remove hardcoded table names * remove shared AE vars * added comment on driver bug * add missing escape characters * get rid of unnecessary bvtTestSetup * more hardcode * fixed issue with AE tests * fixed issue with AE tests * removed more hardcoded values and modified BatchExecutionWithBulkCopyTest to be consistent with other tests * drop all tables after tests * format * remove unused imports * cleanup procedures * Fix compilation error * fix merge errors * removed junk file * add comments * Update DNSKerberosLocator.java fixed typo * Update KerbAuthentication.java removed change of logging level * Update DNSKerberosLocator.java cosmetics * Update DNSKerberosLocator.java removed stupid tab!!
microsoft · Oct 27, 2018 · 7f097cd · 7f097cd
1 parent 7679dbe
commit 7f097cd
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 68 deletions.
diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/KerbAuthentication.java b/src/main/java/com/microsoft/sqlserver/jdbc/KerbAuthentication.java
@@ -251,7 +251,7 @@ private String enrichSpnWithRealm(String spn, boolean allowHostnameCanonicalizat
         }
         String dnsName = m.group(1);
         String portOrInstance = m.group(2);
-        RealmValidator realmValidator = getRealmValidator(dnsName);
+        RealmValidator realmValidator = getRealmValidator();
         String realm = findRealmFromHostname(realmValidator, dnsName);
         if (realm == null && allowHostnameCanonicalization) {
             // We failed, try with canonical host name to find a better match
@@ -277,50 +277,15 @@ private String enrichSpnWithRealm(String spn, boolean allowHostnameCanonicalizat
     private static RealmValidator validator;
 
     /**
-     * Find a suitable way of validating a REALM for given JVM.
+     * Get validator to validate REALM for given JVM.
      *
-     * @param hostnameToTest
-     *        an example hostname we are gonna use to test our realm validator.
-     * @return a not null realm Validator.
+     * @return a not null realm validator.
      */
-    static RealmValidator getRealmValidator(String hostnameToTest) {
+    static RealmValidator getRealmValidator() {
         if (validator != null) {
             return validator;
         }
-        // JVM Specific, here Sun/Oracle JVM
-        try {
-            Class<?> clz = Class.forName("sun.security.krb5.Config");
-            Method getInstance = clz.getMethod("getInstance", new Class[0]);
-            final Method getKDCList = clz.getMethod("getKDCList", new Class[] {String.class});
-            final Object instance = getInstance.invoke(null);
-            RealmValidator oracleRealmValidator = new RealmValidator() {
-
-                @Override
-                public boolean isRealmValid(String realm) {
-                    try {
-                        Object ret = getKDCList.invoke(instance, realm);
-                        return ret != null;
-                    } catch (Exception err) {
-                        return false;
-                    }
-                }
-            };
-            validator = oracleRealmValidator;
-            // As explained here: https://github.com/Microsoft/mssql-jdbc/pull/40#issuecomment-281509304
-            // The default Oracle Resolution mechanism is not bulletproof
-            // If it resolves a non-existing name, drop it.
-            if (!validator.isRealmValid("this.might.not.exist." + hostnameToTest)) {
-                // Our realm validator is well working, return it
-                authLogger.fine("Kerberos Realm Validator: Using Built-in Oracle Realm Validation method.");
-                return oracleRealmValidator;
-            }
-            authLogger
-                    .fine("Kerberos Realm Validator: Detected buggy Oracle Realm Validator, using DNSKerberosLocator.");
-        } catch (ReflectiveOperationException notTheRightJVMException) {
-            // Ignored, we simply are not using the right JVM
-            authLogger.fine("Kerberos Realm Validator: No Oracle Realm Validator Available, using DNSKerberosLocator.");
-        }
-        // No implementation found, default one, not any realm is valid
+
         validator = new RealmValidator() {
             @Override
             public boolean isRealmValid(String realm) {

diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/dns/DNSKerberosLocator.java b/src/main/java/com/microsoft/sqlserver/jdbc/dns/DNSKerberosLocator.java
@@ -9,7 +9,6 @@
 import javax.naming.NameNotFoundException;
 import javax.naming.NamingException;
 
-
 /**
  * Represents a DNS Kerberos Locator
  */
@@ -18,7 +17,9 @@ public final class DNSKerberosLocator {
     private DNSKerberosLocator() {}
 
     /**
-     * Returns whether a realm is valid.
+     * Returns whether a realm is valid by retrieving the KDC list in DNS SRV records.
+     * This will only work if DNS lookup is setup properly or the realms are properly defined in krb5 config file.
+     * Otherwise this will fail since the realm cannot be found.
      *
      * @param realmName
      *        the realm to test
@@ -37,6 +38,7 @@ public static boolean isRealmValid(String realmName) throws NamingException {
             Set<DNSRecordSRV> records = DNSUtilities.findSrvRecords("_kerberos._udp." + realmName);
             return !records.isEmpty();
         } catch (NameNotFoundException wrongDomainException) {
+            // config error - domain controller cannot be located via DNS 
             return false;
         }
     }

diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/dns/DNSRealmsTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/dns/DNSRealmsTest.java