Improve troubleshooting for LDAP authentication errors

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.

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

@@ -440,6 +440,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)
@@ -1228,7 +1229,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
@@ -1256,6 +1258,7 @@ func (s *WindowsService) generateCredentials(ctx context.Context, request genera
 		AuthClient:         s.cfg.AuthClient,
 		CreateUser:         request.createUser,
 		Groups:             request.groups,
+		OmitCDP:            request.omitCDP,
 	})
 }