irods / irods_rule_engine_plugin_metadata_guard

BSD 3-Clause "New" or "Revised" License
0 stars 7 forks source link

Plugin allows non-admins to remove guarded metadata with `imeta rmw` #40

Closed mstfdkmn closed 9 months ago

mstfdkmn commented 2 years ago

I'm running iRODS 4.2.10 on CentOS 7. The plugin is 4.2.10.1 Release

Any metadata that is wanted to be protected seems vulnerable against imeta rmw -C myCollection % % and imeta rmw -d test.txt % %

I guess seems tested only by imeta rm https://github.com/irods/irods_rule_engine_plugin_metadata_guard/blob/main/packaging/test_rule_engine_plugin_metadata_guard.py#L218

korydraughn commented 2 years ago

We see three ways to implement this.

  1. Remove all unprotected metadata and leave the protected ones in place
    • similar to rm * in the terminal
    • requires iterating through the resultset and attempting a delete on each item
    • returns an error message to the client for each AVU that was guarded
      • return code will be non-zero
    • slower than today, but correct
  2. We treat the operation as an atomic request and exit if some of the metadata is protected (i.e. the operation becomes a NOP)
    • getting the resultset
    • then, attempting to remove all of them atomically
      • returns non-zero on failure
    • slower than today, but correct
  3. Do not allow users to use rmw
    • detect the use of the wildcard option (option = 1) and return an error (not supported)

We think (1) is the way to go.

mstfdkmn commented 2 years ago

It seems the option 1 is a more generic solution, however I have some non expertize questions:

trel commented 2 years ago

Yes, 'slower than today' because this would need to find all matching AVUs, then iterate through them one-by-one, and try to remove each. So it will be slower linearly with the number of existing AVUs that match the wildcard. If 10 AVUs match the wildcard, then there will be 1 operation to find the 10, and then 10 individual roundtrips to the database to remove the 10 matches (so total of 11). If there are 100 AVUs that match, then 101 roundtrips total.

No, metadata is just metadata - it only 'matches' or is 'guarded' once the string comparison is performed by the PEP which is part of the metadata guard plugin.

mstfdkmn commented 2 years ago

Then is the following technically doable?

Remove all attached metadata, treat special for the protected metadata (somehow keep somewhere - pep_api_mod_avu_metadata_pre may send to pep_api_mod_avu_metadata_post to add) and add the protected one.

trel commented 2 years ago

I'm not sure what you mean. As a rodsadmin, metadata would not be guarded, it can be manipulated/edited/removed.

As a rodsuser, if things are working as designed/intended... then guarded metadata should not be allowed to be changed.

mstfdkmn commented 2 years ago

Sorry for this late reply and the imperfect question. I see what I wanted to say is impossible because pep_api_mod_avu_metadata_pre doesn't have any attributes (avu) in regards toimeta rmw % % whereas it has for the imeta add and imeta rm operations.

If it had, I would be then wondering an option/possibility keeping in mind Kory's first option for the possible solution and your explanation about slowness. So I meant rmw % % would have removed all metadata and so that would trigger pep_api_mod_avu_metadata_pre and save/pass guarded metadata somewhere and finally add them back to the object.

Thanks.

trel commented 2 years ago

It would not 'save' them anywhere and 'add' them back - each would be attempted (to be deleted) but would error out, just like rm * on the command line if you don't have permission to remove something.

If there are five AVUs on a data object, and two are guarded... option 1 above would match all five, walk through each and try to remove them, two would throw errors because they were guarded/protected, the other three would be removed, and the overall return code would be non-zero.

korydraughn commented 11 months ago

As of iRODS 4.3.0, imeta rmw is deprecated.

That means the metadata guard no longer needs to handle that case. Users of imeta rmw should use an alternative command/API.

With that said, we should at least make the plugin block use of imeta rmw entirely.

alanking commented 9 months ago

Noting here that addw has also been deprecated (see https://github.com/irods/irods/issues/6766), in case that affects anything.

trel commented 9 months ago

deprecating lsw now has an issue as well... https://github.com/irods/irods/issues/7488