internal/ldap: Fix double response on anonymous bind

internal/ldap/bind.go

@@ -25,6 +25,7 @@ func (s *server) handleBind(w ldap.ResponseWriter, m *ldap.Message) {
 	if s.allowAnon && r.Name() == "" {
 		res := ldap.NewBindResponse(ldap.LDAPResultSuccess)
 		w.Write(res)
+		return
 	}
 
 	entityID, err := s.entityIDFromDN(r.Name())

internal/ldap/bind_test.go

@@ -0,0 +1,123 @@
+package ldap
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	ldap "github.com/ps78674/ldapserver"
+
+	pb "github.com/netauth/protocol"
+)
+
+var _ naClient = (*fakeClient)(nil)
+
+type fakeClient struct {
+	authErr   error
+	authCalls []string
+}
+
+func (f *fakeClient) AuthEntity(ctx context.Context, id, password string) error {
+	f.authCalls = append(f.authCalls, id)
+	return f.authErr
+}
+func (f *fakeClient) EntitySearch(context.Context, string) ([]*pb.Entity, error) { return nil, nil }
+func (f *fakeClient) EntityGroups(context.Context, string) ([]*pb.Group, error)  { return nil, nil }
+func (f *fakeClient) GroupSearch(context.Context, string) ([]*pb.Group, error)   { return nil, nil }
+func (f *fakeClient) GroupMembers(context.Context, string) ([]*pb.Entity, error) { return nil, nil }
+
+func TestAnonBindSingleResponse(t *testing.T) {
+	c := &fakeClient{}
+	s := &server{l: testLogger(), c: c}
+	s.nc = []string{"dc=netauth", "dc=example", "dc=com"}
+	s.allowAnon = true
+
+	w := &recordingWriter{}
+	s.handleBind(w, mustDecode(t, berBindRequest("", "")))
+
+	if len(w.responses) != 1 {
+		t.Fatalf("got %d responses for an anonymous bind, want exactly 1", len(w.responses))
+	}
+	if code := resultCode(t, w.responses[0]); code != ldap.LDAPResultSuccess {
+		t.Errorf("result code is %d, want %d", code, ldap.LDAPResultSuccess)
+	}
+	if len(c.authCalls) != 0 {
+		t.Errorf("an anonymous bind reached NetAuth with %v", c.authCalls)
+	}
+}
+
+func TestAnonBindDisabled(t *testing.T) {
+	s := &server{l: testLogger(), c: &fakeClient{}}
+	s.nc = []string{"dc=netauth", "dc=example", "dc=com"}
+
+	w := &recordingWriter{}
+	s.handleBind(w, mustDecode(t, berBindRequest("", "")))
+
+	if len(w.responses) != 1 {
+		t.Fatalf("got %d responses, want exactly 1", len(w.responses))
+	}
+	if code := resultCode(t, w.responses[0]); code != ldap.LDAPResultInvalidDNSyntax {
+		t.Errorf("result code is %d, want %d", code, ldap.LDAPResultInvalidDNSyntax)
+	}
+}
+
+func TestBindValidCredentials(t *testing.T) {
+	c := &fakeClient{}
+	s := &server{l: testLogger(), c: c}
+	s.nc = []string{"dc=netauth", "dc=example", "dc=com"}
+
+	w := &recordingWriter{}
+	s.handleBind(w, mustDecode(t,
+		berBindRequest("uid=jsmith,ou=entities,dc=netauth,dc=example,dc=com", "hunter2")))
+
+	if len(w.responses) != 1 {
+		t.Fatalf("got %d responses, want exactly 1", len(w.responses))
+	}
+	if code := resultCode(t, w.responses[0]); code != ldap.LDAPResultSuccess {
+		t.Errorf("result code is %d, want %d", code, ldap.LDAPResultSuccess)
+	}
+	if len(c.authCalls) != 1 || c.authCalls[0] != "jsmith" {
+		t.Errorf("NetAuth authenticated %v, want [jsmith]", c.authCalls)
+	}
+}
+
+func TestBindInvalidCredentials(t *testing.T) {
+	c := &fakeClient{authErr: errors.New("invalid password")}
+	s := &server{l: testLogger(), c: c}
+	s.nc = []string{"dc=netauth", "dc=example", "dc=com"}
+
+	w := &recordingWriter{}
+	s.handleBind(w, mustDecode(t,
+		berBindRequest("uid=jsmith,ou=entities,dc=netauth,dc=example,dc=com", "wrong")))
+
+	if len(w.responses) != 1 {
+		t.Fatalf("got %d responses, want exactly 1", len(w.responses))
+	}
+	if code := resultCode(t, w.responses[0]); code != ldap.LDAPResultInvalidCredentials {
+		t.Errorf("result code is %d, want %d", code, ldap.LDAPResultInvalidCredentials)
+	}
+}
+
+func TestBindUnsupportedMechanism(t *testing.T) {
+	s := &server{l: testLogger(), c: &fakeClient{}}
+	s.nc = []string{"dc=netauth", "dc=example", "dc=com"}
+
+	// A bind request carrying SASL credentials rather than a simple
+	// password must be refused outright.
+	sasl := berElement(0x60,
+		berInteger(3),
+		berOctetString("uid=jsmith,ou=entities,dc=netauth,dc=example,dc=com"),
+		berElement(0xa3, berOctetString("GSSAPI")),
+	)
+	msg := berElement(0x30, berInteger(1), sasl)
+
+	w := &recordingWriter{}
+	s.handleBind(w, mustDecode(t, msg))
+
+	if len(w.responses) != 1 {
+		t.Fatalf("got %d responses, want exactly 1", len(w.responses))
+	}
+	if code := resultCode(t, w.responses[0]); code != ldap.LDAPResultUnwillingToPerform {
+		t.Errorf("result code is %d, want %d", code, ldap.LDAPResultUnwillingToPerform)
+	}
+}

