HDDS-5193. change to use getShortUserName instead of getUsername

[pakapoj-tul]

HDDS-5193. change to use getShortUserName instead of getUsername

What changes were proposed in this pull request?

IMHO, in kerberized cluster identity pakapoj_tul@DEV.TAP and pakapoj_tul are equal so I propose to use getShortUsername() instead of getShortUsername() from UGI interface when assign and/or compare any ACLs. To leverage auth_to_local property to translate any identity from auth:KERBEROS to plain username which should be consistent with identity from auth:TOKEN

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-5193

Please replace this section with the link to the Apache JIRA)

How was this patch tested?

I manually tested on my ozone cluster, bare metal deployment. there are following tests

ozone sh vol create /vol1
ozone sh bucket create /vol1/bucket1
ozone fs -put sample_seq_data.snappy.orc /vol1/bucket1
ozone sh vol addacl --acl=user:pakapoj_tul:a /vol1
ozone sh bucket addacl --acl=user:pakapoj_tul:a /vol1/bucket1
ozone sh key addacl --acl=user:pakapoj_tul:a /vol1/bucket1/sample_seq_data.snappy.orc
ozone sh token get -t ozone.token
ozone s3 getsecret --om-service-id=dev-ozone
aws configure
aws s3 ls --endpoint http://mtg8-devozonem-01.dev.tap:9878 s3://common-bucket
aws s3api --endpoint http://mtg8-devozonem-01.dev.tap:9878 create-bucket --bucket buckettest
aws s3api --endpoint http://mtg8-devozonem-01.dev.tap:9878 delete-bucket --bucket buckettest
aws s3 cp --endpoint http://mtg8-devozonem-01.dev.tap:9878 README.md s3://common-bucket
aws s3 rm --endpoint http://mtg8-devozonem-01.dev.tap:9878 s3://common-bucket/README.md

also test by running spark application in cluster mode

val data = spark.read.format("orc").load(src)
data.write.format("orc").save(des)

given

src = "ofs://dev-ozone/vol1/bucket1/sample_seq_data.snappy.orc"
des = "ofs://dev-ozone/vol1/bucket1/mykey"

[xiaoyuyao]

Thanks @pakapoj-tul for reporting the issue and the detailed investigation report. The current change can't fully address the token user problem.

The token ugi from the client side may not have the proper short user name. For example: you have a token for user
alice@EXAMPLE.COM but the local OS user is bob, ugi.getSHortUserName() will give you bob instead of alice.
After the token is validated on the server side secret manager, the RPC server will recreate a token UGI that has the proper user name as alice@EXAMPLE.COM.

So I think we should move the user/owner ACL from client to server side upon Volume/Bucket/Key request processing.

[pakapoj-tul]

Hi @xiaoyuyao , thank for the reply, I was unable to response earlier due to the workload.
yeah, I think the id should be from the server side also. donno the process earlier :) let me check then.

[pakapoj-tul]

Hi @xiaoyuyao as i checked the ACLs assignment was happened here and where ACLs was given from the client side so just wanna confirm with you first that we ignore this and build a new one from userInfo?
Request payload.txt

Another question is where should I call the getRemoteUser()?, I try to called it in here but it failed tho.

[pakapoj-tul]

Hi, seem to be very odd if the ACL was set using authenticated user ugi.

My thoughts was if we have data_pipeline user as a writer who write data to /vol1/bucket1/mykey and data_sci as a reader who read data at /vol1/bucket1/mykey , we have to reassign /vol1/bucket1/mykey acl = /vol1/bucket1/ anyway…

[pakapoj-tul]

Ref to the case above, user can't to inherit the acl to the children so I think we should inherit the acl

What’s the meaning of aclscope ACCESS and DEFAULT tho?

  enum OzoneAclScope {
    ACCESS = 0;
    DEFAULT = 1;
  }

b/c it filter only DEFAULT acl from what I see here

[xiaoyuyao]

In the failed acceptance test, acls were added with the Full principle name (links.robot line 45-49) so changing the compare here with shortName will mismatch and fail.

------------------------------------------------------------------------------
Can follow link with read access                                      | FAIL |
‘PERMISSION_DENIED User testuser2/scm@EXAMPLE.COM doesn’t have READ permission to access volume 04714-target null null’ does not contain ‘key-in-readable-bucket’
------------------------------------------------------------------------------

[pakapoj-tul]

note for reproduce the test case

cd compose/ozonesecure
docker-compose up -d
docker-compose --scale datanode=3
docker-compose exec scm bash
robot smoketest/basic/links.robot

[DaveTeng0]

hey @pakapoj-tul ~ would you mind rebasing this PR with the latest master branch? Some links referenced from your comments seemed to mismatch with the current master branch codes~ (Tried to follow up with your above comments & found that) Thanks!

[fapifta]

As the user name related part of this PR is already included in #2466 and we have recent progress on that one, and also because the rest of the patch should not be pulled in (see below) I would suggest to close this PR for now.
@pakapoj-tul @kerneltime what do you think?

On the ACL inheritance:
The Posix draft withdrawn but applied by many nix systems do specify that only the default ACLs are inherited from a parent directory. If we start to inherit ACCESS ACLs as well, that is pretty much against this standard, and with that it seems pretty much unexpected by any kind of user, especially as HDFS earlier, and all linux systems inherit just the default ACLs from the parent.
With that I think we should not inherit the ACCESS ACLs.
References:
Usenix: https://www.usenix.org/legacy/publications/library/proceedings/usenix03/tech/freenix03/full_papers/gruenbacher/gruenbacher_html/main.html
Solaris: https://docs.oracle.com/cd/E36784_01/html/E36883/acl-5.html
SLES: https://documentation.suse.com/sles/12-SP4/html/SLES-all/cha-security-acls.html
FreeBSD: https://man.freebsd.org/cgi/man.cgi?query=setfacl&sektion=1&apropos=0&manpath=FreeBSD+13.1-RELEASE+and+Ports

[ChenSammi]

Agree with @fapifta . The getShortUserName part is resolved by #2466. Close this MR.