Browse Source

get host addresses

set addresses in AS and TGS REQ

option to require host address

fix TGS_REP validation check

remove obsolete argument from krb5Token.NewAuthenticator func

updated contribution guide

fix krb5token test

fix krb5token
Jonathan Turner 8 years ago
parent
commit
cdbf51975b

+ 1 - 2
CONTRIBUTING.md

@@ -55,8 +55,7 @@ incorporated faster this way.
 ```
 <short summary starting with a verb in lowercase and less than 50 characters>
 
-More detailed explanatory text, if necessary.  Wrap it to about 72 
-characters or so.
+More detailed explanatory text. This should reference the related issue.
 ```
 This to adhere to the [git best practice](https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project) and 
 mirror the [contribution guidelines for the Go standard libarary](https://golang.org/doc/contribute.html).

+ 0 - 1
client/client_dns_test.go

@@ -26,7 +26,6 @@ func TestResolveKDC(t *testing.T) {
 	}
 	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)
-	t.Logf("res: %v", res)
 	expected := []string{
 		"kdc.test.gokrb5:88",
 		"kdc1a.test.gokrb5:88",

+ 15 - 7
config/krb5conf.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/jcmturner/gofork/encoding/asn1"
 	"gopkg.in/jcmturner/gokrb5.v4/iana/etypeID"
+	"net"
 )
 
 // Config represents the KRB5 configuration.
