fix: packet referral decoding

james-d-elliott · james-d-elliott · commit 49701f374d27 · 2023-01-24T09:50:16.000+11:00
This adjusts the packet referral decoding behavior to more appropriately handle edge cases and allows to more easily obtain the underlying packet in the event of a decoding error. Instead of matching ASN.1 BER packets on the tag for referrals we match on the class and type. This is particularly important in regards to OpenLDAP which returns a ASN.1 BER Generalized Time tag for referrals.
diff --git a/modify.go b/modify.go
@@ -160,9 +160,7 @@ func (l *Conn) ModifyWithResult(modifyRequest *ModifyRequest) (*ModifyResult, er
 	switch packet.Children[1].Tag {
 	case ApplicationModifyResponse:
 		if err = GetLDAPError(packet); err != nil {
-			if referral, referralErr := getReferral(err, packet); referralErr != nil {
-				return result, referralErr
-			} else {
+			if referral := getReferral(err, packet); referral != "" {
 				result.Referral = referral
 			}
 
diff --git a/passwdmodify.go b/passwdmodify.go
@@ -70,7 +70,6 @@ func (req *PasswordModifyRequest) appendTo(envelope *ber.Packet) error {
 // newPassword is the desired user's password. If empty the server can return
 // an error or generate a new password that will be available in the
 // PasswordModifyResult.GeneratedPassword
-//
 func NewPasswordModifyRequest(userIdentity string, oldPassword string, newPassword string) *PasswordModifyRequest {
 	return &PasswordModifyRequest{
 		UserIdentity: userIdentity,
@@ -96,9 +95,7 @@ func (l *Conn) PasswordModify(passwordModifyRequest *PasswordModifyRequest) (*Pa
 
 	if packet.Children[1].Tag == ApplicationExtendedResponse {
 		if err = GetLDAPError(packet); err != nil {
-			if referral, referralErr := getReferral(err, packet); referralErr != nil {
-				return result, referralErr
-			} else {
+			if referral := getReferral(err, packet); referral != "" {
 				result.Referral = referral
 			}
 
diff --git a/request.go b/request.go
@@ -2,7 +2,6 @@ package ldap
 
 import (
 	"errors"
-	"fmt"
 
 	ber "github.com/go-asn1-ber/asn1-ber"
 )
@@ -71,28 +70,33 @@ func (l *Conn) readPacket(msgCtx *messageContext) (*ber.Packet, error) {
 	return packet, nil
 }
 
-func getReferral(err error, packet *ber.Packet) (referral string, e error) {
+func getReferral(err error, packet *ber.Packet) (referral string) {
 	if !IsErrorWithCode(err, LDAPResultReferral) {
-		return "", nil
+		return ""
 	}
 
 	if len(packet.Children) < 2 {
-		return "", fmt.Errorf("ldap: returned error indicates the packet contains a referral but it doesn't have sufficient child nodes: %w", err)
+		return ""
 	}
 
-	if packet.Children[1].Tag != ber.TagObjectDescriptor {
-		return "", fmt.Errorf("ldap: returned error indicates the packet contains a referral but the relevant child node isn't an object descriptor: %w", err)
+	children := len(packet.Children[1].Children)
+
+	if children == 0 || (packet.Children[1].TagType != ber.TypeConstructed || packet.Children[1].ClassType != ber.ClassApplication) {
+		return ""
 	}
 
 	var ok bool
 
-	for _, child := range packet.Children[1].Children {
-		if child.Tag == ber.TagBitString && len(child.Children) >= 1 {
-			if referral, ok = child.Children[0].Value.(string); ok {
-				return referral, nil
-			}
+	for i := 0; i < children; i++ {
+		if (packet.Children[1].Children[i].Tag != ber.TagBitString && packet.Children[1].Children[i].Tag != ber.TagPrintableString) ||
+			packet.Children[1].Children[i].TagType != ber.TypeConstructed || packet.Children[1].Children[i].ClassType != ber.ClassContext {
+			continue
+		}
+
+		if referral, ok = packet.Children[1].Children[i].Children[0].Value.(string); ok {
+			return referral
 		}
 	}
 
-	return "", fmt.Errorf("ldap: returned error indicates the packet contains a referral but the referral couldn't be decoded: %w", err)
+	return ""
 }
diff --git a/v3/modify.go b/v3/modify.go
@@ -160,9 +160,7 @@ func (l *Conn) ModifyWithResult(modifyRequest *ModifyRequest) (*ModifyResult, er
 	switch packet.Children[1].Tag {
 	case ApplicationModifyResponse:
 		if err = GetLDAPError(packet); err != nil {
-			if referral, referralErr := getReferral(err, packet); referralErr != nil {
-				return result, referralErr
-			} else {
+			if referral := getReferral(err, packet); referral != "" {
 				result.Referral = referral
 			}
 
diff --git a/v3/passwdmodify.go b/v3/passwdmodify.go
@@ -70,7 +70,6 @@ func (req *PasswordModifyRequest) appendTo(envelope *ber.Packet) error {
 // newPassword is the desired user's password. If empty the server can return
 // an error or generate a new password that will be available in the
 // PasswordModifyResult.GeneratedPassword
-//
 func NewPasswordModifyRequest(userIdentity string, oldPassword string, newPassword string) *PasswordModifyRequest {
 	return &PasswordModifyRequest{
 		UserIdentity: userIdentity,
@@ -96,9 +95,7 @@ func (l *Conn) PasswordModify(passwordModifyRequest *PasswordModifyRequest) (*Pa
 
 	if packet.Children[1].Tag == ApplicationExtendedResponse {
 		if err = GetLDAPError(packet); err != nil {
-			if referral, referralErr := getReferral(err, packet); referralErr != nil {
-				return result, referralErr
-			} else {
+			if referral := getReferral(err, packet); referral != "" {
 				result.Referral = referral
 			}
 
diff --git a/v3/request.go b/v3/request.go
@@ -2,7 +2,6 @@ package ldap
 
 import (
 	"errors"
-	"fmt"
 
 	ber "github.com/go-asn1-ber/asn1-ber"
 )
@@ -71,28 +70,33 @@ func (l *Conn) readPacket(msgCtx *messageContext) (*ber.Packet, error) {
 	return packet, nil
 }
 
-func getReferral(err error, packet *ber.Packet) (referral string, e error) {
+func getReferral(err error, packet *ber.Packet) (referral string) {
 	if !IsErrorWithCode(err, LDAPResultReferral) {
-		return "", nil
+		return ""
 	}
 
 	if len(packet.Children) < 2 {
-		return "", fmt.Errorf("ldap: returned error indicates the packet contains a referral but it doesn't have sufficient child nodes: %w", err)
+		return ""
 	}
 
-	if packet.Children[1].Tag != ber.TagObjectDescriptor {
-		return "", fmt.Errorf("ldap: returned error indicates the packet contains a referral but the relevant child node isn't an object descriptor: %w", err)
+	children := len(packet.Children[1].Children)
+
+	if children == 0 || (packet.Children[1].TagType != ber.TypeConstructed || packet.Children[1].ClassType != ber.ClassApplication) {
+		return ""
 	}
 
 	var ok bool
 
-	for _, child := range packet.Children[1].Children {
-		if child.Tag == ber.TagBitString && len(child.Children) >= 1 {
-			if referral, ok = child.Children[0].Value.(string); ok {
-				return referral, nil
-			}
+	for i := 0; i < children; i++ {
+		if (packet.Children[1].Children[i].Tag != ber.TagBitString && packet.Children[1].Children[i].Tag != ber.TagPrintableString) ||
+			packet.Children[1].Children[i].TagType != ber.TypeConstructed || packet.Children[1].Children[i].ClassType != ber.ClassContext {
+			continue
+		}
+
+		if referral, ok = packet.Children[1].Children[i].Children[0].Value.(string); ok {
+			return referral
 		}
 	}
 
-	return "", fmt.Errorf("ldap: returned error indicates the packet contains a referral but the referral couldn't be decoded: %w", err)
+	return ""
 }

Original file line number	Diff line number	Diff line change
`@@ -2,7 +2,6 @@ package ldap`
`2`	`2`
`3`	`3`	`import (`
`4`	`4`	`"errors"`
`5`		`- "fmt"`
`6`	`5`
`7`	`6`	`ber "github.com/go-asn1-ber/asn1-ber"`
`8`	`7`	`)`
`@@ -71,28 +70,33 @@ func (l Conn) readPacket(msgCtx messageContext) (*ber.Packet, error) {`
`71`	`70`	`return packet, nil`
`72`	`71`	`}`
`73`	`72`
`74`		`-func getReferral(err error, packet *ber.Packet) (referral string, e error) {`
	`73`	`+func getReferral(err error, packet *ber.Packet) (referral string) {`
`75`	`74`	`if !IsErrorWithCode(err, LDAPResultReferral) {`
`76`		`- return "", nil`
	`75`	`+ return ""`
`77`	`76`	`}`
`78`	`77`
`79`	`78`	`if len(packet.Children) < 2 {`
`80`		`- return "", fmt.Errorf("ldap: returned error indicates the packet contains a referral but it doesn't have sufficient child nodes: %w", err)`
	`79`	`+ return ""`
`81`	`80`	`}`
`82`	`81`
`83`		`- if packet.Children[1].Tag != ber.TagObjectDescriptor {`
`84`		`- return "", fmt.Errorf("ldap: returned error indicates the packet contains a referral but the relevant child node isn't an object descriptor: %w", err)`
	`82`	`+ children := len(packet.Children[1].Children)`
	`83`	`+`
	`84`	`+ if children == 0 \|\| (packet.Children[1].TagType != ber.TypeConstructed \|\| packet.Children[1].ClassType != ber.ClassApplication) {`
	`85`	`+ return ""`
`85`	`86`	`}`
`86`	`87`
`87`	`88`	`var ok bool`
`88`	`89`
`89`		`- for _, child := range packet.Children[1].Children {`
`90`		`- if child.Tag == ber.TagBitString && len(child.Children) >= 1 {`
`91`		`- if referral, ok = child.Children[0].Value.(string); ok {`
`92`		`- return referral, nil`
`93`		`- }`
	`90`	`+ for i := 0; i < children; i++ {`
	`91`	`+ if (packet.Children[1].Children[i].Tag != ber.TagBitString && packet.Children[1].Children[i].Tag != ber.TagPrintableString) \|\|`
	`92`	`+ packet.Children[1].Children[i].TagType != ber.TypeConstructed \|\| packet.Children[1].Children[i].ClassType != ber.ClassContext {`
	`93`	`+ continue`
	`94`	`+ }`
	`95`	`+`
	`96`	`+ if referral, ok = packet.Children[1].Children[i].Children[0].Value.(string); ok {`
	`97`	`+ return referral`
`94`	`98`	`}`
`95`	`99`	`}`
`96`	`100`
`97`		`- return "", fmt.Errorf("ldap: returned error indicates the packet contains a referral but the referral couldn't be decoded: %w", err)`
	`101`	`+ return ""`
`98`	`102`	`}`