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.

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

[FIXED] Do not bypass authorization blocks when turning on $SYS account access #4605

Merged
merged 1 commit into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion server/opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,9 @@ type Options struct {

// OCSP Cache config enables next-gen cache for OCSP features
OCSPCacheConfig *OCSPResponseCacheConfig

// Used to mark that we had a top level authorization block.
authBlockDefined bool
}

// WebsocketOpts are options for websocket
Expand Down Expand Up @@ -885,7 +888,7 @@ func (o *Options) processConfigFileLine(k string, v interface{}, errors *[]error
*errors = append(*errors, err)
return
}

o.authBlockDefined = true
o.Username = auth.user
o.Password = auth.pass
o.Authorization = auth.token
Expand Down
17 changes: 10 additions & 7 deletions server/opts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ func TestConfigFile(t *testing.T) {
LameDuckDuration: 4 * time.Minute,
ConnectErrorReports: 86400,
ReconnectErrorReports: 5,
authBlockDefined: true,
}

opts, err := ProcessConfigFile("./configs/test.conf")
Expand All @@ -132,13 +133,14 @@ func TestConfigFile(t *testing.T) {

func TestTLSConfigFile(t *testing.T) {
golden := &Options{
ConfigFile: "./configs/tls.conf",
Host: "127.0.0.1",
Port: 4443,
Username: "derek",
Password: "foo",
AuthTimeout: 1.0,
TLSTimeout: 2.0,
ConfigFile: "./configs/tls.conf",
Host: "127.0.0.1",
Port: 4443,
Username: "derek",
Password: "foo",
AuthTimeout: 1.0,
TLSTimeout: 2.0,
authBlockDefined: true,
}
opts, err := ProcessConfigFile("./configs/tls.conf")
if err != nil {
Expand Down Expand Up @@ -283,6 +285,7 @@ func TestMergeOverrides(t *testing.T) {
LameDuckDuration: 4 * time.Minute,
ConnectErrorReports: 86400,
ReconnectErrorReports: 5,
authBlockDefined: true,
}
fopts, err := ProcessConfigFile("./configs/test.conf")
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion server/routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ func TestRouteConfig(t *testing.T) {
NoAdvertise: true,
ConnectRetries: 2,
},
PidFile: "/tmp/nats-server/nats_cluster_test.pid",
PidFile: "/tmp/nats-server/nats_cluster_test.pid",
authBlockDefined: true,
}

// Setup URLs
Expand Down
4 changes: 2 additions & 2 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1239,8 +1239,8 @@ func (s *Server) configureAccounts(reloading bool) (map[string]struct{}, error)
// If we have defined a system account here check to see if its just us and the $G account.
// We would do this to add user/pass to the system account. If this is the case add in
// no-auth-user for $G.
// Only do this if non-operator mode.
if len(opts.TrustedOperators) == 0 && numAccounts == 2 && opts.NoAuthUser == _EMPTY_ {
// Only do this if non-operator mode and we did not have an authorization block defined.
if len(opts.TrustedOperators) == 0 && numAccounts == 2 && opts.NoAuthUser == _EMPTY_ && !opts.authBlockDefined {
// If we come here from config reload, let's not recreate the fake user name otherwise
// it will cause currently clients to be disconnected.
uname := s.sysAccOnlyNoAuthUser
Expand Down
27 changes: 27 additions & 0 deletions server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"context"
"crypto/tls"
"encoding/json"
"errors"
"flag"
"fmt"
"io"
Expand Down Expand Up @@ -2080,3 +2081,29 @@ func TestServerRateLimitLogging(t *testing.T) {

checkLog(c1, c2)
}

// https://github.com/nats-io/nats-server/discussions/4535
func TestServerAuthBlockAndSysAccounts(t *testing.T) {
conf := createConfFile(t, []byte(`
listen: 127.0.0.1:-1
server_name: s-test
authorization {
users = [ { user: "u", password: "pass"} ]
}
accounts {
$SYS: { users: [ { user: admin, password: pwd } ] }
}
`))

s, _ := RunServerWithConfig(conf)
defer s.Shutdown()

// This should work of course.
nc, err := nats.Connect(s.ClientURL(), nats.UserInfo("u", "pass"))
require_NoError(t, err)
defer nc.Close()

// This should not.
_, err = nats.Connect(s.ClientURL())
require_Error(t, err, nats.ErrAuthorization, errors.New("nats: Authorization Violation"))
}