Skip to content

Commit

Permalink
[v17] Improve Teleport's ability to reconnect to LDAP (#48007)
Browse files Browse the repository at this point in the history
* 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

* Fix some LDAP connection bugs

In #36281 we made some improvements to the LDAP reconnect behavior.
These changes considered the case where we had a connection to the
LDAP server but then got disconnected. They did not consider the case
where we never succesfully established a connection at all.
  • Loading branch information
zmb3 authored Oct 29, 2024
1 parent 038a159 commit c3a475e
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 9 deletions.
15 changes: 12 additions & 3 deletions lib/srv/desktop/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,15 @@ 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")
// Check whether we've ever successfully initialized our LDAP client.
s.mu.Lock()
if !s.ldapInitialized {
s.cfg.Logger.DebugContext(context.Background(), "LDAP not ready, skipping discovery and attempting to reconnect")
s.mu.Unlock()
s.initializeLDAP()
return nil
}
s.mu.Unlock()

filter := s.ldapSearchFilter()
s.cfg.Logger.DebugContext(context.Background(), "searching for desktops", "filter", filter)
Expand Down Expand Up @@ -251,7 +256,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))
Expand Down
56 changes: 50 additions & 6 deletions lib/srv/desktop/windows_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,10 +425,60 @@ 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():
// First check if we have successfully initialized the LDAP client.
// If not, then do that now and return.
// (This mimics the check that is performed when LDAP discovery is enabled.)
s.mu.Lock()
if !s.ldapInitialized {
s.cfg.Logger.DebugContext(context.Background(), "LDAP not ready, attempting to reconnect")
s.mu.Unlock()
s.initializeLDAP()
return
}
s.mu.Unlock()

// If we have initizlied the LDAP client, then try to use it to make sure we're still connected
// by attempting 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),
Expand Down Expand Up @@ -693,12 +743,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.
Expand Down

0 comments on commit c3a475e

Please sign in to comment.