From a790e6adac80175e10e70b4622331e889055408c Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Thu, 7 Sep 2023 21:27:29 +0000 Subject: [PATCH 1/5] Send token and namespace ad to director --- director/origin_api.go | 17 +++++++++++------ origin_ui/advertise.go | 30 +++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/director/origin_api.go b/director/origin_api.go index e30d1c0ee..a30e26ae7 100644 --- a/director/origin_api.go +++ b/director/origin_api.go @@ -89,17 +89,22 @@ func VerifyAdvertiseToken(token, namespace string) (bool, error) { return false, err } var ar *jwk.Cache - { + + // defer statements are scoped to function, not lexical enclosure, + // which is why we wrap these defer statements in anon funcs + func () { namespaceKeysMutex.RLock() - defer namespaceKeysMutex.Unlock() + defer namespaceKeysMutex.RUnlock() item := namespaceKeys.Get(namespace) - if !item.IsExpired() { - ar = item.Value() + if item != nil { + if !item.IsExpired() { + ar = item.Value() + } } - } + }() ctx := context.Background() if ar == nil { - ar := jwk.NewCache(ctx) + ar = jwk.NewCache(ctx) if err = ar.Register(issuer_url, jwk.WithMinRefreshInterval(15*time.Minute)); err != nil { return false, err } diff --git a/origin_ui/advertise.go b/origin_ui/advertise.go index 25fc4c322..5c636adb1 100644 --- a/origin_ui/advertise.go +++ b/origin_ui/advertise.go @@ -54,16 +54,40 @@ func PeriodicAdvertiseOrigin() error { func AdvertiseOrigin() error { name := viper.GetString("Sitename") + + fmt.Printf("\n\n\nORIGIN SITENAME: %s\n\n\n", name) if name == "" { return errors.New("Origin name isn't set") } // TODO: waiting on a different branch to merge origin URL generation originUrl := "https://localhost:8444" + // Here we instantiate the namespaceAd slice, but we still need to define the namespace + namespaceUrl, err := url.Parse(viper.GetString("NamespaceUrl")) + if err != nil { + return errors.New("Bad namespaceUrl") + } + if namespaceUrl.String() == "" { + return errors.New("No NamespaceUrl is set") + } + + prefix := viper.GetString("NamespacePrefix") + + // TODO: Need to figure out where to get some of these values + // so that they aren't hardcoded... + nsAd := director.NamespaceAd{ + RequireToken: false, + Path: prefix, + Issuer: *namespaceUrl, + MaxScopeDepth: 3, + Strategy: "OAuth2", + BasePath: "/", + } ad := director.OriginAdvertise{ Name: name, URL: originUrl, - Namespaces: make([]director.NamespaceAd, 0), + // Namespaces: make([]director.NamespaceAd, 0), + Namespaces: []director.NamespaceAd{nsAd}, } body, err := json.Marshal(ad) @@ -81,12 +105,16 @@ func AdvertiseOrigin() error { } directorUrl.Path = "/api/v1.0/director/registerOrigin" + token, err := director.CreateAdvertiseToken(prefix) + log.Debugln("Signed advertise token:", token) + req, err := http.NewRequest("POST", directorUrl.String(), bytes.NewBuffer(body)) if err != nil { return errors.Wrap(err, "Failed to create POST request for director registration") } req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer " + token) client := http.Client{} if viper.GetBool("TLSSkipVerify") { From 6f6e8bf03bef877585ac1dc7a75a0fc3c0b2530d Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Thu, 7 Sep 2023 21:36:57 +0000 Subject: [PATCH 2/5] Remove print statement --- origin_ui/advertise.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/origin_ui/advertise.go b/origin_ui/advertise.go index 5c636adb1..b9fb5e2c0 100644 --- a/origin_ui/advertise.go +++ b/origin_ui/advertise.go @@ -54,8 +54,6 @@ func PeriodicAdvertiseOrigin() error { func AdvertiseOrigin() error { name := viper.GetString("Sitename") - - fmt.Printf("\n\n\nORIGIN SITENAME: %s\n\n\n", name) if name == "" { return errors.New("Origin name isn't set") } From 7bf66fd9e74828d27d62451b4652db13afd2be6b Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Fri, 8 Sep 2023 15:02:47 +0000 Subject: [PATCH 3/5] Fix token verification With this commit, almost all of the pieces for an origin to advertise directly to the namespace registry are in place. A few final things should be tidied up, but the basic functionality now exists. Before we can call this "done for now", we need to look over line 60/61 of `origin_ui/advertise.go`. There's an important TODO there that it looks like someone else has sitting in a branch. Cleanup linter errors Linter's stern decree, Merge awaits its nod, unsure, PR in limbo --- director/origin_api.go | 59 +++++++++++++++++++----- director/redirect.go | 4 +- namespace-registry/registry-db.go | 2 +- namespace-registry/registry.go | 74 ++++++++++++++++++------------- origin_ui/advertise.go | 10 +++-- 5 files changed, 101 insertions(+), 48 deletions(-) diff --git a/director/origin_api.go b/director/origin_api.go index a30e26ae7..6213c9ea0 100644 --- a/director/origin_api.go +++ b/director/origin_api.go @@ -20,7 +20,9 @@ package director import ( "context" - "errors" + "crypto/tls" + "encoding/json" + "net/http" "net/url" "path" "strings" @@ -32,6 +34,8 @@ import ( "github.com/lestrrat-go/jwx/v2/jwk" "github.com/lestrrat-go/jwx/v2/jwt" "github.com/pelicanplatform/pelican/config" + "github.com/pkg/errors" + log "github.com/sirupsen/logrus" "github.com/spf13/viper" ) @@ -49,11 +53,10 @@ var ( ) func CreateAdvertiseToken(namespace string) (string, error) { - key, err := config.GetOriginJWK() - if err != nil { - return "", err - } - issuer_url, err := GetIssuerURL(namespace) + // TODO: Need to come back and carefully consider a few naming practices. + // Here, issuerUrl is actually the registry database url, and not + // the token issuer url for this namespace + issuerUrl, err := GetIssuerURL(namespace) if err != nil { return "", err } @@ -64,7 +67,7 @@ func CreateAdvertiseToken(namespace string) (string, error) { tok, err := jwt.NewBuilder(). Claim("scope", "pelican.advertise"). - Issuer(issuer_url). + Issuer(issuerUrl). Audience([]string{director}). Subject("origin"). Expiration(time.Now().Add(time.Minute)). @@ -73,7 +76,20 @@ func CreateAdvertiseToken(namespace string) (string, error) { return "", err } - signed, err := jwt.Sign(tok, jwt.WithKey(jwa.ES512, key)) + key, err := config.GetOriginJWK() + if err != nil { + return "", errors.Wrap(err, "failed to load the origin's JWK") + } + + // Get/assign the kid, needed for verification of the token by the director + // TODO: Create more generic "tokenCreate" functions so we don't have to do + // this by hand all the time + err = jwk.AssignKeyID(*key) + if err != nil { + return "", errors.Wrap(err, "Failed to assign kid to the token") + } + + signed, err := jwt.Sign(tok, jwt.WithKey(jwa.ES512, *key)) if err != nil { return "", err } @@ -84,7 +100,7 @@ func CreateAdvertiseToken(namespace string) (string, error) { // see if the entity is authorized to advertise an origin for the // namespace func VerifyAdvertiseToken(token, namespace string) (bool, error) { - issuer_url, err := GetIssuerURL(namespace) + issuerUrl, err := GetIssuerURL(namespace) if err != nil { return false, err } @@ -105,14 +121,33 @@ func VerifyAdvertiseToken(token, namespace string) (bool, error) { ctx := context.Background() if ar == nil { ar = jwk.NewCache(ctx) - if err = ar.Register(issuer_url, jwk.WithMinRefreshInterval(15*time.Minute)); err != nil { + // This should be switched to use the common transport, but that must first be exported + client := &http.Client{} + if viper.GetBool("TLSSkipVerify") { + tr := &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + } + client = &http.Client{Transport: tr} + } + if err = ar.Register(issuerUrl, jwk.WithMinRefreshInterval(15*time.Minute), jwk.WithHTTPClient(client)); err != nil { return false, err } namespaceKeysMutex.Lock() defer namespaceKeysMutex.Unlock() namespaceKeys.Set(namespace, ar, ttlcache.DefaultTTL) } - keyset, err := ar.Get(ctx, issuer_url) + log.Debugln("Attempting to fetch keys from ", issuerUrl) + keyset, err := ar.Get(ctx, issuerUrl) + + if log.IsLevelEnabled(log.DebugLevel) { + // Let's check that we can convert to JSON and get the right thing... + jsonbuf, err := json.Marshal(keyset) + if err != nil { + return false, errors.Wrap(err, "failed to marshal the public keyset into JWKS JSON") + } + log.Debugln("Constructed JWKS from fetching jwks:", string(jsonbuf)) + } + if err != nil { return false, err } @@ -150,6 +185,6 @@ func GetIssuerURL(prefix string) (string, error) { if err != nil { return "", err } - namespace_url.Path = path.Join(namespace_url.Path, "namespaces", prefix) + namespace_url.Path = path.Join(namespace_url.Path, "api", "v1.0", "registry", prefix, ".well-known", "issuer.jwks") return namespace_url.String(), nil } diff --git a/director/redirect.go b/director/redirect.go index 61635caea..db2cf1f50 100644 --- a/director/redirect.go +++ b/director/redirect.go @@ -229,7 +229,9 @@ func RegisterOrigin(ctx *gin.Context) { } for _, namespace := range ad.Namespaces { - ok, err := VerifyAdvertiseToken(tokens[0], namespace.Path) + // We're assuming there's only one token in the slice + token := strings.TrimPrefix(tokens[0], "Bearer ") + ok, err := VerifyAdvertiseToken(token, namespace.Path) if err != nil { log.Warningln("Failed to verify token:", err) ctx.JSON(400, gin.H{"error": "Authorization token verification failed"}) diff --git a/namespace-registry/registry-db.go b/namespace-registry/registry-db.go index 2b8f31b55..091e9d780 100644 --- a/namespace-registry/registry-db.go +++ b/namespace-registry/registry-db.go @@ -82,7 +82,7 @@ func namespaceExists(prefix string) (bool, error) { return found, nil } -func getPrefixJwks(prefix string) (*jwk.Set, error) { +func dbGetPrefixJwks(prefix string) (*jwk.Set, error) { jwksQuery := `SELECT pubkey FROM namespace WHERE prefix = ?` var pubkeyStr string err := db.QueryRow(jwksQuery, prefix).Scan(&pubkeyStr) diff --git a/namespace-registry/registry.go b/namespace-registry/registry.go index 37b63b28e..7151ee363 100644 --- a/namespace-registry/registry.go +++ b/namespace-registry/registry.go @@ -30,6 +30,7 @@ import ( "net/http" "net/url" "os" + "path/filepath" "strings" "sync" @@ -614,7 +615,7 @@ func dbDeleteNamespace(ctx *gin.Context) { delTokenStr := strings.TrimPrefix(authHeader, "Bearer ") // Have the token, now we need to load the JWKS for the prefix - originJwks, err := getPrefixJwks(prefix) + originJwks, err := dbGetPrefixJwks(prefix) if err != nil { ctx.JSON(http.StatusInternalServerError, gin.H{"error": "server encountered an error loading the prefix's stored jwks"}) log.Errorf("Failed to get prefix's stored jwks: %v", err) @@ -688,46 +689,57 @@ func dbGetAllNamespaces(ctx *gin.Context) { nss, err := getAllNamespaces() if err != nil { ctx.JSON(http.StatusInternalServerError, gin.H{"error": "server encountered an error trying to list all namespaces"}) + log.Errorln("Failed to get all namespaces: ", err) return } ctx.JSON(http.StatusOK, nss) } -// func metadataHandler(ctx *gin.Context) { -// path := ctx.Param("wildcard") - -// // A weird feature of gin is that wildcards always -// // add a preceding /. We need to trim it here... -// path = strings.TrimPrefix(path, "/") -// log.Debug("Working with path ", path) +func metadataHandler(ctx *gin.Context) { + // A weird feature of gin is that wildcards always + // add a preceding /. Since the prefix / was trimmed + // out during the url parsing, we can just leave the + // new / here! + path := ctx.Param("wildcard") + + // Get the prefix's JWKS + if filepath.Base(path) == "issuer.jwks" { + // do something + prefix := strings.TrimSuffix(path, "/.well-known/issuer.jwks") + jwks, err := dbGetPrefixJwks(prefix) + if err != nil { + ctx.JSON(http.StatusInternalServerError, gin.H{"error": "server encountered an error trying to get jwks for prefix"}) + log.Errorf("Failed to load jwks for prefix %s: %v", prefix, err) + return + } + ctx.JSON(http.StatusOK, jwks) + } + + // // Get OpenID config info + // match, err := filepath.Match("*/\\.well-known/openid-configuration", path) + // if err != nil { + // log.Errorf("Failed to check incoming path for match: %v", err) + // return + // } + // if match { + // // do something + // } else { + // log.Errorln("Unknown request") + // return + // } -// // Get JWKS -// if filepath.Base(path) == "issuer.jwks" { -// // do something -// } +} -// // Get OpenID config info -// match, err := filepath.Match("*/\\.well-known/openid-configuration", path) +// func getJwks(prefix string) (*jwk.Set, error) { +// jwks, err := dbGetPrefixJwks(prefix) // if err != nil { -// log.Errorf("Failed to check incoming path for match: %v", err) -// return -// } -// if match { -// // do something -// } else { -// log.Errorln("Unknown request") -// return +// return nil, errors.Wrapf(err, "Could not load jwks for prefix %s", prefix) // } - +// return jwks, nil // } -/** - * Commenting out until we're ready to use it. -BB -func getJwks(c *gin.Context) { - prefix := c.Param("prefix") - c.JSON(http.StatusOK, gin.H{"status": "Get JWKS is not implemented", "prefix": prefix}) -} - +/* + Commenting out until we're ready to use it. -BB func getOpenIDConfiguration(c *gin.Context) { prefix := c.Param("prefix") c.JSON(http.StatusOK, gin.H{"status": "getOpenIDConfiguration is not implemented", "prefix": prefix}) @@ -740,7 +752,7 @@ func RegisterNamespaceRegistry(router *gin.RouterGroup) { registry.POST("", cliRegisterNamespace) registry.GET("", dbGetAllNamespaces) // Will handle getting jwks, openid config, and listing namespaces - // registry.GET("/*wildcard", metadataHandler) + registry.GET("/*wildcard", metadataHandler) registry.DELETE("/*wildcard", dbDeleteNamespace) } diff --git a/origin_ui/advertise.go b/origin_ui/advertise.go index b9fb5e2c0..b72bc5679 100644 --- a/origin_ui/advertise.go +++ b/origin_ui/advertise.go @@ -78,13 +78,12 @@ func AdvertiseOrigin() error { Path: prefix, Issuer: *namespaceUrl, MaxScopeDepth: 3, - Strategy: "OAuth2", + Strategy: "OAuth2", BasePath: "/", } ad := director.OriginAdvertise{ Name: name, URL: originUrl, - // Namespaces: make([]director.NamespaceAd, 0), Namespaces: []director.NamespaceAd{nsAd}, } @@ -104,6 +103,9 @@ func AdvertiseOrigin() error { directorUrl.Path = "/api/v1.0/director/registerOrigin" token, err := director.CreateAdvertiseToken(prefix) + if err != nil { + return errors.Wrap(err, "Failed to generate advertise token") + } log.Debugln("Signed advertise token:", token) req, err := http.NewRequest("POST", directorUrl.String(), bytes.NewBuffer(body)) @@ -112,8 +114,10 @@ func AdvertiseOrigin() error { } req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer " + token) + req.Header.Set("Authorization", "Bearer "+token) + // We should switch this over to use the common transport, but for that to happen + // that function needs to be exported from pelican client := http.Client{} if viper.GetBool("TLSSkipVerify") { tr := &http.Transport{ From 60f627fecb76f7bd35a7e5ac1772b1a1695b0450 Mon Sep 17 00:00:00 2001 From: Justin Hiemstra Date: Fri, 8 Sep 2023 15:25:10 +0000 Subject: [PATCH 4/5] Fix linter errors --- director/origin_api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/director/origin_api.go b/director/origin_api.go index 6213c9ea0..321438495 100644 --- a/director/origin_api.go +++ b/director/origin_api.go @@ -108,7 +108,7 @@ func VerifyAdvertiseToken(token, namespace string) (bool, error) { // defer statements are scoped to function, not lexical enclosure, // which is why we wrap these defer statements in anon funcs - func () { + func() { namespaceKeysMutex.RLock() defer namespaceKeysMutex.RUnlock() item := namespaceKeys.Get(namespace) From 2364c817868425d66ec1924eb2855b48af38fbf7 Mon Sep 17 00:00:00 2001 From: Brian P Bockelman Date: Fri, 8 Sep 2023 16:55:43 -0500 Subject: [PATCH 5/5] Even for stubs, default to `RequireToken`. --- origin_ui/advertise.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/origin_ui/advertise.go b/origin_ui/advertise.go index b72bc5679..d9d02b7ed 100644 --- a/origin_ui/advertise.go +++ b/origin_ui/advertise.go @@ -74,7 +74,7 @@ func AdvertiseOrigin() error { // TODO: Need to figure out where to get some of these values // so that they aren't hardcoded... nsAd := director.NamespaceAd{ - RequireToken: false, + RequireToken: true, Path: prefix, Issuer: *namespaceUrl, MaxScopeDepth: 3,