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

Fleet commander: store deskprofiles under user running SSSD #7119

Closed
wants to merge 1 commit into from

Conversation

alexey-tikhonov
Copy link
Member

Integrated feature was never oficially released, but the latest development status was:

org.freedesktop.FleetCommanderClient is run as root

and can read profiles doesn't matter files ownership ( https://lists.fedorahosted.org/archives/list/[email protected]/message/IG3MIET5MILWJZRS3JQWMTVOPGNY6XWI/ )

Actual status is that 'FleetCommanderClient' isn't really maintained.

Storing profiles under user that runs SSSD doesn't break anything but removes the need for CAP_SET_?ID and CAP_CHOWN (in this code).

@alexey-tikhonov alexey-tikhonov added the no-backport This should go to target branch only. label Jan 8, 2024
@alexey-tikhonov alexey-tikhonov force-pushed the fc branch 2 times, most recently from c64d6c3 to df49404 Compare January 8, 2024 11:39
@alexey-tikhonov
Copy link
Member Author

Fails at F40 are unrelated.

@justin-stephenson
Copy link
Contributor

Actual status is that 'FleetCommanderClient' isn't really maintained.

But do users actively use it? I've never used Fleet Commander, is there someone willing to test this PR does not break their working setup?

@alexey-tikhonov
Copy link
Member Author

alexey-tikhonov commented Jan 9, 2024

Actual status is that 'FleetCommanderClient' isn't really maintained.

But do users actively use it?

Depends on definition of "actively" but in general "I don't think so".

I've never used Fleet Commander, is there someone willing to test this PR does not break their working setup?

About 1st patch: I think @stanislavlevin said they use something similar in ALT Linux and this works for them (at least it was my understanding from #5888 (comment))

About 2nd patch: this, of course, does break working config - it will require to set session_provider config option explicitly. This patch does two things:

  • session_provider option was simply ignored, it's fixed now
  • default changed to 'none'; I don't insist on this, but, imo, it makes sense because vast majority of IPA users do not use fleet commander (IIUC) yet it generates unnecessary LDAP traffic.

@alexey-tikhonov
Copy link
Member Author

alexey-tikhonov commented Jan 10, 2024

Comments from @abbra about changing defaults:

Technically, you can identify this feature by presence of its LDAP schema. This is something easily discoverable but would require storing something in a domain struct, probably. https://github.com/abbra/freeipa-desktop-profile/blob/master/plugin/schema.d/75-deskprofile.ldif
I am worried about existing deployments. These tend to be not a single PC style ones, so updating deployed systems is a bit hard.
Changing the code to only query whether the scheme is present at domain online check would achieve pretty much the same but would reduce LDAP traffic as you wanted.

@alexey-tikhonov
Copy link
Member Author

Ok, I removed "default changed to 'none'" part from the 2nd patch and created #7123 to implement run time detection.

@alexey-tikhonov
Copy link
Member Author

For the sake of completeness, "non-privileged" runs are in #7120. But I don't think it matters much because I don't think tests hit those codepaths.

Copy link
Contributor

@justin-stephenson justin-stephenson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, thank you.

Copy link
Member

@pbrezina pbrezina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we talked on slack, the second patch is not needed. The target constructor is only run if the provider is ipa.

Integrated feature was never oficially released, but the latest
development status was:
```
org.freedesktop.FleetCommanderClient is run as root
```
and can read profiles doesn't matter files ownership
( https://lists.fedorahosted.org/archives/list/[email protected]/message/IG3MIET5MILWJZRS3JQWMTVOPGNY6XWI/ )

Actual status is that 'FleetCommanderClient' isn't really maintained.

Storing profiles under user that runs SSSD doesn't break anything
but removes the need for CAP_SET_?ID and CAP_CHOWN (in this code).

Resolves: SSSD#4659
@alexey-tikhonov
Copy link
Member Author

As we talked on slack, the second patch is not needed. The target constructor is only run if the provider is ipa.

Removed 2nd patch

@alexey-tikhonov alexey-tikhonov linked an issue Jan 29, 2024 that may be closed by this pull request
Copy link
Contributor

@sumit-bose sumit-bose left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

sorry for miss-reading the patch, I have no further comments, ACK.

bye,
Sumit

@alexey-tikhonov
Copy link
Member Author

Pushed PR: #7119

  • master
    • c11734e - Fleet commander: store deskprofiles under user running SSSD

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-backport This should go to target branch only. non-privileged Pushed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Fleet Commander related code work for unprivileged users
4 participants