rwm: add storage_list and other policing cosmetics and hardening

README.md

@@ -33,22 +33,16 @@ RWM can:
 * restic with S3 repository
 * configurable backup manager/executor
 
+* create, delete and list policed storage buckets
+* check if used bucket is configured with expected policies
 
-todo:
-
-* check if used bucket is configured for versioning
-* check if used access_key does not have administrator privileges to manipulate
-  with WORM policies
+TODO:
 * generate and store current bucket state state-data
 * recreate bucket contents on local filesystem (or remote bucket) acording to specified
   state data
 * ??? check completeness of the current state of the bucket
 * prune all non-recent object versions to reclaim storage space
-
-
-TBD:
 * unlike in other backup solutions, attacker with credentials can restore any old data from the repository/bucket
-* number of object files vs size
 
 
 ## Usage
@@ -61,45 +55,14 @@ make install
 ```
 
 
-### Low-level S3
-
-```
-cp examples/rwm-rclone.conf rwm.conf
-rwm aws s3 ls s3://
-rwm aws s3api list-buckets
-rwm rclone lsd rwmbe:/
-```
-
-
-### Simple copy: rclone with crypt overlay
-
-rclone_crypt defines single default remote named "rwmbe:/" pointed to `rwm_rclone_crypt_bucket` path.
-
-```
-cp examples/rwm-rclone.conf rwm.conf
-rwm rclone_crypt sync /data rwmbe:/
-rwm rclone_crypt lsl rwmbe:/
-```
-
-
-### Restic: manual restic backup
-
-```
-cp examples/rwm-restic.conf rwm.conf
-rwm restic init
-rwm restic backup /data
-rwm restic snapshots
-rwm restic mount /mnt/restore
-```
-
-
 ### RWM: simple backups
 
-backups follows standard restic procedures, but adds profile like configuration to easily run in schedulers
+Backups follows standard restic procedures, but adds profile like configuration
+to easily run in schedulers.
 
 ```
 cp examples/rwm-backups.conf rwm.conf
-rwm restic init  # should create bucket on it's own
+rwm restic init
 
 rwm backup_all
 rwm restic snapshots
@@ -109,7 +72,7 @@ rwm restic mount /mnt/restore
 
 ### RWM: backups with policed buckets
 
-Have two S3 accounts (*admin* and *user1*), create storage bucket and use it.
+Two distinct S3 accounts required (*admin*, *user1*)
 
 ```
 cp examples/rwm-admin.conf admin.conf
@@ -127,6 +90,38 @@ rwm restic mount /mnt/restore
 ```
 
 
+### Other usages
+
+#### AWS cli
+
+```
+cp examples/rwm-rclone.conf rwm.conf
+rwm aws s3 ls s3://
+rwm aws s3api list-buckets
+rwm rclone lsd rwmbe:/
+```
+
+#### rclone with crypt overlay
+
+rclone_crypt defines single default remote named "rwmbe:/" pointed to `rwm_rclone_crypt_bucket` path.
+
+```
+cp examples/rwm-rclone.conf rwm.conf
+rwm rclone_crypt sync /data rwmbe:/
+rwm rclone_crypt lsl rwmbe:/
+```
+
+#### Restic: manual restic backup
+
+```
+cp examples/rwm-restic.conf rwm.conf
+rwm restic init
+rwm restic backup /data
+rwm restic snapshots
+rwm restic mount /mnt/restore
+```
+
+
 ## Notes
 
 * executed tools stdout is buffered, eg. `restic mount` does not print immediate output as normal

rwm.py

@@ -105,6 +105,18 @@ class BackupResult:
 class StorageManager:
     """s3 policed bucket manager"""
 
+    USER_BUCKET_POLICY_ACTIONS = [
+        # backups
+        "s3:ListBucket",
+        "s3:GetObject",
+        "s3:PutObject",
+        "s3:DeleteObject",
+        # check policies
+        "s3:GetBucketPolicy",
+        "s3:ListBucketVersions",
+        "s3:GetBucketVersioning"
+    ]
+
     def __init__(self, url, access_key, secret_key):
         self.url = url
         self.access_key = access_key
