tequeter's pull request #21: "IFP: expose user and group unique IDs through DBus" was opened
PR body: """ This adds a uniqueID property on User and Group InfoPipe objects. It has a useful value on AD- and IPA-backed domains. For Active Directory, this is the GUID. """
See the full pull-request at https://github.com/SSSD/sssd/pull/21 ... or pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/21/head:pr21 git checkout pr21
pbrezina commented on a pull request
""" Hi, thank you for the patch. It looks good and works fine. What do you plan to use it for? """
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-245878446
tequeter's pull request #21: "IFP: expose user and group unique IDs through DBus" label *Accepted* has been added
See the full pull-request at https://github.com/SSSD/sssd/pull/21
tequeter commented on a pull request
""" One of my customers has an in-house GNU/Linux desktop application they use on many remote tiny sites with unreliable WAN links. So far the application was doing local authn/authz using a database, but the customer is migrating their remote employees to the central AD for easier access to new centralized applications.
I will deploy SSSD on the remote desktops to ensure application availability in case of WAN failures, and the customer will update their application to authn with a SSSD-enabled PAM service and authz with InfoPipe. However, the application still needs to map the AD groups of the users to its internal permission system.
I considered using the gid provided by SSSD for that purpose (but it is not guaranteed to be consistent on all computers, from sssd-ldap(5)/ID MAPPING), or the group name (but that felt even more brittle). So, we decided to use the more robust GUID at the cost of creating this patch. """
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-245950785
lslebodn commented on a pull request
""" On (09/09/16 08:39), tequeter wrote:
One of my customers has an in-house GNU/Linux desktop application they use on many remote tiny sites with unreliable WAN links. So far the application was doing local authn/authz using a database, but the customer is migrating their remote employees to the central AD for easier access to new centralized applications.
I will deploy SSSD on the remote desktops to ensure application availability in case of WAN failures, and the customer will update their application to authn with a SSSD-enabled PAM service and authz with InfoPipe. However, the application still needs to map the AD groups of the users to its internal permission system.
I considered using the gid provided by SSSD for that purpose (but it is not guaranteed to be consistent on all computers, from sssd-ldap(5)/ID MAPPING),
Could you quote please?
I can imagine inconsistent GID only in case of different id mapping configuration on different computers. But I do not understant why someone would do that. The ideal is to use default id mapping on all machines. and you would get the same GID everywhere.
LS
"""
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-245996198
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
jhrozek commented: """ I also wonder if the SIDs are needed. I don't have anything to add them in principle, except I would prefer to not add more attributes to the bus unless necessary, because then we have to keep them.
So if you can find out the ID mapping is good enough for you, I think just going with ID numbers might be better. """
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-247596888
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
lslebodn commented: """ On (16/09/16 06:12), Jakub Hrozek wrote:
I also wonder if the SIDs are needed. I don't have anything to add them in principle, except I would prefer to not add more attributes to the bus unless necessary, because then we have to keep them.
So if you can find out the ID mapping is good enough for you, I think just going with ID numbers might be better.
BTW, I think that even with current version of sssd you could get UUID That was a purpose of config option ```user_attributes```. Please check man sssd-ifp -> user_attributes
LS
"""
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-247605525
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
tequeter commented: """
I considered using the gid provided by SSSD for that purpose (but it is not guaranteed to be consistent on all computers, from sssd-ldap(5)/ID MAPPING),
Could you quote please?
From sssd-ldap(5):
NOTE: It is possible to encounter collisions in the hash and subsequent modulus. In these situations, we will select the next available slice, but it may not be possible to reproduce the same exact set of slices on other machines (since the order that they are encountered will determine their slice).
The customer will be performing authorization at application level by matching the group identifiers to identifiers "well known" to the application. Thus they must have a value guaranteed to be identical everywhere.
In that regard GUIDs seem rock-solid, while hashed values sound more leaving a ticking bomb behind me (new domains, mergers etc.)
As for ```user_attributes```: it's not available for groups, only for users. It would have fit the bill perfectly otherwise. """
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-247951686
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
jhrozek commented: """ On Mon, Sep 19, 2016 at 02:49:21AM -0700, tequeter wrote:
I considered using the gid provided by SSSD for that purpose (but it is not guaranteed to be consistent on all computers, from sssd-ldap(5)/ID MAPPING),
Could you quote please?
From sssd-ldap(5):
NOTE: It is possible to encounter collisions in the hash and subsequent modulus. In these situations, we will select the next available slice, but it may not be possible to reproduce the same exact set of slices on other machines (since the order that they are encountered will determine their slice).
The customer will be performing authorization at application level by matching the group identifiers to identifiers "well known" to the application. Thus they must have a value guaranteed to be identical everywhere.
In that regard GUIDs seem rock-solid, while hashed values sound more leaving a ticking bomb behind me (new domains, mergers etc.)
As for ```user_attributes```: it's not available for groups, only for users. It would have fit the bill perfectly otherwise.
I wonder if it was more systematic to implement "group_attributes".
And another question -- why did you choose GUIDs and not SIDs?
"""
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-247958333
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
tequeter commented: """ Yes, I considered creating a ```group_attributes``` instead. However it would have taken more refactoring to avoid duplicating the ```user_attributes``` code than I was comfortable with for a first contribution.
I went for uniqueID (GUID) instead of SIDs for three reasons: - ```uniqueID``` has a meaningful value both in IPA and AD, so it'll be useful to more users. - SIDs can change over time in multi-domain configurations. - GUIDs are supposedly world-unique, while SIDs can collide in forest configurations. """
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-247979288
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
pbrezina commented: """ On 09/19/2016 02:26 PM, tequeter wrote:
Yes, I considered creating a |group_attributes| instead. However it would have taken more refactoring to avoid duplicating the |user_attributes| code than I was comfortable with for a first contribution.
I went for uniqueID (GUID) instead of SIDs for three reasons:
- |uniqueID| has a meaningful value both in IPA and AD, so it'll be useful to more users.
- SIDs can change over time in multi-domain configurations.
- GUIDs are supposedly world-unique, while SIDs can collide in forest configurations.
I think we can accept this patch.
user_attributes is meant mostly for custom attributes that are not downloaded by sssd by default and sssd is not aware of their content. Since this attribute is managed by sssd it is better to have control over it by publishing it directly.
It is also useful to have access to SID/GUID since it is far better concept than UNIX ID and we should make it easily accessible for application developers.
"""
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-248259998
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
jhrozek commented: """ On Tue, Sep 20, 2016 at 03:11:59AM -0700, Pavel Březina wrote:
On 09/19/2016 02:26 PM, tequeter wrote:
Yes, I considered creating a |group_attributes| instead. However it would have taken more refactoring to avoid duplicating the |user_attributes| code than I was comfortable with for a first contribution.
I went for uniqueID (GUID) instead of SIDs for three reasons:
- |uniqueID| has a meaningful value both in IPA and AD, so it'll be useful to more users.
- SIDs can change over time in multi-domain configurations.
- GUIDs are supposedly world-unique, while SIDs can collide in forest configurations.
I think we can accept this patch.
user_attributes is meant mostly for custom attributes that are not downloaded by sssd by default and sssd is not aware of their content. Since this attribute is managed by sssd it is better to have control over it by publishing it directly.
It is also useful to have access to SID/GUID since it is far better concept than UNIX ID
No argument here :)
and we should make it easily accessible for application developers.
The only last thing I'm wondering is if there is any information leak in exposing these information. On one hand, the IFP interface is normally only accessible to root. On the other hand, currently it's all-or-nothing, right? We either publish all information or none...
With the SIDs we already have a library thay pretty much anyone can call and retrieve the SID for ID. But not for GUIDs..
CC @sbose-rh for another opinion..
"""
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-248272085
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
pbrezina commented: """ On 09/20/2016 01:13 PM, Jakub Hrozek wrote:
On Tue, Sep 20, 2016 at 03:11:59AM -0700, Pavel Březina wrote:
On 09/19/2016 02:26 PM, tequeter wrote:
Yes, I considered creating a |group_attributes| instead. However it would have taken more refactoring to avoid duplicating the |user_attributes| code than I was comfortable with for a first
contribution.
I went for uniqueID (GUID) instead of SIDs for three reasons:
- |uniqueID| has a meaningful value both in IPA and AD, so it'll be
useful to more users.
- SIDs can change over time in multi-domain configurations.
- GUIDs are supposedly world-unique, while SIDs can collide in forest
configurations.
I think we can accept this patch.
user_attributes is meant mostly for custom attributes that are not downloaded by sssd by default and sssd is not aware of their content. Since this attribute is managed by sssd it is better to have control over it by publishing it directly.
It is also useful to have access to SID/GUID since it is far better concept than UNIX ID
No argument here :)
and we should make it easily accessible for application developers.
The only last thing I'm wondering is if there is any information leak in exposing these information. On one hand, the IFP interface is normally only accessible to root. On the other hand, currently it's all-or-nothing, right? We either publish all information or none...
AFAIK we use whitelist for user attributes.
With the SIDs we already have a library thay pretty much anyone can call and retrieve the SID for ID. But not for GUIDs..
CC @sbose-rh for another opinion..
"""
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-248278313
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
lslebodn commented: """ On (20/09/16 04:46), Pavel Březina wrote:
On 09/20/2016 01:13 PM, Jakub Hrozek wrote:
The only last thing I'm wondering is if there is any information leak in exposing these information. On one hand, the IFP interface is normally only accessible to root. On the other hand, currently it's all-or-nothing, right? We either publish all information or none...
AFAIK we use whitelist for user attributes.
that's the problem. We use whitelist just for *user* attributes and not for group attributes.
LS
"""
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-248278808
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
sumit-bose commented: """
With the SIDs we already have a library thay pretty much anyone can call and retrieve the SID for ID. But not for GUIDs.. CC @sbose-rh for another opinion..
In general the GUIDs are even less informative than the SID, e.g. you cannot derive the domain form it, it is just a random strings created with some rules to try to avoid collisions. So I cannot see a leak here. Additionally I think there is only special protection on the LDAP side on the GUID attribute, e.g. ipaUniqueID can be read anonymously.
Only if the GUID is misused, e.g. as initial password, there would be an issue but imo not on our side. """
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-248285945
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
jhrozek commented: """ On Tue, Sep 20, 2016 at 05:24:45AM -0700, sumit-bose wrote:
With the SIDs we already have a library thay pretty much anyone can call and retrieve the SID for ID. But not for GUIDs.. CC @sbose-rh for another opinion..
In general the GUIDs are even less informative than the SID, e.g. you cannot derive the domain form it, it is just a random strings created with some rules to try to avoid collisions. So I cannot see a leak here. Additionally I think there is only special protection on the LDAP side on the GUID attribute, e.g. ipaUniqueID can be read anonymously.
Only if the GUID is misused, e.g. as initial password, there would be an issue but imo not on our side.
Thank you.
Then I don't see a reason to not accept this PR. I tested it and it seems to work:
$ dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users org.freedesktop.sssd.infopipe.Users.FindByName string:admin@ipa.test method return time=1474453460.862332 sender=:1.305 -> destination=:1.308 serial=9 reply_serial=2 object path "/org/freedesktop/sssd/infopipe/Users/ipa_2etest/935600000"
$ dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users/ipa_2etest/935600000 org.freedesktop.DBus.Properties.Get string:org.freedesktop.sssd.infopipe.Users.User string:uniqueID method return time=1474453484.184180 sender=:1.305 -> destination=:1.309 serial=11 reply_serial=2 variant string "4e80c946-436e-11e6-8fbd-525400f71478"
If anyone thinks having group_attributes should block this PR, I'm OK with implementing it..
So I'll just wait a bit to see if there is any opposition and if not, I'll push the patch..
"""
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-248572630
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
pbrezina commented: """ On 09/21/2016 12:29 PM, Jakub Hrozek wrote:
On Tue, Sep 20, 2016 at 05:24:45AM -0700, sumit-bose wrote:
With the SIDs we already have a library thay pretty much anyone can
call and retrieve the SID for ID. But not for GUIDs.. CC @sbose-rh for another opinion..
In general the GUIDs are even less informative than the SID, e.g. you
cannot derive the domain form it, it is just a random strings created with some rules to try to avoid collisions. So I cannot see a leak here. Additionally I think there is only special protection on the LDAP side on the GUID attribute, e.g. ipaUniqueID can be read anonymously.
Only if the GUID is misused, e.g. as initial password, there would be
an issue but imo not on our side.
Thank you.
Then I don't see a reason to not accept this PR. I tested it and it seems to work:
$ dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users org.freedesktop.sssd.infopipe.Users.FindByName string:admin@ipa.test method return time=1474453460.862332 sender=:1.305 -> destination=:1.308 serial=9 reply_serial=2 object path "/org/freedesktop/sssd/infopipe/Users/ipa_2etest/935600000"
$ dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users/ipa_2etest/935600000 org.freedesktop.DBus.Properties.Get string:org.freedesktop.sssd.infopipe.Users.User string:uniqueID method return time=1474453484.184180 sender=:1.305 -> destination=:1.309 serial=11 reply_serial=2 variant string "4e80c946-436e-11e6-8fbd-525400f71478"
If anyone thinks having group_attributes should block this PR, I'm OK with implementing it..
So I'll just wait a bit to see if there is any opposition and if not, I'll push the patch..
Feel free to push it.
We can create a ticket to trac more detailed access control in IFP. I remember that we talked about usign polkit for this, since IFP is slowly growing we should look into this.
"""
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-248578041
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
Label: -Accepted
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
Label: +Pushed
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
jhrozek commented: """ * master: e9a2e7afbd09c23dd8748246e09831ed7b17d7c5
@tequeter thank you very much for the contribution """
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-248847785
URL: https://github.com/SSSD/sssd/pull/21 Author: tequeter Title: #21: IFP: expose user and group unique IDs through DBus Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/21/head:pr21 git checkout pr21
URL: https://github.com/SSSD/sssd/pull/21 Title: #21: IFP: expose user and group unique IDs through DBus
jhrozek commented: """ On Wed, Sep 21, 2016 at 03:57:19AM -0700, Pavel Březina wrote:
On 09/21/2016 12:29 PM, Jakub Hrozek wrote:
On Tue, Sep 20, 2016 at 05:24:45AM -0700, sumit-bose wrote:
With the SIDs we already have a library thay pretty much anyone can
call and retrieve the SID for ID. But not for GUIDs.. CC @sbose-rh for another opinion..
In general the GUIDs are even less informative than the SID, e.g. you
cannot derive the domain form it, it is just a random strings created with some rules to try to avoid collisions. So I cannot see a leak here. Additionally I think there is only special protection on the LDAP side on the GUID attribute, e.g. ipaUniqueID can be read anonymously.
Only if the GUID is misused, e.g. as initial password, there would be
an issue but imo not on our side.
Thank you.
Then I don't see a reason to not accept this PR. I tested it and it seems to work:
$ dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users org.freedesktop.sssd.infopipe.Users.FindByName string:admin@ipa.test method return time=1474453460.862332 sender=:1.305 -> destination=:1.308 serial=9 reply_serial=2 object path "/org/freedesktop/sssd/infopipe/Users/ipa_2etest/935600000"
$ dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Users/ipa_2etest/935600000 org.freedesktop.DBus.Properties.Get string:org.freedesktop.sssd.infopipe.Users.User string:uniqueID method return time=1474453484.184180 sender=:1.305 -> destination=:1.309 serial=11 reply_serial=2 variant string "4e80c946-436e-11e6-8fbd-525400f71478"
If anyone thinks having group_attributes should block this PR, I'm OK with implementing it..
So I'll just wait a bit to see if there is any opposition and if not, I'll push the patch..
Feel free to push it.
We can create a ticket to trac more detailed access control in IFP. I remember that we talked about usign polkit for this, since IFP is slowly growing we should look into this.
We have https://fedorahosted.org/sssd/ticket/2328 for polkit so I increased its priority. I also filed https://fedorahosted.org/sssd/ticket/3195 for group_attributes (which may or may not be relevant after #2328, but I didn't want to forget..)
"""
See the full comment at https://github.com/SSSD/sssd/pull/21#issuecomment-248857307
sssd-devel@lists.fedorahosted.org