Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scitoken bug with scope including basepath or / #2132

Closed
Jo-stfc opened this issue Nov 23, 2023 · 20 comments · Fixed by #2152
Closed

Scitoken bug with scope including basepath or / #2132

Jo-stfc opened this issue Nov 23, 2023 · 20 comments · Fixed by #2152
Assignees
Milestone

Comments

@Jo-stfc
Copy link
Collaborator

Jo-stfc commented Nov 23, 2023

Hi,
I found a weird interaction with CMS tokens.
RAL uses lfn2pfn mapping to map cms's /store root folder to the pfn cms:/store.
Testing with a user token, I've confirmed the correct permissions are given (can only access files in the cms pool)
This follows the usual flow of request comes in, lfn2pfn maps /store to cms:/store and permission is granted.
This however caused CMS SAM token tests (external site functional tests) to fail, and from server logs the flow seems to be: request comes in, there is no lfn2pfn mapping and then the request gets denied.
Turning on scitokens.trace all fixed this behaviour and made the SAM tests pass again

@bbockelm
Copy link
Contributor

Hi @Jo-stfc!

Happy to help but there's not a lot to chew on from the report. The two components aren't typically interconnected so I don't have an immediate guess as to what's happening.

As a starting point, can you share the logs from when the scitokens.trace is turned on and the config (including the scitokens config and any mapfile)?

@Jo-stfc
Copy link
Collaborator Author

Jo-stfc commented Nov 30, 2023

Hi @bbockelm
the CMS tests started failing again with the trace enabled..
I'll do some more debugging on my side, the logs when the trace is turned on are:
231129 10:41:04 1868130 XrdLinkXeq: anon.0:687@etf-01.cern.ch connection upgraded to TLSv1.2 231129 10:41:04 1868130 XrootdXeq: u06.1351139:687@etf-01.cern.ch pub IPv4 TLSv1.2 login as 08ca855e-d715-410e-a6ff-ad77306e1763 231129 10:41:04 1868130 scitokens_Access: Trying token-based access control 231129 10:41:04 1868130 scitokens_Access: Cached token mapped_username=, subject=08ca855e-d715-410e-a6ff-ad77306e1763, issuer=https://cms-auth.web.cern.ch/, authorizations=/store/store/mc/SAM:read,dir,stat;/store/store/test/xrootd:read,dir,stat;/store/store/temp/user:excl_create,mkdir,mv,excl_insert,read,dir,stat,create,mkdir,mv,insert,update,chmod,del 231129 10:41:04 1868130 acc_Audit: deny *@? stat /store/test/xrootd/T1_UK_RAL/store/mc/SAM/GenericTTbar/AODSIM/CMSSW_9_2_6_91X_mcRun1_realistic_v2-v1/00000/AE237916-5D76-E711-A48C-FA163EEEBFED.root 231129 10:41:04 1868130 ofs_stat: u06.1351139:687@etf-01.cern.ch Unable to locate /store/test/xrootd/T1_UK_RAL/store/mc/SAM/GenericTTbar/AODSIM/CMSSW_9_2_6_91X_mcRun1_realistic_v2-v1/00000/AE237916-5D76-E711-A48C-FA163EEEBFED.root; permission denied 231129 10:41:04 1594215 XrdLinkXeq: anon.0:688@etf-01.cern.ch connection upgraded to TLSv1.2 231129 10:41:04 1594215 XrootdXeq: u07.1351139:688@etf-01.cern.ch pub IPv4 TLSv1.2 login as 08ca855e-d715-410e-a6ff-ad77306e1763 231129 10:41:04 1594215 scitokens_Access: Trying token-based access control 231129 10:41:04 1594215 scitokens_Access: Cached token mapped_username=, subject=08ca855e-d715-410e-a6ff-ad77306e1763, issuer=https://cms-auth.web.cern.ch/, authorizations=/store/store/mc/SAM:read,dir,stat;/store/store/test/xrootd:read,dir,stat;/store/store/temp/user:excl_create,mkdir,mv,excl_insert,read,dir,stat,create,mkdir,mv,insert,update,chmod,del 231129 10:41:04 1594215 acc_Audit: deny *@? read /store/test/xrootd/T1_UK_RAL/store/mc/SAM/GenericTTbar/AODSIM/CMSSW_9_2_6_91X_mcRun1_realistic_v2-v1/00000/CE860B10-5D76-E711-BCA8-FA163EAA761A.root 231129 10:41:04 1594215 ofs_open: u07.1351139:688@etf-01.cern.ch Unable to open /store/test/xrootd/T1_UK_RAL/store/mc/SAM/GenericTTbar/AODSIM/CMSSW_9_2_6_91X_mcRun1_realistic_v2-v1/00000/CE860B10-5D76-E711-BCA8-FA163EAA761A.root; permission denied

