diff --git a/internal/ldap/dn.go b/internal/ldap/dn.go index 2bd46d8c..a207f120 100644 --- a/internal/ldap/dn.go +++ b/internal/ldap/dn.go @@ -5,6 +5,7 @@ package ldap import ( "bytes" "crypto/x509/pkix" + "encoding/asn1" enchex "encoding/hex" "errors" "fmt" @@ -12,14 +13,14 @@ import ( ) var attributeTypeNames = map[string]string{ - "2.5.4.6": "C", - "2.5.4.10": "O", - "2.5.4.11": "OU", "2.5.4.3": "CN", "2.5.4.5": "SERIALNUMBER", + "2.5.4.6": "C", "2.5.4.7": "L", "2.5.4.8": "ST", "2.5.4.9": "STREET", + "2.5.4.10": "O", + "2.5.4.11": "OU", "2.5.4.17": "POSTALCODE", // FIXME: Add others. "0.9.2342.19200300.100.1.25": "DC", @@ -44,7 +45,7 @@ type DN struct { } // FromCertSubject takes a pkix.Name from a cert and returns a DN -// that uses the same set. +// that uses the same set. Does not support multi value RDNs. func FromCertSubject(subject pkix.Name) (*DN, error) { dn := &DN{ RDNs: make([]*RelativeDN, 0), @@ -73,6 +74,53 @@ func FromCertSubject(subject pkix.Name) (*DN, error) { return dn, nil } +// FromRawCertSubject takes a raw subject from a certificate +// and uses asn1.Unmarshal to get the individual RDNs in the +// original order, including multi-value RDNs. +func FromRawCertSubject(rawSubject []byte) (*DN, error) { + dn := &DN{ + RDNs: make([]*RelativeDN, 0), + } + var rdns pkix.RDNSequence + _, err := asn1.Unmarshal(rawSubject, &rdns) + if err != nil { + return nil, err + } + + for i := len(rdns) - 1; i >= 0; i-- { + rdn := rdns[i] + if len(rdn) == 0 { + continue + } + + r := &RelativeDN{} + attrs := make([]*AttributeTypeAndValue, 0) + for j := len(rdn) - 1; j >= 0; j-- { + atv := rdn[j] + + typeName := "" + name := atv.Type.String() + typeName, ok := attributeTypeNames[name] + if !ok { + return nil, fmt.Errorf("invalid type name: %+v", name) + } + value, ok := atv.Value.(string) + if !ok { + return nil, fmt.Errorf("invalid type value: %+v", atv.Value) + } + attr := &AttributeTypeAndValue{ + Type: typeName, + Value: value, + } + attrs = append(attrs, attr) + } + r.Attributes = attrs + dn.RDNs = append(dn.RDNs, r) + } + + return dn, nil +} + // ParseDN returns a distinguishedName or an error. // The function respects https://tools.ietf.org/html/rfc4514 func ParseDN(str string) (*DN, error) { diff --git a/server/auth.go b/server/auth.go index 7043b2cc..342a049a 100644 --- a/server/auth.go +++ b/server/auth.go @@ -419,7 +419,7 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo } else if hasUsers { // Check if we are tls verify and are mapping users from the client_certificate. if tlsMap { - authorized := checkClientTLSCertSubject(c, func(u string, certRDN *ldap.DN, _ bool) (string, bool) { + authorized := checkClientTLSCertSubject(c, func(u string, certDN *ldap.DN, _ bool) (string, bool) { // First do literal lookup using the resulting string representation // of RDNSequence as implemented by the pkix package from Go. if u != "" { @@ -431,7 +431,7 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo return usr.Username, ok } - if certRDN == nil { + if certDN == nil { return "", false } @@ -443,11 +443,11 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo } // TODO: Use this utility to make a full validation pass // on start in case tlsmap feature is being used. - inputRDN, err := ldap.ParseDN(usr.Username) + inputDN, err := ldap.ParseDN(usr.Username) if err != nil { continue } - if inputRDN.Equal(certRDN) { + if inputDN.Equal(certDN) { user = usr return usr.Username, true } @@ -724,8 +724,9 @@ func checkClientTLSCertSubject(c *client, fn tlsMapAuthFn) bool { // the domain components in case there are any. rdn := cert.Subject.ToRDNSequence().String() - // Match that follows original order from the subject takes precedence. - dn, err := ldap.FromCertSubject(cert.Subject) + // Match using the raw subject to avoid ignoring attributes. + // https://github.com/golang/go/issues/12342 + dn, err := ldap.FromRawCertSubject(cert.RawSubject) if err == nil { if match, ok := fn("", dn, false); ok { c.Debugf("Using DistinguishedNameMatch for auth [%q]", match) diff --git a/test/configs/certs/rdns/client-f.key b/test/configs/certs/rdns/client-f.key new file mode 100644 index 00000000..c3dd4743 --- /dev/null +++ b/test/configs/certs/rdns/client-f.key @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDReroKzpObO1KW +SiPX3bcu6Z20oE53dBGURo58kEcZAErRSkqiieoS/d1xWiVnmTc4oIsJmrIIyNk4 +rO3l3Mu2lCGfVxhNtFhHrPCdtFctXYRu3c7QAcQCr/HjEzcq2oEtQD83wA16D6PH +4PmDfpMhJ0N43FvbDoFOSA61NxxnFkeOyamy6D4QnrIYBxpTvIHdEKCbt2oZsnyD +jlmpZdkkTuW51SNXlxz1o++g0+D9R/nzhnojkU4kVVFe0njlLnWrAuN0c2Y4FQuw +P1rsMOlUHzpdyVc+KT/Kl33oaEr5pxZujhazDCag47SZgPal9EFyS7VfpyMy457m +XEGUHoczAgMBAAECggEAeG6kcv4c4owSiREK1lpDrJbm8iePtSFn0eVWmcqg9YCz +guvBSP0dM9n76+U1x//QPaAfD2B+popCSFEzXIm6HLfBNMhv0oyyjFKi6yf5Tr2L +G+otsmyxchIRcMllWB/TUF61eanSlbBUKt/u02h70f2uztdxf9kxAf5vZkPO8nxT +JSWdkYM4aha56TGFrSxAgLFpmz5rHcp8CH54C0GD4GSafL56hdOV3lfGH2OqwBRf +DrXVR857JvTPyoSlkuUTAtQbGTrLVyvIRSbkYHZOFk9JeNkEHl1j1wuvLf5+BHk4 +olEb4vGjWnuy2EDAStL6QROuyPlQW+Ngd0YPUiiwEQKBgQD+gzh5V1Wp/gRc706a +9D22njMmNd/9qJL9n80GJnpRNrb8Cp9vXf50SeFMFBlMy6VJbnh9qRcGtI9fvUae +ifmWiM08+oFwyAO4HcSY182TDZMokvARy37XtR7OZVkDootoLz+zlHa0s2eLyIa8 +NcUzJI5OEeQeq+o+UXlgZ3FbRwKBgQDStCGlm6ptY9RDCDhV2ruep3j5G/V3T8Bm +93gbHp+2EBQfUu9fSY1h2K/6bcZff4Thqcd5TtF/zpG0DQ7jseYcg9nwlKzUF0w8 +4tYCpQJj+4cHjx9bHNsDLw/7a92jesU4kjah7Tt8JUmEt8lYdXYTzokE/2hs7Ay2 +NsevLgIStQKBgQDJdsusWXKI5ndDrXaWeAGl3eJ1O64710XLl8QuOyUVxm7gYfRE +rq2uFZFOrJY+UPFciCK+rat5dlILogMVmfhErbNwsobl5J31DzNBHYov/k3fjziT +jXaxf0CMdnMYyoD5jnUpTLsOXPj5EFl/AD1CN4yhxc3Cbak1fT7MDfYQHwKBgAPN +qpnRsIbe+XLoUBQEqcRYY4+jmI+5ydBSAUIEEH/51FMobRe8PSgaADs2BhGtPJnS +Nb6T1KZI9UpZvf4QNQYovyNfm6sMbJzgv1o23k8tuCdDxx4e7DknfVNdhBeyXKMD +yKatoJhCGAykQKcvH52F6eVEMv9cV3JmlL4tx23NAoGBAJCrkp0nZhYdeIGNAOqW +vHgHhnXxsqiK+yZ+V4tAUR7c0tr+l7xWxFlVZiXax3bXEMfoyzS3yGlEQoFANvVw +afN3t/pIox6IYChUrJiRHy58rSOGEEhgWfVOQ9xD1qLfd1aDCtroFjTvbsRYv7Vg +mvMiw5rle3jP3qtjDkg+9U9T +-----END PRIVATE KEY----- diff --git a/test/configs/certs/rdns/client-f.pem b/test/configs/certs/rdns/client-f.pem new file mode 100644 index 00000000..b35785c8 --- /dev/null +++ b/test/configs/certs/rdns/client-f.pem @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDOjCCAiKgAwIBAgIUKMTp3zRsyUOYWPKNF1VeFwIIiNMwDQYJKoZIhvcNAQEL +BQAwEjEQMA4GA1UEAwwHTkFUUyBDQTAeFw0yMDExMTUyMzQ4NTVaFw0yNTExMTQy +MzQ4NTVaMGQxEzARBgoJkiaJk/IsZAEZFgNvcmcxFzAVBgoJkiaJk/IsZAEZFgdP +cGVuU1NMMSEwDAYDVQQKDAV1c2VyczARBgoJkiaJk/IsZAEZFgNERVYxETAPBgNV +BAMMCEpvaG4gRG9lMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0Xq6 +Cs6TmztSlkoj1923LumdtKBOd3QRlEaOfJBHGQBK0UpKoonqEv3dcVolZ5k3OKCL +CZqyCMjZOKzt5dzLtpQhn1cYTbRYR6zwnbRXLV2Ebt3O0AHEAq/x4xM3KtqBLUA/ +N8ANeg+jx+D5g36TISdDeNxb2w6BTkgOtTccZxZHjsmpsug+EJ6yGAcaU7yB3RCg +m7dqGbJ8g45ZqWXZJE7ludUjV5cc9aPvoNPg/Uf584Z6I5FOJFVRXtJ45S51qwLj +dHNmOBULsD9a7DDpVB86XclXPik/ypd96GhK+acWbo4WswwmoOO0mYD2pfRBcku1 +X6cjMuOe5lxBlB6HMwIDAQABozYwNDAyBgNVHREEKzApgglsb2NhbGhvc3SCC2V4 +YW1wbGUuY29tgg93d3cuZXhhbXBsZS5jb20wDQYJKoZIhvcNAQELBQADggEBAAtb +cxttYaief3XTMyFa2/dqF6JO47pTCYuCF1i3jL6sPokm2k0L4aCJZTthuUwHBppK +fWWGEfERpD1GnEJPW65BRZYFEYFdLEGvT8u9UAnuwbZK/rGSp6K/P2bhCrWZt2qy +eA/WQNnWDJ9mXAH6nrJCDPd1ReFr/gidvtw7PJI7pqvu6/oi0H5VpR/RWRHZieWd +UplTd2yt3vLaBX592oybfaA5bGQ/lbNaxvZniwUjmR069EvyW5WCGCR0+AON1NxP +y5x22lf2HxvxHVDxUb1FTHYvRduy0zbpmYKpjfRXS2IY6fto246Xhv90NBDzgWP1 +K3erVOvmsj6nNnfcE/A= +-----END CERTIFICATE----- diff --git a/test/tls_test.go b/test/tls_test.go index 0e3dc189..d6ee95bf 100644 --- a/test/tls_test.go +++ b/test/tls_test.go @@ -1574,6 +1574,69 @@ func TestTLSClientAuthWithRDNSequence(t *testing.T) { nil, nil, }, + { + "connect with tls and DN includes a multi value RDN", + ` + port: -1 + %s + + authorization { + users = [ + { user = "CN=John Doe,DC=DEV+O=users,DC=OpenSSL,DC=org" } + ] + } + `, + // + // OpenSSL: -subj "/DC=org/DC=OpenSSL/DC=DEV+O=users/CN=John Doe" + // Go: CN=John Doe,O=users + // RFC2253: CN=John Doe,DC=DEV+O=users,DC=OpenSSL,DC=org + // + nats.ClientCert("./configs/certs/rdns/client-f.pem", "./configs/certs/rdns/client-f.key"), + nil, + nil, + }, + { + "connect with tls and DN includes a multi value RDN but there is no match", + ` + port: -1 + %s + + authorization { + users = [ + { user = "CN=John Doe,DC=DEV,DC=OpenSSL,DC=org" } + ] + } + `, + // + // OpenSSL: -subj "/DC=org/DC=OpenSSL/DC=DEV+O=users/CN=John Doe" -multivalue-rdn + // Go: CN=John Doe,O=users + // RFC2253: CN=John Doe,DC=DEV+O=users,DC=OpenSSL,DC=org + // + nats.ClientCert("./configs/certs/rdns/client-f.pem", "./configs/certs/rdns/client-f.key"), + errors.New("nats: Authorization Violation"), + nil, + }, + { + "connect with tls and DN includes a multi value RDN that are reordered", + ` + port: -1 + %s + + authorization { + users = [ + { user = "CN=John Doe,O=users+DC=DEV,DC=OpenSSL,DC=org" } + ] + } + `, + // + // OpenSSL: -subj "/DC=org/DC=OpenSSL/DC=DEV+O=users/CN=John Doe" -multivalue-rdn + // Go: CN=John Doe,O=users + // RFC2253: CN=John Doe,DC=DEV+O=users,DC=OpenSSL,DC=org + // + nats.ClientCert("./configs/certs/rdns/client-f.pem", "./configs/certs/rdns/client-f.key"), + nil, + nil, + }, } { t.Run(test.name, func(t *testing.T) { content := fmt.Sprintf(test.config, `