internal/ldap/result_test.go

@@ -0,0 +1,56 @@
+package ldap
+
+import (
+	"testing"
+
+	"github.com/ps78674/goldap/message"
+)
+
+// resultCode round-trips a result response through its wire encoding
+// and pulls the result code back out of it. The goldap result types
+// expose setters but no getters, so the encoding is the only way to
+// read it back.
+func resultCode(t *testing.T, po message.ProtocolOp) int {
+	t.Helper()
+
+	m := message.NewLDAPMessageWithProtocolOp(po)
+	message.SetMessageID(m, 1)
+	b, err := m.Write()
+	if err != nil {
+		t.Fatalf("encoding response: %s", err)
+	}
+	raw := b.Bytes()
+
+	// Descend from the LDAPMessage SEQUENCE into the protocolOp,
+	// whose first element is always the resultCode for the result
+	// types this server sends.
+	var e error
+	off := 0
+	if _, off, e = message.ParseTagAndLength(raw, off); e != nil {
+		t.Fatalf("message tag: %s", e)
+	}
+	var id message.TagAndLength
+	if id, off, e = message.ParseTagAndLength(raw, off); e != nil {
+		t.Fatalf("messageID tag: %s", e)
+	}
+	off += id.Length
+	if _, off, e = message.ParseTagAndLength(raw, off); e != nil {
+		t.Fatalf("protocolOp tag: %s", e)
+	}
+
+	codeTL, off, e := message.ParseTagAndLength(raw, off)
+	if e != nil {
+		t.Fatalf("resultCode tag: %s", e)
+	}
+	if codeTL.Tag != 0x0a {
+		t.Fatalf("first element of the protocolOp is tag %d, want 10 (ENUMERATED)", codeTL.Tag)
+	}
+	if off+codeTL.Length > len(raw) {
+		t.Fatal("result code truncated")
+	}
+	code := 0
+	for _, b := range raw[off : off+codeTL.Length] {
+		code = code<<8 | int(b)
+	}
+	return code
+}

internal/ldap/testutil_test.go

@@ -0,0 +1,49 @@
+package ldap
+
+import (
+	"testing"
+
+	"github.com/ps78674/goldap/message"
+	ldap "github.com/ps78674/ldapserver"
+)
+
+// The goldap message types have unexported fields with no exported
+// constructors, so test messages are built by hand-encoding the DER
+// for the requests we care about and running them through the real
+// decoder.
+
+func berElement(tag byte, body ...[]byte) []byte {
+	length := 0
+	for _, b := range body {
+		length += len(b)
+	}
+	out := []byte{tag, byte(length)}
+	for _, b := range body {
+		out = append(out, b...)
+	}
+	return out
+}
+
+func berInteger(v byte) []byte { return berElement(0x02, []byte{v}) }
+
+func berOctetString(s string) []byte { return berElement(0x04, []byte(s)) }
+
+// berBindRequest encodes an LDAPMessage containing a simple
+// BindRequest for the given DN and password.
+func berBindRequest(dn, password string) []byte {
+	bind := berElement(0x60, // [APPLICATION 0] constructed
+		berInteger(3),
+		berOctetString(dn),
+		berElement(0x80, []byte(password)), // [0] simple, primitive
+	)
+	return berElement(0x30, berInteger(1), bind)
+}
+
+func mustDecode(t *testing.T, raw []byte) *ldap.Message {
+	t.Helper()
+	m, err := message.ReadLDAPMessage(message.NewBytes(0, raw))
+	if err != nil {
+		t.Fatalf("decoding test message: %s", err)
+	}
+	return &ldap.Message{LDAPMessage: &m, Done: make(chan bool, 2)}
+}