From cf189090d927c8e6fe07d1741912925280968cbd Mon Sep 17 00:00:00 2001 From: Christopher Cooper Date: Mon, 31 Oct 2022 18:49:00 -0400 Subject: [PATCH] This commit refactors some parts of Windows Desktop into lib/auth/windows in order to prepare it for more general use; in particular, pkinit/x509 authentication for SQL Server has the same certificate requirements, and it does not make sense to import desktop into database code. (#17870) --- lib/auth/windows/windows.go | 261 +++++++++++++++++++++++++ lib/auth/windows/windows_test.go | 104 ++++++++++ lib/service/desktop.go | 3 +- lib/srv/desktop/ldap.go | 4 +- lib/srv/desktop/windows_server.go | 211 +------------------- lib/srv/desktop/windows_server_test.go | 21 +- 6 files changed, 389 insertions(+), 215 deletions(-) create mode 100644 lib/auth/windows/windows.go create mode 100644 lib/auth/windows/windows_test.go diff --git a/lib/auth/windows/windows.go b/lib/auth/windows/windows.go new file mode 100644 index 00000000000..d64d536e0b9 --- /dev/null +++ b/lib/auth/windows/windows.go @@ -0,0 +1,261 @@ +// Copyright 2022 Gravitational, Inc +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package windows + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/asn1" + "encoding/pem" + "fmt" + "strings" + "time" + + "github.com/gravitational/trace" + + "github.com/gravitational/teleport/api/client/proto" + "github.com/gravitational/teleport/lib/auth" +) + +const ( + // CertTTL is the TTL for Teleport-issued Windows Certificates. + // Certificates are requested on each connection attempt, so the TTL is + // deliberately set to a small value to give enough time to establish a + // single desktop session. + CertTTL = 5 * time.Minute +) + +// LDAPConfig contains parameters for connecting to an LDAP server. +type LDAPConfig struct { + // Addr is the LDAP server address in the form host:port. + // Standard port is 636 for LDAPS. + Addr string + // Domain is an Active Directory domain name, like "example.com". + Domain string + // Username is an LDAP username, like "EXAMPLE\Administrator", where + // "EXAMPLE" is the NetBIOS version of Domain. + Username string + // InsecureSkipVerify decides whether we skip verifying with the LDAP server's CA when making the LDAPS connection. + InsecureSkipVerify bool + // ServerName is the name of the LDAP server for TLS. + ServerName string + // CA is an optional CA cert to be used for verification if InsecureSkipVerify is set to false. + CA *x509.Certificate +} + +// Check verifies this LDAPConfig +func (cfg LDAPConfig) Check() error { + if cfg.Addr == "" { + return trace.BadParameter("missing Addr in LDAPConfig") + } + if cfg.Domain == "" { + return trace.BadParameter("missing Domain in LDAPConfig") + } + if cfg.Username == "" { + return trace.BadParameter("missing Username in LDAPConfig") + } + return nil +} + +// DomainDN returns the distinguished name for the domain +func (cfg LDAPConfig) DomainDN() string { + var sb strings.Builder + parts := strings.Split(cfg.Domain, ".") + for _, p := range parts { + if sb.Len() > 0 { + sb.WriteString(",") + } + sb.WriteString("DC=") + sb.WriteString(p) + } + return sb.String() +} + +func crlContainerDN(config LDAPConfig) string { + return "CN=Teleport,CN=CDP,CN=Public Key Services,CN=Services,CN=Configuration," + config.DomainDN() +} + +func crlDN(clusterName string, config LDAPConfig) string { + return "CN=" + clusterName + "," + crlContainerDN(config) +} + +// GenerateCredentials generates a private key / certificate pair for the given +// Windows username. The certificate has certain special fields different from +// the regular Teleport user certificate, to meet the requirements of Active +// Directory. See: +// https://docs.microsoft.com/en-us/windows/security/identity-protection/smart-cards/smart-card-certificate-requirements-and-enumeration +func GenerateCredentials(ctx context.Context, username, domain string, ttl time.Duration, clusterName string, ldapConfig LDAPConfig, authClient auth.ClientI) (certDER, keyDER []byte, err error) { + // Important: rdpclient currently only supports 2048-bit RSA keys. + // If you switch the key type here, update handle_general_authentication in + // rdp/rdpclient/src/piv.rs accordingly. + rsaKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + return nil, nil, trace.Wrap(err) + } + // Also important: rdpclient expects the private key to be in PKCS1 format. + keyDER = x509.MarshalPKCS1PrivateKey(rsaKey) + + // Generate the Windows-compatible certificate, see + // https://docs.microsoft.com/en-us/troubleshoot/windows-server/windows-security/enabling-smart-card-logon-third-party-certification-authorities + // for requirements. + san, err := SubjectAltNameExtension(username, domain) + if err != nil { + return nil, nil, trace.Wrap(err) + } + csr := &x509.CertificateRequest{ + Subject: pkix.Name{CommonName: username}, + // We have to pass SAN and ExtKeyUsage as raw extensions because + // crypto/x509 doesn't support what we need: + // - x509.ExtKeyUsage doesn't have the Smartcard Logon variant + // - x509.CertificateRequest doesn't have OtherName SAN fields (which + // is a type of SAN distinct from DNSNames, EmailAddresses, IPAddresses + // and URIs) + ExtraExtensions: []pkix.Extension{ + EnhancedKeyUsageExtension, + san, + }, + } + csrBytes, err := x509.CreateCertificateRequest(rand.Reader, csr, rsaKey) + if err != nil { + return nil, 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(clusterName, ldapConfig) + genResp, err := authClient.GenerateWindowsDesktopCert(ctx, &proto.WindowsDesktopCertRequest{ + CSR: 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: fmt.Sprintf("ldap:///%s?certificateRevocationList?base?objectClass=cRLDistributionPoint", crlDN), + TTL: proto.Duration(ttl), + }) + if err != nil { + return nil, nil, trace.Wrap(err) + } + certBlock, _ := pem.Decode(genResp.Cert) + certDER = certBlock.Bytes + return certDER, keyDER, nil +} + +// CertKeyPEM returns certificate and private key bytes encoded in PEM format for use with `kinit` +func CertKeyPEM(ctx context.Context, username, domain string, ttl time.Duration, clusterName string, ldapConfig LDAPConfig, authClient auth.ClientI) (certPEM, keyPEM []byte, err error) { + certDER, keyDER, err := GenerateCredentials(ctx, username, domain, ttl, clusterName, ldapConfig, authClient) + if err != nil { + return nil, nil, trace.Wrap(err) + } + + certPEM = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) + keyPEM = pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: keyDER}) + + return +} + +// The following vars contain the various object identifiers required for smartcard +// login certificates. +// +// https://docs.microsoft.com/en-us/troubleshoot/windows-server/windows-security/enabling-smart-card-logon-third-party-certification-authorities +var ( + // EnhancedKeyUsageExtensionOID is the object identifier for a + // certificate's enhanced key usage extension + EnhancedKeyUsageExtensionOID = asn1.ObjectIdentifier{2, 5, 29, 37} + + // SubjectAltNameExtensionOID is the object identifier for a + // certificate's subject alternative name extension + SubjectAltNameExtensionOID = asn1.ObjectIdentifier{2, 5, 29, 17} + + // ClientAuthenticationOID is the object idnetifier that is used to + // include client SSL authentication in a certificate's enhanced + // key usage + ClientAuthenticationOID = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 3, 2} + + // SmartcardLogonOID is the object identifier that is used to include + // smartcard login in a certificate's enhanced key usage + SmartcardLogonOID = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 311, 20, 2, 2} + + // UPNOtherNameOID is the object identifier that is used to include + // the user principal name in a certificate's subject alternative name + UPNOtherNameOID = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 311, 20, 2, 3} +) + +// EnhancedKeyUsageExtension is a set of required extended key fields specific for Microsoft certificates +var EnhancedKeyUsageExtension = pkix.Extension{ + Id: EnhancedKeyUsageExtensionOID, + Value: func() []byte { + val, err := asn1.Marshal([]asn1.ObjectIdentifier{ + ClientAuthenticationOID, + SmartcardLogonOID, + }) + if err != nil { + panic(err) + } + return val + }(), +} + +// SubjectAltNameExtension fills in the SAN for a Windows certificate +func SubjectAltNameExtension(user, domain string) (pkix.Extension, error) { + // Setting otherName SAN according to + // https://samfira.com/2020/05/16/golang-x-509-certificates-and-othername/ + // + // othernName SAN is needed to pass the UPN of the user, per + // https://docs.microsoft.com/en-us/troubleshoot/windows-server/windows-security/enabling-smart-card-logon-third-party-certification-authorities + ext := pkix.Extension{Id: SubjectAltNameExtensionOID} + var err error + ext.Value, err = asn1.Marshal( + SubjectAltName{ + OtherName: otherName{ + OID: UPNOtherNameOID, + Value: upn{ + Value: fmt.Sprintf("%s@%s", user, domain), // TODO(zmb3): sanitize username to avoid domain spoofing + }, + }, + }, + ) + if err != nil { + return ext, trace.Wrap(err) + } + return ext, nil +} + +// Types for ASN.1 SAN serialization. + +// SubjectAltName is a struct for marshaling the SAN field in a certificate +type SubjectAltName struct { + OtherName otherName `asn1:"tag:0"` +} + +type otherName struct { + OID asn1.ObjectIdentifier + Value upn `asn1:"tag:0"` +} + +type upn struct { + Value string `asn1:"utf8"` +} diff --git a/lib/auth/windows/windows_test.go b/lib/auth/windows/windows_test.go new file mode 100644 index 00000000000..278d2ce7409 --- /dev/null +++ b/lib/auth/windows/windows_test.go @@ -0,0 +1,104 @@ +// Copyright 2022 Gravitational, Inc +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package windows + +import ( + "context" + "crypto/x509" + "encoding/asn1" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/auth" +) + +// 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()) + }) + + ldapConfig := LDAPConfig{ + Domain: domain, + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + certb, keyb, err := GenerateCredentials(ctx, user, domain, CertTTL, clusterName, ldapConfig, client) + 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 + 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 + _, err = asn1.Unmarshal(extension.Value, &san) + require.NoError(t, err) + + require.Equal(t, san.OtherName.OID, UPNOtherNameOID) + require.Equal(t, san.OtherName.Value.Value, user+"@"+domain) + } + } + require.True(t, foundKeyUsage) + require.True(t, foundAltName) +} diff --git a/lib/service/desktop.go b/lib/service/desktop.go index 4a2b991b183..e211af35b9a 100644 --- a/lib/service/desktop.go +++ b/lib/service/desktop.go @@ -29,6 +29,7 @@ import ( "github.com/gravitational/teleport/api/types" apiutils "github.com/gravitational/teleport/api/utils" "github.com/gravitational/teleport/lib/auth" + "github.com/gravitational/teleport/lib/auth/windows" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/limiter" "github.com/gravitational/teleport/lib/reversetunnel" @@ -214,7 +215,7 @@ func (process *TeleportProcess) initWindowsDesktopServiceRegistered(log *logrus. StaticHosts: cfg.WindowsDesktop.Hosts, OnHeartbeat: process.onHeartbeat(teleport.ComponentWindowsDesktop), }, - LDAPConfig: desktop.LDAPConfig(cfg.WindowsDesktop.LDAP), + LDAPConfig: windows.LDAPConfig(cfg.WindowsDesktop.LDAP), DiscoveryBaseDN: cfg.WindowsDesktop.Discovery.BaseDN, DiscoveryLDAPFilters: cfg.WindowsDesktop.Discovery.Filters, DiscoveryLDAPAttributeLabels: cfg.WindowsDesktop.Discovery.LabelAttributes, diff --git a/lib/srv/desktop/ldap.go b/lib/srv/desktop/ldap.go index 30bfd5550ea..17010a32ee9 100644 --- a/lib/srv/desktop/ldap.go +++ b/lib/srv/desktop/ldap.go @@ -23,11 +23,13 @@ import ( "github.com/go-ldap/ldap/v3" "github.com/gravitational/trace" + + "github.com/gravitational/teleport/lib/auth/windows" ) // Note: if you want to browse LDAP on the Windows machine, run ADSIEdit.msc. type ldapClient struct { - cfg LDAPConfig + cfg windows.LDAPConfig mu sync.Mutex client ldap.Client diff --git a/lib/srv/desktop/windows_server.go b/lib/srv/desktop/windows_server.go index a11c5e7cd63..d214bf743d3 100644 --- a/lib/srv/desktop/windows_server.go +++ b/lib/srv/desktop/windows_server.go @@ -19,12 +19,8 @@ package desktop import ( "bytes" "context" - "crypto/rand" - "crypto/rsa" "crypto/tls" "crypto/x509" - "crypto/x509/pkix" - "encoding/asn1" "encoding/pem" "fmt" "net" @@ -40,11 +36,11 @@ import ( "github.com/sirupsen/logrus" "github.com/gravitational/teleport" - "github.com/gravitational/teleport/api/client/proto" apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/types/events" "github.com/gravitational/teleport/lib/auth" + "github.com/gravitational/teleport/lib/auth/windows" "github.com/gravitational/teleport/lib/defaults" libevents "github.com/gravitational/teleport/lib/events" "github.com/gravitational/teleport/lib/events/filesessions" @@ -74,12 +70,6 @@ const ( // Active Directory environments may take longer to complete. ldapRequestTimeout = 20 * time.Second - // windowsDesktopCertTTL is the TTL for Teleport-issued Windows Certificates. - // Certificates are requested on each connection attempt, so the TTL is - // deliberately set to a small value to give enough time to establish a - // single desktop session. - windowsDesktopCertTTL = 5 * time.Minute - // windowsDesktopServiceCertTTL is the TTL for certificates issued to the // Windows Desktop Service in order to authenticate with the LDAP server. // It is set longer than the Windows certificates for users because it is @@ -155,7 +145,7 @@ type WindowsServiceConfig struct { // HostLabelsFn gets labels that should be applied to a Windows host. HostLabelsFn func(host string) map[string]string // LDAPConfig contains parameters for connecting to an LDAP server. - LDAPConfig + windows.LDAPConfig // DiscoveryBaseDN is the base DN for searching for Windows Desktops. // Desktop discovery is disabled if this field is empty. DiscoveryBaseDN string @@ -173,50 +163,6 @@ type WindowsServiceConfig struct { Labels map[string]string } -// LDAPConfig contains parameters for connecting to an LDAP server. -type LDAPConfig struct { - // Addr is the LDAP server address in the form host:port. - // Standard port is 636 for LDAPS. - Addr string - // Domain is an Active Directory domain name, like "example.com". - Domain string - // Username is an LDAP username, like "EXAMPLE\Administrator", where - // "EXAMPLE" is the NetBIOS version of Domain. - Username string - // InsecureSkipVerify decides whether whether we skip verifying with the LDAP server's CA when making the LDAPS connection. - InsecureSkipVerify bool - // ServerName is the name of the LDAP server for TLS. - ServerName string - // CA is an optional CA cert to be used for verification if InsecureSkipVerify is set to false. - CA *x509.Certificate -} - -func (cfg LDAPConfig) check() error { - if cfg.Addr == "" { - return trace.BadParameter("missing Addr in LDAPConfig") - } - if cfg.Domain == "" { - return trace.BadParameter("missing Domain in LDAPConfig") - } - if cfg.Username == "" { - return trace.BadParameter("missing Username in LDAPConfig") - } - return nil -} - -func (cfg LDAPConfig) domainDN() string { - var sb strings.Builder - parts := strings.Split(cfg.Domain, ".") - for _, p := range parts { - if sb.Len() > 0 { - sb.WriteString(",") - } - sb.WriteString("DC=") - sb.WriteString(p) - } - return sb.String() -} - // HeartbeatConfig contains the configuration for service heartbeats. type HeartbeatConfig struct { // HostUUID is the UUID of the host that this service runs on. Used as the @@ -233,7 +179,7 @@ type HeartbeatConfig struct { func (cfg *WindowsServiceConfig) checkAndSetDiscoveryDefaults() error { switch { case cfg.DiscoveryBaseDN == types.Wildcard: - cfg.DiscoveryBaseDN = cfg.domainDN() + cfg.DiscoveryBaseDN = cfg.DomainDN() case len(cfg.DiscoveryBaseDN) > 0: if _, err := ldap.ParseDN(cfg.DiscoveryBaseDN); err != nil { return trace.BadParameter("WindowsServiceConfig contains an invalid base_dn: %v", err) @@ -280,7 +226,7 @@ func (cfg *WindowsServiceConfig) CheckAndSetDefaults() error { if err := cfg.Heartbeat.CheckAndSetDefaults(); err != nil { return trace.Wrap(err) } - if err := cfg.LDAPConfig.check(); err != nil { + if err := cfg.LDAPConfig.Check(); err != nil { return trace.Wrap(err) } if err := cfg.checkAndSetDiscoveryDefaults(); err != nil { @@ -839,7 +785,7 @@ func (s *WindowsService) connectRDP(ctx context.Context, log logrus.FieldLogger, GenerateUserCert: func(ctx context.Context, username string, ttl time.Duration) (certDER, keyDER []byte, err error) { return s.generateCredentials(ctx, username, desktop.GetDomain(), ttl) }, - CertTTL: windowsDesktopCertTTL, + CertTTL: windows.CertTTL, Addr: desktop.GetAddr(), Conn: tdpConn, AuthorizeFn: authorize, @@ -1119,7 +1065,7 @@ func (s *WindowsService) updateCA(ctx context.Context) error { func (s *WindowsService) updateCAInNTAuthStore(ctx context.Context, caDER []byte) error { // Check if our CA is already in the store. The LDAP entry for NTAuth store // is constant and it should always exist. - ntAuthDN := "CN=NTAuthCertificates,CN=Public Key Services,CN=Services,CN=Configuration," + s.cfg.LDAPConfig.domainDN() + ntAuthDN := "CN=NTAuthCertificates,CN=Public Key Services,CN=Services,CN=Configuration," + s.cfg.LDAPConfig.DomainDN() entries, err := s.lc.read(ntAuthDN, "certificationAuthority", []string{"cACertificate"}) if err != nil { return trace.Wrap(err, "fetching existing CAs: %v", err) @@ -1209,7 +1155,7 @@ func (s *WindowsService) crlDN() string { // crlContainerDN generates the LDAP distinguished name (DN) of the container // where the certificate revocation list is published func (s *WindowsService) crlContainerDN() string { - return "CN=Teleport,CN=CDP,CN=Public Key Services,CN=Services,CN=Configuration," + s.cfg.LDAPConfig.domainDN() + return "CN=Teleport,CN=CDP,CN=Public Key Services,CN=Services,CN=Configuration," + s.cfg.LDAPConfig.DomainDN() } // generateCredentials generates a private key / certificate pair for the given @@ -1218,68 +1164,7 @@ func (s *WindowsService) crlContainerDN() string { // Directory. See: // https://docs.microsoft.com/en-us/windows/security/identity-protection/smart-cards/smart-card-certificate-requirements-and-enumeration func (s *WindowsService) generateCredentials(ctx context.Context, username, domain string, ttl time.Duration) (certDER, keyDER []byte, err error) { - // Important: rdpclient currently only supports 2048-bit RSA keys. - // If you switch the key type here, update handle_general_authentication in - // rdp/rdpclient/src/piv.rs accordingly. - rsaKey, err := rsa.GenerateKey(rand.Reader, 2048) - if err != nil { - return nil, nil, trace.Wrap(err) - } - // Also important: rdpclient expects the private key to be in PKCS1 format. - keyDER = x509.MarshalPKCS1PrivateKey(rsaKey) - - // Generate the Windows-compatible certificate, see - // https://docs.microsoft.com/en-us/troubleshoot/windows-server/windows-security/enabling-smart-card-logon-third-party-certification-authorities - // for requirements. - san, err := subjectAltNameExtension(username, domain) - if err != nil { - return nil, nil, trace.Wrap(err) - } - csr := &x509.CertificateRequest{ - Subject: pkix.Name{CommonName: username}, - // We have to pass SAN and ExtKeyUsage as raw extensions because - // crypto/x509 doesn't support what we need: - // - x509.ExtKeyUsage doesn't have the Smartcard Logon variant - // - x509.CertificateRequest doesn't have OtherName SAN fields (which - // is a type of SAN distinct from DNSNames, EmailAddresses, IPAddresses - // and URIs) - ExtraExtensions: []pkix.Extension{ - enhancedKeyUsageExtension, - san, - }, - } - csrBytes, err := x509.CreateCertificateRequest(rand.Reader, csr, rsaKey) - if err != nil { - return nil, 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 := s.crlDN() - genResp, err := s.cfg.AuthClient.GenerateWindowsDesktopCert(ctx, &proto.WindowsDesktopCertRequest{ - CSR: 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: fmt.Sprintf("ldap:///%s?certificateRevocationList?base?objectClass=cRLDistributionPoint", crlDN), - TTL: proto.Duration(ttl), - }) - if err != nil { - return nil, nil, trace.Wrap(err) - } - certBlock, _ := pem.Decode(genResp.Cert) - certDER = certBlock.Bytes - return certDER, keyDER, nil + return windows.GenerateCredentials(ctx, username, domain, ttl, s.clusterName, s.cfg.LDAPConfig, s.cfg.AuthClient) } // trackSession creates a session tracker for the given sessionID and @@ -1325,86 +1210,6 @@ func (s *WindowsService) trackSession(ctx context.Context, id *tlsca.Identity, w return nil } -// The following vars contain the various object identifiers required for smartcard -// login certificates. -// -// https://docs.microsoft.com/en-us/troubleshoot/windows-server/windows-security/enabling-smart-card-logon-third-party-certification-authorities -var ( - // enhancedKeyUsageExtensionOID is the object identifier for a - // certificate's enhanced key usage extension - enhancedKeyUsageExtensionOID = asn1.ObjectIdentifier{2, 5, 29, 37} - - // subjectAltNameExtensionOID is the object identifier for a - // certificate's subject alternative name extension - subjectAltNameExtensionOID = asn1.ObjectIdentifier{2, 5, 29, 17} - - // clientAuthenticationOID is the object idnetifier that is used to - // include client SSL authentication in a certificate's enhanced - // key usage - clientAuthenticationOID = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 3, 2} - - // smartcardLogonOID is the object identifier that is used to include - // smartcard login in a certificate's enhanced key usage - smartcardLogonOID = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 311, 20, 2, 2} - - // upnOtherNameOID is the object identifier that is used to include - // the user principal name in a certificate's subject alternative name - upnOtherNameOID = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 311, 20, 2, 3} -) - -var enhancedKeyUsageExtension = pkix.Extension{ - Id: enhancedKeyUsageExtensionOID, - Value: func() []byte { - val, err := asn1.Marshal([]asn1.ObjectIdentifier{ - clientAuthenticationOID, - smartcardLogonOID, - }) - if err != nil { - panic(err) - } - return val - }(), -} - -func subjectAltNameExtension(user, domain string) (pkix.Extension, error) { - // Setting otherName SAN according to - // https://samfira.com/2020/05/16/golang-x-509-certificates-and-othername/ - // - // othernName SAN is needed to pass the UPN of the user, per - // https://docs.microsoft.com/en-us/troubleshoot/windows-server/windows-security/enabling-smart-card-logon-third-party-certification-authorities - ext := pkix.Extension{Id: subjectAltNameExtensionOID} - var err error - ext.Value, err = asn1.Marshal( - subjectAltName{ - OtherName: otherName{ - OID: upnOtherNameOID, - Value: upn{ - Value: fmt.Sprintf("%s@%s", user, domain), // TODO(zmb3): sanitize username to avoid domain spoofing - }, - }, - }, - ) - if err != nil { - return ext, trace.Wrap(err) - } - return ext, nil -} - -// Types for ASN.1 SAN serialization. - -type subjectAltName struct { - OtherName otherName `asn1:"tag:0"` -} - -type otherName struct { - OID asn1.ObjectIdentifier - Value upn `asn1:"tag:0"` -} - -type upn struct { - Value string `asn1:"utf8"` -} - // monitorErrorSender implements the io.StringWriter // interface in order to allow us to pass connection // monitor disconnect messages back to the frontend diff --git a/lib/srv/desktop/windows_server_test.go b/lib/srv/desktop/windows_server_test.go index 2562af93817..822c95b1e46 100644 --- a/lib/srv/desktop/windows_server_test.go +++ b/lib/srv/desktop/windows_server_test.go @@ -30,6 +30,7 @@ import ( "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/types/events" "github.com/gravitational/teleport/lib/auth" + "github.com/gravitational/teleport/lib/auth/windows" libevents "github.com/gravitational/teleport/lib/events" "github.com/gravitational/teleport/lib/events/eventstest" "github.com/gravitational/teleport/lib/srv/desktop/tdp" @@ -38,7 +39,7 @@ import ( func TestConfigWildcardBaseDN(t *testing.T) { cfg := &WindowsServiceConfig{ DiscoveryBaseDN: "*", - LDAPConfig: LDAPConfig{ + LDAPConfig: windows.LDAPConfig{ Domain: "test.goteleport.com", }, } @@ -109,7 +110,7 @@ func TestCRLDN(t *testing.T) { w := &WindowsService{ clusterName: test.clusterName, cfg: WindowsServiceConfig{ - LDAPConfig: LDAPConfig{ + LDAPConfig: windows.LDAPConfig{ Domain: "test.goteleport.com", }, }, @@ -152,7 +153,7 @@ func TestGenerateCredentials(t *testing.T) { w := &WindowsService{ clusterName: clusterName, cfg: WindowsServiceConfig{ - LDAPConfig: LDAPConfig{ + LDAPConfig: windows.LDAPConfig{ Domain: domain, }, AuthClient: client, @@ -162,7 +163,7 @@ func TestGenerateCredentials(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - certb, keyb, err := w.generateCredentials(ctx, user, domain, windowsDesktopCertTTL) + certb, keyb, err := w.generateCredentials(ctx, user, domain, windows.CertTTL) require.NoError(t, err) require.NotNil(t, certb) require.NotNil(t, keyb) @@ -179,22 +180,22 @@ func TestGenerateCredentials(t *testing.T) { foundAltName := false for _, extension := range cert.Extensions { switch { - case extension.Id.Equal(enhancedKeyUsageExtensionOID): + case extension.Id.Equal(windows.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) + require.Contains(t, oids, windows.ClientAuthenticationOID) + require.Contains(t, oids, windows.SmartcardLogonOID) - case extension.Id.Equal(subjectAltNameExtensionOID): + case extension.Id.Equal(windows.SubjectAltNameExtensionOID): foundAltName = true - var san subjectAltName + var san windows.SubjectAltName _, err = asn1.Unmarshal(extension.Value, &san) require.NoError(t, err) - require.Equal(t, san.OtherName.OID, upnOtherNameOID) + require.Equal(t, san.OtherName.OID, windows.UPNOtherNameOID) require.Equal(t, san.OtherName.Value.Value, user+"@"+domain) } }