Improve troubleshooting for LDAP authentication errors (#48947)

This introduces two small changes:

1. Use an aggregate error to make sure the original error is included
   along with our attempt at providing a better error message. This
   change should help distinguish between bad authn/invalid cert
   and valid authentication but insufficient user permissions.
2. Make the CRL Distribution Point in Windows certs optional. This
   metadata is required in the certs we issue for users to RDP,
   but it doesn't need to be present in the certs we use to
   authenticate our service account. The problem with including it
   when it is not needed is it causes windows to perform a revocation
   check and log a failure, which can lead to wasted time when
   troubleshooting.

Author: zmb3

lib/auth/desktop.go

@@ -71,8 +71,13 @@ func (a *Server) GenerateWindowsDesktopCert(ctx context.Context, req *proto.Wind
 		NotAfter:        a.clock.Now().UTC().Add(req.TTL.Get()),
 		ExtraExtensions: csr.Extensions,
 		KeyUsage:        x509.KeyUsageDigitalSignature,
-		// CRL is required for Windows smartcard certs.
-		CRLDistributionPoints: []string{req.CRLEndpoint},
+	}
+
+	// CRL Distribution Points (CDP) are required for Windows smartcard certs
+	// for users wanting to RDP. They are not required for the service account
+	// cert that Teleport itself uses to authenticate for LDAP.
+	if req.CRLEndpoint != "" {
+		certReq.CRLDistributionPoints = []string{req.CRLEndpoint}
 	}
 
 	limitExceeded, err := a.desktopsLimitExceeded(ctx)

lib/auth/windows/ldap.go

@@ -184,9 +184,9 @@ func convertLDAPError(err error) error {
 			return trace.ConnectionProblem(err, "network error")
 		case ldap.LDAPResultOperationsError:
 			if strings.Contains(err.Error(), "successful bind must be completed") {
-				return trace.AccessDenied(
-					"the LDAP server did not accept Teleport's client certificate, " +
-						"has the Teleport CA been imported correctly?")
+				return trace.NewAggregate(trace.AccessDenied(
+					"the LDAP server did not accept Teleport's client certificate, "+
+						"has the Teleport CA been imported correctly?"), err)
 			}
 		case ldap.LDAPResultEntryAlreadyExists:
 			return trace.AlreadyExists("LDAP object already exists: %v", err)

lib/auth/windows/windows.go

@@ -128,19 +128,26 @@ func getCertRequest(req *GenerateCredentialsRequest) (*certRequest, error) {
 		return nil, trace.Wrap(err)
 	}
 	csrPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrBytes})
-	// Note: this CRL DN may or may not be the same DN published in updateCRL.
-	//
-	// There can be multiple AD domains connected to Teleport. Each
-	// windows_desktop_service is connected to a single AD domain and publishes
-	// CRLs in it. Each service can also handle RDP connections for a different
-	// domain, with the assumption that some other windows_desktop_service
-	// published a CRL there.
-	crlDN := crlDN(req.ClusterName, req.LDAPConfig, req.CAType)
-	return &certRequest{
-		csrPEM:      csrPEM,
-		crlEndpoint: fmt.Sprintf("ldap:///%s?certificateRevocationList?base?objectClass=cRLDistributionPoint", crlDN),
-		keyDER:      keyDER,
-	}, nil
+	cr := &certRequest{
+		csrPEM: csrPEM,
+		keyDER: keyDER,
+	}
+
+	if !req.OmitCDP {
+		// Note: this CRL DN may or may not be the same DN published in updateCRL.
+		//
+		// There can be multiple AD domains connected to Teleport. Each
+		// windows_desktop_service is connected to a single AD domain and publishes
+		// CRLs in it. Each service can also handle RDP connections for a different
+		// domain, with the assumption that some other windows_desktop_service
+		// published a CRL there.
+		crlDN := crlDN(req.ClusterName, req.LDAPConfig, req.CAType)
+
+		// TODO(zmb3) consider making Teleport itself the CDP (via HTTP) instead of LDAP
+		cr.crlEndpoint = fmt.Sprintf("ldap:///%s?certificateRevocationList?base?objectClass=cRLDistributionPoint", crlDN)
+	}
+
+	return cr, nil
 }
 
 // AuthInterface is a subset of auth.ClientI
@@ -181,6 +188,11 @@ type GenerateCredentialsRequest struct {
 	CreateUser bool
 	// Groups are groups that user should be member of
 	Groups []string
+
+	// OmitCDP can be used to prevent Teleport from issuing certs with a
+	// CRL Distribution Point (CDP). CDPs are required in user certificates
+	// for RDP, but they can be omitted for certs that are used for LDAP binds.
+	OmitCDP bool
 }
 
 // GenerateWindowsDesktopCredentials generates a private key / certificate pair for the given

lib/srv/desktop/windows_server.go

@@ -443,6 +443,7 @@ func (s *WindowsService) tlsConfigForLDAP() (*tls.Config, error) {
 		domain:             s.cfg.Domain,
 		ttl:                windowsDesktopServiceCertTTL,
 		activeDirectorySID: s.cfg.SID,
+		omitCDP:            true,
 	})
 	if err != nil {
 		return nil, trace.Wrap(err)
@@ -1226,7 +1227,8 @@ type generateCredentialsRequest struct {
 	// createUser specifies if Windows user should be created if missing
 	createUser bool
 	// groups are groups that user should be member of
-	groups []string
+	groups  []string
+	omitCDP bool
 }
 
 // generateCredentials generates a private key / certificate pair for the given
@@ -1254,6 +1256,7 @@ func (s *WindowsService) generateCredentials(ctx context.Context, request genera
 		AuthClient:         s.cfg.AuthClient,
 		CreateUser:         request.createUser,
 		Groups:             request.groups,
+		OmitCDP:            request.omitCDP,
 	})
 }
 

tool/tctl/common/auth_command.go

@@ -79,6 +79,7 @@ type AuthCommand struct {
 	windowsDomain              string
 	windowsPKIDomain           string
 	windowsSID                 string
+	omitCDP                    bool
 	signOverwrite              bool
 	password                   string
 	caType                     string
@@ -150,6 +151,7 @@ func (a *AuthCommand) Initialize(app *kingpin.Application, config *servicecfg.Co
 	a.authSign.Flag("windows-domain", `Active Directory domain for which this cert is valid. Only used when --format is set to "windows"`).StringVar(&a.windowsDomain)
 	a.authSign.Flag("windows-pki-domain", `Active Directory domain where CRLs will be located. Only used when --format is set to "windows"`).StringVar(&a.windowsPKIDomain)
 	a.authSign.Flag("windows-sid", `Optional Security Identifier to embed in the certificate. Only used when --format is set to "windows"`).StringVar(&a.windowsSID)
+	a.authSign.Flag("omit-cdp", `Omit CRL Distribution Points from the cert. Only used when --format is set to "windows"`).BoolVar(&a.omitCDP)
 
 	a.authRotate = auth.Command("rotate", "Rotate certificate authorities in the cluster.")
 	a.authRotate.Flag("grace-period", "Grace period keeps previous certificate authorities signatures valid, if set to 0 will force users to re-login and nodes to re-register.").
@@ -363,6 +365,7 @@ func (a *AuthCommand) generateWindowsCert(ctx context.Context, clusterAPI certif
 		TTL:                a.genTTL,
 		ClusterName:        cn.GetClusterName(),
 		LDAPConfig:         windows.LDAPConfig{Domain: domain},
+		OmitCDP:            a.omitCDP,
 		AuthClient:         clusterAPI,
 	})
 	if err != nil {