XrdMacaroons: Allow chaining user defined XrdAccAuthorize plugin #1147
Conversation
…thorize plugin and not only the default XrdAcc plugin. The library holding the user defined plugin can be specified as a paramter to the XrdMacaroons using a special key "chain_authz=". All previous behavior remains unchanged. Eg. http.exthandler xrdmacaroons libXrdMacaroons.so chain_authz=libCustomXrdAuthorize.so The same mechanism holds when the Macaroons::Authz plugin is loaded, it can be chained to any other XrdAccAuthorize plugin as follows: ofs.authlib libXrdMacaroons.so chain_authz=libCustomXrdAuthorize.so
|
We cannot let this go in Elvin. It completely conflicts with the way this is done in R5 and we can't have two different mechanisms competing in the same code base. I know you spent a lot oftme on this but it really completely undoes everything we did in R5 to solve this problem. In your particular case, either port the R5 back to 4.11 or please branch the code. |
|
Hi Andy, I had a look at R5, more specifically at this commit which adds support for stacking different OFS plugins: but I don't quite see how this applies in this case where I would like to control the authorization plugin which is loaded by an XrdHttpExtHandler plugin namely XrdMacaroons? I looked at the XrdMacaroons code in R5 and it's no different to what is now in 4.11. I'm happy to adapt my approach if you can tell me how I can have the XrdMacaroons load a different authz plugin here: Thanks! |
|
Hi Elvin, You are dealing with some pretty nasty code. I agree that the stacking mechanism in Macaroons is pretty ad hoc and we are trying to standardize how plugins are stacked. There seems to be precious little information on how macaroons fits into the whole authorization scheme. The idea of simply stacking the default authorization is bad and it's even worse by using the linking trick (I know you tried to correct that but the code got even more obscure). You are correct that there was no change to Macaroons but the whole stacking phenomena should follow the convention: ofs.authlib ++ See: https://xrootd.slac.stanford.edu/doc/dev50/ofs_config.htm#_Toc8244755 It's not clear to me where the initial load goes and what is the directive being used to load this plugin. But how it manages stacking is pretty bad (as I said) and I don't want it to get even worse by plastering it over with patches. The other issue is that we normally do back-ports not forward ports. The pull request should have been against git head not a stable release, especially when it's in a release cycle. It also puts us in the position of dropping it unless we port forwards. So, what directive controls this Macaroons plugin? How does it relate to XrdAccAuthorize? It really seem pretty hacky the way it is now (which has nothing to do with your patches). |
|
FWIW - a lot of the direct linking code is due to the fact there's not a public API (and all this code originated as an external plugin). That's an opportunity for code cleanup now that this plugin has access to the private Xrootd APIs. From the So, authorizations are first examined by @esindril - can you kindly remind me of the use case here? I recall talking about it but don't think I wrote it down. That is, if you have ? |
|
OK, so in R5 all of this is handled automatically and no direct linking is
required. To get the same result (with an arbitrary level of stacking),
assuming the base authorization plugin should be the default one, as
follows:
ofs.authlib ++ libXrdAccSciTokens.so
ofs.authlib ++ libXrdMacaroons.so
Then libXrdMacaroons.so would be called first. If it doesn't like the
thing it sees, it passes the call to libXrdAccSciTokens.so (it gets a
pointer to the next stacked plugin). That, in turn, can pass it off to the
default plugin if it wishes. You can, of course, declare one of those as
the base plugin by not specifying "++". That keeps backward compatability.
It's all structured to allow one to write a plugin with only the pulic
API's. So, yes, all of that should be cleaned up. The whole scheme is
available in git head and documented in the XrdAccAuthorize.hh file on how
to structure the code for base or push mode (or both).
Andy
…On Wed, 4 Mar 2020, Brian P Bockelman wrote:
FWIW - a lot of the direct linking code is due to the fact there's not a public API (and all this code originated as an external plugin). That's an opportunity for code cleanup now that this plugin has access to the private Xrootd APIs.
> From the `ofs.authlib` point-of-view, an example of how it's loaded now is:
```
ofs.authlib libXrdMacaroons.so libXrdAccSciTokens.so
```
So, authorizations are first examined by `libXrdMacaroons`; if the result is inconclusive, then it's passed to `libXrdAccSciTokens`; if that result is inconclusive, then it is passed to the default plugin.
@esindril - can you kindly remind me of the use case here? I recall talking about it but don't think I wrote it down. That is, if you have `libXrdAccEOS`, why doesn't the following work:
```
ofs.authlib libXrdMacaroons.so libXrdAccEOS.so
```
?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#1147 (comment)
########################################################################
Use REPLY-ALL to reply to list
To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1
|
|
This sounds very promising! (One aside on the manual text for this - I was very confused by this sentence in the manual: I read this as saying "this can only be used once", which somewhat contradicts what you wrote above. A clarification in the manual would be appreciated.) The second unpleasant piece in this code is the fact that, within the HTTP handler, we need to have access to the Is there such a way to do that? This is currently accomplished by directly constructing the object and via a reimplementation of the Thanks! |
|
On Wed, 4 Mar 2020, Brian P Bockelman wrote:
This sounds very promising!
Yes, it was written specifically to handle multi-mode authorization which
SciTokens essentially introduces (didn'y know Macaroons did the same).
(One aside on the manual text for this - I was very confused by this
sentence in the manual:
```
++ The specified plug-in should stack on top of the existing
plug-in or default. Once specified, it cannot be overridden by a
subsequent directive,
```
I read this as saying "this can only be used once", which somewhat
contradicts what you wrote above. A clarification in the manual would
be appreciated.)
OK, poor wording on my part. You can always override the base plugin with
a subsequent directive. That's the one without a "++". You can't eally
extend this o any plugin that was stacked with a "++". Once stacked, it
reains stacked. All you can do is add another one to the stack. That's
what I was trying to say. So, I'll reword that in all the places it
appears. I say that because the syntax was extended to most plugins. Yes,
you can stack all the plugins that matter at he mment and we can extend it
to additional ones should the need arrise.
The second unpleasant piece in this code is the fact that, within the
HTTP handler, we need to have access to the `XrdAccAuthorize` object
used by the OFS to generate the appropriate permissions for the
Macaroon.
Yes, I see the problem. I was going to add an "Access(const path[, mode])"
to the whole thing so you can get that information. Then I had second
thoughts as I didn't see who would use that. I guess I was wrong here. I
can add that feature and it will be available via the bridge as an fsctl()
operation. I assume that Access() as defined by POSIX will do the trick
for you. Either it says yes or no or returns the mode bits allowed. Right?
Is there such a way to do that? This is currently accomplished by
directly constructing the object and via a reimplementation of the
ofs.authlib` logic. There's not much gain if we significantly clean up
this usage only to add more complexity elsewhere. What you point out
only helps the case of making an authorization object for the `ofs` and
not recreating it in the plugin.
Yes, I am trying to desparately avoid doing this and I think we can do
that pretty easily.
Thanks!
You're welcome.
Andy
|
|
Hi Brian,
Just realized, your plugin doesn't know anything about the bridge, right?
If so, I can plop a pointer to an authorization object in the XrdOucEnv
object passed to the protocol plugin. Then you'll have access to all the
methods using whatever the config was in the ofs. Will that be better for
you?
Andy
…On Wed, 4 Mar 2020, Brian P Bockelman wrote:
This sounds very promising!
(One aside on the manual text for this - I was very confused by this sentence in the manual:
```
++ The specified plug-in should stack on top of the existing plug-in or default. Once specified, it cannot be overridden by a subsequent directive,
```
I read this as saying "this can only be used once", which somewhat contradicts what you wrote above. A clarification in the manual would be appreciated.)
The second unpleasant piece in this code is the fact that, within the HTTP handler, we need to have access to the `XrdAccAuthorize` object used by the OFS to generate the appropriate permissions for the Macaroon.
Is there such a way to do that? This is currently accomplished by directly constructing the object and via a reimplementation of the `ofs.authlib` logic. There's not much gain if we significantly clean up this usage only to add more complexity elsewhere. What you point out only helps the case of making an authorization object for the `ofs` and not recreating it in the plugin.
Thanks!
--
You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub:
#1147 (comment)
|
|
@bbockelm Concerning your question:
I think you already provided the answer in you subsequent comment:
I actually have a similar patch for the XrdSciTokens library, but I'll for the the things to settle on a cleaner solution for all this chaining business.
I think this looks reasonable and it's already cascaded from the XrdHttpProtocol to any XrdHttpExtHandler object. Anyhow, this will only be available in R5, right? |
|
It's a fairly simple change. It could be easily back ported. We already have an issue with 4.11.3 that will likely mean another RC. Not clear that it would be appropriate sneaking this in there. I hate releasing 4.11.4 because every time we create a new 4x release it postpones the 5 release and we can't go on like this forever. So, how critical is this for you so that you don't want to wait for R5? |
I think this would work; at least, would be willing to test for v5. I do eventually need both a pointer to the authorization object and for the filesystem object (for For v4 versus v5: I'm OK with showing this works with v5 and seeing how much nasty code this allows us to remove -- and then making a decision about a backport. |
|
@abh3 - any updates on this? |
|
Yes, the env passed around will have three additional keys: XrdAccAuthorize* pointer to the authorization plugin The patches are already in R5 and will be back ported to 4.11.3. |
|
@abh3 -- I think #1159 has a better approach for Xrootd 5 (and keeps the backward compat). So the question is - what should we do for the 4.11 branch? @esindril - do you have any workarounds? I would be OK with something somewhat hack-ish going in just to get EOS un-stuck, especially if we have something better going forward. |
|
Yes, I agree that 1159 is much cleaner. BTW you know that the pointer will be zero if authorization was not enabled. So, you may want to rethink how to handle that. I suppose I should have added yet another envar to tell you that. Is not configuring authorization something that is acceptable to the Macaroon support? As for a work around, I'm looking for suggestions. I guess you need the stacking framework to make it all workable, right? |
This extends the current behavior and keeps full backwards compatibility with the way XrdMacaroons was loading the (default) authorization plugin or was chaining the XrdAccSciTokens library. I've just added support for specifying a custom library which provides a XrdAccAuthorization plugin to be chained to XrdMacaroons (both to the http and authz plugins).
This is done by using as configuration parameter the key "chain_authz=libCustomAcc.so". We need this since EOS for example does not use at all the default XrdAcc plugin but provides its own authorization plugin. This will be useful also for other projects that want to rely on XrdMacaroons but have their own authorization plugin.
It would be great if this gets merged into 4.11.3.
@bbockelm, could you please review it?
Thanks!