diff --git a/integrations/event-handler/go.mod b/integrations/event-handler/go.mod index cb8d2c545588e..4bb55fed9fb6e 100644 --- a/integrations/event-handler/go.mod +++ b/integrations/event-handler/go.mod @@ -50,6 +50,7 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/sql/armsql v1.2.0 // indirect github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/subscription/armsubscription v1.2.0 // indirect github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c // indirect + github.com/Azure/go-ntlmssp v0.0.0-20221128193559-754e69321358 // indirect github.com/AzureAD/microsoft-authentication-library-for-go v1.4.2 // indirect github.com/BurntSushi/toml v1.5.0 // indirect github.com/MakeNowJust/heredoc v1.0.0 // indirect @@ -141,12 +142,14 @@ require ( github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/fxamacker/cbor/v2 v2.8.0 // indirect github.com/ghodss/yaml v1.0.0 // indirect + github.com/go-asn1-ber/asn1-ber v1.5.8-0.20250403174932-29230038a667 // indirect github.com/go-chi/chi v4.1.2+incompatible // indirect github.com/go-chi/chi/v5 v5.2.2 // indirect github.com/go-errors/errors v1.4.2 // indirect github.com/go-gorp/gorp/v3 v3.1.0 // indirect github.com/go-jose/go-jose/v3 v3.0.4 // indirect github.com/go-jose/go-jose/v4 v4.1.1 // indirect + github.com/go-ldap/ldap/v3 v3.4.11 // indirect github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-openapi/analysis v0.23.0 // indirect diff --git a/integrations/event-handler/go.sum b/integrations/event-handler/go.sum index 7e29306ff5662..102e033a56a71 100644 --- a/integrations/event-handler/go.sum +++ b/integrations/event-handler/go.sum @@ -116,6 +116,8 @@ github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 h1:s6gZFSlWYmbqAu github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137/go.mod h1:OMCwj8VM1Kc9e19TLln2VL61YJF0x1XFtfdL4JdbSyE= github.com/alessio/shellescape v1.4.1 h1:V7yhSDDn8LP4lc4jS8pFkt0zCnzVJlG5JXy9BVKJUX0= github.com/alessio/shellescape v1.4.1/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPpyFjDCtHLS30= +github.com/alexbrainman/sspi v0.0.0-20231016080023-1a75b4708caa h1:LHTHcTQiSGT7VVbI0o4wBRNQIgn917usHWOd6VAffYI= +github.com/alexbrainman/sspi v0.0.0-20231016080023-1a75b4708caa/go.mod h1:cEWa1LVoE5KvSD9ONXsZrj0z6KqySlCCNKHlLzbqAt4= github.com/armon/go-radix v1.0.0 h1:F4z6KzEeeQIMeLFa97iZU6vupzoecKdU5TX24SNppXI= github.com/armon/go-radix v1.0.0/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 0a4f2ec037c32..0cd21f6ba820d 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -32,13 +32,13 @@ import ( "crypto/rand" "crypto/subtle" "crypto/x509" + "encoding/base32" "encoding/pem" "errors" "fmt" "io" "log/slog" "maps" - "math/big" mathrand "math/rand/v2" "net" "os" @@ -3127,7 +3127,7 @@ func (a *Server) augmentUserCertificates( } // Issue audit event on success, same as [Server.generateCert]. - a.emitCertCreateEvent(ctx, newIdentity, notAfter) + a.emitCertCreateEvent(ctx, tlsCA, newIdentity, notAfter) return &proto.Certs{ SSH: newAuthorizedKey, @@ -3536,6 +3536,7 @@ func generateCert(ctx context.Context, a *Server, req certRequest, caType types. } var signedTLSCert []byte + var tlsIssuer *tlsca.CertAuthority if req.tlsPublicKey != nil { tlsCryptoPubKey, err := keys.ParsePublicKey(req.tlsPublicKey) if err != nil { @@ -3563,9 +3564,10 @@ func generateCert(ctx context.Context, a *Server, req certRequest, caType types. if err != nil { return nil, trace.Wrap(err) } + tlsIssuer = tlsCA } - a.emitCertCreateEvent(ctx, &identity, notAfter) + a.emitCertCreateEvent(ctx, tlsIssuer, &identity, notAfter) // create certs struct to return to user certs := &proto.Certs{ @@ -3750,9 +3752,17 @@ func (a *Server) getSigningCAs(ctx context.Context, domainName string, caType ty return tlsCA, sshSigner, ca, nil } -func (a *Server) emitCertCreateEvent(ctx context.Context, identity *tlsca.Identity, notAfter time.Time) { +func (a *Server) emitCertCreateEvent(ctx context.Context, issuer *tlsca.CertAuthority, identity *tlsca.Identity, notAfter time.Time) { eventIdentity := identity.GetEventIdentity() eventIdentity.Expires = notAfter + var certAuthority *apievents.CertificateAuthority + if issuer != nil { + certAuthority = &apievents.CertificateAuthority{ + Type: string(types.UserCA), + Domain: issuer.Cert.Issuer.CommonName, + SubjectKeyID: base32.HexEncoding.EncodeToString(issuer.Cert.SubjectKeyId), + } + } if err := a.emitter.EmitAuditEvent(a.closeCtx, &apievents.CertificateCreate{ Metadata: apievents.Metadata{ Type: events.CertificateCreateEvent, @@ -3765,6 +3775,7 @@ func (a *Server) emitCertCreateEvent(ctx context.Context, identity *tlsca.Identi // fetched. Need to propagate user-agent from HTTP calls. UserAgent: trimUserAgent(metadata.UserAgentFromContext(ctx)), }, + CertificateAuthority: certAuthority, }); err != nil { a.logger.WarnContext(ctx, "Failed to emit certificate create event", "error", err) } @@ -6683,13 +6694,8 @@ func (a *Server) GenerateCertAuthorityCRL(ctx context.Context, caType types.Cert if err != nil { return nil, trace.Wrap(err) } - // Empty CRL valid for 1yr. - template := &x509.RevocationList{ - Number: big.NewInt(1), - ThisUpdate: time.Now().Add(-1 * time.Minute), // 1 min in the past to account for clock skew. - NextUpdate: time.Now().Add(365 * 24 * time.Hour), - } - crl, err := x509.CreateRevocationList(rand.Reader, template, tlsAuthority.Cert, tlsAuthority.Signer) + + crl, err := keystore.GenerateCRL(tlsAuthority.Cert, tlsAuthority.Signer) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/auth/auth_test.go b/lib/auth/auth_test.go index f5d72f8eba486..da97e0e85e356 100644 --- a/lib/auth/auth_test.go +++ b/lib/auth/auth_test.go @@ -2583,9 +2583,14 @@ func TestGenerateUserCertWithCertExtension(t *testing.T) { ClientMetadata: apievents.ClientMetadata{ UserAgent: "test-user-agent/1.0", }, + CertificateAuthority: &apievents.CertificateAuthority{ + Type: "user", + Domain: "test.localhost", + }, }, lastEvent.(*apievents.CertificateCreate), cmpopts.IgnoreFields(apievents.Identity{}, "Logins", "Expires"), + cmpopts.IgnoreFields(apievents.CertificateAuthority{}, "SubjectKeyID"), )) } diff --git a/lib/auth/bot_test.go b/lib/auth/bot_test.go index a8e49a496894f..1cba20e8217f2 100644 --- a/lib/auth/bot_test.go +++ b/lib/auth/bot_test.go @@ -497,10 +497,15 @@ func TestRegisterBotInstance(t *testing.T) { BotName: "test", BotInstanceID: ident.BotInstanceID, }, + CertificateAuthority: &events.CertificateAuthority{ + Type: "user", + Domain: "localhost", + }, }, cmpopts.IgnoreFields(events.Metadata{}, "Time"), cmpopts.IgnoreFields(events.Identity{}, "Logins", "Expires"), cmpopts.IgnoreFields(events.ClientMetadata{}, "UserAgent"), + cmpopts.IgnoreFields(events.CertificateAuthority{}, "SubjectKeyID"), cmpopts.EquateEmpty(), ), ) diff --git a/lib/auth/db.go b/lib/auth/db.go index e6446a03e7bff..47edc16e188ec 100644 --- a/lib/auth/db.go +++ b/lib/auth/db.go @@ -45,6 +45,7 @@ import ( "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/tlsca" + "github.com/gravitational/teleport/lib/winpki" ) // GenerateDatabaseCert generates client certificate used by a database @@ -155,14 +156,28 @@ func (a *Server) generateDatabaseCert(ctx context.Context, req *proto.DatabaseCe Subject: csr.Subject, NotAfter: a.clock.Now().UTC().Add(req.TTL.Get()), } + if req.CertificateExtensions == proto.DatabaseCertRequest_WINDOWS_SMARTCARD { // Pass through ExtKeyUsage (which we need for Smartcard Logon usage) // and SubjectAltName (which we need for otherName SAN, not supported // out of the box in crypto/x509) extensions only. certReq.ExtraExtensions = filterExtensions(a.CloseContext(), a.logger, csr.Extensions, oidExtKeyUsage, oidSubjectAltName, oidADUserMapping) certReq.KeyUsage = x509.KeyUsageDigitalSignature - // CRL is required for Windows smartcard certs. - certReq.CRLDistributionPoints = []string{req.CRLEndpoint} + // CRL Distribution Points (CDP) are required for Windows smartcard certs. + // The CDP is computed here by the auth server issuing the cert and not provided + // by the client because the CDP is based on the identity of the issuer, which is + // necessary in order to support clusters with multiple issuing certs (HSMs). + // If there's only 1 active key we don't include SKID in CDP for backward compatibility. + if req.CRLDomain != "" { + includeSKID := len(ca.GetActiveKeys().TLS) > 1 + cdp := winpki.CRLDistributionPoint(req.CRLDomain, types.DatabaseClientCA, tlsCA, includeSKID) + certReq.CRLDistributionPoints = []string{cdp} + } else if req.CRLEndpoint != "" { + // legacy clients will specify CRL endpoint instead of CRL domain + // DELETE IN v20 (zmb3) + certReq.CRLDistributionPoints = []string{req.CRLEndpoint} + a.logger.DebugContext(ctx, "Generating Database cert with legacy CDP") + } } else { // Include provided server names as SANs in the certificate, CommonName // has been deprecated since Go 1.15: diff --git a/lib/auth/desktop.go b/lib/auth/desktop.go index e795d8c32e2a9..b478a57a4c18e 100644 --- a/lib/auth/desktop.go +++ b/lib/auth/desktop.go @@ -39,6 +39,7 @@ import ( "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/tlsca" "github.com/gravitational/teleport/lib/utils" + "github.com/gravitational/teleport/lib/winpki" ) // GenerateWindowsDesktopCert generates client certificate for Windows RDP @@ -48,6 +49,12 @@ func (a *Server) GenerateWindowsDesktopCert(ctx context.Context, req *proto.Wind return nil, trace.AccessDenied( "this Teleport cluster is not licensed for desktop access, please contact the cluster administrator") } + + limitExceeded, err := a.desktopsLimitExceeded(ctx) + if err != nil { + return nil, trace.Wrap(err) + } + csr, err := tlsca.ParseCertificateRequestPEM(req.CSR) if err != nil { return nil, trace.Wrap(err) @@ -71,6 +78,7 @@ func (a *Server) GenerateWindowsDesktopCert(ctx context.Context, req *proto.Wind if err != nil { return nil, trace.Wrap(err) } + // See https://docs.microsoft.com/en-us/troubleshoot/windows-server/windows-security/enabling-smart-card-logon-third-party-certification-authorities // for cert requirements for Windows authn. certReq := tlsca.CertificateRequest{ @@ -85,14 +93,20 @@ func (a *Server) GenerateWindowsDesktopCert(ctx context.Context, req *proto.Wind // 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 != "" { + // + // The CDP is computed here by the auth server issuing the cert and not provided + // by the client because the CDP is based on the identity of the issuer, which is + // necessary in order to support clusters with multiple issuing certs (HSMs). + if req.CRLDomain != "" { + cdp := winpki.CRLDistributionPoint(req.CRLDomain, types.UserCA, tlsCA, true) + certReq.CRLDistributionPoints = []string{cdp} + } else if req.CRLEndpoint != "" { + // legacy clients will specify CRL endpoint instead of CRL domain + // DELETE IN v21 (zmb3) certReq.CRLDistributionPoints = []string{req.CRLEndpoint} + a.logger.DebugContext(ctx, "Generating Windows desktop cert with legacy CDP") } - limitExceeded, err := a.desktopsLimitExceeded(ctx) - if err != nil { - return nil, trace.Wrap(err) - } certReq.ExtraExtensions = append(certReq.ExtraExtensions, pkix.Extension{ Id: tlsca.LicenseOID, Value: []byte(modules.GetModules().BuildType()), diff --git a/lib/auth/init.go b/lib/auth/init.go index 692c9a0a4d754..dd1327755687c 100644 --- a/lib/auth/init.go +++ b/lib/auth/init.go @@ -764,6 +764,35 @@ func initializeAuthority(ctx context.Context, asrv *Server, caID types.CertAuthI } } + // Add [empty] CRLs to any issuers that are missing them. + // These are valid for 10 years and regenerated on CA rotation. + updated := false + for _, kp := range ca.GetActiveKeys().TLS { + if kp.CRL != nil { + continue + } + certBytes, signer, err := asrv.keyStore.GetTLSCertAndSigner(ctx, ca) + if err != nil { + return nil, nil, trace.Wrap(err) + } + cert, err := tlsca.ParseCertificatePEM(certBytes) + if err != nil { + return nil, nil, trace.Wrap(err) + } + crl, err := keystore.GenerateCRL(cert, signer) + if err != nil { + return nil, nil, trace.Wrap(err) + } + kp.CRL = crl + updated = true + } + + if updated { + if ca, err = asrv.UpdateCertAuthority(ctx, ca); err != nil { + return nil, nil, trace.Wrap(err) + } + } + // Make sure the keystore has usable keys. This is a bit redundant if the CA // was just generated above, but cheap relative to generating the CA, and // it's nice to get the usableKeysResult. diff --git a/lib/auth/keystore/manager.go b/lib/auth/keystore/manager.go index 9f95f93560d7e..f280eb6141327 100644 --- a/lib/auth/keystore/manager.go +++ b/lib/auth/keystore/manager.go @@ -25,6 +25,7 @@ import ( "crypto/ecdsa" "crypto/ed25519" "crypto/elliptic" + "crypto/rand" "crypto/rsa" "crypto/x509" "crypto/x509/pkix" @@ -32,7 +33,9 @@ import ( "io" "log/slog" "maps" + "math/big" "slices" + "time" kms "cloud.google.com/go/kms/apiv1" "github.com/gravitational/trace" @@ -595,15 +598,41 @@ func (m *Manager) newTLSKeyPair(ctx context.Context, clusterName string, alg cry if err != nil { return nil, trace.Wrap(err) } + + certificate, err := tlsca.ParseCertificatePEM(tlsCert) + if err != nil { + return nil, trace.Wrap(err) + } + + crl, err := GenerateCRL(certificate, signer) + if err != nil { + return nil, err + } return &types.TLSKeyPair{ Cert: tlsCert, Key: tlsKey, KeyType: keyType(tlsKey), + CRL: crl, }, nil } -// New JWTKeyPair create a new JWT keypair in the keystore backend and returns -// it. +// GenerateCRL generates an empty x509 certificate revocation list. +func GenerateCRL(caCert *x509.Certificate, signer crypto.Signer) ([]byte, error) { + revocationList := &x509.RevocationList{ + Number: big.NewInt(1), + ThisUpdate: time.Now().Add(-1 * time.Minute), // 1 min in the past to account for clock skew. + + // Note the 10 year expiration date. CRLs are always empty, so they don't need to change frequently. + NextUpdate: time.Now().Add(10 * 365 * 24 * time.Hour), + } + crl, err := x509.CreateRevocationList(rand.Reader, revocationList, caCert, signer) + if err != nil { + return nil, trace.Wrap(err, "generating CRL") + } + return crl, nil +} + +// NewJWTKeyPair create a new JWT keypair in the keystore backend and returns it. func (m *Manager) NewJWTKeyPair(ctx context.Context, purpose cryptosuites.KeyPurpose) (*types.JWTKeyPair, error) { alg, err := cryptosuites.AlgorithmForKey(ctx, m.currentSuiteGetter, purpose) if err != nil { diff --git a/lib/srv/desktop/windows_server_test.go b/lib/srv/desktop/windows_server_test.go index 0471ee0648abd..c3594d73c53ca 100644 --- a/lib/srv/desktop/windows_server_test.go +++ b/lib/srv/desktop/windows_server_test.go @@ -22,6 +22,8 @@ import ( "context" "crypto/rand" "crypto/x509" + "encoding/asn1" + "encoding/base32" "io" "log/slog" "os" @@ -39,6 +41,7 @@ import ( "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/service/servicecfg" "github.com/gravitational/teleport/lib/srv/desktop/tdp" + "github.com/gravitational/teleport/lib/tlsca" logutils "github.com/gravitational/teleport/lib/utils/log" "github.com/gravitational/teleport/lib/winpki" ) @@ -121,8 +124,6 @@ func TestGenerateCredentials(t *testing.T) { domain = "test.example.com" ) - testSid := "S-1-5-21-1329593140-2634913955-1900852804-500" - authServer, err := auth.NewTestAuthServer(auth.TestAuthServerConfig{ ClusterName: clusterName, Dir: t.TempDir(), @@ -138,6 +139,17 @@ func TestGenerateCredentials(t *testing.T) { require.NoError(t, tlsServer.Close()) }) + ca, err := authServer.AuthServer.GetCertAuthorities(t.Context(), types.UserCA, false) + require.NoError(t, err) + require.Len(t, ca, 1) + + keys := ca[0].GetActiveKeys() + require.Len(t, keys.TLS, 1) + + cert, err := tlsca.ParseCertificatePEM(keys.TLS[0].Cert) + require.NoError(t, err) + commonName := base32.HexEncoding.EncodeToString(cert.SubjectKeyId) + "_" + clusterName + client, err := tlsServer.NewClient(auth.TestServerID(types.RoleWindowsDesktop, "test-host-id")) require.NoError(t, err) t.Cleanup(func() { @@ -147,75 +159,75 @@ func TestGenerateCredentials(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() + testSID := "S-1-5-21-1329593140-2634913955-1900852804-500" + for _, test := range []struct { name string activeDirectorySID string - cdp string - configure func(*WindowsServiceConfig) }{ { name: "no ad sid", activeDirectorySID: "", - cdp: `ldap:///CN=test,CN=Teleport,CN=CDP,CN=Public Key Services,CN=Services,CN=Configuration,DC=test,DC=example,DC=com?certificateRevocationList?base?objectClass=cRLDistributionPoint`, }, { name: "with ad sid", - activeDirectorySID: testSid, - cdp: `ldap:///CN=test,CN=Teleport,CN=CDP,CN=Public Key Services,CN=Services,CN=Configuration,DC=test,DC=example,DC=com?certificateRevocationList?base?objectClass=cRLDistributionPoint`, - }, - { - name: "separate PKI domain", - activeDirectorySID: "", - configure: func(cfg *WindowsServiceConfig) { cfg.PKIDomain = "pki.example.com" }, - cdp: `ldap:///CN=test,CN=Teleport,CN=CDP,CN=Public Key Services,CN=Services,CN=Configuration,DC=pki,DC=example,DC=com?certificateRevocationList?base?objectClass=cRLDistributionPoint`, + activeDirectorySID: testSID, }, } { - w := &WindowsService{ - clusterName: clusterName, - cfg: WindowsServiceConfig{ - LDAPConfig: winpki.LDAPConfig{ - Domain: domain, - }, - AuthClient: client, - }, - } - if test.configure != nil { - test.configure(&w.cfg) - } - - certb, keyb, err := w.generateCredentials(ctx, generateCredentialsRequest{ - username: user, - domain: domain, - ttl: windowsUserCertTTL, - activeDirectorySID: test.activeDirectorySID, - }) - require.NoError(t, err) - require.NotNil(t, certb) - require.NotNil(t, keyb) - - cert, err := x509.ParseCertificate(certb) - require.NoError(t, err) - require.NotNil(t, cert) - - require.Equal(t, user, cert.Subject.CommonName) - require.ElementsMatch(t, cert.CRLDistributionPoints, []string{test.cdp}) - - foundKeyUsage := false - foundAltName := false - foundAdUserMapping := false - for _, extension := range cert.Extensions { - switch { - case extension.Id.Equal(winpki.EnhancedKeyUsageExtensionOID): - foundKeyUsage = true - case extension.Id.Equal(winpki.SubjectAltNameExtensionOID): - foundAltName = true - case extension.Id.Equal(winpki.ADUserMappingExtensionOID): - foundAdUserMapping = true + t.Run(test.name, func(t *testing.T) { + certb, keyb, err := winpki.GenerateWindowsDesktopCredentials(ctx, client, &winpki.GenerateCredentialsRequest{ + Username: user, + Domain: domain, + TTL: 5 * time.Minute, + ClusterName: clusterName, + ActiveDirectorySID: test.activeDirectorySID, + }) + require.NoError(t, err) + require.NotNil(t, certb) + require.NotNil(t, keyb) + + cert, err := x509.ParseCertificate(certb) + require.NoError(t, err) + require.NotNil(t, cert) + + require.Equal(t, user, cert.Subject.CommonName) + require.Contains(t, cert.CRLDistributionPoints, + `ldap:///CN=`+commonName+`,CN=Teleport,CN=CDP,CN=Public Key Services,CN=Services,CN=Configuration,DC=test,DC=example,DC=com?certificateRevocationList?base?objectClass=cRLDistributionPoint`) + + foundKeyUsage := false + foundAltName := false + foundAdUserMapping := false + for _, extension := range cert.Extensions { + switch { + case extension.Id.Equal(winpki.EnhancedKeyUsageExtensionOID): + foundKeyUsage = true + var oids []asn1.ObjectIdentifier + _, err = asn1.Unmarshal(extension.Value, &oids) + require.NoError(t, err) + require.Len(t, oids, 2) + require.Contains(t, oids, winpki.ClientAuthenticationOID) + require.Contains(t, oids, winpki.SmartcardLogonOID) + case extension.Id.Equal(winpki.SubjectAltNameExtensionOID): + foundAltName = true + var san winpki.SubjectAltName[winpki.UPN] + _, err = asn1.Unmarshal(extension.Value, &san) + require.NoError(t, err) + require.Equal(t, winpki.UPNOtherNameOID, san.OtherName.OID) + require.Equal(t, user+"@"+domain, san.OtherName.Value.Value) + case extension.Id.Equal(winpki.ADUserMappingExtensionOID): + foundAdUserMapping = true + var adUserMapping winpki.SubjectAltName[winpki.ADSid] + _, err = asn1.Unmarshal(extension.Value, &adUserMapping) + require.NoError(t, err) + require.Equal(t, winpki.ADUserMappingInternalOID, adUserMapping.OtherName.OID) + require.Equal(t, []byte(testSID), adUserMapping.OtherName.Value.Value) + + } } - } - require.True(t, foundKeyUsage) - require.True(t, foundAltName) - require.Equal(t, test.activeDirectorySID != "", foundAdUserMapping) + require.True(t, foundKeyUsage) + require.True(t, foundAltName) + require.Equal(t, test.activeDirectorySID != "", foundAdUserMapping) + }) } } diff --git a/lib/tlsca/ca.go b/lib/tlsca/ca.go index 8a4ce8b5ffd8e..08777f0e074e9 100644 --- a/lib/tlsca/ca.go +++ b/lib/tlsca/ca.go @@ -27,6 +27,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/asn1" + "encoding/base32" "encoding/pem" "fmt" "math/big" @@ -1372,6 +1373,7 @@ func (ca *CertAuthority) GenerateCertificate(req CertificateRequest) ([]byte, er "dns_names", req.DNSNames, "key_usage", req.KeyUsage, "common_name", req.Subject.CommonName, + "issuer_skid", base32.HexEncoding.EncodeToString(ca.Cert.SubjectKeyId), ) template := &x509.Certificate{ diff --git a/lib/tlsca/parsegen.go b/lib/tlsca/parsegen.go index bbdb19010d139..e867c2ed86266 100644 --- a/lib/tlsca/parsegen.go +++ b/lib/tlsca/parsegen.go @@ -92,8 +92,8 @@ func GenerateSelfSignedCAWithConfig(config GenerateCAConfig) (certPEM []byte, er // signed by the same private key and having the same subject (happens in tests) config.Entity.SerialNumber = serialNumber.String() - // Note: KeyUsageCRLSign is set only to generate empty CRLs for Desktop - // Access authentication with Windows. + // Note: KeyUsageCRLSign is set only to generate empty CRLs for + // desktop and database access on Windows keyUsage := x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign | x509.KeyUsageCRLSign if _, isRSA := config.Signer.Public().(*rsa.PublicKey); isRSA { // The KeyEncipherment bit is necessary for RSA key exchanges diff --git a/lib/winpki/certificate_authority.go b/lib/winpki/certificate_authority.go index 147b34f0b29b1..7f193cb79f3ec 100644 --- a/lib/winpki/certificate_authority.go +++ b/lib/winpki/certificate_authority.go @@ -20,11 +20,13 @@ package winpki import ( "context" + "encoding/base32" "log/slog" "github.com/gravitational/trace" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/tlsca" ) // NewCertificateStoreClient returns a new structure for modifying windows certificates in a Windows CA. @@ -43,6 +45,8 @@ type CertificateStoreClient struct { type CRLGenerator interface { // GenerateCertAuthorityCRL returns an empty CRL for a CA. GenerateCertAuthorityCRL(ctx context.Context, caType types.CertAuthType) ([]byte, error) + // GetCertAuthorities returns a list of cert authorities + GetCertAuthorities(ctx context.Context, caType types.CertAuthType, loadKeys bool) ([]types.CertAuthority, error) } // CertificateStoreConfig is a config structure for a Windows Certificate Authority @@ -64,11 +68,6 @@ type CertificateStoreConfig struct { func (c *CertificateStoreClient) Update(ctx context.Context) error { caType := types.UserCA - crlDER, err := c.cfg.AccessPoint.GenerateCertAuthorityCRL(ctx, caType) - if err != nil { - return trace.Wrap(err, "generating CRL") - } - // TODO(zmb3): check for the presence of Teleport's CA in the NTAuth store // To make the CA trusted, we need 3 things: @@ -76,15 +75,48 @@ func (c *CertificateStoreClient) Update(ctx context.Context) error { // 2. put the CA cert into NTAuth store in LDAP // 3. put the CRL of the CA into a dedicated LDAP entry // - // #1 and #2 are done manually as part of the set up process (see public docs). + // #1 and #2 are done manually as part of the set-up process (see public docs). // Below we do #3. - if err := c.updateCRL(ctx, crlDER, caType); err != nil { - return trace.Wrap(err, "updating CRL over LDAP") + + hasCRL := false + certAuthorities, err := c.cfg.AccessPoint.GetCertAuthorities(ctx, caType, false) + if err != nil { + return trace.Wrap(err) + } + for _, ca := range certAuthorities { + for _, keyPair := range ca.GetActiveKeys().TLS { + if len(keyPair.CRL) == 0 { + continue + } + hasCRL = true + cert, err := tlsca.ParseCertificatePEM(keyPair.Cert) + if err != nil { + return trace.Wrap(err) + } + subjectID := base32.HexEncoding.EncodeToString(cert.SubjectKeyId) + issuer := subjectID + "_" + c.cfg.ClusterName + if err := c.updateCRL(ctx, issuer, keyPair.CRL, caType); err != nil { + return trace.Wrap(err) + } + } + } + + // All authorities are missing CRL, let's fall back to legacy behavior + // TODO(probakowski): DELETE IN v21.0.0 + if !hasCRL { + crlDER, err := c.cfg.AccessPoint.GenerateCertAuthorityCRL(ctx, caType) + if err != nil { + return trace.Wrap(err, "generating CRL") + } + + if err := c.updateCRL(ctx, c.cfg.ClusterName, crlDER, caType); err != nil { + return trace.Wrap(err, "updating CRL over LDAP") + } } return nil } -func (c *CertificateStoreClient) updateCRL(ctx context.Context, crlDER []byte, caType types.CertAuthType) error { +func (c *CertificateStoreClient) updateCRL(ctx context.Context, issuer string, crlDER []byte, caType types.CertAuthType) error { // Publish the CRL for current cluster CA. For trusted clusters, their // respective windows_desktop_services will publish CRLs of their CAs so we // don't have to do it here. @@ -99,7 +131,7 @@ func (c *CertificateStoreClient) updateCRL(ctx context.Context, crlDER []byte, c // CA will be placed at: // ... > CDP > Teleport > prod containerDN := crlContainerDN(c.cfg.Domain, caType) - crlDN := crlDN(c.cfg.ClusterName, c.cfg.Domain, caType) + crlDN := CRLDN(issuer, c.cfg.Domain, caType) // Create the parent container. if err := c.cfg.LC.CreateContainer(containerDN); err != nil { diff --git a/lib/winpki/ldap.go b/lib/winpki/ldap.go index 3a1f6494711a2..b1efcf077be93 100644 --- a/lib/winpki/ldap.go +++ b/lib/winpki/ldap.go @@ -20,6 +20,7 @@ package winpki import ( "crypto/x509" + "encoding/base32" "errors" "fmt" "strings" @@ -29,6 +30,7 @@ import ( "github.com/gravitational/trace" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/tlsca" ) // LDAPConfig contains parameters for connecting to an LDAP server. @@ -268,8 +270,20 @@ func crlContainerDN(domain string, caType types.CertAuthType) string { return fmt.Sprintf("CN=%s,CN=CDP,CN=Public Key Services,CN=Services,CN=Configuration,%s", crlKeyName(caType), DomainDN(domain)) } -func crlDN(clusterName string, activeDirectoryDomain string, caType types.CertAuthType) string { - return "CN=" + clusterName + "," + crlContainerDN(activeDirectoryDomain, caType) +// CRLDN computes the distinguished name for a Teleport issuer in Windows environments. +func CRLDN(issuerID string, activeDirectoryDomain string, caType types.CertAuthType) string { + return "CN=" + issuerID + "," + crlContainerDN(activeDirectoryDomain, caType) +} + +// CRLDistributionPoint computes the CRL distribution point for certs issued. +func CRLDistributionPoint(activeDirectoryDomain string, caType types.CertAuthType, issuer *tlsca.CertAuthority, includeSKID bool) string { + name := issuer.Cert.Subject.CommonName + if includeSKID { + id := base32.HexEncoding.EncodeToString(issuer.Cert.SubjectKeyId) + name = id + "_" + name + } + crlDN := CRLDN(name, activeDirectoryDomain, caType) + return fmt.Sprintf("ldap:///%s?certificateRevocationList?base?objectClass=cRLDistributionPoint", crlDN) } // crlKeyName returns the appropriate LDAP key given the CA type. diff --git a/lib/winpki/windows.go b/lib/winpki/windows.go index 21c8ccd3f69e8..41ec817ffab1e 100644 --- a/lib/winpki/windows.go +++ b/lib/winpki/windows.go @@ -40,9 +40,12 @@ import ( ) type certRequest struct { - csrPEM []byte - crlEndpoint string - keyDER []byte + csrPEM []byte + keyDER []byte + + // Optionally specifies the AD domain where CRLs are published. + // If omitted then the cert will not specify a CRL distribution point. + cdpDomain string } func createUsersExtension(groups []string) (pkix.Extension, error) { @@ -107,10 +110,10 @@ func getCertRequest(req *GenerateCredentialsRequest) (*certRequest, error) { } if req.ActiveDirectorySID != "" { - adUserMapping, err := asn1.Marshal(SubjectAltName[adSid]{ - otherName[adSid]{ + adUserMapping, err := asn1.Marshal(SubjectAltName[ADSid]{ + otherName[ADSid]{ OID: ADUserMappingInternalOID, - Value: adSid{ + Value: ADSid{ Value: []byte(req.ActiveDirectorySID), }, }}) @@ -134,17 +137,7 @@ func getCertRequest(req *GenerateCredentialsRequest) (*certRequest, error) { } 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, cmp.Or(req.PKIDomain, req.Domain), 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) + cr.cdpDomain = cmp.Or(req.PKIDomain, req.Domain) } return cr, nil @@ -206,17 +199,9 @@ func GenerateWindowsDesktopCredentials(ctx context.Context, auth AuthInterface, return nil, nil, trace.Wrap(err) } genResp, err := auth.GenerateWindowsDesktopCert(ctx, &proto.WindowsDesktopCertRequest{ - CSR: certReq.csrPEM, - // LDAP URI pointing at the CRL created with updateCRL. - // - // The full format is: - // ldap://domain_controller_addr/distinguished_name_and_parameters. - // - // Using ldap:///distinguished_name_and_parameters (with empty - // domain_controller_addr) will cause Windows to fetch the CRL from any - // of its current domain controllers. - CRLEndpoint: certReq.crlEndpoint, - TTL: proto.Duration(req.TTL), + CSR: certReq.csrPEM, + CRLDomain: certReq.cdpDomain, + TTL: proto.Duration(req.TTL), }) if err != nil { return nil, nil, trace.Wrap(err) @@ -250,7 +235,7 @@ func generateDatabaseCredentials(ctx context.Context, auth AuthInterface, req *G // Using ldap:///distinguished_name_and_parameters (with empty // domain_controller_addr) will cause Windows to fetch the CRL from any // of its current domain controllers. - CRLEndpoint: certReq.crlEndpoint, + CRLDomain: certReq.cdpDomain, TTL: proto.Duration(req.TTL), CertificateExtensions: proto.DatabaseCertRequest_WINDOWS_SMARTCARD, }) @@ -341,10 +326,10 @@ func SubjectAltNameExtension(user, domain string) (pkix.Extension, error) { ext := pkix.Extension{Id: SubjectAltNameExtensionOID} var err error ext.Value, err = asn1.Marshal( - SubjectAltName[upn]{ - OtherName: otherName[upn]{ + SubjectAltName[UPN]{ + OtherName: otherName[UPN]{ OID: UPNOtherNameOID, - Value: upn{ + Value: UPN{ Value: fmt.Sprintf("%s@%s", user, domain), // TODO(zmb3): sanitize username to avoid domain spoofing }, }, @@ -374,11 +359,11 @@ type ( Value T `asn1:"tag:0"` } - upn struct { + UPN struct { Value string `asn1:"utf8"` } - adSid struct { + ADSid struct { // Value is the bytes representation of the user's SID string, // e.g. []byte("S-1-5-21-1329593140-2634913955-1900852804-500") Value []byte // Gets encoded as an asn1 octet string diff --git a/lib/winpki/windows_test.go b/lib/winpki/windows_test.go index ca5a129c9c5a8..d248771640b51 100644 --- a/lib/winpki/windows_test.go +++ b/lib/winpki/windows_test.go @@ -19,17 +19,12 @@ package winpki import ( - "context" - "crypto/x509" - "encoding/asn1" "os" "testing" - "time" "github.com/stretchr/testify/require" "github.com/gravitational/teleport/api/types" - "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/modules" ) @@ -38,111 +33,6 @@ func TestMain(m *testing.M) { os.Exit(m.Run()) } -// TestGenerateCredentials verifies that the smartcard certificates generated -// by Teleport meet the requirements for Windows logon. -func TestGenerateCredentials(t *testing.T) { - const ( - clusterName = "test" - user = "test-user" - domain = "test.example.com" - ) - - authServer, err := auth.NewTestAuthServer(auth.TestAuthServerConfig{ - ClusterName: clusterName, - Dir: t.TempDir(), - }) - require.NoError(t, err) - t.Cleanup(func() { - require.NoError(t, authServer.Close()) - }) - - tlsServer, err := authServer.NewTestTLSServer() - require.NoError(t, err) - t.Cleanup(func() { - require.NoError(t, tlsServer.Close()) - }) - - client, err := tlsServer.NewClient(auth.TestServerID(types.RoleWindowsDesktop, "test-host-id")) - require.NoError(t, err) - t.Cleanup(func() { - require.NoError(t, client.Close()) - }) - - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - - testSid := "S-1-5-21-1329593140-2634913955-1900852804-500" - - for _, test := range []struct { - name string - activeDirectorySID string - }{ - { - name: "no ad sid", - activeDirectorySID: "", - }, - { - name: "with ad sid", - activeDirectorySID: testSid, - }, - } { - t.Run(test.name, func(t *testing.T) { - certb, keyb, err := GenerateWindowsDesktopCredentials(ctx, client, &GenerateCredentialsRequest{ - Username: user, - Domain: domain, - TTL: 5 * time.Minute, - ClusterName: clusterName, - ActiveDirectorySID: test.activeDirectorySID, - }) - require.NoError(t, err) - require.NotNil(t, certb) - require.NotNil(t, keyb) - - cert, err := x509.ParseCertificate(certb) - require.NoError(t, err) - require.NotNil(t, cert) - - require.Equal(t, user, cert.Subject.CommonName) - require.Contains(t, cert.CRLDistributionPoints, - `ldap:///CN=test,CN=Teleport,CN=CDP,CN=Public Key Services,CN=Services,CN=Configuration,DC=test,DC=example,DC=com?certificateRevocationList?base?objectClass=cRLDistributionPoint`) - - foundKeyUsage := false - foundAltName := false - foundAdUserMapping := false - for _, extension := range cert.Extensions { - switch { - case extension.Id.Equal(EnhancedKeyUsageExtensionOID): - foundKeyUsage = true - var oids []asn1.ObjectIdentifier - _, err = asn1.Unmarshal(extension.Value, &oids) - require.NoError(t, err) - require.Len(t, oids, 2) - require.Contains(t, oids, ClientAuthenticationOID) - require.Contains(t, oids, SmartcardLogonOID) - case extension.Id.Equal(SubjectAltNameExtensionOID): - foundAltName = true - var san SubjectAltName[upn] - _, err = asn1.Unmarshal(extension.Value, &san) - require.NoError(t, err) - require.Equal(t, UPNOtherNameOID, san.OtherName.OID) - require.Equal(t, user+"@"+domain, san.OtherName.Value.Value) - case extension.Id.Equal(ADUserMappingExtensionOID): - foundAdUserMapping = true - var adUserMapping SubjectAltName[adSid] - _, err = asn1.Unmarshal(extension.Value, &adUserMapping) - require.NoError(t, err) - require.Equal(t, ADUserMappingInternalOID, adUserMapping.OtherName.OID) - require.Equal(t, []byte(testSid), adUserMapping.OtherName.Value.Value) - - } - } - require.True(t, foundKeyUsage) - require.True(t, foundAltName) - require.Equal(t, test.activeDirectorySID != "", foundAdUserMapping) - }) - } -} - func TestCRLDN(t *testing.T) { for _, test := range []struct { name string @@ -174,7 +64,7 @@ func TestCRLDN(t *testing.T) { }, } { t.Run(test.name, func(t *testing.T) { - require.Equal(t, test.crlDN, crlDN(test.clusterName, "test.goteleport.com", test.caType)) + require.Equal(t, test.crlDN, CRLDN(test.clusterName, "test.goteleport.com", test.caType)) }) } }