@bbockelm
Copy link
Contributor

@Jo-stfc - can you post your scitokens.cfg as well?

From this line:

231129 10:41:04 1594215 scitokens_Access: Cached token mapped_username=, subject=08ca855e-d715-410e-a6ff-ad77306e1763, issuer=https://cms-auth.web.cern.ch/, authorizations=/store/store/mc/SAM:read,dir,stat;/store/store/test/xrootd:read,dir,stat;/store/store/temp/user:excl_create,mkdir,mv,excl_insert,read,dir,stat,create,mkdir,mv,insert,update,chmod,del

You'll see the authorization generated is for /store/store/test/xrootd while the test is opening /store/test/xrootd/. That suggests there's a misconfiguration where you're setting a base_path = /store but you want base_path = / and restricted_path = /store.

@Jo-stfc
Copy link
Collaborator Author

Jo-stfc commented Nov 30, 2023

Hi Brian,
this is the config I'm using. I did set base_path = / initially, but that allowed cms token to read files outside the restricted path, which is why I changed to this one, which was working as intended with a user token

[Issuer CMS_IAM]
issuer = https://cms-auth.web.cern.ch/
base_path = /store
map_subject = False
restricted_path=cms:/store

@Jo-stfc
Copy link
Collaborator Author

Jo-stfc commented Dec 6, 2023

this is a summary of the user token testing:

base path | restricted_path | cms file | non-cms file

/store | cms:/store | yes | no
/store | /store | no | no
/ | cms:/store | yes | yes
/ | /store | no | no

@Jo-stfc
Copy link
Collaborator Author

Jo-stfc commented Dec 12, 2023

from that check, restricted_path doesn't seem to work as intended - it either allows the token to access all data (when the pfn is specified) or none (when the lfn is specified) when the base_path is set to /.

@Jo-stfc
Copy link
Collaborator Author

Jo-stfc commented Dec 12, 2023

@bbockelm we found the issue - restricted_path is acting as a check on the scope of the token, not the filepath.
That is, a token with scope storage.read:/ attempting to read a file /store/xyz will get rejected when the restricted_path is set to /store

@Jo-stfc
Copy link
Collaborator Author

Jo-stfc commented Dec 14, 2023

@bbockelm Additionaly, if a token's scope already includes the base_path currently it will still get appended to the scope when parsing authorizations. I've drafted a PR fixing this issue
#2151

@abh3
Copy link
Member

abh3 commented Dec 14, 2023

So, I a trying to understand this as the title of this issue mentioning lfn2pfn doesn't seem relevant here, especially considering the pull request. The question is why would the incoming file path sometimes have the base path and sometimes not? Only one of them should be valid not both it would seem. The pull request makes both valid which is sort of odd. @bbockelm could you review the pull request and sign off on it if it's correct?

@Jo-stfc Jo-stfc changed the title Scitoken bug with lfn2pfn mapping Scitoken bug with scope including basepath or / Dec 15, 2023
@Jo-stfc
Copy link
Collaborator Author

Jo-stfc commented Dec 15, 2023