@@ -131,6 +143,7 @@ class StorageManager:
         try:
             return json.loads(self.s3.Bucket(name).Policy().policy)
         except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as exc:
+            if "NoSuchBucketPolicy" not in str(exc):
                 logger.error("rwm bucket_policy error, %s", (exc))
             return None
 
@@ -149,16 +162,16 @@ class StorageManager:
             raise ValueError("must specify value for bucket and user")
 
         bucket = self.bucket_create(bucket_name)
-        tenant, manager_username = bucket.Acl().owner["ID"].split("$")
+        tenant, admin_username = bucket.Acl().owner["ID"].split("$")
 
         # grants basic RW access to user in same tenant
         bucket_policy = {
             "Version": "2012-10-17",
             "Statement": [
-                # full access to manager
+                # full access to admin
                 {
                     "Effect": "Allow",
-                    "Principal": {"AWS": [f"arn:aws:iam::{tenant}:user/{manager_username}"]},
+                    "Principal": {"AWS": [f"arn:aws:iam::{tenant}:user/{admin_username}"]},
                     "Action": ["*"],
                     "Resource": [f"arn:aws:s3:::{bucket.name}", f"arn:aws:s3:::{bucket.name}/*"]
                 },
@@ -166,10 +179,7 @@ class StorageManager:
                 {
                     "Effect": "Allow",
                     "Principal": {"AWS": [f"arn:aws:iam::{tenant}:user/{target_username}"]},
-                    "Action": [
-                        "s3:ListBucket", "s3:GetObject", "s3:PutObject", "s3:DeleteObject",
-                        "s3:GetBucketPolicy", "s3:ListBucketVersions", "s3:GetBucketVersioning"
-                    ],
+                    "Action": self.USER_BUCKET_POLICY_ACTIONS,
                     "Resource": [f"arn:aws:s3:::{bucket.name}", f"arn:aws:s3:::{bucket.name}/*"]
                 }
             ]
@@ -189,21 +199,62 @@ class StorageManager:
         bucket.object_versions.all().delete()
         bucket.delete()
 
+    @staticmethod
+    def _policy_statements_admin(policy):
+        """policy helper"""
+        return list(filter(lambda stmt: stmt["Action"] == ["*"], policy["Statement"]))
+
+    @staticmethod
+    def _policy_statements_user(policy):
+        """policy helper"""
+        return list(filter(lambda stmt: stmt["Action"] != ["*"], policy["Statement"]))
+
     def storage_check_policy(self, name):
         """storage check bucket policy"""
 
         if not (policy := self.bucket_policy(name)):
             return False
 
-        if (
+        admin_statements = self._policy_statements_admin(policy)
+        user_statements = self._policy_statements_user(policy)
+
+        if (  # pylint: disable=too-many-boolean-expressions
+            # only two expected statements should be present on a bucket
             len(policy["Statement"]) == 2
-            and len(list(filter(lambda stmt: stmt["Action"] == ["*"], policy["Statement"]))) == 1
+            and len(admin_statements) == 1
+            and len(user_statements) == 1
+            # with distinct identities for admin and user
+            and admin_statements[0]["Principal"] != user_statements[0]["Principal"]
+            # user should have only limited access
+            and sorted(self.USER_BUCKET_POLICY_ACTIONS) == sorted(user_statements[0]["Action"])
+            # the bucket should be versioned
             and self.s3.Bucket(name).Versioning().status == "Enabled"
         ):
             return True
 
         return False
 
+    def storage_list(self):
+        """storage list"""
+
+        output = []
+        for item in self.list_buckets():
+            result = {
+                "name": item.name,
+                "policy": "OK" if self.storage_check_policy(item.name) else "FAILED",
+                "owner": self.bucket_owner(item.name).split("$")[-1]
+            }
+
+            if result["policy"] == "OK":
+                user_statement = self._policy_statements_user(self.bucket_policy(item.name))[0]
+                result["target_user"] = user_statement["Principal"]["AWS"][0].split("/")[-1]
+            else:
+                result["target_user"] = None
+
+            output.append(result)
+
+        return output
+
 
 class RWM:
     """rwm impl"""
@@ -311,8 +362,8 @@ class RWM:
     def backup_cmd(self, name) -> subprocess.CompletedProcess:
         """backup command"""
 
-        # TODO: check target backup policy, restic automatically creates 
-        # bucket if ot does not exist with null-policy
+        if not self.storage_manager.storage_check_policy(self.config["rwm_restic_bucket"]):
+            logger.warning("used bucket does not have expected policy")
 
         wrap_output(backup_proc := self._restic_backup(name))
         if backup_proc.returncode != 0:
@@ -383,11 +434,14 @@ class RWM:
         return ret
 
     def storage_list_cmd(self):
-        pass
+        """storage_list command"""
 
-    def storage_restore(self, bucket_name, target_username):
-        """https://gitlab.cesnet.cz/709/public/restic/aws/-/blob/main/bucket_copy.sh?ref_type=heads"""
-        pass
+        print(tabulate(
+            self.storage_manager.storage_list(),
+            headers="keys",
+            numalign="left"
+        ))
+        return 0
 
 
 def configure_logging(debug):
@@ -425,7 +479,7 @@ def parse_arguments(argv):
 
     backup_cmd_parser = subparsers.add_parser("backup", help="backup command")
     backup_cmd_parser.add_argument("name", help="backup config name")
-    subparsers.add_parser("backup_all", help="backup all command")
+    _ = subparsers.add_parser("backup_all", help="backup all command")
 
     storage_create_cmd_parser = subparsers.add_parser("storage_create", help="storage_create command")
     storage_create_cmd_parser.add_argument("bucket_name", help="bucket name")
@@ -434,6 +488,7 @@ def parse_arguments(argv):
     storage_delete_cmd_parser.add_argument("bucket_name", help="bucket name")
     storage_check_policy_cmd_parser = subparsers.add_parser("storage_check_policy", help="storage_check_policy command; use --debug to show policy")
     storage_check_policy_cmd_parser.add_argument("bucket_name", help="bucket name")
+    _ = subparsers.add_parser("storage_list", help="storage_list command")
 
     return parser.parse_args(argv)
 
@@ -478,6 +533,8 @@ def main(argv=None):
         ret = rwmi.storage_delete_cmd(args.bucket_name)
     if args.command == "storage_check_policy":
         ret = rwmi.storage_check_policy_cmd(args.bucket_name)
+    if args.command == "storage_list":
+        ret = rwmi.storage_list_cmd()
 
     logger.debug("rwm finished with %s (ret %d)", "success" if ret == 0 else "errors", ret)
     return ret

tests/test_default.py

@@ -34,16 +34,16 @@ def test_main(tmpworkdir: str):  # pylint: disable=unused-argument
     mock_proc = Mock(return_value=CompletedProcess(args='dummy', returncode=0))
     mock_ok = Mock(return_value=0)
 
-    with patch.object(rwm.RWM, f"aws_cmd", mock_proc):
+    with patch.object(rwm.RWM, "aws_cmd", mock_proc):
         assert rwm_main(["aws", "dummy"]) == 0
-    with patch.object(rwm.RWM, f"rclone_cmd", mock_proc):
+    with patch.object(rwm.RWM, "rclone_cmd", mock_proc):
         assert rwm_main(["rclone", "dummy"]) == 0
-    with patch.object(rwm.RWM, f"rclone_crypt_cmd", mock_proc):
+    with patch.object(rwm.RWM, "rclone_crypt_cmd", mock_proc):
         assert rwm_main(["rclone_crypt", "dummy"]) == 0
-    with patch.object(rwm.RWM, f"restic_cmd", mock_proc):
+    with patch.object(rwm.RWM, "restic_cmd", mock_proc):
         assert rwm_main(["restic", "dummy"]) == 0
 
-    with patch.object(rwm.RWM, f"backup_cmd", mock_proc):
+    with patch.object(rwm.RWM, "backup_cmd", mock_proc):
         assert rwm_main(["backup", "dummy"]) == 0
     with patch.object(rwm.RWM, "backup_all_cmd", mock_ok):
         assert rwm_main(["backup_all"]) == 0
@@ -54,3 +54,5 @@ def test_main(tmpworkdir: str):  # pylint: disable=unused-argument
         assert rwm_main(["storage_delete", "bucket"]) == 0
     with patch.object(rwm.RWM, "storage_check_policy_cmd", mock_ok):
         assert rwm_main(["storage_check_policy", "bucket"]) == 0
+    with patch.object(rwm.RWM, "storage_list_cmd", mock_ok):
+        assert rwm_main(["storage_list"]) == 0

tests/test_rwm.py

@@ -142,7 +142,6 @@ def test_backup_cmd(tmpworkdir: str, motoserver: str):  # pylint: disable=unused
 def test_backup_cmd_excludes(tmpworkdir: str, motoserver: str):  # pylint: disable=unused-argument
     """test backup command"""
 
-    import os
     trwm = rwm.RWM({
         "rwm_s3_endpoint_url": motoserver,
         "rwm_s3_access_key": "dummy",
@@ -187,6 +186,7 @@ def test_backup_cmd_error_handling(tmpworkdir: str, motoserver: str):  # pylint:
     """test backup command err cases"""
 
     rwm_conf = {
+        "rwm_restic_bucket": "restictest",
         "rwm_backups": {
             "dummycfg": {"filesdirs": ["dummydir"]}
         }
@@ -285,3 +285,17 @@ def test_storage_check_policy_cmd(tmpworkdir: str, microceph: str, radosuser_adm
     mock = Mock(return_value=False)
     with patch.object(rwm.StorageManager, "storage_check_policy", mock):
         assert trwm.storage_check_policy_cmd("dummy") == 1
+
+
+def test_storage_list_cmd(tmpworkdir: str, microceph: str, radosuser_admin: rwm.StorageManager):  # pylint: disable=unused-argument
+    """test storage check policy command"""
+
+    trwm = rwm.RWM({
+        "rwm_s3_endpoint_url": radosuser_admin.url,
+        "rwm_s3_access_key": radosuser_admin.access_key,
+        "rwm_s3_secret_key": radosuser_admin.secret_key,
+    })
+
+    mock = Mock(return_value=[])
+    with patch.object(rwm.StorageManager, "storage_list", mock):
+        assert trwm.storage_list_cmd() == 0

tests/test_storage.py

@@ -70,7 +70,6 @@ def test_storage_delete(
     target_username = "test1"
     bucket = radosuser_admin.storage_create(bucket_name, target_username)
 
-    # 
     bucket = radosuser_test1.s3.Bucket(bucket.name)
     bucket.upload_fileobj(BytesIO(b"dummydata"), "dummykey")
     assert len(radosuser_test1.list_objects(bucket.name)) == 1
@@ -145,3 +144,18 @@ def test_storage_backup_usage(
 
     with pytest.raises(radosuser_test1.s3.meta.client.exceptions.ClientError, match=r"AccessDenied"):
         assert radosuser_test1.storage_delete(bucket_name)
+
+
+def test_storage_list(
+    tmpworkdir: str,
+    microceph: str,
+    radosuser_admin: rwm.StorageManager,
+):  # pylint: disable=unused-argument
+    """test managet list storage"""
+
+    bucket_name = "rwmbackup-test1"
+    target_username = "test1"
+
+    radosuser_admin.bucket_create("no-acl-dummy")
+    radosuser_admin.storage_create(bucket_name, target_username)
+    assert radosuser_admin.storage_list()