Enable key chaining to protect super/sub namespaces

config/config.go

@@ -440,6 +440,7 @@ func InitServer(sType ServerType) error {
 	viper.SetDefault("OIDC.ClientIDFile", filepath.Join(configDir, "oidc-client-id"))
 	viper.SetDefault("OIDC.ClientSecretFile", filepath.Join(configDir, "oidc-client-secret"))
 	viper.SetDefault("Cache.ExportLocation", "/")
+	viper.SetDefault("Registry.RequireKeyChaining", true)
 	if IsRootExecution() {
 		viper.SetDefault("Xrootd.RunLocation", filepath.Join("/run", "pelican", "xrootd", xrootdPrefix))
 		viper.SetDefault("Cache.DataLocation", "/run/pelican/xcache")

docs/parameters.yaml

@@ -466,6 +466,15 @@ root_default: /var/lib/pelican/registry.sqlite
 default: $ConfigBase/ns-registry.sqlite
 components: ["nsregistry"]
 ---
+name: Registry.RequireKeyChaining
+description: >-
+  Specifies whether namespaces requesting registration must possess a key matching any already-registered super/sub namespaces. For
+  example, if true and a namespace `/foo/bar` is already registered, then registration of `/foo` or `/foo/bar/baz` can only be done
+  using keys registered to `/foo/bar`.
+type: bool
+default: true
+components: ["nsregistry"]
+---
 
 ############################
 #   Server-level configs   #

namespace_registry/client_commands_test.go

@@ -32,11 +32,9 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestServeNamespaceRegistry(t *testing.T) {
+func registryMockup(t *testing.T) *httptest.Server {
 	issuerTempDir := t.TempDir()
 
-	viper.Reset()
-	config.InitConfig()
 	ikey := filepath.Join(issuerTempDir, "issuer.jwk")
 	viper.Set("IssuerKey", ikey)
 	viper.Set("Registry.DbLocation", filepath.Join(issuerTempDir, "test.sql"))
@@ -45,29 +43,36 @@ func TestServeNamespaceRegistry(t *testing.T) {
 
 	err = InitializeDB()
 	require.NoError(t, err)
-	defer ShutdownDB()
 
 	gin.SetMode(gin.TestMode)
 	engine := gin.Default()
 
-	_, err = config.GetIssuerPublicJWKS()
-	require.NoError(t, err)
-	privKey, err := config.GetIssuerPrivateJWK()
-	require.NoError(t, err)
-
 	//Configure registry
 	RegisterNamespaceRegistry(engine.Group("/"))
 
 	//Set up a server to use for testing
 	svr := httptest.NewServer(engine)
-	defer svr.CloseClientConnections()
-	defer svr.Close()
-
 	viper.Set("Federation.NamespaceUrl", svr.URL)
-	viper.Set("Origin.NamespacePrefix", "/test")
+	return svr
+}
+
+func TestServeNamespaceRegistry(t *testing.T) {
+	viper.Reset()
+
+	svr := registryMockup(t)
+	defer func() {
+		ShutdownDB()
+		svr.CloseClientConnections()
+		svr.Close()
+	}()
+
+	_, err := config.GetIssuerPublicJWKS()
+	require.NoError(t, err)
+	privKey, err := config.GetIssuerPrivateJWK()
+	require.NoError(t, err)
 
 	//Test functionality of registering a namespace (without identity)
-	err = NamespaceRegister(privKey, svr.URL+"/api/v1.0/registry", "", "/test")
+	err = NamespaceRegister(privKey, svr.URL+"/api/v1.0/registry", "", "/foo/bar")
 	require.NoError(t, err)
 
 	//Test we can list the namespace without an error
@@ -87,7 +92,7 @@ func TestServeNamespaceRegistry(t *testing.T) {
 		capturedOutput := make([]byte, 1024)
 		n, _ := r.Read(capturedOutput)
 		stdoutCapture = string(capturedOutput[:n])
-		assert.Contains(t, stdoutCapture, `"Prefix":"/test"`)
+		assert.Contains(t, stdoutCapture, `"Prefix":"/foo/bar"`)
 	})
 
 	//Test functionality of namespace get
@@ -106,12 +111,12 @@ func TestServeNamespaceRegistry(t *testing.T) {
 		capturedOutput := make([]byte, 1024)
 		n, _ := r.Read(capturedOutput)
 		stdoutCapture = string(capturedOutput[:n])
-		assert.Contains(t, stdoutCapture, `"Prefix":"/test"`)
+		assert.Contains(t, stdoutCapture, `"Prefix":"/foo/bar"`)
 	})
 
 	t.Run("Test namespace delete", func(t *testing.T) {
 		//Test functionality of namespace delete
-		err = NamespaceDelete(svr.URL+"/api/v1.0/registry/test", "/test")
+		err = NamespaceDelete(svr.URL+"/api/v1.0/registry/foo/bar", "/foo/bar")
 		require.NoError(t, err)
 		var stdoutCapture string
 		oldStdout := os.Stdout
@@ -129,3 +134,57 @@ func TestServeNamespaceRegistry(t *testing.T) {
 	})
 	viper.Reset()
 }
+
+func TestRegistryKeyChaining(t *testing.T) {
+	viper.Reset()
+	// On by default, but just to make things explicit
+	viper.Set("Registry.RequireKeyChaining", true)
+	svr := registryMockup(t)
+	defer func() {
+		ShutdownDB()
+		svr.CloseClientConnections()
+		svr.Close()
+	}()
+
+	_, err := config.GetIssuerPublicJWKS()
+	require.NoError(t, err)
+	privKey, err := config.GetIssuerPrivateJWK()
+	require.NoError(t, err)
+
+	//Test we register /foo/bar with the default key
+	err = NamespaceRegister(privKey, svr.URL+"/api/v1.0/registry", "", "/foo/bar")
+	require.NoError(t, err)
+
+	// Perform one test with a subspace and the same key -- should succeed
+	err = NamespaceRegister(privKey, svr.URL+"/api/v1.0/registry", "", "/foo/bar/test")
+	require.NoError(t, err)
+
+	// Now we create a new key and try to use it to register a super/sub space. These shouldn't succeed
+	viper.Set("IssuerKey", t.TempDir()+"/keychaining")
+	_, err = config.GetIssuerPublicJWKS()
+	require.NoError(t, err)
+	privKey, err = config.GetIssuerPrivateJWK()
+	require.NoError(t, err)
+
+	err = NamespaceRegister(privKey, svr.URL+"/api/v1.0/registry", "", "/foo/bar/baz")
+	require.ErrorContains(t, err, "Cannot register a namespace that is suffixed or prefixed")
+
+	err = NamespaceRegister(privKey, svr.URL+"/api/v1.0/registry", "", "/foo")
+	require.ErrorContains(t, err, "Cannot register a namespace that is suffixed or prefixed")
+
+	// Make sure we can register things similar but distinct in prefix and suffix
+	err = NamespaceRegister(privKey, svr.URL+"/api/v1.0/registry", "", "/fo")
+	require.NoError(t, err)
+	err = NamespaceRegister(privKey, svr.URL+"/api/v1.0/registry", "", "/foo/barz")
+	require.NoError(t, err)
+
+	// Now turn off token chaining and retry -- no errors should occur
+	viper.Set("Registry.RequireKeyChaining", false)
+	err = NamespaceRegister(privKey, svr.URL+"/api/v1.0/registry", "", "/foo/bar/baz")
+	require.NoError(t, err)
+
+	err = NamespaceRegister(privKey, svr.URL+"/api/v1.0/registry", "", "/foo")
+	require.NoError(t, err)
+
+	viper.Reset()
+}

namespace_registry/registry.go

@@ -96,6 +96,42 @@ type registrationData struct {
 	Prefix           string          `json:"prefix"`
 }
 
+func matchKeys(incomingKey jwk.Key, registeredNamespaces []string) (bool, error) {
+	// If this is the case, we want to make sure that at least one of the superspaces has the
+	// same registration key as the incoming. This guarantees the owner of the superspace is
+	// permitting the action (assuming their keys haven't been stolen!)
+	foundMatch := false
+	for _, ns := range registeredNamespaces {
+		keyset, err := dbGetPrefixJwks(ns)
+		if err != nil {
+			return false, errors.Wrapf(err, "Cannot get keyset for %s from the database", ns)
+		}
+
+		// A super inelegant way to compare keys, but for whatever reason the keyset.Index(key) method
+		// doesn't seem to actually recognize when a key is in the keyset, even if that key decodes to
+		// the exact same JSON as a key in the set...
+		for it := (*keyset).Keys(context.Background()); it.Next(context.Background()); {
+			pair := it.Pair()
+			registeredKey := pair.Value.(jwk.Key)
+			registeredKeyBuf, err := json.Marshal(registeredKey)
+			if err != nil {
+				return false, errors.Wrapf(err, "failed to marshal a key registered to %s into JSON", ns)
+			}
+			incomingKeyBuf, err := json.Marshal(incomingKey)
+			if err != nil {
+				return false, errors.Wrap(err, "failed to marshal the incoming key into JSON")
+			}
+
+			if string(registeredKeyBuf) == string(incomingKeyBuf) {
+				foundMatch = true
+				break
+			}
+		}
+	}
+
+	return foundMatch, nil
+}
+
 func keySignChallenge(ctx *gin.Context, data *registrationData, action string) error {
 	if data.ClientNonce != "" && data.ClientPayload != "" && data.ClientSignature != "" &&
 		data.ServerNonce != "" && data.ServerPayload != "" && data.ServerSignature != "" {
@@ -258,6 +294,51 @@ func keySignChallengeCommit(ctx *gin.Context, data *registrationData, action str
 				return nil
 			}
 
+			if param.Registry_RequireKeyChaining.GetBool() {
+				superspaces, subspaces, err := namespaceSupSubChecks(data.Prefix)
+				if err != nil {
+					log.Errorf("Failed to check if namespace suffixes or prefixes another registered namespace: %v", err)
+					return errors.Wrap(err, "Server encountered an error checking if namespace already exists")
+				}
+
+				// If we make the assumption that namespace prefixes are heirarchical, eg that the owner of /foo should own
+				// everything under /foo (/foo/bar, /foo/baz, etc), then it makes sense to check for superspaces first. If any
+				// superspace is found, they logically "own" the incoming namespace.
+				if len(superspaces) > 0 {
+					// If this is the case, we want to make sure that at least one of the superspaces has the
+					// same registration key as the incoming. This guarantees the owner of the superspace is
+					// permitting the action (assuming their keys haven't been stolen!)
+					matched, err := matchKeys(key, superspaces)
+					if err != nil {
+						ctx.JSON(500, gin.H{"error": "Server encountered an error checking for key matches in the database"})
+						return errors.Errorf("%v: Unable to check if the incoming key for %s matched any public keys for %s", err, data.Prefix, subspaces)
+					}
+					if !matched {
+						_ = ctx.AbortWithError(403, errors.New("Cannot register a namespace that is suffixed or prefixed by an already-registered namespace unless the incoming public key matches a registered key"))
+						return errors.New("Cannot register a namespace that is suffixed or prefixed by an already-registered namespace unless the incoming public key matches a registered key")
+					}
+
+				} else if len(subspaces) > 0 {
+					// If there are no superspaces, we can check the subspaces.
+
+					// TODO: Eventually we might want to check only the highest level subspaces and use those keys for matching. For example,
+					// if /foo/bar and /foo/bar/baz are registered with two keysets such that the complement of their intersections is not null,
+					// it may be the case that the only key we match against belongs to /foo/bar/baz. If we go ahead with registration at that
+					// point, we're essentially saying /foo/bar/baz, the logical subspace of /foo/bar, has authorized a superspace for both.
+					// More interestingly, if /foo/bar and /foo/baz are both registered, should they both be consulted before adding /foo?
+
+					// For now, we'll just check for any key match.
+					matched, err := matchKeys(key, subspaces)
+					if err != nil {
+						ctx.JSON(500, gin.H{"error": "Server encountered an error checking for key matches in the database"})
+						return errors.Errorf("%v: Unable to check if the incoming key for %s matched any public keys for %s", err, data.Prefix, subspaces)
+					}
+					if !matched {
+						_ = ctx.AbortWithError(403, errors.New("Cannot register a namespace that is suffixed or prefixed by an already-registered namespace unless the incoming public key matches a registered key"))
+						return errors.New("Cannot register a namespace that is suffixed or prefixed by an already-registered namespace unless the incoming public key matches a registered key")
+					}
+				}
+			}
 			reqPrefix, err := validateNSPath(data.Prefix)
 			if err != nil {
 				err = errors.Wrapf(err, "Requested namespace %s failed validation", reqPrefix)
@@ -367,17 +448,17 @@ func cliRegisterNamespace(ctx *gin.Context) {
 		reqData.Identity = string(body)
 		err = keySignChallenge(ctx, &reqData, "register")
 		if err != nil {
-			ctx.JSON(http.StatusInternalServerError, gin.H{"error": "server encountered an error during key-sign challenge" + err.Error()})
-			log.Errorf("Failed to complete key sign challenge with identity requirement: %v", err)
+			ctx.JSON(http.StatusInternalServerError, gin.H{"error": "server encountered an error during key-sign challenge: " + err.Error()})
+			log.Warningf("Failed to complete key sign challenge with identity requirement: %v", err)
 		}
 		return
 	}
 
 	if reqData.IdentityRequired == "false" || reqData.IdentityRequired == "" {
 		err := keySignChallenge(ctx, &reqData, "register")
 		if err != nil {
-			ctx.JSON(http.StatusInternalServerError, gin.H{"error": "server encountered an error during key-sign challenge " + err.Error()})
-			log.Errorf("Failed to complete key sign challenge without identity requirement: %v", err)
+			ctx.JSON(http.StatusInternalServerError, gin.H{"error": "server encountered an error during key-sign challenge: " + err.Error()})
+			log.Warningf("Failed to complete key sign challenge without identity requirement: %v", err)
 		}
 		return
 	}

namespace_registry/registry_db.go

@@ -83,6 +83,44 @@ func namespaceExists(prefix string) (bool, error) {
 	return found, nil
 }
 
+func namespaceSupSubChecks(prefix string) (superspaces []string, subspaces []string, err error) {
+	// Check if any registered namespaces already superspace the incoming namespace,
+	// eg if /foo is already registered, this will be true for an incoming /foo/bar because
+	// /foo is logically above /foo/bar (according to my logic, anyway)
+	superspaceQuery := `SELECT prefix FROM namespace WHERE (? || '/') LIKE (prefix || '/%')`
+	superspaceResults, err := db.Query(superspaceQuery, prefix)
+	if err != nil {
+		return
+	}
+	defer superspaceResults.Close()
+
+	for superspaceResults.Next() {
+		var foundSuperspace string
+		if err := superspaceResults.Scan(&foundSuperspace); err == nil {
+			superspaces = append(superspaces, foundSuperspace)
+		}
+	}
+
+	// Check if any registered namespaces already subspace the incoming namespace,
+	// eg if /foo/bar is already registered, this will be true for an incoming /foo because
+	// /foo/bar is logically below /foo
+	subspaceQuery := `SELECT prefix FROM namespace WHERE (prefix || '/') LIKE (? || '/%')`
+	subspaceResults, err := db.Query(subspaceQuery, prefix)
+	if err != nil {
+		return
+	}
+	defer subspaceResults.Close()
+
+	for subspaceResults.Next() {
+		var foundSubspace string
+		if err := subspaceResults.Scan(&foundSubspace); err == nil {
+			subspaces = append(subspaces, foundSubspace)
+		}
+	}
+
+	return
+}
+
 func dbGetPrefixJwks(prefix string) (*jwk.Set, error) {
 	jwksQuery := `SELECT pubkey FROM namespace WHERE prefix = ?`
 	var pubkeyStr string