Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve troubleshooting for LDAP authentication errors #42948

Merged
merged 2 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions lib/auth/desktop.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,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)
Expand Down
6 changes: 3 additions & 3 deletions lib/auth/windows/ldap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
38 changes: 25 additions & 13 deletions lib/auth/windows/windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion lib/srv/desktop/windows_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,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)
Expand Down Expand Up @@ -1281,7 +1282,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
Expand Down Expand Up @@ -1309,6 +1311,7 @@ func (s *WindowsService) generateCredentials(ctx context.Context, request genera
AuthClient: s.cfg.AuthClient,
CreateUser: request.createUser,
Groups: request.groups,
OmitCDP: request.omitCDP,
})
}

Expand Down
3 changes: 3 additions & 0 deletions tool/tctl/common/auth_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ type AuthCommand struct {
windowsDomain string
windowsPKIDomain string
windowsSID string
omitCDP bool
signOverwrite bool
password string
caType string
Expand Down Expand Up @@ -152,6 +153,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.").
Expand Down Expand Up @@ -367,6 +369,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 {
Expand Down
Loading