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.

Sign up for GitHub

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

Jump to bottom

Authorization Callouts #3719

Merged
merged 2 commits into from Jan 3, 2023
Merged

Authorization Callouts #3719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2daf904
Authentication and Authorization callouts for server configuration mode.
derekcollison Nov 30, 2022
a63929c
Add in account scoped auth error event. If external auth, supply reas…
derekcollison Jan 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions go.mod
View file
Open in desktop
Expand Up @@ -5,13 +5,13 @@ go 1.19
require (
github.com/klauspost/compress v1.15.11
github.com/minio/highwayhash v1.0.2
github.com/nats-io/jwt/v2 v2.3.0
github.com/nats-io/jwt/v2 v2.3.1-0.20221227170542-bdf40fa3627b
github.com/nats-io/nats.go v1.19.0
github.com/nats-io/nkeys v0.3.0
github.com/nats-io/nkeys v0.3.1-0.20221215194120-47c7408e7546
github.com/nats-io/nuid v1.0.1
go.uber.org/automaxprocs v1.5.1
golang.org/x/crypto v0.0.0-20220926161630-eccd6366d1be
golang.org/x/sys v0.0.0-20220928140112-f11e5e49a4ec
golang.org/x/crypto v0.3.0
golang.org/x/sys v0.2.0
golang.org/x/time v0.0.0-20220922220347-f3bd1da661af
)

