Skip to content

Commit

Permalink
Merge pull request #149 from nats-io/cve
Browse files Browse the repository at this point in the history 
[FIXED] validation to return error when token are in the wrong context
  • Loading branch information
@kozlovic
kozlovic committed Mar 14, 2021
2 parents a84d3be + a826c77 commit 6c72fdd
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 93 deletions.
9 changes: 8 additions & 1 deletion activation_claims.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,14 @@ func (a *ActivationClaims) Payload() interface{} {

// Validate checks the claims
func (a *ActivationClaims) Validate(vr *ValidationResults) {
a.ClaimsData.Validate(vr)
a.validateWithTimeChecks(vr, true)
}

// Validate checks the claims
func (a *ActivationClaims) validateWithTimeChecks(vr *ValidationResults, timeChecks bool) {
if timeChecks {
a.ClaimsData.Validate(vr)
}
a.Activation.Validate(vr)
if a.IssuerAccount != "" && !nkeys.IsValidPublicAccountKey(a.IssuerAccount) {
vr.AddError("account_id is not an account public key")
Expand Down
35 changes: 21 additions & 14 deletions imports.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (i *Import) Validate(actPubKey string, vr *ValidationResults) {
}

if i.Account == "" {
vr.AddWarning("account to import from is not specified")
vr.AddError("account to import from is not specified")
}

i.Subject.Validate(vr)
Expand All @@ -78,46 +78,53 @@ func (i *Import) Validate(actPubKey string, vr *ValidationResults) {

if i.Token != "" {
// Check to see if its an embedded JWT or a URL.
if url, err := url.Parse(i.Token); err == nil && url.Scheme != "" {
if u, err := url.Parse(i.Token); err == nil && u.Scheme != "" {
c := &http.Client{Timeout: 5 * time.Second}
resp, err := c.Get(url.String())
resp, err := c.Get(u.String())
if err != nil {
vr.AddWarning("import %s contains an unreachable token URL %q", i.Subject, i.Token)
vr.AddError("import %s contains an unreachable token URL %q", i.Subject, i.Token)
}

if resp != nil {
defer resp.Body.Close()
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
vr.AddWarning("import %s contains an unreadable token URL %q", i.Subject, i.Token)
vr.AddError("import %s contains an unreadable token URL %q", i.Subject, i.Token)
} else {
act, err = DecodeActivationClaims(string(body))
if err != nil {
vr.AddWarning("import %s contains a url %q with an invalid activation token", i.Subject, i.Token)
vr.AddError("import %s contains a URL %q with an invalid activation token", i.Subject, i.Token)
}
}
}
} else {
var err error
act, err = DecodeActivationClaims(i.Token)
if err != nil {
vr.AddWarning("import %q contains an invalid activation token", i.Subject)
vr.AddError("import %q contains an invalid activation token", i.Subject)
}
}
}

if act != nil {
if act.Issuer != i.Account {
vr.AddWarning("activation token doesn't match account for import %q", i.Subject)
if !(act.Issuer == i.Account || act.IssuerAccount == i.Account) {
vr.AddError("activation token doesn't match account for import %q", i.Subject)
}

if act.ClaimsData.Subject != actPubKey {
vr.AddWarning("activation token doesn't match account it is being included in, %q", i.Subject)
vr.AddError("activation token doesn't match account it is being included in, %q", i.Subject)
}
if act.ImportType != i.Type {
vr.AddError("mismatch between token import type %s and type of import %s", act.ImportType, i.Type)
}
act.validateWithTimeChecks(vr, false)
subj := i.Subject
if i.IsService() && i.To != "" {
subj = i.To
}
if !subj.IsContainedIn(act.ImportSubject) {
vr.AddError("activation token import subject %q doesn't match import %q", act.ImportSubject, i.Subject)
}
} else {
vr.AddWarning("no activation provided for import %s", i.Subject)
}

}

// Imports is a list of import structs
Expand Down
69 changes: 22 additions & 47 deletions imports_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,29 +34,19 @@ func TestImportValidation(t *testing.T) {
vr := CreateValidationResults()
i.Validate("", vr)

if vr.IsEmpty() {
t.Errorf("imports without token or url should warn the caller")
}

if vr.IsBlocking(true) {
t.Errorf("imports without token or url should not be blocking")
if !vr.IsEmpty() {
t.Errorf("imports should not generate an issue")
}

i.Type = Service
vr = CreateValidationResults()
i.Validate("", vr)

if vr.IsEmpty() {
t.Errorf("imports without token or url should warn the caller")
}

if vr.IsBlocking(true) {
t.Errorf("imports without token or url should not be blocking")
if !vr.IsEmpty() {
t.Errorf("imports should not generate an issue")
}

activation := NewActivationClaims(akp)
activation.Max = 1024 * 1024
activation.Expires = time.Now().Add(time.Duration(time.Hour)).UTC().Unix()
activation.Expires = time.Now().Add(time.Hour).UTC().Unix()

activation.ImportSubject = "test"
activation.ImportType = Stream
Expand Down Expand Up @@ -96,19 +86,15 @@ func TestInvalidImportToken(t *testing.T) {
vr := CreateValidationResults()
i.Validate("", vr)

if vr.IsEmpty() {
t.Errorf("imports with a bad token or url should warn the caller")
}

if vr.IsBlocking(true) {
t.Errorf("invalid type shouldnt be blocking")
if !vr.IsBlocking(true) {
t.Errorf("bad token should be blocking")
}
}

func TestInvalidImportURL(t *testing.T) {
ak := createAccountNKey(t)
akp := publicKey(ak, t)
i := &Import{Subject: "foo", Account: akp, Token: "foo://bad token url", To: "bar", Type: Stream}
i := &Import{Subject: "foo", Account: akp, Token: "foo://bad-token-url", To: "bar", Type: Stream}

vr := CreateValidationResults()
i.Validate("", vr)
Expand All @@ -117,8 +103,8 @@ func TestInvalidImportURL(t *testing.T) {
t.Errorf("imports with a bad token or url should warn the caller")
}

if vr.IsBlocking(true) {
t.Errorf("invalid type shouldnt be blocking")
if !vr.IsBlocking(true) {
t.Errorf("invalid type should be blocking")
}
}

Expand All @@ -127,15 +113,11 @@ func TestInvalidImportTokenValuesValidation(t *testing.T) {
ak2 := createAccountNKey(t)
akp := publicKey(ak, t)
akp2 := publicKey(ak2, t)
i := &Import{Subject: "test", Account: akp2, To: "bar", Type: Stream}
i := &Import{Subject: "bar", Account: akp2, To: "test", Type: Service}

vr := CreateValidationResults()
i.Validate("", vr)

if vr.IsEmpty() {
t.Errorf("imports without token or url should warn the caller")
}

if vr.IsBlocking(true) {
t.Errorf("imports without token or url should not be blocking")
}
Expand All @@ -144,10 +126,6 @@ func TestInvalidImportTokenValuesValidation(t *testing.T) {
vr = CreateValidationResults()
i.Validate("", vr)

if vr.IsEmpty() {
t.Errorf("imports without token or url should warn the caller")
}

if vr.IsBlocking(true) {
t.Errorf("imports without token or url should not be blocking")
}
Expand All @@ -157,7 +135,7 @@ func TestInvalidImportTokenValuesValidation(t *testing.T) {
activation.Expires = time.Now().Add(time.Duration(time.Hour)).UTC().Unix()

activation.ImportSubject = "test"
activation.ImportType = Stream
activation.ImportType = Service
actJWT := encode(activation, ak2, t)

i.Token = actJWT
Expand Down Expand Up @@ -187,18 +165,19 @@ func TestInvalidImportTokenValuesValidation(t *testing.T) {
t.Errorf("imports with wrong issuer")
}
}

func TestMissingAccountInImport(t *testing.T) {
i := &Import{Subject: "foo", To: "bar", Type: Stream}

vr := CreateValidationResults()
i.Validate("", vr)

if len(vr.Issues) != 2 {
t.Errorf("imports without token or url should warn the caller, as should missing account")
if len(vr.Issues) != 1 {
t.Errorf("expected only one issue")
}

if vr.IsBlocking(true) {
t.Errorf("Missing Account is not blocking, must import failures are warnings")
if !vr.IsBlocking(true) {
t.Errorf("Missing Account is blocking")
}
}

Expand All @@ -210,10 +189,6 @@ func TestServiceImportWithWildcard(t *testing.T) {
vr := CreateValidationResults()
i.Validate("", vr)

if len(vr.Issues) != 2 {
t.Errorf("imports without token or url should warn the caller, as should wildcard service")
}

if !vr.IsBlocking(true) {
t.Errorf("expected service import with a wildcard subject to be a blocking error")
}
Expand All @@ -225,8 +200,8 @@ func TestStreamImportWithWildcardPrefix(t *testing.T) {
vr := CreateValidationResults()
i.Validate("", vr)

if len(vr.Issues) != 3 {
t.Errorf("should have registered 3 issues with this import, got %d", len(vr.Issues))
if len(vr.Issues) != 2 {
t.Errorf("should have registered 2 issues with this import, got %d", len(vr.Issues))
}

if !vr.IsBlocking(true) {
Expand All @@ -246,8 +221,8 @@ func TestImportsValidation(t *testing.T) {
vr := CreateValidationResults()
imports.Validate("", vr)

if len(vr.Issues) != 3 {
t.Errorf("imports without token or url should warn the caller x2, wildcard service as well")
if len(vr.Issues) != 1 {
t.Errorf("warn about wildcard service")
}

if !vr.IsBlocking(true) {
Expand Down Expand Up @@ -349,7 +324,7 @@ func TestImportSubjectValidation(t *testing.T) {
vr = CreateValidationResults()
i.Validate(akp, vr)

if !vr.IsEmpty() {
if vr.IsEmpty() {
t.Errorf("imports with non-contains subject should be not valid")
}

Expand Down
2 changes: 1 addition & 1 deletion v2/account_claims_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestNewAccountClaims(t *testing.T) {
account.InfoURL = "http://localhost/my-account/doc"
account.Description = "my account"
account.Imports = Imports{}
account.Imports.Add(&Import{Subject: "test", Name: "test import", Account: apk2, Token: actJWT, To: "my", Type: Stream})
account.Imports.Add(&Import{Subject: "test", Name: "test import", Account: apk2, Token: actJWT, LocalSubject: "my", Type: Stream})

vr := CreateValidationResults()
account.Validate(vr)
Expand Down
19 changes: 16 additions & 3 deletions v2/imports.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,11 @@ func (i *Import) Validate(actPubKey string, vr *ValidationResults) {
}

if i.Account == "" {
vr.AddWarning("account to import from is not specified")
vr.AddError("account to import from is not specified")
}

if i.GetTo() != "" {
vr.AddWarning("the field to has been deprecated (use LocalSubject instead)")
}

i.Subject.Validate(vr)
Expand All @@ -88,19 +92,28 @@ func (i *Import) Validate(actPubKey string, vr *ValidationResults) {
var err error
act, err = DecodeActivationClaims(i.Token)
if err != nil {
vr.AddWarning("import %q contains an invalid activation token", i.Subject)
vr.AddError("import %q contains an invalid activation token", i.Subject)
}
}

if act != nil {
if !(act.Issuer == i.Account || act.IssuerAccount == i.Account) {
vr.AddError("activation token doesn't match account for import %q", i.Subject)
}

if act.ClaimsData.Subject != actPubKey {
vr.AddError("activation token doesn't match account it is being included in, %q", i.Subject)
}
if act.ImportType != i.Type {
vr.AddError("mismatch between token import type %s and type of import %s", act.ImportType, i.Type)
}
act.validateWithTimeChecks(vr, false)
subj := i.Subject
if i.IsService() && i.To != "" {
subj = i.To
}
if !subj.IsContainedIn(act.ImportSubject) {
vr.AddError("activation token import subject %q doesn't match import %q", act.ImportSubject, i.Subject)
}
}
}

Expand Down
Loading

0 comments on commit 6c72fdd

Please sign in to comment.