internal/ldap: Require a bind before serving searches

internal/ldap/bind.go

@@ -14,6 +14,7 @@ func (s *server) handleBind(w ldap.ResponseWriter, m *ldap.Message) {
 	// fancy because we are after all just fronting another
 	// protocol.
 	if r.AuthenticationChoice() != "simple" {
+		s.markUnbound(m)
 		res := ldap.NewBindResponse(ldap.LDAPResultUnwillingToPerform)
 		res.SetDiagnosticMessage("Authentication choice not supported")
 		w.Write(res)
@@ -23,6 +24,7 @@ func (s *server) handleBind(w ldap.ResponseWriter, m *ldap.Message) {
 	s.l.Debug("Bind from dn", "dn", r.Name())
 
 	if s.allowAnon && r.Name() == "" {
+		s.markBound(m)
 		res := ldap.NewBindResponse(ldap.LDAPResultSuccess)
 		w.Write(res)
 		return
@@ -30,6 +32,7 @@ func (s *server) handleBind(w ldap.ResponseWriter, m *ldap.Message) {
 
 	entityID, err := s.entityIDFromDN(r.Name())
 	if err != nil {
+		s.markUnbound(m)
 		res := ldap.NewBindResponse(ldap.LDAPResultInvalidDNSyntax)
 		res.SetDiagnosticMessage(err.Error())
 		s.l.Warn("Request with invalid DN", "dn", r.Name())
@@ -38,12 +41,14 @@ func (s *server) handleBind(w ldap.ResponseWriter, m *ldap.Message) {
 	}
 
 	if err := s.c.AuthEntity(ctx, entityID, string(r.AuthenticationSimple())); err != nil {
+		s.markUnbound(m)
 		res := ldap.NewBindResponse(ldap.LDAPResultInvalidCredentials)
 		res.SetDiagnosticMessage("invalid credentials")
 		w.Write(res)
 		return
 	}
 
+	s.markBound(m)
 	res := ldap.NewBindResponse(ldap.LDAPResultSuccess)
 	w.Write(res)
 }

internal/ldap/conn.go

@@ -0,0 +1,89 @@
+package ldap
+
+import (
+	"sync"
+
+	ldap "github.com/ps78674/ldapserver"
+)
+
+// connection is the part of the underlying server's client type that
+// this bridge uses for state belonging to a connection rather than to
+// a single request. That type is unexported, so it is reached through
+// the message and abstracted here so tests can supply their own.
+type connection interface {
+	GetAddData() interface{}
+	SetAddData(interface{})
+}
+
+// boundState is the per-connection record stored in the client's
+// additional data, which releases it together with the connection.
+type boundState struct {
+	sync.Mutex
+	bound bool
+}
+
+// messageConn returns the connection a request arrived on.
+func messageConn(m *ldap.Message) connection {
+	if m == nil || m.Client == nil {
+		return nil
+	}
+	return m.Client
+}
+
+// connOf returns the connection for a request. It is a field so that
+// tests can inject a connection without the server's unexported
+// client type.
+func (s *server) connOf(m *ldap.Message) connection {
+	if s.conn == nil {
+		return messageConn(m)
+	}
+	return s.conn(m)
+}
+
+// bound reports whether the request arrived on a connection that has
+// already been authenticated.
+func (s *server) bound(m *ldap.Message) bool {
+	st := existingState(s.connOf(m))
+	if st == nil {
+		return false
+	}
+	st.Lock()
+	defer st.Unlock()
+	return st.bound
+}
+
+// markBound records that the connection has been authenticated.
+func (s *server) markBound(m *ldap.Message) {
+	c := s.connOf(m)
+	if c == nil {
+		return
+	}
+	if st := existingState(c); st != nil {
+		st.Lock()
+		defer st.Unlock()
+		st.bound = true
+		return
+	}
+	c.SetAddData(&boundState{bound: true})
+}
+
+// markUnbound clears the authentication state of a connection. RFC
+// 4511 requires a failed bind to leave the connection unauthenticated
+// so that a client cannot hold on to access it has lost.
+func (s *server) markUnbound(m *ldap.Message) {
+	st := existingState(s.connOf(m))
+	if st == nil {
+		return
+	}
+	st.Lock()
+	defer st.Unlock()
+	st.bound = false
+}
+
+func existingState(c connection) *boundState {
+	if c == nil {
+		return nil
+	}
+	st, _ := c.GetAddData().(*boundState)
+	return st
+}

internal/ldap/search.go

@@ -32,6 +32,14 @@ func (s *server) handleSearchEntities(w ldap.ResponseWriter, m *ldap.Message) {
 	ctx := context.Background()
 	s.l.Debug("Search Entities")
 
+	if !s.bound(m) {
+		s.l.Warn("Search from an unauthenticated connection")
+		res := ldap.NewSearchResultDoneResponse(ldap.LDAPResultInsufficientAccessRights)
+		res.SetDiagnosticMessage("a bind is required before searching")
+		w.Write(res)
+		return
+	}
+
 	r := m.GetSearchRequest()
 	expr, err := s.buildBleveQuery(r.Filter())
 	if err != nil {
@@ -100,6 +108,14 @@ func (s *server) handleSearchGroups(w ldap.ResponseWriter, m *ldap.Message) {
 	ctx := context.Background()
 	s.l.Debug("Search Groups")
 
+	if !s.bound(m) {
+		s.l.Warn("Search from an unauthenticated connection")
+		res := ldap.NewSearchResultDoneResponse(ldap.LDAPResultInsufficientAccessRights)
+		res.SetDiagnosticMessage("a bind is required before searching")
+		w.Write(res)
+		return
+	}
+
 	r := m.GetSearchRequest()
 
 	expr, err := s.buildBleveQuery(r.Filter())

internal/ldap/search_test.go

@@ -0,0 +1,78 @@
+package ldap
+
+import (
+	"errors"
+	"testing"
+
+	ldap "github.com/ps78674/ldapserver"
+)
+
+// fakeConn stands in for the server's unexported client type so that
+// per-connection bind state can be exercised in tests.
+type fakeConn struct {
+	data interface{}
+}
+
+func (c *fakeConn) GetAddData() interface{}  { return c.data }
+func (c *fakeConn) SetAddData(d interface{}) { c.data = d }
+
+func boundTestServer(c naClient) *server {
+	fc := &fakeConn{}
+	s := &server{l: testLogger(), c: c}
+	s.nc = []string{"dc=netauth", "dc=example", "dc=com"}
+	s.conn = func(*ldap.Message) connection { return fc }
+	return s
+}
+
+func TestSearchRequiresBind(t *testing.T) {
+	s := boundTestServer(&fakeClient{})
+
+	w := &recordingWriter{}
+	s.handleSearchEntities(w, mustDecode(t, berSearchRequest("ou=entities,dc=netauth,dc=example,dc=com")))
+
+	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.LDAPResultInsufficientAccessRights {
+		t.Errorf("entities: result code is %d, want %d", code, ldap.LDAPResultInsufficientAccessRights)
+	}
+
+	w = &recordingWriter{}
+	s.handleSearchGroups(w, mustDecode(t, berSearchRequest("ou=groups,dc=netauth,dc=example,dc=com")))
+	if code := resultCode(t, w.responses[0]); code != ldap.LDAPResultInsufficientAccessRights {
+		t.Errorf("groups: result code is %d, want %d", code, ldap.LDAPResultInsufficientAccessRights)
+	}
+}
+
+func TestSearchAfterBindSucceeds(t *testing.T) {
+	s := boundTestServer(&fakeClient{})
+
+	// A successful bind marks the connection, so a follow-up search
+	// is no longer refused for lack of authentication.
+	s.handleBind(&recordingWriter{}, mustDecode(t,
+		berBindRequest("uid=jsmith,ou=entities,dc=netauth,dc=example,dc=com", "hunter2")))
+
+	w := &recordingWriter{}
+	s.handleSearchEntities(w, mustDecode(t, berSearchRequest("ou=entities,dc=netauth,dc=example,dc=com")))
+	if code := resultCode(t, w.responses[len(w.responses)-1]); code == ldap.LDAPResultInsufficientAccessRights {
+		t.Errorf("a bound connection was still refused")
+	}
+}
+
+func TestFailedRebindClearsConnection(t *testing.T) {
+	c := &fakeClient{authErr: errors.New("invalid password")}
+	s := boundTestServer(c)
+
+	// Mark the connection bound, then have a rebind fail; RFC 4511
+	// requires the connection to drop back to unauthenticated.
+	s.markBound(&ldap.Message{})
+	w := &recordingWriter{}
+	s.handleBind(w, mustDecode(t,
+		berBindRequest("uid=jsmith,ou=entities,dc=netauth,dc=example,dc=com", "wrong")))
+	if code := resultCode(t, w.responses[0]); code != ldap.LDAPResultInvalidCredentials {
+		t.Fatalf("bind result code is %d, want %d", code, ldap.LDAPResultInvalidCredentials)
+	}
+	if s.bound(&ldap.Message{}) {
+		t.Error("connection still marked bound after a failed bind")
+	}
+}

internal/ldap/testutil_test.go

@@ -39,6 +39,22 @@ func berBindRequest(dn, password string) []byte {
 	return berElement(0x30, berInteger(1), bind)
 }
 
+// berSearchRequest encodes an LDAPMessage containing a SearchRequest
+// with an attribute-present filter on uid and no attribute selection.
+func berSearchRequest(baseDN string) []byte {
+	search := berElement(0x63, // [APPLICATION 3] constructed
+		berOctetString(baseDN),          // baseObject
+		berElement(0x0a, []byte{2}),     // scope: wholeSubtree
+		berElement(0x0a, []byte{0}),     // derefAliases: never
+		berInteger(0),                   // sizeLimit
+		berInteger(0),                   // timeLimit
+		berElement(0x01, []byte{0x00}),  // typesOnly: false
+		berElement(0x87, []byte("uid")), // filter: (uid=*)
+		berElement(0x30),                // attributes: empty selection
+	)
+	return berElement(0x30, berInteger(2), search)
+}
+
 func mustDecode(t *testing.T, raw []byte) *ldap.Message {
 	t.Helper()
 	m, err := message.ReadLDAPMessage(message.NewBytes(0, raw))

internal/ldap/type.go

@@ -28,5 +28,10 @@ type server struct {
 
 	nc []string
 
+	// conn resolves the connection a request arrived on so that
+	// per-connection bind state can be tracked. Tests override it
+	// since the server's client type is unexported.
+	conn func(*ldap.Message) connection
+
 	allowAnon bool
 }