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

ZTN and Scitokens auth #1584

Closed
jthiltges opened this issue Jan 10, 2022 · 40 comments
Closed

ZTN and Scitokens auth #1584

jthiltges opened this issue Jan 10, 2022 · 40 comments
Assignees

Comments

@jthiltges
Copy link
Contributor

When using ZTN to pass the scitoken, the token reaches XrdAccSciTokens::Validate() as expected. However, a later call to XrdAccSciTokens::Access() fails.

const char *authz = env ? env->Get("authz") : nullptr;
if (authz == nullptr) {
return OnMissing(Entity, path, oper, env);

Access() expects to find the token with env->Get("authz") which I believe is only true for HTTPS.

With the current flow, it seems Validate() does a scitoken_deserialize() to process the token, and leaves ACLs to Access(). Then Access() generates ACLs, and puts them into a cache (keyed by the JWT).

As for possible solutions, the token could be stored in Validate() (which seems like something to avoid). Or we could refactor to instead generate the ACLs in Validate().

@xrootd-dev
Copy link

xrootd-dev commented Jan 10, 2022 via email

@bbockelm
Copy link
Contributor

bbockelm commented Feb 8, 2022

Hi all,

Took a quick look at the setup this morning. I think it's behaving correctly but, unfortunately, the correct behavior isn't all that useful.

@abh3 - when we discussed ZTN with the dCache folks, one item discussed is what the ZTN token should do beyond authorizing a session creation (which is all it does today).

Particularly, my recollection was that if the user didn't present a token in a subsequent operation, we wanted to utilize the token from the session to determine whether the operation was authorized.

Implementing the "session token" behavior is fairly straightforward (preserve the session token in a XrdSecEntity attribute, utilize the token later in the authorization later) -- any objections to doing that?

Otherwise, we probably need an XrdCl update to always send a token with operations in addition to the session token.

Thoughts?

Brian

@abh3
Copy link
Member

abh3 commented Feb 9, 2022

I don't have a problem with recording the session token in the attributes. However, we already have a field in the secentity structure for this (i.e. creds and credslen). So, I can do that which is what all other protocols do that are credentials based. When do you want it?

@bbockelm
Copy link
Contributor

Ah, an existing mechanism is even better! The code change is relatively simple, just wanted input on how to approach it.

@alrossi -- does the above proposal match what dCache does?

@alrossi
Copy link

alrossi commented Feb 10, 2022

No, it is precisely what I thought we were not supposed to do. Perhaps I misunderstood the discussion I had with @abh3 but the ZTN token is considered a "throw away" ... that the authorization token which appears as the opaque 'authz=' value needs to be there. I remember this because I originally implemented authorization to fallback on the ZTN token if there is no authz= and then changed it. Obviously, if the fallback is what we want, I can reimplement.

@jthiltges
Copy link
Contributor Author

Agreed that storing the full token seems risky. If a vulnerability exposes memory, or core dumps are shared, valid tokens could be exposed.

An option would be storing tokens in a "de-fanged" state. Validate the token, remove the signature, and store the token into creds. Clients reading from creds trust that the signature was valid. (This might be tricky if libraries can't separate signature checks from expiration checks.)

@bbockelm
Copy link
Contributor

:) @alrossi - Well I'm really happy I brought it up then!

I think it comes down to whether-or-not the client should be changed to append the token to the CGI for all requests. This seems ... well, complex to be honest. The CGI should be inserted at all call points if and only if the user didn't override the authz setting and ZTN module was used for authentication - and the logic would need to be duplicated for token discovery (or a new API between the client and the ZTN module added).

I can talk myself into saying @alrossi was right the first time and this should be used as a session token and allow the client to override it if desired.

@alrossi
Copy link

alrossi commented Feb 10, 2022

Then the original ZTN needs things like the subject attribute which was not required to be there originally.

One of the other issues about the current way it works is that of redirect to dCache pools. We are compelled to force TLS on the pools to protect the token sent with the opaque data, even though dCache does not reauthorize/re-authenticate on the pool. It would be great if the client had a way not to send the token if it does not receive an authentication data request on login.

(Of course, if we ever do implement TPC with tokens in xroot, then all bets are off because the TPC channel would need TLS to protect the token needed by the client on the destination side to authenticate with the source server ...)

@alrossi
Copy link

alrossi commented Feb 10, 2022

(Of course, a way around that issue is to implement control vs data channel kXR_bind, which I have considered and is on the back burner ... )

@bbockelm
Copy link
Contributor

Then the original ZTN needs things like the subject attribute which was not required to be there originally.

I'm not quite following. There's a subject in the token (among many other things, some of which may or may not be useful for authorization). I assume the semantics should be, if the token was not presented with a request, then the token from the session can be used as if it was presented with the request.

We are compelled to force TLS on the pools to protect the token sent with the opaque data

I think TLS should be forced on the pools regardless, token or no token, unless you differentiate between trusted LAN and WAN transfers.

Redirection is another case where using the session token is probably useful - the client doesn't have to keep track as to whether the authz CGI header should be sent or dropped.

@alrossi
Copy link

alrossi commented Feb 10, 2022

Redirection is another case where using the session token is probably useful - the client doesn't have to keep track as to whether the authz CGI header should be sent or dropped.

Yes that is what I was implying (fallback would have that advantage).

I'm not quite following. There's a subject in the token 

ZTN does not check for the user. It only checks issuer and audience. A ZTN token does not require authorization data, only enough to demonstrate it is from a trusted issuer. At least that is how I understood our original discussion.

@bbockelm
Copy link
Contributor

ZTN does not check for the user. It only checks issuer and audience. A ZTN token does not require authorization data, only enough to demonstrate it is from a trusted issuer. At least that is how I understood our original discussion.

Right, that much I was following -- the piece I missed is why reusing the token layer implies a subject attribute is now required (or whether there's a protocol issue). That's all up to what the later authorization framework wants to see to evaluate the authorization of a request. Maybe it decides to allow read requests if and only if the first letter of the token is B!

@alrossi
Copy link

alrossi commented Feb 10, 2022

OK, I get you. But in practical terms ... Anyway, restoring the old behavior should not be an issue.

As for @jthiltges concerns, dcache does not write out the token to disk, so only a memory dump of the JVM would expose the token, though that would be true for any of the token processing.

@abh3
Copy link
Member

abh3 commented Feb 10, 2022

OK, there is a lot of stuff going on here. So, let's go through what zrn was supposed to be for and what it seems people want it to do.

If you go back to the original discussion, we wanted to know that a client connecting to a server was capable of getting valid tokens. That's all. We didn't care who that was, what they were going to do, or anything else. All we cared about is showing proof of capability.

This is why it was implemented the way it was. The token only needs to be validated. We don't need any generated acls. All we need is that the token is for the acceptable audience issued by a known token issuer. Period, nothing more. That's why we never saved that token.

Now, it seems people want feature creep and want access to the token "just in case". We can do that by using the standard mechanism of placing the token in the creds field of the SecEntity structure and I would strongly resist any other approach as this is the standard way X509, Krb5 and other credential protocols do. As for exposure due to a core file, the token has an expiration date and hopefully these tokens are issued with a reasonably short lifetime so by the time the core file is exposed to unauthorized people (which would have a lot of other security implications outside of SciTokens), the token would have expired. So, I don't see this as very serious concern beyond exposing a core file no matter what.

So, my understanding that all that is wanted is to be able to access the ztn token in the case that a request does not have a url token. This, of course, runs in the face of narrow tokens and we might get stuck with fat tokens which is counter to what everyone wants. So, I don't see how this will play well in the future.

Have I missed something here?

@bbockelm
Copy link
Contributor

Have I missed something here?

I think the question is "how does one use token authorization using XrdCl or xrdcp?"

With the various issues around redirection @alrossi pointed out above, it's not clear that there's a great way to do this as a user. The proposal above, to utilize the ZTN token as a session token, rather neatly addresses things with neither client nor protocol changes.

Let's say that a user has a single token and would like to download a single file with xrdcp. How does one do this?

@abh3
Copy link
Member

abh3 commented Feb 19, 2022

The way you would do it is to place the token on the url in the standard way:

xroot;//host//my/path?authz=[brearer%20]

Yes, this is very inconvenient but no worse than what you have to do for AWS. The nit here is "bearer%20" you may need to add that if the token you got doesn't have it. I've seen tokens both ways and I wish we could standardize that because it will confuse people to no end. Generally, the idea was that a url provider would give you a fully formed url so you don't think twice about it. Of course, for the one-off copies or whatever there is no url provider and, in fact, I have no idea how a person would get a token for a one-off copy (probably harder than we think; enough to make a user want x509 back).

Now, to other stuff. Whatever cgi a user specified should be carried across redirection. If that is not being done then it is a bug. I think we corrected that problem for certain edge cases some time ago. I don't know if that is also true for dCache but it should be as it's part of the protocol specification.

As for @alrossi getting confused, he should be confused because the "backup token" idea came out of the blue. So, Al you are right, the original plan was exactly what you thought it was. That said, I don't mind extending things. I just want to make sure it doesn't create new problems. I think @alrossi is on board with that and can make the appropriate changes. I can place the token in the creds field as it's a no-brainer. Then you can use it as you see fit in the SciTokens plug-in.

Anyway, let me know whether or not you want access to the ztn token. Please @bbockelm finish off the review comments in the voms mapfile pull request. We are almost there.

@alrossi
Copy link

alrossi commented Feb 19, 2022 via email

@bbockelm
Copy link
Contributor

The way you would do it is to place the token on the url in the standard way:
xroot;//host//my/path?authz=[brearer%20]

But this doesn't really work, does it? You can't put secrets into the command line because they are visible across the system. One would need to re-implement the token discovery logic external to the ZTN library and put it in xrdcp and XrdCl to have it auto-discover the token and/or have the user explicitly provide a token file.

Secondly, this doesn't work because the above invocation is only safe when TLS is used on the transport layer. Again, this is only known at the XrdCl level and not a priori by the individual invoking the client.

@abh3
Copy link
Member

abh3 commented Feb 28, 2022

Well, i believe it does because that's what the code allows. Indeed, you are correct that the user should use TLS to transmit the token via the URL. Unfortunately, nothing stops the user from not using TLS. I suppose we could test for the presence of 'authz' cgi in the client and force using TLS if it's there. I think the basic problem is that there is no defined way of how a user gets a one-off token, is there? ATLAS (and likely CMS) plans on having Rucio add the authz to the UR before handing back to the user who asked for it. Well, that's the idea anyway. Like I said, I have no issues of pointing to the ztn token from the SecEntity object. My question is whether the ztn token is valid for all file accesses the user wants to make; and if so, doesn't it make it a fat token?

@abh3
Copy link
Member

abh3 commented Mar 5, 2022

OK, so now ztn will point to the initial token via the Entity.creds field. Can I close this?

@abh3 abh3 self-assigned this Mar 9, 2022
@abh3
Copy link
Member

abh3 commented Mar 9, 2022

After a 2.5 hour discussion on this topic, I do see the benefit of extending what ztn does as it's very much the same what a user would do in many circumstances. There is one caveat, if we allow the ztn token to be used as the default token(which is now possible to do in he SciToken authorization plug-in) then it either we do it always or we never do it. There is nothing in between if we want to provide reliable and consistent SciToken authorization. That mean a bit of work for @alrossi but I don't think he minds as long as the result leads to a better user experience. So, what is the verdict here?

@wyang007
Copy link
Member

wyang007 commented Mar 9, 2022

In is currently way, I have to put a token in both $BEARER_TOKEN, etc. and in CGI. Perhaps there is good reason behind it but this is not what users are expected.

In fact, what appear in the CGI doesn't have to be a SciToken. It can be a macaroon token (passing a macaroon token is independent from Sec). I wonder whether there is a value to explore this - I view a macaroon token as something similar to the function of a signed URL. Maybe this one belongs to a different topic.

@alrossi
Copy link

alrossi commented Mar 9, 2022 via email

@wyang007
Copy link
Member

wyang007 commented Mar 9, 2022

I expect the native xrootd TPC will not dead. xrootd TPC operations in two modes: X509 delegation and rendezvous key. The latter is used in cases where X509 is not used.

@alrossi
Copy link

alrossi commented Mar 9, 2022 via email

@wyang007
Copy link
Member

wyang007 commented Mar 9, 2022

robocert is not rendezvous key. It is something prior to x509 delegation so we no longer need robocert. rendezvous key is a time limited shared secret sent by the client to both endpoints, to facilitate TPC. It needs TLS.
Whether dCache want to support rendezvous key is something to be discussed. The primary TPC in HEP is http though not exclusive. The rendezvous key in xrootd is used where x509 infrastructure is not available (and likely the token infrastructure is also not available, somewhere outside of HEP).
When x509 goes away, in principle, we can use token for authentication and then use rendezvous key for TPC (to avoid a more complexed token chain reaction), though someone will say that token is only for authorization :-)

@alrossi
Copy link

alrossi commented Mar 9, 2022 via email

@abh3
Copy link
Member

abh3 commented Mar 9, 2022 via email

@bbockelm
Copy link
Contributor

OK, so now ztn will point to the initial token via the Entity.creds field. Can I close this?

Yes, I think this is good enough to close! Will try to send a corresponding PR today / tomorrow.

@alrossi
Copy link

alrossi commented Mar 11, 2022 via email

@abh3
Copy link
Member

abh3 commented Mar 11, 2022 via email

@alrossi
Copy link

alrossi commented Mar 11, 2022 via email

@alrossi
Copy link

alrossi commented Mar 23, 2022 via email

@abh3
Copy link
Member

abh3 commented Mar 24, 2022

Hi Albert,

Correct, while TLS would have been preferable the additional safeguards put in place to protect the rendezvous token made it highly unlikely to be subverted (you would need to accomplish at least three other subversion before the token became vulnerable). Now that TLS is widely available nothing prevents the token being transmitted using TLS, though for backward compatibility, it's not required.

As you state it, the door is not in a position to use the rendezvous token presented by the destination since it needs to authenticate the destination first. So, if that's how the code sits then, yes, without skipping authorization in the presence of a token, it would not work. Mind you rendezvous tokens are only meant to be used for pull requests so they are incapable of performing and destructive action.

So, presumably you are wondering that one could get around the problem by having the destination use a JWT along with the rendezvous token. The issue here is how does one get the JWT? I was told that it can't be the requesting clients JWT as these are not delegable, though I still don't see what prevents them from being used by someone else, hence why they need TLS. The destination server is also not really in a position of getting a JWT. Seems like an issue here.

Of course, that all hinges on whether the requesting client's JWT cannot be used by the destination server. If it can be (or at least if the client can get a delegable token) then, yes, we could handle the rendezvous in that way. That said, why bother with a rendezvous token at all since we could do the same thing with JWT's can't we?

So, in the end we need to get all of this straightened out to figure out the positioning of a rendezvous TPC. Maybe @bbockelm will see this post and expand on how a requesting client's JWT can and cannot be used in a TPC context by the destination server.

@alrossi
Copy link

alrossi commented Mar 24, 2022 via email

@alrossi
Copy link

alrossi commented Mar 24, 2022 via email

@alrossi
Copy link

alrossi commented Mar 24, 2022 via email

@bbockelm
Copy link
Contributor

bbockelm commented Apr 6, 2022

Chatting with Al at the WLCG BDT meeting today -- we think this ticket can be closed. The xrootd-TPC discussion is alive and healthy on the mailing list but not exactly relevant to the original ticket item. Would be good to create a fresh issue if we want to bring it to GitHub.

@abh3
Copy link
Member

abh3 commented Sep 30, 2022

As requested.

@abh3 abh3 closed this as completed Sep 30, 2022
@alrossi
Copy link

alrossi commented Oct 11, 2022 via email

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

6 participants