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

[XrdCl] Make xrd.secuid take effect only if euid == 0 #629

Closed
wants to merge 1 commit into from

Conversation

gbitzes
Copy link
Contributor

@gbitzes gbitzes commented Nov 22, 2017

No description provided.

@bbockelm
Copy link
Contributor

Is that the right approach? Shouldn't you instead check for the correct capability?

After all, I can construct situations where user xrootd is allowed to do the UID-switch and situations where root is not.

@abh3
Copy link
Member

abh3 commented Nov 22, 2017

I think that Brian is correct here. Having euid 0 doesn't necessarily mean much in certain OS's. For instance, in BSD I can deny certain capabilities even for root.

@gbitzes
Copy link
Contributor Author

gbitzes commented Nov 23, 2017

I agree this is not the best approach: It's a workaround for the fact "xrd.secuid" is passed on to the server, even though it's only needed locally on the client. Checking if euid == 0 is a workaround to stop our disk servers from getting confused, as they aren't running as root.

I had a chat with Michal, he thinks it should be easy to add internal CGI parameters not passed on to the server, acting strictly as directives for the client.. This would solve the issue we're seeing on EOS.

Let's postpone this pull request for now, as the above is far cleaner, let's see how it goes.

@simonmichal
Copy link
Contributor

@gbitzes : could you please test 2be1931 and let me know if it solves your problem?

@gbitzes
Copy link
Contributor Author

gbitzes commented Nov 23, 2017

@simonmichal works great, thanks a lot. I also verified xrdcl.secuid doesn't end up on the server side by checking the logs, everything seems OK.

No more need for the ugly hack contained in this pull request, I close it.

@gbitzes gbitzes closed this Nov 23, 2017
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

4 participants