From cfc934a81713b26518f1ae0fa94900a2da77553b Mon Sep 17 00:00:00 2001 From: Greg Steuck Date: Mon, 5 Jul 2021 15:53:01 -0700 Subject: dashboard/app: cover checkClient and fix for the exposed bug Required checkClient to take the config as a parameter. --- dashboard/app/api.go | 2 +- dashboard/app/auth.go | 32 +++++++++++------------ dashboard/app/auth_test.go | 64 ++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 78 insertions(+), 20 deletions(-) diff --git a/dashboard/app/api.go b/dashboard/app/api.go index 53803a5c2..7bdd3731b 100644 --- a/dashboard/app/api.go +++ b/dashboard/app/api.go @@ -111,7 +111,7 @@ func handleAPI(c context.Context, r *http.Request) (reply interface{}, err error return nil, err } // Somewhat confusingly the "key" parameter is the password. - ns, err := checkClient(client, r.PostFormValue("key"), subj) + ns, err := checkClient(config, client, r.PostFormValue("key"), subj) if err != nil { if client != "" { log.Errorf(c, "%v", err) diff --git a/dashboard/app/auth.go b/dashboard/app/auth.go index b096ebf38..7492f8ded 100644 --- a/dashboard/app/auth.go +++ b/dashboard/app/auth.go @@ -7,29 +7,27 @@ // The client // The VM that wants to invoke the API: // 1) Gets a token from the metainfo server with this http request: -// curl -sH 'Metadata-Flavor: Google' 'http://metadata.google.internal/computeMetadata/v1/instance/service-accounts/default/identity?audience=https://syzkaller.appspot.com/api' +// META=http://metadata.google.internal/computeMetadata/v1 +// AUD=https://syzkaller.appspot.com/api +// curl -sH 'Metadata-Flavor: Google' \ +// "$META/instance/service-accounts/default/identity?audience=$AUD" // 2) Invokes /api with header 'Authorization: Bearer ' - -// Maybe we can use -// https://pkg.go.dev/golang.org/x/oauth2/google - +// // The AppEngine api server: // 1) Receive the token, invokes this http request: // curl -s "https://oauth2.googleapis.com/tokeninfo?id_token=" // 2) Checks the resulting JSON having the expected audience and expiration. // 3) Looks up the permissions in the config using the value of sub. // -// https://cloud.google.com/iap/docs/signed-headers-howto#retrieving_the_user_identity from the IAP docs agrees to trust sub. - -// TODO: private key caching and local verification? -// +// https://cloud.google.com/iap/docs/signed-headers-howto#retrieving_the_user_identity +// from the IAP docs agrees to trust sub. package main import ( "encoding/json" - "io/ioutil" "fmt" + "io/ioutil" "net/http" "net/url" "strconv" @@ -58,7 +56,7 @@ func mkAuthEndpoint(u string) authEndpoint { return authEndpoint{url: u} } -// The JSON representaion of JWT claims. +// The JSON representation of JWT claims. type jwtClaimsParse struct { Subject string `json:"sub"` Audience string `json:"aud"` @@ -116,11 +114,11 @@ func (auth *authEndpoint) determineAuthSubj(authHeader []string) (string, error) return "", err } if claims.Audience != dashapi.DashboardAudience { - err := fmt.Errorf("Unexpected audience %v %v", claims.Audience, claims) + err := fmt.Errorf("unexpected audience %v %v", claims.Audience, claims) return "", err } if claims.Expiration.Before(time.Now()) { - err := fmt.Errorf("Token past expiration %v", claims.Expiration) + err := fmt.Errorf("token past expiration %v", claims.Expiration) return "", err } return oauthMagic + claims.Subject, nil @@ -128,9 +126,9 @@ func (auth *authEndpoint) determineAuthSubj(authHeader []string) (string, error) // Verifies that the given credentials are acceptable and returns the // corresponding namespace. -func checkClient(name0, secretPassword, oauthSubject string) (string, error) { +func checkClient(conf *GlobalConfig, name0, secretPassword, oauthSubject string) (string, error) { checkAuth := func(ns, a string) (string, error) { - if strings.HasPrefix(oauthMagic, a) && a == oauthSubject { + if strings.HasPrefix(a, oauthMagic) && a == oauthSubject { return ns, nil } if a != secretPassword { @@ -138,12 +136,12 @@ func checkClient(name0, secretPassword, oauthSubject string) (string, error) { } return ns, nil } - for name, authenticator := range config.Clients { + for name, authenticator := range conf.Clients { if name == name0 { return checkAuth("", authenticator) } } - for ns, cfg := range config.Namespaces { + for ns, cfg := range conf.Namespaces { for name, authenticator := range cfg.Clients { if name == name0 { return checkAuth(ns, authenticator) diff --git a/dashboard/app/auth_test.go b/dashboard/app/auth_test.go index 02d7ad9fc..b8457449f 100644 --- a/dashboard/app/auth_test.go +++ b/dashboard/app/auth_test.go @@ -1,3 +1,6 @@ +// Copyright 2017 syzkaller project authors. All rights reserved. +// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. + package main import ( @@ -54,7 +57,7 @@ func TestBearerWrongAudience(t *testing.T) { defer ts.Close() _, err := dut.determineAuthSubj([]string{"Bearer x"}) - if !strings.HasPrefix(err.Error(), "Unexpected audience") { + if !strings.HasPrefix(err.Error(), "unexpected audience") { t.Fatalf("Unexpected error %v", err) } } @@ -68,7 +71,7 @@ func TestBearerExpired(t *testing.T) { defer ts.Close() _, err := dut.determineAuthSubj([]string{"Bearer x"}) - if !strings.HasPrefix(err.Error(), "Token past expiration") { + if !strings.HasPrefix(err.Error(), "token past expiration") { t.Fatalf("Unexpected error %v", err) } } @@ -90,3 +93,60 @@ func TestBadHeader(t *testing.T) { t.Errorf("Unexpected error %v %v", got, err) } } + +func TestClientSecretOK(t *testing.T) { + got, err := checkClient(&GlobalConfig{ + Clients: map[string]string{ + "user": "secr1t", + }, + }, "user", "secr1t", "") + if err != nil || got != "" { + t.Errorf("Unexpected error %v %v", got, err) + } +} + +func TestClientOauthOK(t *testing.T) { + got, err := checkClient(&GlobalConfig{ + Clients: map[string]string{ + "user": "OauthSubject:public", + }, + }, "user", "", "OauthSubject:public") + if err != nil || got != "" { + t.Errorf("Unexpected error %v %v", got, err) + } +} + +func TestClientSecretFail(t *testing.T) { + got, err := checkClient(&GlobalConfig{ + Clients: map[string]string{ + "user": "secr1t", + }, + }, "user", "wrong", "") + if err != ErrAccess || got != "" { + t.Errorf("Unexpected error %v %v", got, err) + } +} + +func TestClientSecretMissing(t *testing.T) { + got, err := checkClient(&GlobalConfig{ + Clients: map[string]string{}, + }, "user", "ignored", "") + if err != ErrAccess || got != "" { + t.Errorf("Unexpected error %v %v", got, err) + } +} + +func TestClientNamespaceOK(t *testing.T) { + got, err := checkClient(&GlobalConfig{ + Namespaces: map[string]*Config{ + "ns1": { + Clients: map[string]string{ + "user": "secr1t", + }, + }, + }, + }, "user", "secr1t", "") + if err != nil || got != "ns1" { + t.Errorf("Unexpected error %v %v", got, err) + } +} -- cgit mrf-deployment