use configured KDCs even if "dns_lookup_kdc" is true

use configured KDCs even if "dns_lookup_kdc" is true

client/client_dns_test.go

@@ -2,7 +2,6 @@ package client
 
 import (
 	"encoding/hex"
-	"github.com/stretchr/testify/assert"
 	"gopkg.in/jcmturner/gokrb5.v7/config"
 	"gopkg.in/jcmturner/gokrb5.v7/keytab"
 	"gopkg.in/jcmturner/gokrb5.v7/test"
@@ -10,48 +9,6 @@ import (
 	"testing"
 )
 
-func TestResolveKDC(t *testing.T) {
-	test.Privileged(t)
-
-	//ns := os.Getenv("DNSUTILS_OVERRIDE_NS")
-	//if ns == "" {
-	//	os.Setenv("DNSUTILS_OVERRIDE_NS", testdata.TEST_NS)
-	//}
-	c, _ := config.NewConfigFromString(testdata.TEST_KRB5CONF)
-	c.LibDefaults.DNSLookupKDC = true
-	var cl Client
-	cl.Config = c
-	count, res, err := cl.Config.GetKDCs(c.LibDefaults.DefaultRealm, true)
-	if err != nil {
-		t.Errorf("error resolving KDC via DNS TCP: %v", err)
-	}
-	assert.Equal(t, 5, count, "Number of SRV records not as expected: %v", res)
-	assert.Equal(t, count, len(res), "Map size does not match: %v", res)
-	expected := []string{
-		"kdc.test.gokrb5:88",
-		"kdc1a.test.gokrb5:88",
-		"kdc2a.test.gokrb5:88",
-		"kdc1b.test.gokrb5:88",
-		"kdc2b.test.gokrb5:88",
-	}
-	for _, s := range expected {
-		var found bool
-		for _, v := range res {
-			if s == v {
-				found = true
-				break
-			}
-		}
-		assert.True(t, found, "Record %s not found in results", s)
-	}
-	c.LibDefaults.DNSLookupKDC = false
-	_, res, err = cl.Config.GetKDCs(c.LibDefaults.DefaultRealm, true)
-	if err != nil {
-		t.Errorf("error resolving KDCs from config: %v", err)
-	}
-	assert.Equal(t, "127.0.0.1:88", res[1], "KDC not read from config as expected")
-}
-
 func TestClient_Login_DNSKDCs(t *testing.T) {
 	test.Privileged(t)
 

config/hosts.go

@@ -18,37 +18,41 @@ func (c *Config) GetKDCs(realm string, tcp bool) (int, map[int]string, error) {
 	kdcs := make(map[int]string)
 	var count int
 
-	// Use DNS to resolve kerberos SRV records if configured to do so in krb5.conf.
-	if c.LibDefaults.DNSLookupKDC {
-		proto := "udp"
-		if tcp {
-			proto = "tcp"
-		}
-		c, addrs, err := dnsutils.OrderedSRV("kerberos", proto, realm)
-		if err != nil {
-			return count, kdcs, err
-		}
-		if len(addrs) < 1 {
-			return count, kdcs, fmt.Errorf("no KDC SRV records found for realm %s", realm)
-		}
-		count = c
-		for k, v := range addrs {
-			kdcs[k] = strings.TrimRight(v.Target, ".") + ":" + strconv.Itoa(int(v.Port))
-		}
-	} else {
-		// Get the KDCs from the krb5.conf an order them randomly for preference.
-		var ks []string
-		for _, r := range c.Realms {
-			if r.Realm == realm {
-				ks = r.KDC
-				break
-			}
-		}
-		count = len(ks)
-		if count < 1 {
-			return count, kdcs, fmt.Errorf("no KDCs defined in configuration for realm %s", realm)
+	// Get the KDCs from the krb5.conf.
+	var ks []string
+	for _, r := range c.Realms {
+		if r.Realm != realm {
+			continue
 		}
+		ks = r.KDC
+	}
+	count = len(ks)
+
+	if count > 0 {
+		// Order the kdcs randomly for preference.
 		kdcs = randServOrder(ks)
+		return count, kdcs, nil
+	}
+
+	if !c.LibDefaults.DNSLookupKDC {
+		return count, kdcs, fmt.Errorf("no KDCs defined in configuration for realm %s", realm)
+	}
+
+	// Use DNS to resolve kerberos SRV records.
+	proto := "udp"
+	if tcp {
+		proto = "tcp"
+	}
+	index, addrs, err := dnsutils.OrderedSRV("kerberos", proto, realm)
+	if err != nil {
+		return count, kdcs, err
+	}
+	if len(addrs) < 1 {
+		return count, kdcs, fmt.Errorf("no KDC SRV records found for realm %s", realm)
+	}
+	count = index
+	for k, v := range addrs {
+		kdcs[k] = strings.TrimRight(v.Target, ".") + ":" + strconv.Itoa(int(v.Port))
 	}
 	return count, kdcs, nil
 }

config/hosts_test.go

@@ -0,0 +1,91 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"gopkg.in/jcmturner/gokrb5.v7/test"
+	"gopkg.in/jcmturner/gokrb5.v7/test/testdata"
+)
+
+func TestConfig_GetKDCsUsesConfiguredKDC(t *testing.T) {
+	t.Parallel()
+
+	// This test is meant to cover the fix for
+	// https://github.com/jcmturner/gokrb5/issues/332
+	krb5ConfWithKDCAndDNSLookupKDC := `
+[libdefaults]
+ dns_lookup_kdc = true
+
+[realms]
+ TEST.GOKRB5 = {
+  kdc = kdc2b.test.gokrb5:88
+ }
+`
+
+	c, err := NewConfigFromString(krb5ConfWithKDCAndDNSLookupKDC)
+	if err != nil {
+		t.Fatalf("Error loading config: %v", err)
+	}
+
+	count, kdcs, err := c.GetKDCs("TEST.GOKRB5", false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if count != 1 {
+		t.Fatalf("expected 1 but received %d", count)
+	}
+	if kdcs[1] != "kdc2b.test.gokrb5:88" {
+		t.Fatalf("expected kdc2b.test.gokrb5:88 but received %s", kdcs[1])
+	}
+}
+
+func TestResolveKDC(t *testing.T) {
+	test.Privileged(t)
+
+	c, err := NewConfigFromString(testdata.TEST_KRB5CONF)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Store the original value for realms since we'll use them in our
+	// second test.
+	originalRealms := c.Realms
+
+	// For our first test, let's check that we discover the expected
+	// KDCs when they're not provided and we should be looking them up.
+	c.LibDefaults.DNSLookupKDC = true
+	c.Realms = make([]Realm, 0)
+	count, res, err := c.GetKDCs(c.LibDefaults.DefaultRealm, true)
+	if err != nil {
+		t.Errorf("error resolving KDC via DNS TCP: %v", err)
+	}
+	assert.Equal(t, 5, count, "Number of SRV records not as expected: %v", res)
+	assert.Equal(t, count, len(res), "Map size does not match: %v", res)
+	expected := []string{
+		"kdc.test.gokrb5:88",
+		"kdc1a.test.gokrb5:88",
+		"kdc2a.test.gokrb5:88",
+		"kdc1b.test.gokrb5:88",
+		"kdc2b.test.gokrb5:88",
+	}
+	for _, s := range expected {
+		var found bool
+		for _, v := range res {
+			if s == v {
+				found = true
+				break
+			}
+		}
+		assert.True(t, found, "Record %s not found in results", s)
+	}
+
+	// For our second check, verify that when we shouldn't be looking them up,
+	// we get the expected value.
+	c.LibDefaults.DNSLookupKDC = false
+	c.Realms = originalRealms
+	_, res, err = c.GetKDCs(c.LibDefaults.DefaultRealm, true)
+	if err != nil {
+		t.Errorf("error resolving KDCs from config: %v", err)
+	}
+	assert.Equal(t, "127.0.0.1:88", res[1], "KDC not read from config as expected")
+}