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

Initial code to internally proxy security credentials with a macaroon. #1115

Closed
wants to merge 1 commit into from

Conversation

bbockelm
Copy link
Contributor

This causes HTTP requests coming in using an X509 proxy to generate a new macaroon used internally. Since the macaroon serializes the security session, this avoids problems to forward authorization if this is a gateway.

This causes HTTP requests coming in using an X509 proxy to generate
a new macaroon used internally.  Since the macaroon serializes the
security session, this avoids problems to forward authorization
if this is a gateway.
@bbockelm
Copy link
Contributor Author

@ffurano - this is the code we had been discussing today: it should allow an EOS gateway to forward authorizations along as if the HTTP request came in with a token.

With this, the "HTTPS smoke tests" work when run in gateway mode!

@simonmichal
Copy link
Contributor

simonmichal commented Jan 21, 2020

@bbockelm : you are linking now LIB_XRD_HTTP_UTILS against MACAROONS_LIB :

https://github.com/xrootd/xrootd/pull/1115/files#diff-db7a029d325861c5a1b6bacccc3f20edR55

but there is no guard checking if MACAROONS_LIB has been found in the first place, as a result if the HTTP component is being built and the macaroons were not found the build will fail, e.g. see:

https://travis-ci.org/xrootd/xrootd/jobs/637440081?utm_medium=notification&utm_source=github_status

this needs fixing before we can merge ;-)

@djw8605
Copy link
Contributor

djw8605 commented Jan 30, 2020

If you just want to make the linking to the Macaroons library condition on BUILD_MACAROONS, I attached a patch here that should do that.

0001-Conditional-link-Macaroons-lib.txt
(it's a git am compatible patch file)

@simonmichal
Copy link
Contributor

The question is: after this PR gets merged will XrdHttp be still functional if libmacaroons are missing? this is to be answered by @ffurano and @bbockelm and possibly @abh3.

(we are building for platforms were libmacaroons are not available, though I don't think anyone is using those platforms to run XrdHttp)

@bbockelm
Copy link
Contributor Author

bbockelm commented Feb 4, 2020

Hi @simonmichal -

(Sorry for the delayed response - travel last week really knocked me out of commission)

Cleaning up the various build issues and ensuring things are conditionally compiled only on platforms with libmacaroons present are relatively easy, about an evening's work (in addition to the items @simonmichal pointed out, there are going to be a few #ifdefs in the C++ code.

However, I'm still waiting to hear back from @ffurano on whether this new feature is the way he wants to go with the code. When we originally talked about this at CERN, he was reluctant about the whole approach given that he and Andy had previously discussed a different path based on SSI security instead.

If folks are willing to merge this, I can do the remaining untangling of the build...

Brian

@ffurano
Copy link
Contributor

ffurano commented Feb 24, 2020

Hi,

I think that your comment tells it all:

"useful when an internal filesystem (such as XrdPss) does not have the ability to forward an XrdSecEntity but might understand macaroons."

The approach is solid, it's a pity that the internal theoretically orthodox approach (i.e. through XrdSec) has to be worked around in such ways, and I totally see the point for this kind of things.

I can merge it, will you please fix the build errors Brian?

@abh3
Copy link
Member

abh3 commented Feb 24, 2020 via email

@simonmichal
Copy link
Contributor

For the record, the SSS approach is being tracked in #1138, @abh3 : could you give us a timeline for this development? I suppose if there is a consensus that #1138 makes this PR unnecessary we could close this it, @bbockelm , @ffurano what's your opinion?

@xrootd-dev
Copy link

xrootd-dev commented Feb 25, 2020 via email

@ffurano
Copy link
Contributor

ffurano commented Feb 25, 2020

Hi,
this discussion is very positive and welcome indeed, and we all share the need of forwarding security credentials with a certain level of flexibility.

I understand the direction of willing to keep the xrootd protocol in the back of a gateway, and it makes sense to me.

Even if I like the idea of a macaroon for this use case I'd also vote for the XrdSSS approach, and hope that it becomes able to forward the additional info that an XrdSecEntity instance can hold.

@bbockelm
Copy link
Contributor Author

I’m certainly happy to close this out (SSS is a more comprehensive approach if taken) if the 5.1 timeline works for EOS! Maybe ping Elvin on it?

(At any rate, glad we forced the discussion and didn’t try to merge it into 4.11.2!)

@simonmichal
Copy link
Contributor

I've just crosschecked with Elvin and it seems he no longer needs a proxy for HTTP, hence from EOS side there's no desire for this PR.

@xrootd-dev
Copy link

xrootd-dev commented Feb 26, 2020 via email

@simonmichal
Copy link
Contributor

I suppose we can close this PR?

@bbockelm
Copy link
Contributor Author

bbockelm commented Mar 9, 2020

Yup! Looks like I have the ability to close this ... thanks for the feedback everyone!

@bbockelm bbockelm closed this Mar 9, 2020
@xrootd-dev
Copy link

xrootd-dev commented Mar 9, 2020 via email

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

Successfully merging this pull request may close these issues.

None yet

6 participants