From 239723afc87f8ca636d8c3bfdc1d6442158ebd78 Mon Sep 17 00:00:00 2001
From: Zac Bergquist <zac.bergquist@goteleport.com>
Date: Mon, 28 Oct 2024 09:09:23 -0600
Subject: [PATCH] Improve Teleport's ability to reconnect to LDAP (#36281)

* Always attempt desktop discovery, even if LDAP is not ready

If Teleport loses it's connection to the LDAP server, it will
attempt to initiate a new condition when:

1. The user tries to connect to a desktop and Teleport fails
   to obtain the user's SID.
2. The periodic desktop discovery routine attempts to search
   LDAP for desktops.

In some circumstances, #2 never gets the chance to apply, since
discovery is skipped when LDAP is not ready. Additionally, if
LDAP is not ready, then you can't connect to a desktop, so #1
can't happen either, which means Teleport won't connect again
until it is restarted.

* Periodically use the LDAP connection when discovery is not enabled

If LDAP-based discovery is not enabled then we may go long periods
of time without trying to use the LDAP connection, which prevents
us from detecting disconnects (and restoring the connection) in a
timely manner.

When discovery is disabled, perform a read every 5 minutes and
reconnect if we detect a connection problem.

* Address review comments
---
 lib/srv/desktop/discovery.go      | 11 ++++----
 lib/srv/desktop/windows_server.go | 43 ++++++++++++++++++++++++++-----
 2 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/lib/srv/desktop/discovery.go b/lib/srv/desktop/discovery.go
index c44bcc0a9cabb..6050309b35bbe 100644
--- a/lib/srv/desktop/discovery.go
+++ b/lib/srv/desktop/discovery.go
@@ -99,11 +99,6 @@ func (s *WindowsService) ldapSearchFilter() string {
 
 // getDesktopsFromLDAP discovers Windows hosts via LDAP
 func (s *WindowsService) getDesktopsFromLDAP() map[string]types.WindowsDesktop {
-	if !s.ldapReady() {
-		s.cfg.Logger.WarnContext(context.Background(), "skipping desktop discovery: LDAP not yet initialized")
-		return nil
-	}
-
 	filter := s.ldapSearchFilter()
 	s.cfg.Logger.DebugContext(context.Background(), "searching for desktops", "filter", filter)
 
@@ -250,7 +245,11 @@ func (s *WindowsService) lookupDesktop(ctx context.Context, hostname string) ([]
 
 // ldapEntryToWindowsDesktop generates the Windows Desktop resource
 // from an LDAP search result
-func (s *WindowsService) ldapEntryToWindowsDesktop(ctx context.Context, entry *ldap.Entry, getHostLabels func(string) map[string]string) (types.WindowsDesktop, error) {
+func (s *WindowsService) ldapEntryToWindowsDesktop(
+	ctx context.Context,
+	entry *ldap.Entry,
+	getHostLabels func(string) map[string]string,
+) (types.WindowsDesktop, error) {
 	hostname := entry.GetAttributeValue(windows.AttrDNSHostName)
 	if hostname == "" {
 		attrs := make([]string, len(entry.Attributes))
diff --git a/lib/srv/desktop/windows_server.go b/lib/srv/desktop/windows_server.go
index 791f477861666..77b272acd0696 100644
--- a/lib/srv/desktop/windows_server.go
+++ b/lib/srv/desktop/windows_server.go
@@ -425,10 +425,47 @@ func NewWindowsService(cfg WindowsServiceConfig) (*WindowsService, error) {
 		s.cfg.Logger.InfoContext(ctx, "desktop discovery via LDAP is disabled, set 'base_dn' to enable")
 	}
 
+	// if LDAP-based discovery is not enabled, but we have configured LDAP
+	// then it's important that we periodically try to use the LDAP connection
+	// to detect connection closure
+	if s.ldapConfigured && len(s.cfg.DiscoveryBaseDN) == 0 {
+		s.startLDAPConnectionCheck(ctx)
+	}
+
 	ok = true
 	return s, nil
 }
 
+// startLDAPConnectionCheck starts a background process that
+// periodically reads from the LDAP connection in order to detect
+// connection closure, and reconnects if necessary.
+// This is useful when LDAP-based discovery is disabled, because without
+// discovery the connection goes idle and may be closed by the server.
+func (s *WindowsService) startLDAPConnectionCheck(ctx context.Context) {
+	s.cfg.Logger.DebugContext(ctx, "starting LDAP connection checker")
+	go func() {
+		t := s.cfg.Clock.NewTicker(5 * time.Minute)
+		defer t.Stop()
+
+		for {
+			select {
+			case <-t.Chan():
+				// attempt to read CAs in the NTAuth store (we know we have permissions to do so)
+				ntAuthDN := "CN=NTAuthCertificates,CN=Public Key Services,CN=Services,CN=Configuration," + s.cfg.LDAPConfig.DomainDN()
+				_, err := s.lc.Read(ntAuthDN, "certificationAuthority", []string{"cACertificate"})
+				if trace.IsConnectionProblem(err) {
+					s.cfg.Logger.DebugContext(ctx, "detected broken LDAP connection, will reconnect")
+					if err := s.initializeLDAP(); err != nil {
+						s.cfg.Logger.WarnContext(ctx, "failed to reconnect to LDAP", "error", err)
+					}
+				}
+			case <-ctx.Done():
+				return
+			}
+		}
+	}()
+}
+
 func (s *WindowsService) newSessionRecorder(recConfig types.SessionRecordingConfig, sessionID string) (libevents.SessionPreparerRecorder, error) {
 	return recorder.New(recorder.Config{
 		SessionID:    session.ID(sessionID),
@@ -693,12 +730,6 @@ func (s *WindowsService) readyForConnections() bool {
 	return s.ldapInitialized
 }
 
-func (s *WindowsService) ldapReady() bool {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-	return s.ldapInitialized
-}
-
 // handleConnection handles TLS connections from a Teleport proxy.
 // It authenticates and authorizes the connection, and then begins
 // translating the TDP messages from the proxy into native RDP.