Hi Andy,
I've updated the title of the issue. (My initial hypotesis on why it didn't work was wrong). The issue happens because CMS sets default user read token scopes to '/', while SAM tests and write permissions will have some variant of /store/....
This means that setting restricted path to /store will reject any token with scope '/', failing the default user read token. The PR allows appending the basepath to the scope of the incoming token issued by the provider if the basepath is not at the start of the scope. The resulting behaviour is as follows:

  • Token with '/' will have its scope adjusted to the basepath (meaning you only give access to files in the basepath)
  • Tokens including the basepath will have the scope unchanged
  • Tokens without the basepath will be changed to be relative to the basepath (this behaviour is unchanged)

This is a change of scope, not requested filepath. i.e, if a filepath is outside the scope the request will be rejected, nor will it introduced unplanned changes on the location of the file.

@bbockelm
Copy link
Contributor

I'm not sure the PR is in the right direction. After all, if the problem was as described, then the dozen or so sites that are working wouldn't be working.

The base_path operates on LFN paths and restricted_path operates on paths in scopes. So, for CMS, the only possible solution is base_path=/ and restricted_path=/store. I double checked this on a site that is passing the SAM tests.

In the table above, that case is listed as not working. However, it doesn't match the log file excerpts. Can you share the logging lines in that case?

@snafus
Copy link
Contributor

snafus commented Dec 15, 2023

Hi @bbockelm;
I think the problem is that CMS has two requirements:

  1. production workflows, will supply as scope of "storage.read:/store/blah"
  2. user workflows; these are currently specified to use the "storage.read:/" scope.

We currently can't see a way to simultaneously satisfy both requirements.
either:
we require a /store restricted path and base_path / to correctly restrict Cms to its own pool, making Prod work
or,
we have the restricted path "/" and base_path /store to satisfy user workflow, (but this prepends /store to /store prod LFNs)

There seems to be no way to allow a token scoped with storage.read:/ but only allow it access to LFNs with "/store",
unless I misunderstand things ?
Thanks,
James

@bbockelm
Copy link
Contributor

Two thoughts:

  • we can ask CMS to restrict users to /store. Given all CMS files are in /store, that's hopefully a small ask.
  • instead of ignoring scopes that don't fit within the restricted paths (as is done now), we can simply add restrictions to them. This way, the scope is allowed but the file access to anything outside the restricted scope would fail.

@snafus
Copy link
Contributor

snafus commented Dec 15, 2023

The first I guess is for CMS/you (? :) ) to discuss.

The second is probably more preferable in the longer term. I.e. the implementation details should be hidden from the user, and if they want to specify "storage.read:/" across all SE's that should be OK. On the storage side, there is then the need to restrict them appropriately.
I'm not sure of other considerations at this point though.

James

@Jo-stfc
Copy link
Collaborator Author

Jo-stfc commented Dec 15, 2023

@bbockelm the following are the log excerpt:
with basepath:/ and restrictedpath /store
this is the log with scope:/ , with restricted path

231211 14:36:30 3196764 scitokens_Access: Token not found in recent cache; parsing.
231211 14:36:30 3196764 scitokens_Access: New valid token mapped_username=cmsprod, subject=4c702efd-aa09-42ce-9292-8b79b8b8bb7e, issuer=https://cms-auth.web.cern.ch/
231211 14:36:30 3196764 acc_Audit: deny *@? stat /store/user/kellis/test_file.root
231211 14:36:30 3196764 ofs_stat: unknown.137:34@host-172-16-113-234.nubes.stfc.ac.uk Unable to locate /store/user/kellis/test_file.root; permission denied
231211 14:36:30 3196764 XrootdXeq: unknown.137:34@host-172-16-113-234.nubes.stfc.ac.uk disc 0:00:00 (send failure)

this is when the scope is set to /store on the token

