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

f-stream monitoring does not work with tokens #1987

Closed
bbockelm opened this issue Apr 3, 2023 · 10 comments
Closed

f-stream monitoring does not work with tokens #1987

bbockelm opened this issue Apr 3, 2023 · 10 comments

Comments

@bbockelm
Copy link
Contributor

bbockelm commented Apr 3, 2023

(This is probably true for the other monitoring packet types but I happened to stumble across this with the f-stream parsing in #1985)

When a file-open record is sent in the f-stream, the record references the login session.

However, for token-based access, the access control is done at the file-level, not at the session level. Typically the session is for some anonymous user (HTTP) or, when using ZTN, potentially a different token.

I can see two obvious options:

  1. Extend the file-open packet type to also include the security entity information, much like the LFN record is done today.
  2. Some complex setup where we introduce a new f-stream sub-record type that includes the security entity information and indicated that all following file-open sub-records in the same packet share the same type.

Honestly? I'd prefer simplicity won the day and go with (1). That'd have the side-effect of making the f-stream easier to use as one wouldn't even need to capture user login packets anymore - they'd be embedded in the f-stream.

@abh3
Copy link
Member

abh3 commented Apr 4, 2023

Yes, this is another unfortunate side-effect of tokens. We need to keep architectural integrity here otherwise the whole scheme falls into chaos. So, the solution is not very straightforward. Fortunately, we have a couple of years to figure this out and I am sure we will.

@bbockelm
Copy link
Contributor Author

bbockelm commented Apr 4, 2023

So, the solution is not very straightforward

What's wrong with this:

 struct XrdXrootdMonFileLFN
      {
       kXR_unt32           user;     // dictid for the user
       char                lfn[1032];// Variable length, nul-terminated.
      };

 struct XrdXrootdMonReqAuth
      {
       char                authinfo[1032]; // Variable length, nul-terminated,
                                           // similar to authinfo in the `u` record but remove
                                           // connection-specific items like hostname or appinfo.
      };

struct XrdXrootdMonFileOPN
      {
       XrdXrootdMonFileHdr Hdr;      // recType == isOpen
       long long           fsz;      // file size at open
       XrdXrootdMonFileLFN ufn;      // OPTIONAL, present if recFlag & hasLFN is set.
       XrdXrootdMonReqAuth authn;    // OPTIONAL, present if recFlag & hasAuth is set.
       };

Fortunately, we have a couple of years to figure this out and I am sure we will

I'm not sure why there's a couple of years here. Most of the authenticated data in OSDF uses tokens. Isn't ~99% of the LHC WAN data moved using macaroons (and hence missing f-stream data) as well?

@abh3
Copy link
Member

abh3 commented Apr 6, 2023

Before we get much further let's do some basic stuff. First, what is the actual contents of authinfo when a token is used to populate that field. We already know what it is for non-token scenarios but we need to document what it is for token scenarios. Second, how is this field populated? That is, what should be called with what API or should one get some other structure and populate the filed from that? It's all pretty much a black box right now.

@bbockelm
Copy link
Contributor Author

bbockelm commented Apr 6, 2023

First, what is the actual contents of authinfo when a token is used to populate that field

Of course, currently there is nothing. Here's how the fields are populated from a XrdSecEntity object (from https://github.com/xrootd/xrootd/blob/master/src/XrdXrootd/XrdXrootdXeq.cc#L4037-L4048):

snprintf(Buff,sizeof(Buff),
    "&p=%s&n=%s&h=%s&o=%s&r=%s&g=%s&m=%s%s&I=%c",
    Client->prot,
    (Client->name ? Client->name : ""),
    (Client->host ? Client->host : ""),
    (Client->vorg ? Client->vorg : ""),
    (Client->role ? Client->role : ""),
    (Client->grps ? Client->grps : ""),
    (Client->moninfo ? Client->moninfo : ""),
    (Entity.moninfo  ? Entity.moninfo  : ""),
    (clientPV & XrdOucEI::uIPv4 ? '4' : '6')

For the XrdSciTokens plugin (https://github.com/xrootd/xrootd/blob/master/src/XrdSciTokens/XrdSciTokensAccess.cc#L453), here's what's in those fields:

  • prot: Unfilled by plugin, I consider this owned by the authorization plugin (could be ztn, for example, or https).
  • name: Unfilled by plugin.
    • Instead, request.name in extended attributes is set to the mapped username.
    • token.subject is set to the sub claim of the token.
  • host: Unfilled by plugin, not relevant here.
  • vorg: Set to the issuer (iss claim)
  • grps: Set to the groups in the token (wlcg.groups for WLCG tokens). Unused by macaroons plugin.
  • role: Unused by the SciTokens and Macaroons plugin currently.
  • moninfo: Unused by either plugin.

Here's what I'd propose for an authinfo for a request:

  • n: Set to request.name.
  • o: Set to issuer
  • r: Set to role if non-empty.
  • g: Set to groups if non-empty.
  • m: Set to token.subject.

This is to be taken from the XrdSecEntity object associated with the file-open request. I'd propose it's only populated if the request.name extended attribute is set as that attribute indicates something occurred per-request. Notably, p and h are not included. So, for an existing token from the WLCG issuer, here's a proposed authinfo:

n=bbockelm&o=https://cms-auth.web.cern.ch&m=e608252d-e817-4071-b15c-1a72fe557b3f

About 80 characters total.

@bbockelm
Copy link
Contributor Author

@abh3 - thoughts on the above?

@abh3
Copy link
Member

abh3 commented May 26, 2023

We pretty much have the scheme drawn out but the issue here is that the SciTokens library has to issue a new ident event. ow we provide the hook to do that is still undecided. That should happened after the new re-falgamized secEntity has been used for authorization. So, I assume you are OK with that,

@abh3
Copy link
Member

abh3 commented Jun 18, 2023

OK, I am ready to test this. I need some instructions on how to setup the token scheme and send off an open request for a file with an actual valid token. Can you supply the info @bbockelm ?

@abh3
Copy link
Member

abh3 commented Jun 19, 2023

@bbockelm If you tell me in the next 24 hours we may be able to get this into 5.6.0 this week. Otherwise, it will have to wait for the next release, likely a month or two from from now.

@bbockelm
Copy link
Contributor Author

Hi Andy,

Sorry - yesterday was a holiday here.

The easiest way to test out a token is to use the https://demo.scitokens.org/ service. It allows you to put whatever you want into the token and generates a valid signature (which, obviously, means you shouldn't enable such an insecure service beyond demos or tests). If you click on the "SET PAYLOAD TO ACCESS TO PROTECTED AREA", it'll generate a reasonable-looking token. The two things you probably want to change are:

  • Audience. Set this to be the same as what's in your scitokens.cfg
  • Scope paths. Set to the location your server is exporting.

Here's an example scitokens.cfg:

[Global]
audience = https://xrootd.example.com:8000

[Issuer DEMO]
issuer = https://demo.scitokens.org
# Prefix that's to be exported.
base_path = /protected/foo
# For controlling mapping, probably not needed for tests
default_user = brian

Brian

@abh3
Copy link
Member

abh3 commented Jun 21, 2023

This ticket has been addressed by commit d7f4b61 with documentation to come in the next few days.

@abh3 abh3 closed this as completed Jun 21, 2023
@amadio amadio added this to the 5.6 milestone Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants