-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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.
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
Authorization Callouts #3719
Authorization Callouts #3719
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
|
@@ -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 | ||
|
@@ -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) | ||
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 | ||
|
@@ -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 | ||
|
@@ -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() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
|
@@ -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 | ||
|
@@ -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 | ||
} | ||
|
@@ -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 | ||
} | ||
|
||
|
@@ -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 | ||
} | ||
|
||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.