fix: verify the digest-md5 rspauth value in DigestMD5Bind

v3/bind.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"crypto/md5"
 	"crypto/rand"
+	"crypto/subtle"
 	"encoding/binary"
 	"encoding/hex"
 	"errors"
@@ -240,8 +241,9 @@ func (l *Conn) DigestMD5Bind(digestMD5BindRequest *DigestMD5BindRequest) (*Diges
 		}
 	}
 
+	var wantRspauth string
 	if len(params) > 0 {
-		resp, err := computeResponse(
+		resp, rspauth, err := computeResponse(
 			params,
 			"ldap/"+strings.ToLower(digestMD5BindRequest.Host),
 			digestMD5BindRequest.Username,
@@ -250,6 +252,7 @@ func (l *Conn) DigestMD5Bind(digestMD5BindRequest *DigestMD5BindRequest) (*Diges
 		if err != nil {
 			return nil, fmt.Errorf("compute digest-md5 response: %s", err)
 		}
+		wantRspauth = rspauth
 		packet = ber.Encode(ber.ClassUniversal, ber.TypeConstructed, ber.TagSequence, nil, "LDAP Request")
 		packet.AppendChild(ber.NewInteger(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, l.nextMessageID(), "MessageID"))
 
@@ -322,9 +325,47 @@ func (l *Conn) DigestMD5Bind(digestMD5BindRequest *DigestMD5BindRequest) (*Diges
 	}
 
 	err = GetLDAPError(packet)
+	if err == nil && wantRspauth != "" {
+		err = verifyDigestMD5Rspauth(packet, wantRspauth)
+	}
 	return result, err
 }
 
+// verifyDigestMD5Rspauth checks the response-auth value the server returns in
+// the serverSaslCreds field of the success bind response (RFC 2831 section
+// 2.1.3). This step is what makes DIGEST-MD5 mutually authenticating: the
+// resultCode alone would accept any server that claims success without
+// knowing the password.
+func verifyDigestMD5Rspauth(packet *ber.Packet, want string) error {
+	protocolOp, err := packetChild(packet, 1)
+	if err != nil {
+		return err
+	}
+	for _, child := range protocolOp.Children {
+		// serverSaslCreds is the context-class [7] member of BindResponse.
+		if child == nil || child.ClassType != ber.ClassContext || child.Tag != ber.TagObjectDescriptor {
+			continue
+		}
+		data, err := packetData(child)
+		if err != nil {
+			return err
+		}
+		params, err := parseParams(string(data))
+		if err != nil {
+			return fmt.Errorf("parsing rspauth: %s", err)
+		}
+		rspauth, ok := params["rspauth"]
+		if !ok {
+			break
+		}
+		if subtle.ConstantTimeCompare([]byte(rspauth), []byte(want)) != 1 {
+			return errors.New("digest-md5: rspauth mismatch, server has not authenticated itself")
+		}
+		return nil
+	}
+	return errors.New("digest-md5: server did not send rspauth, server has not authenticated itself")
+}
+
 func parseParams(str string) (map[string]string, error) {
 	m := make(map[string]string)
 	var key, value string
@@ -401,12 +442,12 @@ func parseParams(str string) (map[string]string, error) {
 	return m, nil
 }
 
-func computeResponse(params map[string]string, uri, username, password string) (string, error) {
+func computeResponse(params map[string]string, uri, username, password string) (string, string, error) {
 	nc := "00000001"
 	qop := "auth"
 	rb, err := randomBytes(16)
 	if err != nil {
-		return "", err
+		return "", "", err
 	}
 	cnonce := hex.EncodeToString(rb)
 	x := username + ":" + params["realm"] + ":" + password
@@ -438,7 +479,17 @@ func computeResponse(params map[string]string, uri, username, password string) (
 		qop,
 		quotedStringEscape(uri),
 		resp,
-	), nil
+	), computeRspauth(ha1, params["nonce"], cnonce, uri), nil
+}
+
+// computeRspauth returns the response-auth value the server is expected to
+// send back after validating the digest-response (RFC 2831 section 2.1.3). It
+// is derived like the digest-response, over the same nc and qop
+// computeResponse uses, except that A2 carries no method before the colon.
+func computeRspauth(ha1, nonce, cnonce, uri string) string {
+	ha2 := hex.EncodeToString(md5Hash([]byte(":" + uri)))
+	kd := ha1 + ":" + nonce + ":00000001:" + cnonce + ":auth:" + ha2
+	return hex.EncodeToString(md5Hash([]byte(kd)))
 }
 
 // quotedStringEscape escapes the two characters that may not appear unescaped

v3/bind_test.go

@@ -1,8 +1,12 @@
 package ldap
 
 import (
+	"bytes"
+	"encoding/hex"
+	"strings"
 	"testing"
 
+	ber "github.com/go-asn1-ber/asn1-ber"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -60,7 +64,7 @@
 	// properly escaped DIGEST-MD5 quoted string, otherwise it breaks out of
 	// the username directive and injects further directives into the response.
 	params := map[string]string{"realm": "example.com", "nonce": "abc"}
-	resp, err := computeResponse(params, "ldap/host", `a"b\c`, "secret")
+	resp, _, err := computeResponse(params, "ldap/host", `a"b\c`, "secret")
 	assert.NoError(t, err)
 	assert.Contains(t, resp, `username="a\"b\\c"`)
 }
@@ -69,7 +73,7 @@
 	// realm and nonce come from the server challenge and are echoed back
 	// inside quoted strings, so they need the same escaping.
 	params := map[string]string{"realm": `r"x`, "nonce": "abc"}
-	resp, err := computeResponse(params, "ldap/host", "user", "secret")
+	resp, _, err := computeResponse(params, "ldap/host", "user", "secret")
 	assert.NoError(t, err)
 	assert.Contains(t, resp, `realm="r\"x"`)
 }
@@ -106,6 +110,138 @@
 	assert.Equal(t, "c d", params["nonce"])
 }
 
+func TestComputeRspauthRFC2831Example(t *testing.T) {
+	// Worked example from RFC 2831 section 4: user "chris", password
+	// "secret", with the server's final message carrying
+	// rspauth=ea40f60335c427b5527b84dbabcdfffd.
+	x := "chris:elwood.innosoft.com:secret"
+	a1 := bytes.NewBuffer(md5Hash([]byte(x)))
+	a1.WriteString(":OA6MG9tEQGm2hh:OA6MHXh6VqTrRk")
+	ha1 := hex.EncodeToString(md5Hash(a1.Bytes()))
+
+	rspauth := computeRspauth(ha1, "OA6MG9tEQGm2hh", "OA6MHXh6VqTrRk", "imap/elwood.innosoft.com")
+	assert.Equal(t, "ea40f60335c427b5527b84dbabcdfffd", rspauth)
+}
+
+// digestMD5Rspauth computes, from the digest-response directives a client
+// sent and the shared password, the response-auth value a genuine server
+// would return (RFC 2831 section 2.1.3).
+func digestMD5Rspauth(creds map[string]string, password string) string {
+	x := creds["username"] + ":" + creds["realm"] + ":" + password
+	a1 := bytes.NewBuffer(md5Hash([]byte(x)))
+	a1.WriteString(":" + creds["nonce"] + ":" + creds["cnonce"])
+	ha1 := hex.EncodeToString(md5Hash(a1.Bytes()))
+	return computeRspauth(ha1, creds["nonce"], creds["cnonce"], creds["digest-uri"])
+}
+
+// serveDigestMD5Bind scripts the server side of a DIGEST-MD5 bind on ptc: it
+// answers the first bind request with a challenge and the digest-response
+// with success, carrying the serverSaslCreds produced by rspauthFor (an empty
+// string omits the field). Errors are not reported here: the exchange breaking
+// off surfaces as a bind error on the client side under test.
+func serveDigestMD5Bind(ptc *packetTranslatorConn, rspauthFor func(creds map[string]string) string) {
+	bindResponse := func(msgID int64, code int64, saslCreds string) *ber.Packet {
+		envelope := ber.Encode(ber.ClassUniversal, ber.TypeConstructed, ber.TagSequence, nil, "")
+		envelope.AppendChild(ber.NewInteger(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, msgID, "MessageID"))
+		response := ber.Encode(ber.ClassApplication, ber.TypeConstructed, ApplicationBindResponse, nil, "")
+		response.AppendChild(ber.NewInteger(ber.ClassUniversal, ber.TypePrimitive, ber.TagEnumerated, code, "Result Code"))
+		response.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "", "Matched DN"))
+		response.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "", "Diagnostic Message"))
+		if saslCreds != "" {
+			response.AppendChild(ber.NewString(ber.ClassContext, ber.TypePrimitive, ber.TagObjectDescriptor, saslCreds, "Server SASL Credentials"))
+		}
+		envelope.AppendChild(response)
+		return envelope
+	}
+
+	go func() {
+		req, err := ptc.ReceiveRequest()
+		if err != nil {
+			return
+		}
+		msgID, err := packetInt64At(req, 0)
+		if err != nil {
+			return
+		}
+		challenge := `realm="example.com",nonce="OA6MG9tEQGm2hh",qop="auth",charset=utf-8,algorithm=md5-sess`
+		if err := ptc.SendResponse(bindResponse(msgID, LDAPResultSaslBindInProgress, challenge)); err != nil {
+			return
+		}
+
+		req, err = ptc.ReceiveRequest()
+		if err != nil {
+			return
+		}
+		msgID, err = packetInt64At(req, 0)
+		if err != nil {
+			return
+		}
+		auth, err := packetChild(req.Children[1], 2)
+		if err != nil {
+			return
+		}
+		digestResponse, err := packetStringAt(auth, 1)
+		if err != nil {
+			return
+		}
+		creds, err := parseParams(digestResponse)
+		if err != nil {
+			return
+		}
+		_ = ptc.SendResponse(bindResponse(msgID, LDAPResultSuccess, rspauthFor(creds)))
+	}()
+}
+
+func TestDigestMD5BindVerifiesRspauth(t *testing.T) {
+	// The success bind response of a DIGEST-MD5 bind must carry the
+	// response-auth value computed from the shared secret (RFC 2831 section
+	// 2.1.3); verifying it is what authenticates the server. A server that
+	// omits it or returns a wrong value never proved knowledge of the
+	// password, so the bind has to fail instead of reporting success.
+	tests := []struct {
+		name       string
+		rspauthFor func(creds map[string]string) string
+		wantErr    string
+	}{
+		{
+			name: "valid rspauth",
+			rspauthFor: func(creds map[string]string) string {
+				return "rspauth=" + digestMD5Rspauth(creds, "secret")
+			},
+		},
+		{
+			name:       "missing rspauth",
+			rspauthFor: func(map[string]string) string { return "" },
+			wantErr:    "server did not send rspauth",
+		},
+		{
+			name: "mismatched rspauth",
+			rspauthFor: func(map[string]string) string {
+				return "rspauth=" + strings.Repeat("0", 32)
+			},
+			wantErr: "rspauth mismatch",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ptc := newPacketTranslatorConn()
+			defer ptc.Close()
+			conn := NewConn(ptc, false)
+			conn.Start()
+			defer conn.Close()
+
+			serveDigestMD5Bind(ptc, tt.rspauthFor)
+
+			err := conn.MD5Bind("ldap.example.com", "user", "secret")
+			if tt.wantErr == "" {
+				assert.NoError(t, err)
+			} else {
+				assert.ErrorContains(t, err, tt.wantErr)
+			}
+		})
+	}
+}
+
 func TestConn_UnauthenticatedBind(t *testing.T) {
 	l, err := getTestConnection(false)
 	if err != nil {