231212 11:36:26 3281371 scitokens_Access: Trying token-based access control
231212 11:36:26 3281371 scitokens_Access: Token not found in recent cache; parsing.
231212 11:36:26 3281371 scitokens_Access: New valid token mapped_username=cmsprod, subject=4c702efd-aa09-42ce-9292-8b79b8b8bb7e, issuer=https://cms-auth.web.cern.ch/, authorizations=/store:read,dir,stat
231212 11:36:26 3281371 scitokens_Access: Grant authorization based on scopes for operation=stat, path=/store/user/kellis/test_file.root
231212 11:36:26 3281371 scitokens_Access: Request username cmsprod
Stat path = /store/user/kellis/test_file.root
m_translateFileName - translated '/store/user/kellis/test_file.root' to 'cms:/store/user/kellis/test_file.root'
231212 11:36:26 ceph_posix_stat
231212 11:36:26 ceph_namelib : translated /store/user/kellis/test_file.root to cms:/store/user/kellis/test_file.root

this is with the patch in place and base_path set to /store, without restricted_path:

token scope /

231215 10:56:41 3587219 scitokens_Access: Trying token-based access control
231215 10:56:41 3587219 scitokens_Reconfig: Parsing configuration file: /etc/xrootd/scitokens.cfg
231215 10:56:41 3587219 scitokens_Reconfig: Configuring issuer https://cms-auth.web.cern.ch/
231215 10:56:41 3587219 scitokens_Access: Token not found in recent cache; parsing.
231215 10:56:41 3587219 scitokens_Access: New valid token mapped_username=cmsprod, subject=4c702efd-aa09-42ce-9292-8b79b8b8bb7e, issuer=https://cms-auth.web.cern.ch/, authorizations=/store:read,dir,stat
231215 10:56:41 3587219 scitokens_Access: Grant authorization based on scopes for operation=stat, path=/store/temp/sitetest/test_token_katy.root
231215 10:56:41 3587219 scitokens_Access: Request username cmsprod
Stat path = /store/temp/sitetest/test_token_katy.root
m_translateFileName - translated '/store/temp/sitetest/test_token_katy.root' to 'cms:/store/temp/sitetest/test_token_katy.root'

token scope /store

231215 10:57:27 3587219 scitokens_Access: Trying token-based access control
231215 10:57:27 3587219 scitokens_Access: Token not found in recent cache; parsing.
231215 10:57:27 3587219 scitokens_Access: New valid token mapped_username=cmsprod, subject=4c702efd-aa09-42ce-9292-8b79b8b8bb7e, issuer=https://cms-auth.web.cern.ch/, authorizations=/store:read,dir,stat
231215 10:57:27 3587219 scitokens_Access: Grant authorization based on scopes for operation=stat, path=/store/temp/sitetest/test_token_katy.root
231215 10:57:27 3587219 scitokens_Access: Request username cmsprod
Stat path = /store/temp/sitetest/test_token_katy.root
m_translateFileName - translated '/store/temp/sitetest/test_token_katy.root' to 'cms:/store/temp/sitetest/test_token_katy.root'

we don't see any change in the lfn requested

@bbockelm
Copy link
Contributor

@snafus or @Jo-stfc - would you have time to test #2152 for me in this setup? I had enough time this morning to write down the idea but not do a full reproduction of the issue you're facing.

The patch allows the scopes in the token to be more generic than the restricted paths. Internally, in that case instead of ignoring the scope it generates ACLs based on the restricted paths. So, if you have this case:

  • base_path=/
  • restricted_paths=/store
  • token scope storage:read:/
  • attempted operation read
  • attempted path /store/foo.txt

You'd end up with permission granted (previously, it was a denied).

@Jo-stfc
Copy link
Collaborator Author

Jo-stfc commented Jan 8, 2024

Hi @bbockelm,
I've tested #2152 with the case you mentioned. It doesn't work with the current state of the PR, I've commented the amendment I did to get it working as intended

@abh3
Copy link
Member

abh3 commented Jan 12, 2024

Are we getting any closer to getting this resolved?

@amadio
Copy link
Member

amadio commented Feb 26, 2024

Assigning to @bbockelm, since #2152 fixes this, as confirmed by @Jo-stfc in his comment there.

@amadio
Copy link
Member

amadio commented Feb 26, 2024

Closing, as the fix (commit fd0f772) has just been merged into master and will be released in 5.6.9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants