From a766b52c47d6bee7b34e5e12a70fe8398573b9df Mon Sep 17 00:00:00 2001 From: Waldemar Quevedo Date: Fri, 20 Nov 2020 17:04:22 -0800 Subject: [PATCH] Allow matching DNs regardless of order Signed-off-by: Waldemar Quevedo --- internal/ldap/dn.go | 23 +++++++++ server/auth.go | 15 +++++- test/tls_test.go | 122 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 159 insertions(+), 1 deletion(-) diff --git a/internal/ldap/dn.go b/internal/ldap/dn.go index a207f120..878ffbca 100644 --- a/internal/ldap/dn.go +++ b/internal/ldap/dn.go @@ -229,6 +229,29 @@ func (d *DN) Equal(other *DN) bool { return true } +// RDNsMatch returns true if the individual RDNs of the DNs +// are the same regardless of ordering. +func (d *DN) RDNsMatch(other *DN) bool { + if len(d.RDNs) != len(other.RDNs) { + return false + } + +CheckNextRDN: + for _, irdn := range d.RDNs { + for _, ordn := range other.RDNs { + if (len(irdn.Attributes) == len(ordn.Attributes)) && + (irdn.hasAllAttributes(ordn.Attributes) && ordn.hasAllAttributes(irdn.Attributes)) { + // Found the RDN, check if next one matches. + continue CheckNextRDN + } + } + + // Could not find a matching individual RDN, auth fails. + return false + } + return true +} + // AncestorOf returns true if the other DN consists of at least one RDN followed by all the RDNs of the current DN. // "ou=widgets,o=acme.com" is an ancestor of "ou=sprockets,ou=widgets,o=acme.com" // "ou=widgets,o=acme.com" is not an ancestor of "ou=sprockets,ou=widgets,o=foo.com" diff --git a/server/auth.go b/server/auth.go index 342a049a..2356c334 100644 --- a/server/auth.go +++ b/server/auth.go @@ -435,8 +435,9 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo return "", false } - // Look through the accounts for an RDN that is equal to the one + // Look through the accounts for a DN that is equal to the one // presented by the certificate. + dns := make(map[*User]*ldap.DN) for _, usr := range s.users { if !c.connectionTypeAllowed(usr.AllowedConnectionTypes) { continue @@ -451,6 +452,18 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo user = usr return usr.Username, true } + + // In case it did not match exactly, then collect the DNs + // and try to match later in case the DN was reordered. + dns[usr] = inputDN + } + + // Check in case the DN was reordered. + for usr, inputDN := range dns { + if inputDN.RDNsMatch(certDN) { + user = usr + return usr.Username, true + } } return "", false }) diff --git a/test/tls_test.go b/test/tls_test.go index d6ee95bf..45b0f7d1 100644 --- a/test/tls_test.go +++ b/test/tls_test.go @@ -1540,7 +1540,10 @@ func TestTLSClientAuthWithRDNSequence(t *testing.T) { users = [ { user = "CN=*.example.com,OU=NATS,O=NATS,L=Los Angeles,ST=CA,C=US,DC=example,DC=com", permissions = { subscribe = { deny = ">" }} } + + # This should take precedence since it is in the RFC2253 order. { user = "DC=com,DC=example,CN=*.example.com,O=NATS,OU=NATS,L=Los Angeles,ST=CA,C=US" } + { user = "CN=*.example.com,OU=NATS,O=NATS,L=Los Angeles,ST=CA,C=US", permissions = { subscribe = { deny = ">" }} } ] @@ -1685,6 +1688,125 @@ func TestTLSClientAuthWithRDNSequence(t *testing.T) { } } +func TestTLSClientAuthWithRDNSequenceReordered(t *testing.T) { + for _, test := range []struct { + name string + config string + certs nats.Option + err error + rerr error + }{ + { + "connect with tls and DN includes a multi value RDN that are reordered", + ` + port: -1 + %s + + authorization { + users = [ + { user = "DC=org,DC=OpenSSL,O=users+DC=DEV,CN=John Doe" } + ] + } + `, + // + // 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, + }, + { + "connect with tls and DN includes a multi value RDN that are reordered but not equal RDNs", + ` + port: -1 + %s + + authorization { + users = [ + { user = "DC=org,DC=OpenSSL,O=users,CN=John Doe" } + ] + } + `, + // + // 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 but not equal RDNs", + ` + port: -1 + %s + + authorization { + users = [ + { user = "DC=OpenSSL, DC=org, O=users, CN=John Doe" } + ] + } + `, + // + // 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, + }, + } { + t.Run(test.name, func(t *testing.T) { + content := fmt.Sprintf(test.config, ` + tls { + cert_file: "configs/certs/rdns/server.pem" + key_file: "configs/certs/rdns/server.key" + ca_file: "configs/certs/rdns/ca.pem" + timeout: 5 + verify_and_map: true + } + `) + conf := createConfFile(t, []byte(content)) + defer os.Remove(conf) + s, opts := RunServerWithConfig(conf) + defer s.Shutdown() + + nc, err := nats.Connect(fmt.Sprintf("tls://localhost:%d", opts.Port), + test.certs, + nats.RootCAs("./configs/certs/rdns/ca.pem"), + ) + if test.err == nil && err != nil { + t.Errorf("Expected to connect, got %v", err) + } else if test.err != nil && err == nil { + t.Errorf("Expected error on connect") + } else if test.err != nil && err != nil { + // Error on connect was expected + if test.err.Error() != err.Error() { + t.Errorf("Expected error %s, got: %s", test.err, err) + } + return + } + defer nc.Close() + + nc.Subscribe("ping", func(m *nats.Msg) { + m.Respond([]byte("pong")) + }) + nc.Flush() + + _, err = nc.Request("ping", []byte("ping"), 250*time.Millisecond) + if test.rerr != nil && err == nil { + t.Errorf("Expected error getting response") + } else if test.rerr == nil && err != nil { + t.Errorf("Expected response") + } + }) + } +} + func TestTLSClientSVIDAuth(t *testing.T) { for _, test := range []struct { name string