Skip to content

Commit 49733a2

Browse files
committed
auth: refactor access check and add initial prometheus metrics
Signed-off-by: Aaron Wilson <aawilson@nvidia.com>
1 parent 2400cf1 commit 49733a2

2 files changed

Lines changed: 131 additions & 45 deletions

File tree

ais/prxauth.go

Lines changed: 86 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/NVIDIA/aistore/cmn/nlog"
2222
"github.com/NVIDIA/aistore/core/meta"
2323
"github.com/NVIDIA/aistore/memsys"
24+
"github.com/NVIDIA/aistore/stats"
2425

2526
onexxh "github.com/OneOfOne/xxhash"
2627
)
@@ -227,12 +228,8 @@ func (p *proxy) validateEnableAuth(r *http.Request, clone *cmn.AuthConf, toUpdat
227228
}
228229

229230
claims, err := p.validateToken(r.Context(), r.Header)
230-
if err != nil || claims == nil {
231-
reason := "no claims in provided token"
232-
if err != nil {
233-
reason = err.Error()
234-
}
235-
return http.StatusUnauthorized, fmt.Errorf("enabling JWT/OIDC auth requires a valid token (%s)", reason)
231+
if err != nil {
232+
return http.StatusUnauthorized, fmt.Errorf("enabling JWT/OIDC auth requires a valid token [err: %v]", err)
236233
}
237234
if !claims.IsAdmin {
238235
return http.StatusUnauthorized, errors.New("enabling JWT/OIDC auth requires a token with an 'admin' claim")
@@ -314,17 +311,41 @@ func (p *proxy) delToken(w http.ResponseWriter, r *http.Request) {
314311
}
315312
}
316313

314+
// Validate the token found in the given header, return claims, and update metrics
315+
// Returned claims will always be non-nil if no error is returned
316+
func (p *proxy) validateToken(ctx context.Context, hdr http.Header) (*tok.AISClaims, error) {
317+
claims, err := p.extractAndValidate(ctx, hdr)
318+
319+
if err != nil {
320+
p.statsT.Inc(stats.AuthFailCount)
321+
switch {
322+
case errors.Is(err, tok.ErrNoToken):
323+
p.statsT.Inc(stats.AuthNoTokenCount)
324+
case errors.Is(err, tok.ErrInvalidToken):
325+
p.statsT.Inc(stats.AuthInvalidTokenCount)
326+
case errors.Is(err, tok.ErrTokenExpired):
327+
p.statsT.Inc(stats.AuthExpiredTokenCount)
328+
}
329+
} else {
330+
p.statsT.Inc(stats.AuthSuccessCount)
331+
}
332+
return claims, err
333+
}
334+
317335
// Validates a token from the request header.
318336
// Supports both standard Bearer tokens and X-Amz-Security-Token
319337
// as fallback for AWS SDK compatibility.
320-
func (p *proxy) validateToken(ctx context.Context, hdr http.Header) (*tok.AISClaims, error) {
338+
func (p *proxy) extractAndValidate(ctx context.Context, hdr http.Header) (*tok.AISClaims, error) {
321339
tokenHdr, err := tok.ExtractToken(hdr)
322340
if err != nil {
323341
return nil, err
324342
}
325343
claims, err := p.authn.validateToken(ctx, tokenHdr.Token)
326344
if err != nil {
327-
return nil, fmt.Errorf("invalid token from header %q: %w ", tokenHdr.Header, err)
345+
return nil, fmt.Errorf("invalid token from header %q: %w", tokenHdr.Header, err)
346+
}
347+
if claims == nil {
348+
return nil, fmt.Errorf("token from header %q has no claims: %w", tokenHdr.Header, tok.ErrInvalidToken)
328349
}
329350
return claims, nil
330351
}
@@ -363,54 +384,76 @@ func aceErrToCode(err error) (status int) {
363384
// Validate the given header contains a token allowing access to the given bucket with the requested permissions
364385
// All failures must be logged at this level
365386
func (p *proxy) access(ctx context.Context, hdr http.Header, bck *meta.Bck, ace apc.AccessAttrs) (err error) {
366-
var (
367-
claims *tok.AISClaims
368-
bucket *cmn.Bck
369-
)
387+
// Skip internal calls
370388
if p.checkIntraCall(hdr, false /*from primary*/) == nil {
371389
return nil
372390
}
373-
if cmn.Rom.AuthEnabled() { // config.Auth.Enabled
374-
claims, err = p.validateToken(ctx, hdr)
375-
if err != nil {
376-
// NOTE: making exception to allow 3rd party clients read remote ht://bucket
377-
if errors.Is(err, tok.ErrNoToken) && bck != nil && bck.IsHT() {
378-
err = nil
379-
} else {
380-
nlog.Warningln("token validation failed:", err)
381-
}
382-
return err
391+
392+
// If auth is NOT enabled, only check bucket properties
393+
if !cmn.Rom.AuthEnabled() {
394+
if bck == nil {
395+
return nil
383396
}
384-
uid := p.owner.smap.Get().UUID
385-
if bck != nil {
386-
bucket = bck.Bucket()
397+
// With Auth disabled, always allow read-only access, PATCH, and ACL
398+
ace &^= apc.AcePATCH | apc.AceBckSetACL | apc.AccessRO
399+
err = bck.Allow(ace)
400+
if err != nil {
401+
nlog.Warningln("bucket access check failed:", err)
387402
}
388-
if err = claims.CheckPermissions(uid, bucket, ace); err != nil {
389-
nlog.Warningln("access permission check failed:", err)
390-
return err
403+
return err
404+
}
405+
406+
// Validate token and parse claims ONCE
407+
claims, err := p.validateToken(ctx, hdr)
408+
if err != nil {
409+
// NOTE: making exception to allow 3rd party clients read remote ht://bucket
410+
if errors.Is(err, tok.ErrNoToken) && bck != nil && bck.IsHT() {
411+
err = nil
412+
} else {
413+
nlog.Warningln("token validation failed:", err)
391414
}
415+
return err
392416
}
417+
return p.checkTokenAccess(claims, bck, ace)
418+
}
419+
420+
func (p *proxy) checkTokenAccess(claims *tok.AISClaims, bck *meta.Bck, ace apc.AccessAttrs) (err error) {
393421
if bck == nil {
394-
// cluster ACL: create/list buckets, node management, etc.
395-
return nil
422+
err = p.checkClaimPermissions(claims, nil, ace)
423+
if err != nil {
424+
nlog.Warningln("cluster access check failed:", err)
425+
}
426+
} else {
427+
err = p.checkBucketAccess(claims, bck, ace)
428+
if err != nil {
429+
nlog.Warningln("bucket access check failed:", err)
430+
}
396431
}
397-
398-
// bucket access conventions:
399-
// - without AuthN: read-only access, PATCH, and ACL
400-
// - with AuthN: superuser can PATCH and change ACL
401-
if !cmn.Rom.AuthEnabled() {
402-
ace &^= apc.AcePATCH | apc.AceBckSetACL | apc.AccessRO
403-
} else if claims != nil && claims.IsAdmin {
404-
ace &^= apc.AcePATCH | apc.AceBckSetACL
432+
if err != nil {
433+
p.statsT.Inc(stats.ACLDeniedCount)
405434
}
406-
if ace == 0 {
407-
return nil
435+
return err
436+
}
437+
438+
// checkClaimPermissions validates claims have the required permissions
439+
func (p *proxy) checkClaimPermissions(claims *tok.AISClaims, bucket *cmn.Bck, ace apc.AccessAttrs) error {
440+
if claims == nil {
441+
return tok.ErrInvalidToken
408442
}
409-
err = bck.Allow(ace)
443+
uid := p.owner.smap.Get().UUID
444+
return claims.CheckPermissions(uid, bucket, ace)
445+
}
446+
447+
func (p *proxy) checkBucketAccess(claims *tok.AISClaims, bck *meta.Bck, ace apc.AccessAttrs) error {
448+
err := p.checkClaimPermissions(claims, bck.Bucket(), ace)
410449
if err != nil {
411-
nlog.Warningln("bucket ACL check failed:", err)
450+
return err
412451
}
413-
return err
452+
// If an admin, bucket properties for access still apply, but admin can always patch and set ACL
453+
if claims.IsAdmin {
454+
ace &^= apc.AcePATCH | apc.AceBckSetACL
455+
}
456+
return bck.Allow(ace)
414457
}
415458

416459
/////////////////////

stats/proxy_stats.go

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,20 @@ import (
1414
"github.com/NVIDIA/aistore/cmn/debug"
1515
"github.com/NVIDIA/aistore/cmn/nlog"
1616
"github.com/NVIDIA/aistore/core"
17+
"github.com/NVIDIA/aistore/core/meta"
1718
)
1819

19-
const numProxyStats = 24 // approx. initial
20+
const numProxyStats = 30 // approx. initial
2021

21-
// NOTE: currently, proxy's stats == common and hardcoded
22+
// Authentication and Authorization metrics
23+
const (
24+
AuthSuccessCount = "auth.success.n"
25+
AuthFailCount = "auth.fail.n"
26+
AuthNoTokenCount = "auth.notoken.n"
27+
AuthInvalidTokenCount = "auth.invalidtoken.n"
28+
AuthExpiredTokenCount = "auth.expiredtoken.n"
29+
ACLDeniedCount = "acl.denied.n"
30+
)
2231

2332
type Prunner struct {
2433
runner
@@ -43,6 +52,7 @@ func (r *Prunner) Init(p core.Node) *atomic.Bool {
4352
r.core.init(numProxyStats)
4453

4554
r.regCommon(p.Snode()) // common metrics
55+
r.regAuth(p.Snode())
4656

4757
r.core.statsTime = cmn.GCO.Get().Periodic.StatsTime.D()
4858
r.ctracker = make(copyTracker, numProxyStats)
@@ -56,6 +66,39 @@ func (r *Prunner) Init(p core.Node) *atomic.Bool {
5666
return &r.runner.startedUp
5767
}
5868

69+
func (r *Prunner) regAuth(snode *meta.Snode) {
70+
r.reg(snode, AuthSuccessCount, KindCounter,
71+
&Extra{
72+
Help: "total number of successful authentications",
73+
},
74+
)
75+
r.reg(snode, AuthFailCount, KindCounter,
76+
&Extra{
77+
Help: "total number of failed authentication attempts",
78+
},
79+
)
80+
r.reg(snode, AuthNoTokenCount, KindCounter,
81+
&Extra{
82+
Help: "authentication failures due to missing token",
83+
},
84+
)
85+
r.reg(snode, AuthInvalidTokenCount, KindCounter,
86+
&Extra{
87+
Help: "authentication failures due to invalid token",
88+
},
89+
)
90+
r.reg(snode, AuthExpiredTokenCount, KindCounter,
91+
&Extra{
92+
Help: "authentication failures due to expired token",
93+
},
94+
)
95+
r.reg(snode, ACLDeniedCount, KindCounter,
96+
&Extra{
97+
Help: "total number of ACL permission denials",
98+
},
99+
)
100+
}
101+
59102
//
60103
// statsLogger interface impl
61104
//

0 commit comments

Comments
 (0)