Skip to content

Commit

Permalink
Fix bad sys request for different account
Browse files Browse the repository at this point in the history
When a request for a system service like $SYS.REQ.ACCOUNT.*.CONNZ
is imported/exported we ensured that the requesting account is identical
to the account referenced in the subject.

In #3250 this check was extended from CONNZ to all $SYS.REQ.ACCOUNT.*.*
requests.

In general this check interferes with monitoring accounts that need
to query all other accounts, not just itself.
There the use case is that account A sends a request with account B
in the subject. The check for equal accounts prevents this.

This change removes the check to support these use cases.

Signed-off-by: Matthias Hanel <mh@synadia.com>
  • Loading branch information
matthiashanel committed Aug 19, 2022
1 parent 6bf50db commit cf66b38
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 9 deletions.
10 changes: 1 addition & 9 deletions server/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -893,15 +893,7 @@ func (s *Server) initEventTracking() {
if tk := strings.Split(subject, tsep); len(tk) != accReqTokens {
return _EMPTY_, fmt.Errorf("subject %q is malformed", subject)
} else {
acc := tk[accReqAccIndex]
if ci, _, _, _, err := c.srv.getRequestInfo(c, msg); err == nil && ci.Account != _EMPTY_ {
// Make sure the accounts match.
if ci.Account != acc {
// Do not leak too much here.
return _EMPTY_, fmt.Errorf("bad request")
}
}
return acc, nil
return tk[accReqAccIndex], nil
}
}
monAccSrvc := map[string]msgHandler{
Expand Down
47 changes: 47 additions & 0 deletions server/jwt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4106,6 +4106,53 @@ func TestJWTTimeExpiration(t *testing.T) {
})
}

func TestJWTSysImportForDifferentAccount(t *testing.T) {
sysKp, _ := nkeys.CreateAccount()
sysPub, _ := sysKp.PublicKey()
sysClaim := jwt.NewAccountClaims(sysPub)
sysClaim.Exports.Add(&jwt.Export{
Type: jwt.Service,
Subject: "$SYS.REQ.ACCOUNT.*.INFO",
})
sysJwt, err := sysClaim.Encode(oKp)
require_NoError(t, err)

// create account
aKp, _ := nkeys.CreateAccount()
aPub, _ := aKp.PublicKey()
claim := jwt.NewAccountClaims(aPub)
claim.Imports.Add(&jwt.Import{
Type: jwt.Service,
Subject: "$SYS.REQ.ACCOUNT.*.INFO",
LocalSubject: "COMMON.ADVISORY.SYS.REQ.ACCOUNT.*.INFO",
Account: sysPub,
})
aJwt, err := claim.Encode(oKp)
require_NoError(t, err)

conf := createConfFile(t, []byte(fmt.Sprintf(`
listen: 127.0.0.1:-1
operator: %s
system_account: %s
resolver: MEM
resolver_preload: {
%s: %s
%s: %s
}
`, ojwt, sysPub, sysPub, sysJwt, aPub, aJwt)))
defer removeFile(t, conf)
sA, _ := RunServerWithConfig(conf)
defer sA.Shutdown()

nc := natsConnect(t, sA.ClientURL(), createUserCreds(t, nil, aKp))
// user for account a requests for a different account, the system account
m, err := nc.Request(fmt.Sprintf("COMMON.ADVISORY.SYS.REQ.ACCOUNT.%s.INFO", sysPub), nil, time.Second)

resp := &ServerAPIResponse{}
require_NoError(t, json.Unmarshal(m.Data, resp))
require_True(t, resp.Error == nil)
}

func TestJWTLimits(t *testing.T) {
doNotExpire := time.Now().AddDate(1, 0, 0)
// create account
Expand Down

0 comments on commit cf66b38

Please sign in to comment.