Productionize HDFS rego rules (#11)

sbernauer · siegfriedweber · web-flow · commit ec4997251ae2 · 2024-03-26T15:08:45.000Z
* pull in new rego rules version

* feat: Implement regex support

* default matches_identity to false

* Usernames must match entire regex (not only parts)

* Change bob regex

* Introduce userRegex and shortUserRegex

* fix bob userRegex

* use backticks

* Add groupRegex

* Update rego/hdfs.rego

Co-authored-by: Siegfried Weber &lt;mail@siegfriedweber.net&gt;

* Add missing shortUser test

* fix test name

* fix(tests): Add test-case for shortUserRegex

* Use the new rules also in test stack

* fix: Let tilt stack work again

---------

Co-authored-by: Siegfried Weber &lt;mail@siegfriedweber.net&gt;
diff --git a/rego/hdfs.rego b/rego/hdfs.rego
@@ -2,7 +2,8 @@ package hdfs
 
 import rego.v1
 
-default allow = false
+default allow := false
+default matches_identity(identity) := false
 
 # HDFS authorizer
 allow if {
@@ -12,21 +13,22 @@ allow if {
     action_sufficient_for_operation(acl.action, input.operationName)
 }
 
-# HDFS group mapper (this returns a list of strings)
-groups := {group |
-    raw = groups_for_user[input.username][_]
-    # Keycloak groups have trailing slashes
-    group := trim_prefix(raw, "/")
+# Identity mentions the (long) userName or shortUsername explicitly
+matches_identity(identity) if {
+    identity in {
+        concat("", ["user:", input.callerUgi.userName]),
+        concat("", ["shortUser:", input.callerUgi.shortUserName])
+    }
 }
 
-# Identity mentions the (long) userName explicitly
+# Identity regex matches the (long) userName
 matches_identity(identity) if {
-    identity == concat("", ["user:", input.callerUgi.userName])
+    match_entire(identity, concat("", ["userRegex:", input.callerUgi.userName]))
 }
 
-# Identity mentions the shortUserName explicitly
+# Identity regex matches the shortUsername
 matches_identity(identity) if {
-    identity == concat("", ["shortUser:", input.callerUgi.shortUserName])
+    match_entire(identity, concat("", ["shortUserRegex:", input.callerUgi.shortUserName]))
 }
 
 # Identity mentions group the user is part of (by looking up using the (long) userName)
@@ -35,6 +37,24 @@ matches_identity(identity) if {
     identity == concat("", ["group:", group])
 }
 
+# Identity regex matches group the user is part of (by looking up using the (long) userName)
+matches_identity(identity) if {
+    some group in groups_for_user[input.callerUgi.userName]
+    match_entire(identity, concat("", ["groupRegex:", group]))
+}
+
+# Identity mentions group the user is part of (by looking up using the shortUserName)
+matches_identity(identity) if {
+    some group in groups_for_short_user_name[input.callerUgi.shortUserName]
+    identity == concat("", ["group:", group])
+}
+
+# Identity regex matches group the user is part of (by looking up using the shortUserName)
+matches_identity(identity) if {
+    some group in groups_for_short_user_name[input.callerUgi.shortUserName]
+    match_entire(identity, concat("", ["groupRegex:", group]))
+}
+
 # Resource mentions the file explicitly
 matches_resource(file, resource) if {
     resource == concat("", ["hdfs:file:", file])
@@ -63,6 +83,13 @@ action_hierarchy := {
     "ro": ["ro"],
 }
 
+match_entire(pattern, value) if {
+	# Add the anchors ^ and $
+	pattern_with_anchors := concat("", ["^", pattern, "$"])
+
+	regex.match(pattern_with_anchors, value)
+}
+
 # To get a (hopefully complete) list of actions run "ack 'String operationName = '" in the hadoop source code
 action_for_operation := {
     # The "rename" operation will be actually called on both - the source and the target location.
@@ -182,14 +209,16 @@ groups_for_user := {
     "bob/test-hdfs-permissions.default.svc.cluster.local@CLUSTER.LOCAL": []
 }
 
+groups_for_short_user_name := {}
+
 acls := [
     {
         "identity": "group:admins",
         "action": "full",
         "resource": "hdfs:dir:/",
     },
     {
-        "identity": "group:developers",
+        "identity": "groupRegex:(developers)",
         "action": "rw",
         "resource": "hdfs:dir:/developers/",
     },
@@ -204,7 +233,7 @@ acls := [
         "resource": "hdfs:dir:/alice/",
     },
     {
-        "identity": "user:bob/test-hdfs-permissions.default.svc.cluster.local@CLUSTER.LOCAL",
+        "identity": `userRegex:bob/.+\.default\.svc\.cluster\.local@CLUSTER\.LOCAL`,
         "action": "rw",
         "resource": "hdfs:dir:/bob/",
     },
@@ -216,11 +245,16 @@ acls := [
     {
         "identity": "user:bob/test-hdfs-permissions.default.svc.cluster.local@CLUSTER.LOCAL",
         "action": "rw",
-        "resource": "hdfs:file:/developers/file-from-bob",
+        "resource": "hdfs:file:/developers/file-from-bob-via-user",
     },
     {
         "identity": "shortUser:bob",
         "action": "rw",
-        "resource": "hdfs:file:/developers/file-from-bob",
+        "resource": "hdfs:file:/developers/file-from-bob-via-short-user",
+    },
+    {
+        "identity": "shortUserRegex:(bob|bobby)",
+        "action": "rw",
+        "resource": "hdfs:file:/developers/file-from-bob-via-short-user-regex",
     },
 ]
diff --git a/rego/hdfs_test.rego b/rego/hdfs_test.rego
@@ -141,13 +141,35 @@ test_bob_no_rw_access_to_developers if {
     }
 }
 
-test_bob_rw_access_to_developers_special_file if {
+test_bob_rw_access_to_developers_special_file_via_user if {
     allow with input as {
         "callerUgi": {
             "shortUserName": "bob",
             "userName": "bob/test-hdfs-permissions.default.svc.cluster.local@CLUSTER.LOCAL",
         },
-        "path": "/developers/file-from-bob",
+        "path": "/developers/file-from-bob-via-user",
+        "operationName": "create",
+    }
+}
+
+test_bob_rw_access_to_developers_special_file_via_short_user if {
+    allow with input as {
+        "callerUgi": {
+            "shortUserName": "bob",
+            "userName": "bob/test-hdfs-permissions.default.svc.cluster.local@CLUSTER.LOCAL",
+        },
+        "path": "/developers/file-from-bob-via-short-user",
+        "operationName": "create",
+    }
+}
+
+test_bob_rw_access_to_developers_special_file_via_short_user_regex if {
+    allow with input as {
+        "callerUgi": {
+            "shortUserName": "bob",
+            "userName": "bob/test-hdfs-permissions.default.svc.cluster.local@CLUSTER.LOCAL",
+        },
+        "path": "/developers/file-from-bob-via-short-user-regex",
         "operationName": "create",
     }
 }
diff --git a/test/stack/11-rego-rules.yaml b/test/stack/11-rego-rules.yaml
@@ -11,34 +11,59 @@ data:
 
     import rego.v1
 
-    default allow = false
+    default allow := false
+    default matches_identity(identity) := false
 
     # HDFS authorizer
     allow if {
         some acl in acls
-        matches_identity(input.callerUgi.shortUserName, acl.identity)
+        matches_identity(acl.identity)
         matches_resource(input.path, acl.resource)
         action_sufficient_for_operation(acl.action, input.operationName)
     }
 
-    # HDFS group mapper (this returns a list of strings)
-    groups := {group |
-        raw = groups_for_user[input.username][_]
-        # Keycloak groups have trailing slashes
-        group := trim_prefix(raw, "/")
+    # Identity mentions the (long) userName or shortUsername explicitly
+    matches_identity(identity) if {
+        identity in {
+            concat("", ["user:", input.callerUgi.userName]),
+            concat("", ["shortUser:", input.callerUgi.shortUserName])
+        }
     }
 
-    # Identity mentions the user explicitly
-    matches_identity(user, identity) if {
-        identity == concat("", ["user:", user])
+    # Identity regex matches the (long) userName
+    matches_identity(identity) if {
+        match_entire(identity, concat("", ["userRegex:", input.callerUgi.userName]))
     }
 
-    # Identity mentions group the user is part of
-    matches_identity(user, identity) if {
-        some group in groups_for_user[user]
+    # Identity regex matches the shortUsername
+    matches_identity(identity) if {
+        match_entire(identity, concat("", ["shortUserRegex:", input.callerUgi.shortUserName]))
+    }
+
+    # Identity mentions group the user is part of (by looking up using the (long) userName)
+    matches_identity(identity) if {
+        some group in groups_for_user[input.callerUgi.userName]
+        identity == concat("", ["group:", group])
+    }
+
+    # Identity regex matches group the user is part of (by looking up using the (long) userName)
+    matches_identity(identity) if {
+        some group in groups_for_user[input.callerUgi.userName]
+        match_entire(identity, concat("", ["groupRegex:", group]))
+    }
+
+    # Identity mentions group the user is part of (by looking up using the shortUserName)
+    matches_identity(identity) if {
+        some group in groups_for_short_user_name[input.callerUgi.shortUserName]
         identity == concat("", ["group:", group])
     }
 
+    # Identity regex matches group the user is part of (by looking up using the shortUserName)
+    matches_identity(identity) if {
+        some group in groups_for_short_user_name[input.callerUgi.shortUserName]
+        match_entire(identity, concat("", ["groupRegex:", group]))
+    }
+
     # Resource mentions the file explicitly
     matches_resource(file, resource) if {
         resource == concat("", ["hdfs:file:", file])
@@ -67,6 +92,13 @@ data:
         "ro": ["ro"],
     }
 
+    match_entire(pattern, value) if {
+        # Add the anchors ^ and $
+        pattern_with_anchors := concat("", ["^", pattern, "$"])
+
+        regex.match(pattern_with_anchors, value)
+    }
+
     # To get a (hopefully complete) list of actions run "ack 'String operationName = '" in the hadoop source code
     action_for_operation := {
         # The "rename" operation will be actually called on both - the source and the target location.
@@ -180,7 +212,8 @@ data:
         "transitionToStandby": "full",
     }
 
-    groups_for_user := {"admin": ["admins"], "alice": ["developers"], "bob": []}
+    groups_for_user := {}
+    groups_for_short_user_name := {"admin": ["admins"], "alice": ["developers"], "bob": []}
 
     acls := [
         {
@@ -199,22 +232,22 @@ data:
             "resource": "hdfs:dir:/developers-ro/",
         },
         {
-            "identity": "user:alice",
+            "identity": "shortUser:alice",
             "action": "rw",
             "resource": "hdfs:dir:/alice/",
         },
         {
-            "identity": "user:bob",
+            "identity": "shortUser:bob",
             "action": "rw",
             "resource": "hdfs:dir:/bob/",
         },
         {
-            "identity": "user:bob",
+            "identity": "shortUser:bob",
             "action": "ro",
             "resource": "hdfs:dir:/developers/",
         },
         {
-            "identity": "user:bob",
+            "identity": "shortUser:bob",
             "action": "rw",
             "resource": "hdfs:file:/developers/file-from-bob",
         },
diff --git a/test/stack/20-hdfs.yaml b/test/stack/20-hdfs.yaml
@@ -35,21 +35,11 @@ spec:
       tlsSecretClass: tls # pragma: allowlist secret
       kerberos:
         secretClass: kerberos-default # pragma: allowlist secret
+    authorization:
+      opa:
+        configMapName: opa
+        package: hdfs
   nameNodes:
-    envOverrides: &envOverrides
-      HADOOP_CLASSPATH: "/stackable/hadoop/share/hadoop/tools/lib/*.jar"
-    configOverrides: &configOverrides
-      hdfs-site.xml:
-        dfs.namenode.inode.attributes.provider.class: tech.stackable.hadoop.StackableAuthorizer
-      core-site.xml:
-        # The mapper is only handled on the namenode so no need to apply this config to all roles
-        hadoop.security.group.mapping: tech.stackable.hadoop.StackableGroupMapper
-        hadoop.security.group.mapping.opa.policy.url: http://opa.default.svc.cluster.local:8081/v1/data/hdfs/groups
-        hadoop.security.authorization.opa.policy.url: http://opa.default.svc.cluster.local:8081/v1/data/hdfs/allow
-        # The operator adds a default static mapping when kerberos is activated, see:
-        # https://github.com/stackabletech/hdfs-operator/blob/main/rust/operator-binary/src/kerberos.rs#L97-L101
-        # This should be removed so that the mapping implementation can provide this information instead:
-        hadoop.user.group.static.mapping.overrides: ""
     config:
       logging:
         containers:
@@ -65,14 +55,10 @@ spec:
       default:
         replicas: 2
   dataNodes:
-    configOverrides: *configOverrides
-    envOverrides: *envOverrides
     roleGroups:
       default:
         replicas: 1
   journalNodes:
-    configOverrides: *configOverrides
-    envOverrides: *envOverrides
     roleGroups:
       default:
         replicas: 1
diff --git a/test/stack/30-test-hdfs-permissions.yaml b/test/stack/30-test-hdfs-permissions.yaml
@@ -52,13 +52,8 @@ spec:
               bin/hdfs dfs -ls /developers
               bin/hdfs dfs -ls /developers-ro && exit 1
 
-              sleep infinity
-
-              bin/hdfs dfs -ls /
-              bin/hdfs dfs -rm -f /hosts
-              bin/hdfs dfs -put -f /etc/hosts /hosts
-              bin/hdfs dfs -ls /
-              bin/hdfs dfs -cat /hosts
+              echo "Test passed"
+              exit 0
           volumeMounts:
             - name: hdfs-config
               mountPath: /stackable/conf/hdfs