@@ -58,13 +59,13 @@ type LibDefaults struct {
 	DNSCanonicalizeHostname bool     //default true
 	DNSLookupKDC            bool     //default false
 	DNSLookupRealm          bool
-	//extra_addresses []net.IPAddr //Not implementing yet
-	Forwardable            bool           //default false
-	IgnoreAcceptorHostname bool           //default false
-	K5LoginAuthoritative   bool           //default false
-	K5LoginDirectory       string         //default user's home directory. Must be owned by the user or root
-	KDCDefaultOptions      asn1.BitString //default 0x00000010 (KDC_OPT_RENEWABLE_OK)
-	KDCTimeSync            int            //default 1
+	ExtraAddresses          []net.IP       //Not implementing yet
+	Forwardable             bool           //default false
+	IgnoreAcceptorHostname  bool           //default false
+	K5LoginAuthoritative    bool           //default false
+	K5LoginDirectory        string         //default user's home directory. Must be owned by the user or root
+	KDCDefaultOptions       asn1.BitString //default 0x00000010 (KDC_OPT_RENEWABLE_OK)
+	KDCTimeSync             int            //default 1
 	//kdc_req_checksum_type int //unlikely to implement as for very old KDCs
 	NoAddresses         bool     //default true
 	PermittedEnctypes   []string //default aes256-cts-hmac-sha1-96 aes128-cts-hmac-sha1-96 des3-cbc-sha1 arcfour-hmac-md5 camellia256-cts-cmac camellia128-cts-cmac des-cbc-crc des-cbc-md5 des-cbc-md4
@@ -178,6 +179,13 @@ func (l *LibDefaults) parseLines(lines []string) error {
 				return fmt.Errorf("libdefaults configuration line invalid. %v: %s", err, line)
 			}
 			l.DNSLookupRealm = v
+		case "extra_addresses":
+			ipStr := strings.Replace(p[1], " ", "", -1)
+			for _, ip := range strings.Split(ipStr, ",") {
+				if eip := net.ParseIP(ip); eip != nil {
+					l.ExtraAddresses = append(l.ExtraAddresses, eip)
+				}
+			}
 		case "forwardable":
 			v, err := parseBoolean(p[1])
 			if err != nil {

+ 2 - 1
config/krb5conf_test.go

@@ -81,6 +81,7 @@ const (
  admin_server = FILE:/var/log/kerberos/kadmind.log
 
 [libdefaults]
+ noaddresses = true
  default_realm = TEST.GOKRB5
  dns_lookup_realm = false
 
@@ -228,7 +229,7 @@ func TestLoad2(t *testing.T) {
 
 	assert.Equal(t, "TEST.GOKRB5", c.DomainRealm[".test.gokrb5"], "Domain to realm mapping not as expected")
 	assert.Equal(t, "TEST.GOKRB5", c.DomainRealm["test.gokrb5"], "Domain to realm mapping not as expected")
-
+	assert.True(t, c.LibDefaults.NoAddresses, "No address not set as true")
 }
 
 func TestLoadNoBlankLines(t *testing.T) {

+ 3 - 5
gssapi/krb5Token.go

@@ -128,7 +128,7 @@ func NewAPREQMechToken(creds credentials.Credentials, tkt messages.Ticket, sessi
 	tb, _ := hex.DecodeString(TOK_ID_KRB_AP_REQ)
 	m.TokID = tb
 
-	auth, err := NewAuthenticator(creds, sessionKey.KeyType, GSSAPIFlags)
+	auth, err := NewAuthenticator(creds, GSSAPIFlags)
 	if err != nil {
 		return m, err
 	}
@@ -147,10 +147,8 @@ func NewAPREQMechToken(creds credentials.Credentials, tkt messages.Ticket, sessi
 	return m, nil
 }
 
-// NewAuthenticator (DEPRECATED - this method will be updated in future versions to remove
-// the obsolete keyType argument and may be made private to the gssapi package)
-// creates a new kerberos authenticator for kerberos MechToken
-func NewAuthenticator(creds credentials.Credentials, keyType int32, flags []int) (types.Authenticator, error) {
+// NewAuthenticator creates a new kerberos authenticator for kerberos MechToken
+func NewAuthenticator(creds credentials.Credentials, flags []int) (types.Authenticator, error) {
 	//RFC 4121 Section 4.1.1
 	auth, err := types.NewAuthenticator(creds.Realm, creds.CName)
 	if err != nil {

+ 2 - 3
gssapi/krb5Token_test.go

@@ -49,7 +49,7 @@ func TestMechToken_newAuthenticatorWithSubkeyGeneration(t *testing.T) {
 	creds.CName.NameString = testdata.TEST_PRINCIPALNAME_NAMESTRING
 	var etypeID int32 = 18
 	keyLen := 32 // etypeID 18 refers to AES256 -> 32 bytes key
-	a, err := NewAuthenticator(creds, etypeID, []int{GSS_C_INTEG_FLAG, GSS_C_CONF_FLAG})
+	a, err := NewAuthenticator(creds, []int{GSS_C_INTEG_FLAG, GSS_C_CONF_FLAG})
 	if err != nil {
 		t.Fatalf("Error creating authenticator: %v", err)
 	}
@@ -73,8 +73,7 @@ func TestMechToken_newAuthenticatorWithSubkeyGeneration(t *testing.T) {
 func TestMechToken_newAuthenticator(t *testing.T) {
 	creds := credentials.NewCredentials("hftsai", testdata.TEST_REALM)
 	creds.CName.NameString = testdata.TEST_PRINCIPALNAME_NAMESTRING
-	var etypeID int32 = 18
-	a, err := NewAuthenticator(creds, etypeID, []int{GSS_C_INTEG_FLAG, GSS_C_CONF_FLAG})
+	a, err := NewAuthenticator(creds, []int{GSS_C_INTEG_FLAG, GSS_C_CONF_FLAG})
 	if err != nil {
 		t.Fatalf("Error creating authenticator: %v", err)
 	}

+ 1 - 1
krberror/error.go

@@ -62,7 +62,7 @@ func Errorf(err error, et, format string, a ...interface{}) Krberror {
 func NewErrorf(et, format string, a ...interface{}) Krberror {
 	var s string
 	if len(a) > 0 {
-		s = fmt.Sprintf("%s: %s", et, fmt.Sprintf(format, a))
+		s = fmt.Sprintf("%s: %s", et, fmt.Sprintf(format, a...))
 	} else {
 		s = fmt.Sprintf("%s: %s", et, format)
 	}

+ 1 - 1
messages/KDCRep.go

@@ -297,7 +297,7 @@ func (k *TGSRep) IsValid(cfg *config.Config, tgsReq TGSReq) (bool, error) {
 	if k.DecryptedEncPart.SRealm != tgsReq.ReqBody.Realm {
 		return false, krberror.NewErrorf(krberror.KRBMsgError, "SRealm in response does not match what was requested. Requested: %s; Reply: %s", tgsReq.ReqBody.Realm, k.DecryptedEncPart.SRealm)
 	}
-	if len(tgsReq.ReqBody.Addresses) > 0 {
+	if len(k.DecryptedEncPart.CAddr) > 0 {
 		if !types.HostAddressesEqual(k.DecryptedEncPart.CAddr, tgsReq.ReqBody.Addresses) {
 			return false, krberror.NewErrorf(krberror.KRBMsgError, "Addresses listed in the TGS_REP does not match those listed in the TGS_REQ")
 		}

+ 16 - 1
messages/KDCReq.go

@@ -126,7 +126,14 @@ func NewASReq(realm string, c *config.Config, cname types.PrincipalName) (ASReq,
 		types.SetFlag(&a.ReqBody.KDCOptions, flags.Renewable)
 		a.ReqBody.RTime = t.Add(c.LibDefaults.RenewLifetime)
 		a.ReqBody.RTime = t.Add(time.Duration(48) * time.Hour)
-
+	}
+	if !c.LibDefaults.NoAddresses {
+		ha, err := types.LocalHostAddresses()
+		if err != nil {
+			return a, fmt.Errorf("could not get local addresses: %v", err)
+		}
+		ha = append(ha, types.HostAddressesFromNetIPs(c.LibDefaults.ExtraAddresses)...)
+		a.ReqBody.Addresses = ha
 	}
 	return a, nil
 }
@@ -166,6 +173,14 @@ func NewTGSReq(cname types.PrincipalName, kdcRealm string, c *config.Config, tkt
 		types.SetFlag(&a.ReqBody.KDCOptions, flags.Renewable)
 		a.ReqBody.RTime = t.Add(c.LibDefaults.RenewLifetime)
 	}
+	if !c.LibDefaults.NoAddresses {
+		ha, err := types.LocalHostAddresses()
+		if err != nil {
+			return a, fmt.Errorf("could not get local addresses: %v", err)
+		}
+		ha = append(ha, types.HostAddressesFromNetIPs(c.LibDefaults.ExtraAddresses)...)
+		a.ReqBody.Addresses = ha
+	}
 	if renewal {
 		types.SetFlag(&a.ReqBody.KDCOptions, flags.Renew)
 		types.SetFlag(&a.ReqBody.KDCOptions, flags.Renewable)

+ 4 - 1
service/APExchange.go

@@ -15,7 +15,7 @@ import (
 )
 
 // ValidateAPREQ validates an AP_REQ sent to the service. Returns a boolean for if the AP_REQ is valid and the client's principal name and realm.
-func ValidateAPREQ(APReq messages.APReq, kt keytab.Keytab, sa string, cAddr string) (bool, credentials.Credentials, error) {
+func ValidateAPREQ(APReq messages.APReq, kt keytab.Keytab, sa string, cAddr string, requireHostAddr bool) (bool, credentials.Credentials, error) {
 	var creds credentials.Credentials
 	err := APReq.Ticket.DecryptEncPart(kt, sa)
 	if err != nil {
@@ -50,6 +50,9 @@ func ValidateAPREQ(APReq messages.APReq, kt keytab.Keytab, sa string, cAddr stri
 			err := messages.NewKRBError(APReq.Ticket.SName, APReq.Ticket.Realm, errorcode.KRB_AP_ERR_BADADDR, "Client address not within the list contained in the service ticket")
 			return false, creds, err
 		}
+	} else if requireHostAddr {
+		err := messages.NewKRBError(APReq.Ticket.SName, APReq.Ticket.Realm, errorcode.KRB_AP_ERR_BADADDR, "ticket does not contain HostAddress values required")
+		return false, creds, err
 	}
 
 	// Check the clock skew between the client and the service server

+ 8 - 8
service/APExchange_test.go

@@ -49,7 +49,7 @@ func TestValidateAPREQ(t *testing.T) {
 		t.Fatalf("Error getting test AP_REQ: %v", err)
 	}
 
-	ok, _, err := ValidateAPREQ(APReq, kt, "", "127.0.0.1")
+	ok, _, err := ValidateAPREQ(APReq, kt, "", "127.0.0.1", false)
 	if !ok || err != nil {
 		t.Fatalf("Validation of AP_REQ failed when it should not have: %v", err)
 	}
@@ -92,7 +92,7 @@ func TestValidateAPREQ_KRB_AP_ERR_BADMATCH(t *testing.T) {
 		t.Fatalf("Error getting test AP_REQ: %v", err)
 	}
 
-	ok, _, err := ValidateAPREQ(APReq, kt, "", "127.0.0.1")
+	ok, _, err := ValidateAPREQ(APReq, kt, "", "127.0.0.1", false)
 	if ok || err == nil {
 		t.Fatal("Validation of AP_REQ passed when it should not have")
 	}
@@ -137,7 +137,7 @@ func TestValidateAPREQ_LargeClockSkew(t *testing.T) {
 		t.Fatalf("Error getting test AP_REQ: %v", err)
 	}
 
-	ok, _, err := ValidateAPREQ(APReq, kt, "", "127.0.0.1")
+	ok, _, err := ValidateAPREQ(APReq, kt, "", "127.0.0.1", false)
 	if ok || err == nil {
 		t.Fatal("Validation of AP_REQ passed when it should not have")
 	}
@@ -180,12 +180,12 @@ func TestValidateAPREQ_Replay(t *testing.T) {
 		t.Fatalf("Error getting test AP_REQ: %v", err)
 	}
 
-	ok, _, err := ValidateAPREQ(APReq, kt, "", "127.0.0.1")
+	ok, _, err := ValidateAPREQ(APReq, kt, "", "127.0.0.1", false)
 	if !ok || err != nil {
 		t.Fatalf("Validation of AP_REQ failed when it should not have: %v", err)
 	}
 	// Replay
-	ok, _, err = ValidateAPREQ(APReq, kt, "", "127.0.0.1")
+	ok, _, err = ValidateAPREQ(APReq, kt, "", "127.0.0.1", false)
 	if ok || err == nil {
 		t.Fatal("Validation of AP_REQ passed when it should not have")
 	}
@@ -226,7 +226,7 @@ func TestValidateAPREQ_FutureTicket(t *testing.T) {
 		t.Fatalf("Error getting test AP_REQ: %v", err)
 	}
 
-	ok, _, err := ValidateAPREQ(APReq, kt, "", "127.0.0.1")
+	ok, _, err := ValidateAPREQ(APReq, kt, "", "127.0.0.1", false)
 	if ok || err == nil {
 		t.Fatal("Validation of AP_REQ passed when it should not have")
 	}
@@ -271,7 +271,7 @@ func TestValidateAPREQ_InvalidTicket(t *testing.T) {
 		t.Fatalf("Error getting test AP_REQ: %v", err)
 	}
 
-	ok, _, err := ValidateAPREQ(APReq, kt, "", "127.0.0.1")
+	ok, _, err := ValidateAPREQ(APReq, kt, "", "127.0.0.1", false)
 	if ok || err == nil {
 		t.Fatal("Validation of AP_REQ passed when it should not have")
 	}
@@ -315,7 +315,7 @@ func TestValidateAPREQ_ExpiredTicket(t *testing.T) {
 		t.Fatalf("Error getting test AP_REQ: %v", err)
 	}
 
-	ok, _, err := ValidateAPREQ(APReq, kt, "", "127.0.0.1")
+	ok, _, err := ValidateAPREQ(APReq, kt, "", "127.0.0.1", false)
 	if ok || err == nil {
 		t.Fatal("Validation of AP_REQ passed when it should not have")
 	}

+ 2 - 1
service/authenticator.go

@@ -20,6 +20,7 @@ type SPNEGOAuthenticator struct {
 	Keytab            *keytab.Keytab
 	ServiceAccount    string
 	ClientAddr        string
+	RequireHostAddr   bool
 }
 
 // Authenticate and retrieve a goidentity.Identity. In this case it is a pointer to a credentials.Credentials
@@ -50,7 +51,7 @@ func (a SPNEGOAuthenticator) Authenticate() (i goidentity.Identity, ok bool, err
 		return
 	}
 
-	ok, c, err := ValidateAPREQ(mt.APReq, *a.Keytab, a.ServiceAccount, a.ClientAddr)
+	ok, c, err := ValidateAPREQ(mt.APReq, *a.Keytab, a.ServiceAccount, a.ClientAddr, a.RequireHostAddr)
 	if err != nil {
 		err = fmt.Errorf("SPNEGO validation error: %v", err)
 		return

+ 2 - 2
service/http.go

@@ -31,7 +31,7 @@ const (
 // sa - service account name.
 // If Active Directory is used for the KDC this is the account name you have set the SPN against (setspn.exe -a "HTTP/<fqdn>" <account name>)
 // If the SPN was added to the KDC without associating it with an account pass and empty string "". This is the case if you create the SPN in MIT KDC with: /usr/sbin/kadmin.local -q "add_principal HTTP/<fqdn>"
-func SPNEGOKRB5Authenticate(f http.Handler, kt keytab.Keytab, sa string, l *log.Logger) http.Handler {
+func SPNEGOKRB5Authenticate(f http.Handler, kt keytab.Keytab, sa string, requireHostAddr bool, l *log.Logger) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		s := strings.SplitN(r.Header.Get("Authorization"), " ", 2)
 		if len(s) != 2 || s[0] != "Negotiate" {
@@ -66,7 +66,7 @@ func SPNEGOKRB5Authenticate(f http.Handler, kt keytab.Keytab, sa string, l *log.
 			return
 		}
 
-		if ok, creds, err := ValidateAPREQ(mt.APReq, kt, sa, r.RemoteAddr); ok {
+		if ok, creds, err := ValidateAPREQ(mt.APReq, kt, sa, r.RemoteAddr, requireHostAddr); ok {
 			ctx := r.Context()
 			ctx = context.WithValue(ctx, CTXKeyCredentials, creds)
 			ctx = context.WithValue(ctx, CTXKeyAuthenticated, true)

+ 1 - 1
service/http_test.go

@@ -245,7 +245,7 @@ func httpServer() *httptest.Server {
 	b, _ := hex.DecodeString(testdata.HTTP_KEYTAB)
 	kt, _ := keytab.Parse(b)
 	th := http.HandlerFunc(testAppHandler)
-	s := httptest.NewServer(SPNEGOKRB5Authenticate(th, kt, "", l))
+	s := httptest.NewServer(SPNEGOKRB5Authenticate(th, kt, "", false, l))
 	return s
 }
 

+ 1 - 0
testdata/test_vectors.go

@@ -139,6 +139,7 @@ const (
   forwardable = yes
   default_tkt_enctypes = aes256-cts-hmac-sha1-96
   default_tgs_enctypes = aes256-cts-hmac-sha1-96
+  noaddresses = false
 
 [realms]
  TEST.GOKRB5 = {

+ 70 - 18
types/HostAddress.go

@@ -7,6 +7,7 @@ import (
 	"bytes"
 	"fmt"
 	"github.com/jcmturner/gofork/encoding/asn1"
+	"gopkg.in/jcmturner/gokrb5.v4/iana/addrtype"
 	"net"
 )
 
@@ -34,18 +35,6 @@ address
 	This field encodes a single address of type addr-type.
 */
 
-const (
-	addrTypeIPv4          = 2
-	addrTypeDirectional   = 3
-	addrTypeChaosNet      = 5
-	addrTypeXNS           = 6
-	addrTypeISO           = 7
-	addrTypeDECNETPhaseIV = 12
-	addrTypeAppleTalkDDP  = 16
-	addrTypeNetBios       = 20
-	addrTypeIPv6          = 24
-)
-
 // HostAddresses implements RFC 4120 type: https://tools.ietf.org/html/rfc4120#section-5.2.5
 type HostAddresses []HostAddress
 
@@ -60,20 +49,20 @@ func GetHostAddress(s string) (HostAddress, error) {
 	var h HostAddress
 	cAddr, _, err := net.SplitHostPort(s)
 	if err != nil {
-		return h, fmt.Errorf("Invalid format of client address: %v", err)
+		return h, fmt.Errorf("invalid format of client address: %v", err)
 	}
 	ip := net.ParseIP(cAddr)
 	hb, err := ip.MarshalText()
 	if err != nil {
-		return h, fmt.Errorf("Could not marshal client's address into bytes: %v", err)
+		return h, fmt.Errorf("could not marshal client's address into bytes: %v", err)
 	}
 	var ht int32
 	if ip.To4() != nil {
-		ht = addrTypeIPv4
+		ht = addrtype.IPv4
 	} else if ip.To16() != nil {
-		ht = addrTypeIPv6
+		ht = addrtype.IPv6
 	} else {
-		return h, fmt.Errorf("Could not determine client's address types: %v", err)
+		return h, fmt.Errorf("could not determine client's address types: %v", err)
 	}
 	h = HostAddress{
 		AddrType: ht,
@@ -89,6 +78,70 @@ func (h *HostAddress) GetAddress() (string, error) {
 	return string(b), err
 }
 
+// LocalHostAddresses returns a HostAddresses struct for the local machines interface IP addresses.
+func LocalHostAddresses() (ha HostAddresses, err error) {
+	ifs, err := net.Interfaces()
+	if err != nil {
+		return
+	}
+	for _, iface := range ifs {
+		if iface.Flags&net.FlagLoopback != 0 || iface.Flags&net.FlagUp == 0 {
+			// Interface is either loopback of not up
+			continue
+		}
+		addrs, err := iface.Addrs()
+		if err != nil {
+			continue
+		}
+		for _, addr := range addrs {
+			var ip net.IP
+			switch v := addr.(type) {
+			case *net.IPNet:
+				ip = v.IP
+			case *net.IPAddr:
+				ip = v.IP
+			}
+			var a HostAddress
+			if ip.To16() == nil {
+				//neither IPv4 or IPv6
+				continue
+			}
+			if ip.To4() != nil {
+				//Is IPv4
+				a.AddrType = addrtype.IPv4
+				a.Address = ip.To4()
+			} else {
+				a.AddrType = addrtype.IPv6
+				a.Address = ip.To16()
+			}
+			ha = append(ha, a)
+		}
+	}
+	return ha, nil
+}
+
+func HostAddressesFromNetIPs(ips []net.IP) (ha HostAddresses) {
+	for _, ip := range ips {
+		ha = append(ha, HostAddressFromNetIP(ip))
+	}
+	return ha
+}
+
+func HostAddressFromNetIP(ip net.IP) HostAddress {
+	if ip.To4() != nil {
+		//Is IPv4
+		return HostAddress{
+			AddrType: addrtype.IPv4,
+			Address:  ip.To4(),
+		}
+	} else {
+		return HostAddress{
+			AddrType: addrtype.IPv6,
+			Address:  ip.To16(),
+		}
+	}
+}
+
 // HostAddressesEqual tests if two HostAddress slices are equal.
 func HostAddressesEqual(h, a []HostAddress) bool {
 	if len(h) != len(a) {
@@ -96,7 +149,6 @@ func HostAddressesEqual(h, a []HostAddress) bool {
 	}
 	for _, e := range a {
 		var found bool
-		found = false
 		for _, i := range h {
 			if e.Equal(i) {
 				found = true