Expand Down
15 changes: 8 additions & 7 deletions go.sum
View file
Open in desktop
Expand Up @@ -13,12 +13,13 @@ github.com/klauspost/compress v1.15.11 h1:Lcadnb3RKGin4FYM/orgq0qde+nc15E5Cbqg4B
github.com/klauspost/compress v1.15.11/go.mod h1:QPwzmACJjUTFsnSHH934V6woptycfrDDJnH7hvFVbGM=
github.com/minio/highwayhash v1.0.2 h1:Aak5U0nElisjDCfPSG79Tgzkn2gl66NxOMspRrKnA/g=
github.com/minio/highwayhash v1.0.2/go.mod h1:BQskDq+xkJ12lmlUUi7U0M5Swg3EWR+dLTk+kldvVxY=
github.com/nats-io/jwt/v2 v2.3.0 h1:z2mA1a7tIf5ShggOFlR1oBPgd6hGqcDYsISxZByUzdI=
github.com/nats-io/jwt/v2 v2.3.0/go.mod h1:0tqz9Hlu6bCBFLWAASKhE5vUA4c24L9KPUUgvwumE/k=
github.com/nats-io/jwt/v2 v2.3.1-0.20221227170542-bdf40fa3627b h1:exHeHbghpBp1JvdYq7muaKFvJgLD93UDcmoIbFu/9PA=
github.com/nats-io/jwt/v2 v2.3.1-0.20221227170542-bdf40fa3627b/go.mod h1:DYujvzCMZzUuqB3i1Pnpf1YtkuTwhdI84Aah9wRXkK0=
github.com/nats-io/nats.go v1.19.0 h1:H6j8aBnTQFoVrTGB6Xjd903UMdE7jz6DS4YkmAqgZ9Q=
github.com/nats-io/nats.go v1.19.0/go.mod h1:tLqubohF7t4z3du1QDPYJIQQyhb4wl6DhjxEajSI7UA=
github.com/nats-io/nkeys v0.3.0 h1:cgM5tL53EvYRU+2YLXIK0G2mJtK12Ft9oeooSZMA2G8=
github.com/nats-io/nkeys v0.3.0/go.mod h1:gvUNGjVcM2IPr5rCsRsC6Wb3Hr2CQAm08dsxtV6A5y4=
github.com/nats-io/nkeys v0.3.1-0.20221215194120-47c7408e7546 h1:7ZylVLLiDSFqxJTSibNsO2RjVSXj3QWnDc+zKara2HE=
github.com/nats-io/nkeys v0.3.1-0.20221215194120-47c7408e7546/go.mod h1:JOEZlxMfMnmaLwr+mpmP+RGIYSxLNBFsZykCGaI2PvA=
github.com/nats-io/nuid v1.0.1 h1:5iA8DT8V7q8WK2EScv2padNa/rTESc1KdnPw4TC2paw=
github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OSON2c=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
Expand All @@ -27,13 +28,13 @@ github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMT
go.uber.org/automaxprocs v1.5.1 h1:e1YG66Lrk73dn4qhg8WFSvhF0JuFQF0ERIp4rpuV8Qk=
go.uber.org/automaxprocs v1.5.1/go.mod h1:BF4eumQw0P9GtnuxxovUd06vwm1o18oMzFtK66vU6XU=
golang.org/x/crypto v0.0.0-20210314154223-e6e6c4f2bb5b/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4=
golang.org/x/crypto v0.0.0-20220926161630-eccd6366d1be h1:fmw3UbQh+nxngCAHrDCCztao/kbYFnWjoqop8dHx05A=
golang.org/x/crypto v0.0.0-20220926161630-eccd6366d1be/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.3.0 h1:a06MkbcxBrEFc0w0QIZWXrH/9cCX6KJyWbBOIwAn+7A=
golang.org/x/crypto v0.3.0/go.mod h1:hebNnKkNXi2UzZN1eVRvBB7co0a+JxK6XbPiWVs/3J4=
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
golang.org/x/sys v0.0.0-20190130150945-aca44879d564/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20220928140112-f11e5e49a4ec h1:BkDtF2Ih9xZ7le9ndzTA7KJow28VbQW3odyk/8drmuI=
golang.org/x/sys v0.0.0-20220928140112-f11e5e49a4ec/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.2.0 h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A=
golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/time v0.0.0-20220922220347-f3bd1da661af h1:Yx9k8YCG3dvF87UAn2tu2HQLf2dt/eR1bXxpLMWeH+Y=
Expand Down
75 changes: 75 additions & 0 deletions server/accounts.go
View file
Open in desktop
Expand Up @@ -82,6 +82,7 @@ type Account struct {
expired bool
incomplete bool
signingKeys map[string]jwt.Scope
extAuth *jwt.ExternalAuthorization
srv *Server // server this account is registered with (possibly nil)
lds string // loop detection subject for leaf nodes
siReply []byte // service reply prefix, will form wildcard subscription.
Expand Down Expand Up @@ -1928,6 +1929,13 @@ func (a *Account) subscribeInternal(subject string, cb msgHandler) (*subscriptio
return a.subscribeInternalEx(subject, cb, false)
}

// Unsubscribe from an internal account subscription.
func (a *Account) unsubscribeInternal(sub *subscription) {
if ic := a.internalClient(); ic != nil {
ic.processUnsub(sub.sid)
}
}

// Creates internal subscription for service import responses.
func (a *Account) subscribeServiceImportResponse(subject string) (*subscription, error) {
return a.subscribeInternalEx(subject, a.processServiceImportResponse, true)
Expand Down Expand Up @@ -2984,6 +2992,65 @@ func (a *Account) traceLabel() string {
return a.Name
}

// Check if an account has external auth set.
// Operator/Account Resolver only.
func (a *Account) hasExternalAuth() bool {
if a == nil {
return false
}
a.mu.RLock()
defer a.mu.RUnlock()
return a.extAuth != nil
}

// Deterimine if this is an external auth user.
func (a *Account) isExternalAuthUser(userID string) bool {
if a == nil {
return false
}
a.mu.RLock()
defer a.mu.RUnlock()
if a.extAuth != nil {
for _, u := range a.extAuth.AuthUsers {
if userID == u {
return true
}
}
}
return false
}

// Return the external authorization xkey if external authorization is enabled and the xkey is set.
// Operator/Account Resolver only.
func (a *Account) externalAuthXKey() string {
if a == nil {
return _EMPTY_
}
a.mu.RLock()
defer a.mu.RUnlock()
if a.extAuth != nil && a.extAuth.XKey != _EMPTY_ {
return a.extAuth.XKey
}
return _EMPTY_
}

// Check if an account switch for external authorization is allowed.
func (a *Account) isAllowedAcount(acc string) bool {
if a == nil {
return false
}
a.mu.RLock()
defer a.mu.RUnlock()
if a.extAuth != nil {
for _, a := range a.extAuth.AllowedAccounts {
if a == acc {
return true
}
}
}
return false
}

// updateAccountClaimsWithRefresh will update an existing account with new claims.
// If refreshImportingAccounts is true it will also update incomplete dependent accounts
// This will replace any exports or imports previously defined.
Expand All @@ -3003,6 +3070,14 @@ func (s *Server) updateAccountClaimsWithRefresh(a *Account, ac *jwt.AccountClaim
a.nameTag = ac.Name
a.tags = ac.Tags

// Check for external authorization.
if ac.HasExternalAuthorization() {
a.extAuth = &jwt.ExternalAuthorization{}
a.extAuth.AuthUsers.Add(ac.Authorization.AuthUsers...)
a.extAuth.AllowedAccounts.Add(ac.Authorization.AllowedAccounts...)
a.extAuth.XKey = ac.Authorization.XKey
}

// Reset exports and imports here.

// Exports is creating a whole new map.
Expand Down
119 changes: 102 additions & 17 deletions server/auth.go
View file
Open in desktop
@@ -1,4 +1,4 @@
// Copyright 2012-2022 The NATS Authors
// Copyright 2012-2023 The NATS Authors
// 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
Expand Down Expand Up @@ -262,7 +262,7 @@ func (s *Server) configureAuthorization() {
} else if opts.Nkeys != nil || opts.Users != nil {
s.nkeys, s.users = s.buildNkeysAndUsersFromOptions(opts.Nkeys, opts.Users)
s.info.AuthRequired = true
} else if opts.Username != "" || opts.Authorization != "" {
} else if opts.Username != _EMPTY_ || opts.Authorization != _EMPTY_ {
s.info.AuthRequired = true
} else {
s.users = nil
Expand All @@ -274,6 +274,27 @@ func (s *Server) configureAuthorization() {
s.wsConfigAuth(&opts.Websocket)
// And for mqtt config
s.mqttConfigAuth(&opts.MQTT)

// Check for server configured auth callouts.
if opts.AuthCallout != nil {
// Make sure we have a valid account and auth_users.
_, err := s.lookupAccount(opts.AuthCallout.Account)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it guaranteed that lookupAccount will be fully populated, in all account resolver modes, so that this must succeed during initial parse of the config file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lookupAccount can reach out. Like SYSTEM account, if you are running in operator mode, this account should also be part of the preloads. But if it isn't we will try to fetch it. If that fails we will error.

if err != nil {
s.Errorf("Authorization callout account %q not valid", opts.AuthCallout.Account)
}
for _, u := range opts.AuthCallout.AuthUsers {
// Check for user in users and nkeys since this is server config.
var found bool
if len(s.users) > 0 {
_, found = s.users[u]
} else if len(s.nkeys) > 0 && !found {
derekcollison marked this conversation as resolved.
Show resolved Hide resolved
_, found = s.nkeys[u]
}
if !found {
s.Errorf("Authorization callout user %q not valid: %v", u, err)
}
}
}
}

// Takes the given slices of NkeyUser and User options and build
Expand Down Expand Up @@ -547,7 +568,7 @@ func processUserPermissionsTemplate(lim jwt.UserPermissionLimits, ujwt *jwt.User
return lim, nil
}

func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) bool {
func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) (authorized bool) {
var (
nkey *NkeyUser
juc *jwt.UserClaims
Expand All @@ -557,6 +578,70 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
err error
ao bool // auth override
)

// Check if we have auth callouts enabled at the server level or in the bound account.
defer func() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a lot of security-critical work to be happening inside a deferred function. I'm worried about this being triggered after a panic. Is this something which could be put into a new intermediary function instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be a defer to be able to reference the result of authorized which is a return variable.

// Default reason
reason := AuthenticationViolation.String()
// No-op
if juc == nil && opts.AuthCallout == nil {
if !authorized {
s.sendAccountAuthErrorEvent(c, c.acc, reason)
}
return
}
// We have a juc defined here, check account.
if juc != nil && !acc.hasExternalAuth() {
if !authorized {
s.sendAccountAuthErrorEvent(c, c.acc, reason)
}
return
}

// We have auth callout set here.
var skip bool
// Check if we are on the list of auth_users.
userID := c.getRawAuthUser()
if juc != nil {
skip = acc.isExternalAuthUser(userID)
} else {
for _, u := range opts.AuthCallout.AuthUsers {
if userID == u {
skip = true
break
}
}
}

// If we are here we have an auth callout defined and we have failed auth so far
// so we will callout to our auth backend for processing.
if !skip {
authorized, reason = s.processClientOrLeafCallout(c, opts)
}
// Check if we are authorized and in the auth callout account, and if so add in deny publish permissions for the auth subject.
if authorized {
var authAccountName string
if juc == nil && opts.AuthCallout != nil {
authAccountName = opts.AuthCallout.Account
} else if juc != nil {
authAccountName = acc.Name
}
c.mu.Lock()
neilalexander marked this conversation as resolved.
Show resolved Hide resolved
if c.acc != nil && c.acc.Name == authAccountName {
c.mergeDenyPermissions(pub, []string{AuthCalloutSubject})
}
c.mu.Unlock()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this weren't inside a huge defer doing critical work, then you could make this a more normal/safer deferred call to unlock.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

625 matches the lock on 621. Normal locking unlocking. For small blocks without branching I enclose, for more complicated stuff I try to leverge defer unlock. Defer could be used in hear but is function scoped, not block.

} else {
// If we are here we failed external authorization.
// Send an account scoped event. Server config mode acc will be nil,
// so lookup the auth callout assigned account, that is where this will be sent.
if acc == nil {
acc, _ = s.lookupAccount(opts.AuthCallout.Account)
}
s.sendAccountAuthErrorEvent(c, acc, reason)
}
}()

s.mu.Lock()
authRequired := s.info.AuthRequired
if !authRequired {
Expand Down Expand Up @@ -812,7 +897,7 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
return false
}
if juc.BearerToken && acc.failBearer() {
c.Debugf("Account does not allow bearer token")
c.Debugf("Account does not allow bearer tokens")
return false
}
// skip validation of nonce when presented with a bearer token
Expand Down Expand Up @@ -1037,7 +1122,7 @@ func checkClientTLSCertSubject(c *client, fn tlsMapAuthFn) bool {
// https://github.com/golang/go/issues/12342
dn, err := ldap.FromRawCertSubject(cert.RawSubject)
if err == nil {
if match, ok := fn("", dn, false); ok {
if match, ok := fn(_EMPTY_, dn, false); ok {
c.Debugf("Using DistinguishedNameMatch for auth [%q]", match)
return true
}
Expand Down Expand Up @@ -1120,22 +1205,22 @@ func (s *Server) isRouterAuthorized(c *client) bool {

if opts.Cluster.TLSMap || opts.Cluster.TLSCheckKnownURLs {
return checkClientTLSCertSubject(c, func(user string, _ *ldap.DN, isDNSAltName bool) (string, bool) {
if user == "" {
return "", false
if user == _EMPTY_ {
return _EMPTY_, false
}
if opts.Cluster.TLSCheckKnownURLs && isDNSAltName {
if dnsAltNameMatches(dnsAltNameLabels(user), opts.Routes) {
return "", true
return _EMPTY_, true
}
}
if opts.Cluster.TLSMap && opts.Cluster.Username == user {
return "", true
return _EMPTY_, true
}
return "", false
return _EMPTY_, false
})
}

if opts.Cluster.Username == "" {
if opts.Cluster.Username == _EMPTY_ {
return true
}

Expand All @@ -1156,25 +1241,25 @@ func (s *Server) isGatewayAuthorized(c *client) bool {
// Check whether TLS map is enabled, otherwise use single user/pass.
if opts.Gateway.TLSMap || opts.Gateway.TLSCheckKnownURLs {
return checkClientTLSCertSubject(c, func(user string, _ *ldap.DN, isDNSAltName bool) (string, bool) {
if user == "" {
return "", false
if user == _EMPTY_ {
return _EMPTY_, false
}
if opts.Gateway.TLSCheckKnownURLs && isDNSAltName {
labels := dnsAltNameLabels(user)
for _, gw := range opts.Gateway.Gateways {
if gw != nil && dnsAltNameMatches(labels, gw.URLs) {
return "", true
return _EMPTY_, true
}
}
}
if opts.Gateway.TLSMap && opts.Gateway.Username == user {
return "", true
return _EMPTY_, true
}
return "", false
return _EMPTY_, false
})
}

if opts.Gateway.Username == "" {
if opts.Gateway.Username == _EMPTY_ {
return true
}

Expand Down