Hi, I'm sending a patch set that removes support for fake user entries and add ghost attribute instead:
#117: Trivial patch that only adds the SYSDB_GHOST attribute
#118: This is one of core patches which changes the behavior on LDAP provider when querying for groups. Instead of creating fake user entries, it uses ghost hash table for RFC230bis and SYSDB_GHOST attribute for RFC2307.
#119: Couple of relatively small changes to adapt proxy provider to the change. I haven't actually tested this but the code seems straightforward to me.
#120: Modifications in sysdb: - removed sysdb_add_fake_user() - modified sysdb_add_user() to remove ghost entries for the user that is currently added.
#121: Memberof plugin modifications: the memberof plugin does all the work when it comes to populating memberuid.
#122: Include ghost members as well as memberuid members in results. Alternatively this could be solved in memberof plugin but that would lead to duplicated information - ghost attribute would be copied to memberuid. This approach seems better to me.
#123: This function is no longer necessary since fake user entries don't exist any more.
#124: sss_groupshow utility has been modified to be aware of SYSDB_GHOST
#125: Various small changes in the code - basically nitpicking cleanup.
Thanks Jan
Stephen Gallagher sgallagh@redhat.com wrote:
On Mon, 2012-04-23 at 16:22 +0200, Jan Zelený wrote:
Hi, I'm sending a patch set that removes support for fake user entries and add
ghost attribute instead:
Jan, could you run a few performance tests with large groups so we can see what we got for improvement?
Sure, no problem. I will also include better commit messages (discussed with Simo on IRC) and also a sysdb upgrade function is missing.
Jan
Stephen Gallagher sgallagh@redhat.com wrote:
On Mon, 2012-04-23 at 16:22 +0200, Jan Zelený wrote:
Hi, I'm sending a patch set that removes support for fake user entries and add
ghost attribute instead:
Jan, could you run a few performance tests with large groups so we can see what we got for improvement?
Sure, no problem. I will also include better commit messages (discussed with Simo on IRC) and also a sysdb upgrade function is missing.
Jan
Ok, here is the updated patch set. So far I've run only a simple test:
- IPA server - large-group with 3k users - flat membership structure - log level 10 (not sure how much impact would it have to turn this off) - user testuser who is not part of the large-group
The testing user is queried for establishing connection to the server and taking the connection time out of the equation.
1. service sssd stop 2. rm -f /var/lib/sss/db* /var/log/sssd/* 3. git checkout master 4. reinstall sssd 5. service sssd start 6. getent group testuser 7. time getent group large-group
real 0m8.198s user 0m0.002s sys 0m0.006s
8. service sssd stop 9. rm -f /var/lib/sss/db* /var/log/sssd/* 10. git checkout ghosts 11. reinstall sssd 12. service sssd start 13. getent group testuser 14. time getent group large-group
real 0m4.441s user 0m0.005s sys 0m0.004s
If you have any other scenarios, I'll be happy to run them for you but please don't ask me to add another 3k users to the LDAP server ;-)
Thanks Jan
On Wed, 2012-04-25 at 15:41 +0200, Jan Zelený wrote:
Stephen Gallagher sgallagh@redhat.com wrote:
On Mon, 2012-04-23 at 16:22 +0200, Jan Zelený wrote:
Hi, I'm sending a patch set that removes support for fake user entries and add
ghost attribute instead:
Jan, could you run a few performance tests with large groups so we can see what we got for improvement?
Sure, no problem. I will also include better commit messages (discussed with Simo on IRC) and also a sysdb upgrade function is missing.
Jan
Ok, here is the updated patch set. So far I've run only a simple test:
- IPA server
- large-group with 3k users
- flat membership structure
- log level 10 (not sure how much impact would it have to turn this off)
log level 10 causes a lot of writes to the log file, can be quite a large impact, can you redo the test turning log level down to 0 ?
- user testuser who is not part of the large-group
The testing user is queried for establishing connection to the server and taking the connection time out of the equation.
- service sssd stop
- rm -f /var/lib/sss/db* /var/log/sssd/*
- git checkout master
- reinstall sssd
- service sssd start
- getent group testuser
- time getent group large-group
real 0m8.198s user 0m0.002s sys 0m0.006s
- service sssd stop
- rm -f /var/lib/sss/db* /var/log/sssd/*
- git checkout ghosts
- reinstall sssd
- service sssd start
- getent group testuser
- time getent group large-group
real 0m4.441s user 0m0.005s sys 0m0.004s
So I assume the first test is w/o the patch, and the second w/ the patch ? :)
If you have any other scenarios, I'll be happy to run them for you but please don't ask me to add another 3k users to the LDAP server ;-)
ok, can you add 3k groups? (j/k)
Sound good from what I can see, if log level 0 confirms or exceeds cutting time in a half, this will be a good improvement.
Simo.
On Wed, 2012-04-25 at 15:41 +0200, Jan Zelený wrote:
Stephen Gallagher sgallagh@redhat.com wrote:
On Mon, 2012-04-23 at 16:22 +0200, Jan Zelený wrote:
Hi, I'm sending a patch set that removes support for fake user entries and add
ghost attribute instead:
Jan, could you run a few performance tests with large groups so we can see what we got for improvement?
Sure, no problem. I will also include better commit messages (discussed with Simo on IRC) and also a sysdb upgrade function is missing.
Jan
Ok, here is the updated patch set. So far I've run only a simple test:
- IPA server
- large-group with 3k users
- flat membership structure
- log level 10 (not sure how much impact would it have to turn this off)
log level 10 causes a lot of writes to the log file, can be quite a large impact, can you redo the test turning log level down to 0 ?
- user testuser who is not part of the large-group
The testing user is queried for establishing connection to the server and taking the connection time out of the equation.
- service sssd stop
- rm -f /var/lib/sss/db* /var/log/sssd/*
- git checkout master
- reinstall sssd
- service sssd start
- getent group testuser
- time getent group large-group
real 0m8.198s user 0m0.002s sys 0m0.006s
- service sssd stop
- rm -f /var/lib/sss/db* /var/log/sssd/*
- git checkout ghosts
- reinstall sssd
- service sssd start
- getent group testuser
- time getent group large-group
real 0m4.441s user 0m0.005s sys 0m0.004s
So I assume the first test is w/o the patch, and the second w/ the patch ? :)
If you have any other scenarios, I'll be happy to run them for you but please don't ask me to add another 3k users to the LDAP server ;-)
ok, can you add 3k groups? (j/k)
Sound good from what I can see, if log level 0 confirms or exceeds cutting time in a half, this will be a good improvement.
I have pleasant results:
master (without patches): =========== real 0m5.190s user 0m0.002s sys 0m0.007s
ghosts (with patches): =========== real 0m2.239s user 0m0.005s sys 0m0.005s
The percentual boost is a little better than it was with the log level 10.
Thanks Jan
Stephen Gallagher sgallagh@redhat.com wrote:
On Mon, 2012-04-23 at 16:22 +0200, Jan Zelený wrote:
Hi, I'm sending a patch set that removes support for fake user entries and add
ghost attribute instead:
Jan, could you run a few performance tests with large groups so we can see what we got for improvement?
Sure, no problem. I will also include better commit messages (discussed with Simo on IRC) and also a sysdb upgrade function is missing.
Jan
Ok, here is the updated patch set. So far I've run only a simple test:
- IPA server
- large-group with 3k users
- flat membership structure
- log level 10 (not sure how much impact would it have to turn this off)
- user testuser who is not part of the large-group
The testing user is queried for establishing connection to the server and taking the connection time out of the equation.
- service sssd stop
- rm -f /var/lib/sss/db* /var/log/sssd/*
- git checkout master
- reinstall sssd
- service sssd start
- getent group testuser
- time getent group large-group
real 0m8.198s user 0m0.002s sys 0m0.006s
- service sssd stop
- rm -f /var/lib/sss/db* /var/log/sssd/*
- git checkout ghosts
- reinstall sssd
- service sssd start
- getent group testuser
- time getent group large-group
real 0m4.441s user 0m0.005s sys 0m0.004s
If you have any other scenarios, I'll be happy to run them for you but please don't ask me to add another 3k users to the LDAP server ;-)
Thanks Jan
I'm sending patch set rebased on the current master.
Jan
On Wed, 2012-04-25 at 15:41 +0200, Jan Zelený wrote:
Ok, here is the updated patch set.
117: Ack
118: Nack
In sdap_fill_memberships(), please do not ever store HASH_* value in 'ret'. Create a new variable (I suggest 'hret'). 'ret' is assumed to be of type errno_t in pretty much all of our code and I don't want to conflate the two.
It's probably academic, but ghostel->values[k].length should probably be strlen()+1, since the buffer technically includes the NULL-terminator. In practice, it will likely never matter.
I don't understand why you look up SYSDB_MEMBER a second time if ghostel->num_values == 0. Why would it have changed?
In sdap_save_group() you assign the ghost name to el->values[el->num_values].data but you don't set its length.
119: Ack
120: Nack I know it's not strictly the fault of this patch, but could you please fix the ldb_transaction logic in this function. Right now, it won't cancel if the commit fails.
Please allocate 'msg' on tmp_ctx. Right now it will not be freed if any of the ldb_msg_add_* functions fail.
Please rename 'el' to 'alias_el' so it's clear what it's being used for in the cleanout code. A few more comments there would be handy too, for future maintainers.
121: Looks good, but I'd much prefer if you would add some comments. The memberOf plugin is very complex code.
122: Nack
I disagree with your assertion in the comment that this is rare. In many (maybe most) existing environments, there will be fake users in the DB.
Please do the search within the transaction, not before it.
Please rename 'memberof' to 'memberof_el' so it's clear it contains all of them.
We know that 'name' is a single-valued attribute. Just use ldb_msg_find_attr_as_string() and make 'name' a 'const char *'. We should also ensure that it's non-NULL.
Use ldb_dn_validate() on the msg->dn, just in case the memberOf reference is broken. In that case, we should just log a message, ignore it and hope for the best.
Please prefer talloc_zfree() for variables that get reused (such as in a loop). Specifically, please talloc_zfree(msg) at the end of the deletion loop.
123: Nack I don't see the point of this patch (and I think it's guaranteeing duplicates). Unless I completely misread Patch 121, you've already got the memberOf plugin creating memberUID attributes for ghost users. So with this patch, we're adding all memberUID values as group members, and then going and adding the ghost users a second time. That doesn't make any sense. Please tell me if I've misinterpreted something here.
124: Ack
125: Ack
126: Ack
Updated patch set in the attachment
Stephen Gallagher sgallagh@redhat.com wrote:
On Wed, 2012-04-25 at 15:41 +0200, Jan Zelený wrote:
Ok, here is the updated patch set.
117: Ack
118: Nack
In sdap_fill_memberships(), please do not ever store HASH_* value in 'ret'. Create a new variable (I suggest 'hret'). 'ret' is assumed to be of type errno_t in pretty much all of our code and I don't want to conflate the two.
Well, I have one of my questions about the code in other parts of SSSD explained :) Fixed
It's probably academic, but ghostel->values[k].length should probably be strlen()+1, since the buffer technically includes the NULL-terminator. In practice, it will likely never matter.
You are right. Academic or not, it should be +1. Fixed
I don't understand why you look up SYSDB_MEMBER a second time if ghostel->num_values == 0. Why would it have changed?
That's actually pretty nasty part of the sysdb code. The thing is that sysdb_attrs_get_el() creates an element if it isn't found in element array. That means the element array is reallocated. In such case the original SYSDB_MEMBER element would still be there (even though the array could have been actually moved elsewhere) for some time but then it disappears as the space is reallocated for something else. That leads to "wonderful" heisenbugs, it took me two days to get to the bottom of this.
To sum up: ghoster->num_values == 0 might be indicating that the array of elements has been reallocated. In that case we need to find the SYSDB_MEMBER again so we don't have a pointer to possibly invalid memory.
In sdap_save_group() you assign the ghost name to el->values[el->num_values].data but you don't set its length.
Fixed
119: Ack
120: Nack I know it's not strictly the fault of this patch, but could you please fix the ldb_transaction logic in this function. Right now, it won't cancel if the commit fails.
I'd prefer to fix this in a separate patch since it is on six more places in the code (if I understand correctly what did you mean).
Please allocate 'msg' on tmp_ctx. Right now it will not be freed if any of the ldb_msg_add_* functions fail.
Ah, copy-paste error. Fixed
Please rename 'el' to 'alias_el' so it's clear what it's being used for in the cleanout code. A few more comments there would be handy too, for future maintainers.
Done and done
121: Looks good, but I'd much prefer if you would add some comments. The memberOf plugin is very complex code.
Done
122: Nack
I disagree with your assertion in the comment that this is rare. In many (maybe most) existing environments, there will be fake users in the DB.
I thought that fake users are resolved ASAP, perhaps this basic assumption was wrong?
Please do the search within the transaction, not before it.
Done
Please rename 'memberof' to 'memberof_el' so it's clear it contains all of them.
Done
We know that 'name' is a single-valued attribute. Just use ldb_msg_find_attr_as_string() and make 'name' a 'const char *'. We should also ensure that it's non-NULL.
Done
Use ldb_dn_validate() on the msg->dn, just in case the memberOf reference is broken. In that case, we should just log a message, ignore it and hope for the best.
Done
Please prefer talloc_zfree() for variables that get reused (such as in a loop). Specifically, please talloc_zfree(msg) at the end of the deletion loop.
I original left it as is because it is immediatelly assigned to in the beginning of the loop. Nevertheless it is fixed now.
123: Nack I don't see the point of this patch (and I think it's guaranteeing duplicates). Unless I completely misread Patch 121, you've already got the memberOf plugin creating memberUID attributes for ghost users. So with this patch, we're adding all memberUID values as group members, and then going and adding the ghost users a second time. That doesn't make any sense. Please tell me if I've misinterpreted something here.
Actually no, there are no duplicates. I tried to explain in the commit message. The object that contains users in the ghost attribute doesn't contain them in memberuid attribute. I didn't want to have redundant information in the object. That's why we need to look for both attributes.
124: Ack
125: Ack
126: Ack
Thanks for the review.
Jan
On Wed, May 02, 2012 at 08:44:20PM +0200, Jan Zeleny wrote:
Updated patch set in the attachment
Stephen Gallagher sgallagh@redhat.com wrote:
On Wed, 2012-04-25 at 15:41 +0200, Jan Zelený wrote:
Ok, here is the updated patch set.
117: Ack
118: Nack
In sdap_fill_memberships(), please do not ever store HASH_* value in 'ret'. Create a new variable (I suggest 'hret'). 'ret' is assumed to be of type errno_t in pretty much all of our code and I don't want to conflate the two.
Well, I have one of my questions about the code in other parts of SSSD explained :) Fixed
It's probably academic, but ghostel->values[k].length should probably be strlen()+1, since the buffer technically includes the NULL-terminator. In practice, it will likely never matter.
You are right. Academic or not, it should be +1. Fixed
I don't understand why you look up SYSDB_MEMBER a second time if ghostel->num_values == 0. Why would it have changed?
That's actually pretty nasty part of the sysdb code. The thing is that sysdb_attrs_get_el() creates an element if it isn't found in element array. That means the element array is reallocated. In such case the original SYSDB_MEMBER element would still be there (even though the array could have been actually moved elsewhere) for some time but then it disappears as the space is reallocated for something else. That leads to "wonderful" heisenbugs, it took me two days to get to the bottom of this.
To sum up: ghoster->num_values == 0 might be indicating that the array of elements has been reallocated. In that case we need to find the SYSDB_MEMBER again so we don't have a pointer to possibly invalid memory.
In sdap_save_group() you assign the ghost name to el->values[el->num_values].data but you don't set its length.
Fixed
I can see why you are passing username in one branch and DN in other, but plese rename the username variable (or add another add_group_member_2307 call into the branch that checks aliases), this is quite confusing:
+ username = sysdb_user_strdn(tmp_ctx, state->dom->name, username); + el = state->sysdb_dns; goto done;
done: + ret = sdap_add_group_member_2307(el, state->dom, username);
There are some old-style DEBUG macros in the patch. Otherwise looks good.
119: Ack
Ack+1
120: Nack I know it's not strictly the fault of this patch, but could you please fix the ldb_transaction logic in this function. Right now, it won't cancel if the commit fails.
I'd prefer to fix this in a separate patch since it is on six more places in the code (if I understand correctly what did you mean).
Please allocate 'msg' on tmp_ctx. Right now it will not be freed if any of the ldb_msg_add_* functions fail.
Ah, copy-paste error. Fixed
Please rename 'el' to 'alias_el' so it's clear what it's being used for in the cleanout code. A few more comments there would be handy too, for future maintainers.
Done and done
Looks good to me now, ack
121: Looks good, but I'd much prefer if you would add some comments. The memberOf plugin is very complex code.
Done
Looks good to me, too.
122: Nack
I disagree with your assertion in the comment that this is rare. In many (maybe most) existing environments, there will be fake users in the DB.
I thought that fake users are resolved ASAP, perhaps this basic assumption was wrong?
They are resolved when explicitly requested with getpwnam/pwuid.
Please do the search within the transaction, not before it.
Done
Please rename 'memberof' to 'memberof_el' so it's clear it contains all of them.
Done
We know that 'name' is a single-valued attribute. Just use ldb_msg_find_attr_as_string() and make 'name' a 'const char *'. We should also ensure that it's non-NULL.
Done
Use ldb_dn_validate() on the msg->dn, just in case the memberOf reference is broken. In that case, we should just log a message, ignore it and hope for the best.
Done
Please prefer talloc_zfree() for variables that get reused (such as in a loop). Specifically, please talloc_zfree(msg) at the end of the deletion loop.
I original left it as is because it is immediatelly assigned to in the beginning of the loop. Nevertheless it is fixed now.
Nack, this patch doesn't work. You forgot to set the sysdb version to 0.11, so SSSD startup aborts b/c the version stays at 0.10.
The logic for converting the fake users needs fixing, too. You search for uidNumber == 0, but the fake users have no UID at all, so the filter never matches anything.
Also this looks suspicious: + for (i = 0; i < res->count; i++) { + user = res->msgs[0];
Shouldn't it read res->msgs[i]?
123: Nack I don't see the point of this patch (and I think it's guaranteeing duplicates). Unless I completely misread Patch 121, you've already got the memberOf plugin creating memberUID attributes for ghost users. So with this patch, we're adding all memberUID values as group members, and then going and adding the ghost users a second time. That doesn't make any sense. Please tell me if I've misinterpreted something here.
Actually no, there are no duplicates. I tried to explain in the commit message. The object that contains users in the ghost attribute doesn't contain them in memberuid attribute. I didn't want to have redundant information in the object. That's why we need to look for both attributes.
I saw a discussion yesterday on #sssd that suggested combining the ghost attribute with memberuid, but it wasn't quite clear to me from the back log what the outcome was.
That said, this code looks good to me should we proceed with the approach. Just please change the old DEBUG macros in fill_members().
124: Ack
Ack+1
125: Ack
Ack+1
126: Ack
Ack+1
Ok, new batch of patches:
On Wed, May 02, 2012 at 08:44:20PM +0200, Jan Zeleny wrote:
Updated patch set in the attachment
Stephen Gallagher sgallagh@redhat.com wrote:
On Wed, 2012-04-25 at 15:41 +0200, Jan Zelený wrote:
Ok, here is the updated patch set.
117: Ack
118: Nack
In sdap_fill_memberships(), please do not ever store HASH_* value in 'ret'. Create a new variable (I suggest 'hret'). 'ret' is assumed to be of type errno_t in pretty much all of our code and I don't want to conflate the two.
Well, I have one of my questions about the code in other parts of SSSD explained :) Fixed
It's probably academic, but ghostel->values[k].length should probably be strlen()+1, since the buffer technically includes the NULL-terminator. In practice, it will likely never matter.
You are right. Academic or not, it should be +1. Fixed
Note that I reverted the change. The original approach was in fact correct. This new approach didn't cause any serious issue, it just made the attribute base64 encoded when using ldbsearch. This leads me to conclusion that ldb doesn't expect terminating \0 to be included in the length.
I don't understand why you look up SYSDB_MEMBER a second time if ghostel->num_values == 0. Why would it have changed?
That's actually pretty nasty part of the sysdb code. The thing is that sysdb_attrs_get_el() creates an element if it isn't found in element array. That means the element array is reallocated. In such case the original SYSDB_MEMBER element would still be there (even though the array could have been actually moved elsewhere) for some time but then it disappears as the space is reallocated for something else. That leads to "wonderful" heisenbugs, it took me two days to get to the bottom of this.
To sum up: ghoster->num_values == 0 might be indicating that the array of elements has been reallocated. In that case we need to find the SYSDB_MEMBER again so we don't have a pointer to possibly invalid memory.
In sdap_save_group() you assign the ghost name to el->values[el->num_values].data but you don't set its length.
Fixed
I can see why you are passing username in one branch and DN in other, but plese rename the username variable (or add another add_group_member_2307 call into the branch that checks aliases), this is quite confusing:
username = sysdb_user_strdn(tmp_ctx, state->dom->name, username);el = state->sysdb_dns; goto done;done:
- ret = sdap_add_group_member_2307(el, state->dom, username);
I did what I could. Personally I don't think there is much difference.
There are some old-style DEBUG macros in the patch. Otherwise looks good.
Oops, I forgot one. Fixed.
119: Ack
Ack+1
120: Nack I know it's not strictly the fault of this patch, but could you please fix the ldb_transaction logic in this function. Right now, it won't cancel if the commit fails.
I'd prefer to fix this in a separate patch since it is on six more places in the code (if I understand correctly what did you mean).
Please allocate 'msg' on tmp_ctx. Right now it will not be freed if any of the ldb_msg_add_* functions fail.
Ah, copy-paste error. Fixed
Please rename 'el' to 'alias_el' so it's clear what it's being used for in the cleanout code. A few more comments there would be handy too, for future maintainers.
Done and done
Looks good to me now, ack
121: Looks good, but I'd much prefer if you would add some comments. The memberOf plugin is very complex code.
Done
Looks good to me, too.
The patch in this version changed significantly. It should be in conformance with Simo's suggestion from yesterday IRC discussion.
122: Nack
I disagree with your assertion in the comment that this is rare. In many (maybe most) existing environments, there will be fake users in the DB.
I thought that fake users are resolved ASAP, perhaps this basic assumption was wrong?
They are resolved when explicitly requested with getpwnam/pwuid.
Yes, I looked at the code once more after the first note from Stephen. But thanks for confirming.
Please do the search within the transaction, not before it.
Done
Please rename 'memberof' to 'memberof_el' so it's clear it contains all of them.
Done
We know that 'name' is a single-valued attribute. Just use ldb_msg_find_attr_as_string() and make 'name' a 'const char *'. We should also ensure that it's non-NULL.
Done
Use ldb_dn_validate() on the msg->dn, just in case the memberOf reference is broken. In that case, we should just log a message, ignore it and hope for the best.
Done
Please prefer talloc_zfree() for variables that get reused (such as in a loop). Specifically, please talloc_zfree(msg) at the end of the deletion loop.
I original left it as is because it is immediatelly assigned to in the beginning of the loop. Nevertheless it is fixed now.
Nack, this patch doesn't work. You forgot to set the sysdb version to 0.11, so SSSD startup aborts b/c the version stays at 0.10.
The logic for converting the fake users needs fixing, too. You search for uidNumber == 0, but the fake users have no UID at all, so the filter never matches anything.
Also this looks suspicious:
- for (i = 0; i < res->count; i++) {
user = res->msgs[0];Shouldn't it read res->msgs[i]?
Sorry for that. There was also one other minor bug in the code. Everything should be ok now, the patch is working for me.
123: Nack I don't see the point of this patch (and I think it's guaranteeing duplicates). Unless I completely misread Patch 121, you've already got the memberOf plugin creating memberUID attributes for ghost users. So with this patch, we're adding all memberUID values as group members, and then going and adding the ghost users a second time. That doesn't make any sense. Please tell me if I've misinterpreted something here.
Actually no, there are no duplicates. I tried to explain in the commit message. The object that contains users in the ghost attribute doesn't contain them in memberuid attribute. I didn't want to have redundant information in the object. That's why we need to look for both attributes.
I saw a discussion yesterday on #sssd that suggested combining the ghost attribute with memberuid, but it wasn't quite clear to me from the back log what the outcome was.
That said, this code looks good to me should we proceed with the approach. Just please change the old DEBUG macros in fill_members().
Done. We have to proceed with the approach now, as the basic concept of ghost members shifted a little bit in coparison with the original patch set. Ghost attribute now co-exists with the memberuid, they don't overlap on any level whatsoever.
124: Ack
Ack+1
125: Ack
Ack+1
126: Ack
Ack+1
Thanks for the review Jan
On Fri, 2012-05-04 at 14:30 +0200, Jan Zelený wrote:
123: Nack I don't see the point of this patch (and I think it's
guaranteeing
duplicates). Unless I completely misread Patch 121, you've
already got
the memberOf plugin creating memberUID attributes for ghost
users. So
with this patch, we're adding all memberUID values as group
members,
and then going and adding the ghost users a second time. That
doesn't
make any sense. Please tell me if I've misinterpreted something
here.
Actually no, there are no duplicates. I tried to explain in the
commit
message. The object that contains users in the ghost attribute
doesn't
contain them in memberuid attribute. I didn't want to have
redundant
information in the object. That's why we need to look for both attributes.
I saw a discussion yesterday on #sssd that suggested combining the
ghost
attribute with memberuid, but it wasn't quite clear to me from the
back
log what the outcome was.
That said, this code looks good to me should we proceed with the approach. Just please change the old DEBUG macros in fill_members().
Done. We have to proceed with the approach now, as the basic concept of ghost members shifted a little bit in coparison with the original patch set. Ghost attribute now co-exists with the memberuid, they don't overlap on any level whatsoever.
NACK,
the wrong number of result is sent back to client if both memberuigd and ghosrt are present as memnum is reset each time. The sum of actual members + ghost memebers need to be sent back to the client.
Simo.
On Fri, 2012-05-04 at 14:30 +0200, Jan Zelený wrote:
Done. We have to proceed with the approach now, as the basic concept of ghost members shifted a little bit in coparison with the original patch set. Ghost attribute now co-exists with the memberuid, they don't overlap on any level whatsoever.
One comment here. With either the previous or current code I am having some difficulty determining if the ghost attributes (ane memberuid with the previous approach) of groups are appropriately handled if ghost groups should change after a new group lookup.
Assume a user is never actually resolved, but is deleted or renamed in the LDAP server. Are we properly handling updating the relative ghost group all around ?
It would be nice to see a unit test to check this behavior.
The issue is connected to the way you implemented removing the ghost attribute for the user name (and its aliases) in patch 120. I have the impression that if by the time the real user is looked up it has changed (say it has one more alias) that the operation may fail as not ghost entry with the 'new' alias is found so the delete operation may fail due to the value not being in there.
The delete operation in that case may need to be split into a looks/match/delete operation as a fallback (doing it by default would probably kill a bit performances so perhaps a fallback case is better).
Simo.
Simo Sorce simo@redhat.com wrote:
On Fri, 2012-05-04 at 14:30 +0200, Jan Zelený wrote:
Done. We have to proceed with the approach now, as the basic concept of ghost members shifted a little bit in coparison with the original patch set. Ghost attribute now co-exists with the memberuid, they don't overlap on any level whatsoever.
One comment here. With either the previous or current code I am having some difficulty determining if the ghost attributes (ane memberuid with the previous approach) of groups are appropriately handled if ghost groups should change after a new group lookup.
Assume a user is never actually resolved, but is deleted or renamed in the LDAP server. Are we properly handling updating the relative ghost group all around ?
It would be nice to see a unit test to check this behavior.
The issue is connected to the way you implemented removing the ghost attribute for the user name (and its aliases) in patch 120. I have the impression that if by the time the real user is looked up it has changed (say it has one more alias) that the operation may fail as not ghost entry with the 'new' alias is found so the delete operation may fail due to the value not being in there.
The delete operation in that case may need to be split into a looks/match/delete operation as a fallback (doing it by default would probably kill a bit performances so perhaps a fallback case is better).
Simo.
Ok, here goes another set. This issue should be addressed now. I didn't make the test suite part but I tested the behavior and it works as expected.
The other issue (member count returned to client) is addressed as well.
Jan
On Thu, May 10, 2012 at 10:57:23PM +0200, Jan Zeleny wrote:
Simo Sorce simo@redhat.com wrote:
On Fri, 2012-05-04 at 14:30 +0200, Jan Zelený wrote:
Done. We have to proceed with the approach now, as the basic concept of ghost members shifted a little bit in coparison with the original patch set. Ghost attribute now co-exists with the memberuid, they don't overlap on any level whatsoever.
One comment here. With either the previous or current code I am having some difficulty determining if the ghost attributes (ane memberuid with the previous approach) of groups are appropriately handled if ghost groups should change after a new group lookup.
Assume a user is never actually resolved, but is deleted or renamed in the LDAP server. Are we properly handling updating the relative ghost group all around ?
It would be nice to see a unit test to check this behavior.
The issue is connected to the way you implemented removing the ghost attribute for the user name (and its aliases) in patch 120. I have the impression that if by the time the real user is looked up it has changed (say it has one more alias) that the operation may fail as not ghost entry with the 'new' alias is found so the delete operation may fail due to the value not being in there.
The delete operation in that case may need to be split into a looks/match/delete operation as a fallback (doing it by default would probably kill a bit performances so perhaps a fallback case is better).
Simo.
Ok, here goes another set. This issue should be addressed now. I didn't make the test suite part but I tested the behavior and it works as expected.
The other issue (member count returned to client) is addressed as well.
Jan
Sorry it took so long to get back to the patchset.
Can you rebase the patches on top of current master?
Can you move the src/providers/ldap/sdap_async_groups.c hunk from the last patch to the LDAP patch? The LDAP patch doesn't compile without it (I tried to do a partial review but couldn't compile the patches unless the last one was applied)
From 48a09ace701e3de727912accd399187e052e795b Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 04:45:29 -0400 Subject: [PATCH 01/10] Ghost members - add the ghost attribute to sysdb
src/db/sysdb.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h index a0d176ec7f9c04b467caf7b2c38ffc6fb263497b..9e7d3c5ca2b584ac306891cbec34d63132649e0f 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -68,6 +68,7 @@
#define SYSDB_MEMBER "member" #define SYSDB_MEMBERUID "memberUid" +#define SYSDB_GHOST "ghost" #define SYSDB_POSIX "isPosix" #define SYSDB_USER_CATEGORY "userCategory" #define SYSDB_HOST_CATEGORY "hostCategory" @@ -167,6 +168,7 @@ NULL} #define SYSDB_GRSRC_ATTRS {SYSDB_NAME, SYSDB_GIDNUM, \ SYSDB_MEMBERUID, \
SYSDB_GHOST, \ SYSDB_DEFAULT_ATTRS, \ NULL}#define SYSDB_GRPW_ATTRS {SYSDB_NAME, SYSDB_UIDNUM, \
1.7.7.6
From 96039ae40cd4c61bd2f78f8a23c3b706cbf90749 Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 04:46:33 -0400 Subject: [PATCH 02/10] Ghost members - support in LDAP provider
The original approach was to store name and original DN in an object in sysdb. When later referenced as member of a group, it was retrieved by its original DN and the correct information about its sysdb DN was stored in the group object which referenced it.
The new approach doesn't use fake user objects, therefore this information has to be reached differently when constructing group memberships. The approach is to store all users to a hash table where original DN is used as the key and username as value. When constructing group memberships, the name is retrieved from this hash table instead of sysdb. This hash table is constructed when retrieving user objects from LDAP server - if the user is not present in sysdb, it is automatically stored in the hash table.
Another situation is for rfc2307. Because there is no nesting there, we can construct the SYSDB_GHOST attribute directly and therefore don't need a hash table of ghost users.
src/providers/ldap/sdap_async_groups.c | 472 +++++++++++++++++++------------- 1 files changed, 287 insertions(+), 185 deletions(-)
diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c index 361525037eb270462251fe03d0c5e1df63de73f4..a3cf4d05606559409f3bd9ab20aeae76438a508d 100644 --- a/src/providers/ldap/sdap_async_groups.c +++ b/src/providers/ldap/sdap_async_groups.c @@ -96,12 +96,17 @@ static int sdap_fill_memberships(struct sysdb_attrs *group_attrs, struct sysdb_ctx *ctx, struct sdap_options *opts, struct sss_domain_info *domain,
hash_table_t *ghosts, struct ldb_val *values, int num_values){ struct ldb_message_element *el;
- int i, j;
struct ldb_message_element *ghostel;
int i, j, k; int ret;
errno_t hret;
hash_key_t key;
hash_value_t value;
ret = sysdb_attrs_get_el(group_attrs, SYSDB_MEMBER, &el); if (ret) {
@@ -116,29 +121,76 @@ static int sdap_fill_memberships(struct sysdb_attrs *group_attrs, goto done; }
- for (i = 0, j = el->num_values; i < num_values; i++) {
- ret = sysdb_attrs_get_el(group_attrs, SYSDB_GHOST, &ghostel);
- if (ret) {
goto done;- }
/* sync search entry with this as origDN */ret = sdap_find_entry_by_origDN(el->values, ctx, domain,(char *)values[i].data,(char **)&el->values[j].data);
- if (ghostel->num_values == 0) {
/* Element was probably newly created, look for "member" again */ret = sysdb_attrs_get_el(group_attrs, SYSDB_MEMBER, &el); if (ret != EOK) {
if (ret != ENOENT) {
goto done;}- }
- /* Just allocate both big enough to contain all members for now */
- ghostel->values = talloc_realloc(group_attrs, ghostel->values, struct ldb_val,
ghostel->num_values + num_values);- if (!ghostel->values) {
ret = ENOMEM;goto done;- }
- j = el->num_values;
- k = ghostel->num_values;
- for (i = 0; i < num_values; i++) {
if (ghosts == NULL) {hret = HASH_ERROR_KEY_NOT_FOUND;} else {key.type = HASH_KEY_STRING;key.str = (char *)values[i].data;hret = hash_lookup(ghosts, &key, &value);}if (hret == HASH_ERROR_KEY_NOT_FOUND) {/* sync search entry with this as origDN */ret = sdap_find_entry_by_origDN(el->values, ctx, domain,(char *)values[i].data,(char **)&el->values[j].data);if (ret != EOK) {/* This should never return ENOENT* -> fail if it does*/ goto done; }
DEBUG(7, (" member #%d (%s): not found!\n",i, (char *)values[i].data));} else { DEBUG(7, (" member #%d (%s): [%s]\n", i, (char *)values[i].data, (char *)el->values[j].data)); el->values[j].length = strlen((char *)el->values[j].data); j++;
} else if (hret != HASH_SUCCESS) {ret = EFAULT;goto done;} else {DEBUG(SSSDBG_TRACE_FUNC, (" member #%d (%s): ghost!\n",i, (char *)values[i].data));ghostel->values[k].data = (uint8_t *)talloc_strdup(ghostel->values,value.ptr);if (ghostel->values[k].data == NULL) {ret = ENOMEM;goto done;}ghostel->values[k].length = strlen((char *)ghostel->values[k].data);k++; }- ghostel->num_values = k;
- ret = EOK;
done: @@ -186,6 +238,7 @@ static int sdap_save_group(TALLOC_CTX *memctx, struct sss_domain_info *dom, struct sysdb_attrs *attrs, bool populate_members,
hash_table_t *ghosts, char **_usn_value, time_t now){ @@ -193,12 +246,14 @@ static int sdap_save_group(TALLOC_CTX *memctx, struct sysdb_attrs *group_attrs; const char *name = NULL; gid_t gid;
- int ret;
int ret, cnt, i; char *usn_value = NULL; TALLOC_CTX *tmpctx = NULL; bool posix_group; bool use_id_mapping = dp_opt_get_bool(opts->basic, SDAP_ID_MAPPING); char *sid_str;
hash_key_t key;
hash_value_t value;
tmpctx = talloc_new(memctx); if (!tmpctx) {
@@ -321,16 +376,58 @@ static int sdap_save_group(TALLOC_CTX *memctx,
if (populate_members) { struct ldb_message_element *el1;
struct ldb_message_element *gh; ret = sysdb_attrs_get_el(attrs, opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name, &el1); if (ret != EOK) { goto fail; }ret = sysdb_attrs_get_el(group_attrs, SYSDB_MEMBER, &el); if (ret != EOK) { goto fail; } el->values = el1->values; el->num_values = el1->num_values;ret = sysdb_attrs_get_el(attrs, SYSDB_GHOST, &gh);if (ret != EOK) {goto fail;}ret = sysdb_attrs_get_el(group_attrs, SYSDB_GHOST, &el);if (ret != EOK) {goto fail;}el->values = gh->values;el->num_values = gh->num_values;/* Now process RFC2307bis ghost hash table */if (ghosts != NULL) {cnt = el->num_values + el1->num_values;el->values = talloc_realloc(attrs, el->values, struct ldb_val,cnt);if (el->values == NULL) {ret = ENOMEM;goto fail;}for (i = 0; i < el1->num_values; i++) {key.type = HASH_KEY_STRING;key.str = (char *)el1->values[i].data;ret = hash_lookup(ghosts, &key, &value);if (ret == HASH_ERROR_KEY_NOT_FOUND) {continue;} else if (ret != HASH_SUCCESS) {ret = EFAULT;goto fail;}el->values[el->num_values].data = (uint8_t *)talloc_strdup(el->values, value.ptr);if (el->values[el->num_values].data == NULL) {ret = ENOMEM;goto fail;}el->values[el->num_values].length = strlen(value.ptr)+1;el->num_values++;}}}
ret = sdap_save_all_names(name, attrs, !dom->case_sensitive, group_attrs);
@@ -373,6 +470,7 @@ static int sdap_save_grpmem(TALLOC_CTX *memctx, struct sdap_options *opts, struct sss_domain_info *dom, struct sysdb_attrs *attrs,
hash_table_t *ghosts, time_t now){ struct ldb_message_element *el; @@ -405,6 +503,7 @@ static int sdap_save_grpmem(TALLOC_CTX *memctx, }
ret = sdap_fill_memberships(group_attrs, ctx, opts, dom,
ghosts, el->values, el->num_values); if (ret) { goto fail;@@ -434,6 +533,7 @@ static int sdap_save_groups(TALLOC_CTX *memctx, struct sysdb_attrs **groups, int num_groups, bool populate_members,
hash_table_t *ghosts, char **_usn_value){ TALLOC_CTX *tmpctx; @@ -487,7 +587,8 @@ static int sdap_save_groups(TALLOC_CTX *memctx, /* if 2 pass savemembers = false */ ret = sdap_save_group(tmpctx, sysdb, opts, dom, groups[i],
populate_members, &usn_value, now);
populate_members, ghosts,&usn_value, now); /* Do not fail completely on errors. * Just report the failure to save and go on */@@ -520,7 +621,8 @@ static int sdap_save_groups(TALLOC_CTX *memctx,
for (i = 0; i < nsaved_groups; i++) {
ret = sdap_save_grpmem(tmpctx, sysdb, opts, dom, saved_groups[i], now);
ret = sdap_save_grpmem(tmpctx, sysdb, opts, dom, saved_groups[i],ghosts, now); /* Do not fail completely on errors. * Just report the failure to save and go on */ if (ret) {@@ -557,13 +659,12 @@ struct sdap_process_group_state { struct sysdb_ctx *sysdb;
struct sysdb_attrs *group;
- struct sysdb_attrs **new_members; struct ldb_message_element* sysdb_dns;
- struct ldb_message_element* ghost_dns; char **queued_members; int queue_len; const char **attrs; const char *filter;
- size_t member_idx; size_t queue_idx; size_t count; size_t check_count;
@@ -578,7 +679,32 @@ static int sdap_process_group_members_2307bis(struct tevent_req *req, struct sdap_process_group_state *state, struct ldb_message_element *memberel); static int sdap_process_group_members_2307(struct sdap_process_group_state *state,
struct ldb_message_element *memberel);
struct ldb_message_element *memberel,struct ldb_message_element *ghostel);+static errno_t sdap_process_group_create_dns(TALLOC_CTX *mem_ctx,
size_t num_values,struct ldb_message_element **_dns)+{
- struct ldb_message_element *dns;
- dns = talloc(mem_ctx, struct ldb_message_element);
- if (dns == NULL) {
return ENOMEM;- }
- dns->num_values = 0;
- dns->values = talloc_array(dns, struct ldb_val,
num_values);- if (dns->values == NULL) {
talloc_zfree(dns);return ENOMEM;- }
- *_dns = dns;
- return EOK;
+}
struct tevent_req *sdap_process_group_send(TALLOC_CTX *memctx, struct tevent_context *ev, @@ -590,6 +716,7 @@ struct tevent_req *sdap_process_group_send(TALLOC_CTX *memctx, bool enumeration) { struct ldb_message_element *el;
- struct ldb_message_element *ghostel; struct sdap_process_group_state *grp_state; struct tevent_req *req = NULL; const char **attrs;
@@ -621,8 +748,6 @@ struct tevent_req *sdap_process_group_send(TALLOC_CTX *memctx, grp_state->sysdb = sysdb; grp_state->group = group; grp_state->check_count = 0;
- grp_state->new_members = NULL;
- grp_state->member_idx = 0; grp_state->queue_idx = 0; grp_state->queued_members = NULL; grp_state->queue_len = 0;
@@ -644,27 +769,47 @@ struct tevent_req *sdap_process_group_send(TALLOC_CTX *memctx, goto done; }
- grp_state->sysdb_dns = talloc(grp_state, struct ldb_message_element);
- if (!grp_state->sysdb_dns) {
talloc_zfree(req);return NULL;- }
- grp_state->sysdb_dns->values = talloc_array(grp_state, struct ldb_val,
el->num_values);- if (!grp_state->sysdb_dns->values) {
talloc_zfree(req);return NULL;- }
- grp_state->sysdb_dns->num_values = 0;
ret = sysdb_attrs_get_el(group,
SYSDB_GHOST,&ghostel);if (ret) {
goto done;}
if (ghostel->num_values == 0) {
/* Element was probably newly created, look for "member" again */ret = sysdb_attrs_get_el(group,opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name,&el);if (ret != EOK) {goto done;}}
ret = sdap_process_group_create_dns(grp_state, el->num_values,
&grp_state->sysdb_dns);if (ret != EOK) {
goto done;}
ret = sdap_process_group_create_dns(grp_state, el->num_values,
&grp_state->ghost_dns);if (ret != EOK) {
goto done;}
switch (opts->schema_type) { case SDAP_SCHEMA_RFC2307:
ret = sdap_process_group_members_2307(grp_state, el);
ret = sdap_process_group_members_2307(grp_state, el, ghostel); break; case SDAP_SCHEMA_IPA_V1: case SDAP_SCHEMA_AD: case SDAP_SCHEMA_RFC2307BIS:/* Note that this code branch will be used only if* ldap_nesting_level = 0 is set in config file*/ ret = sdap_process_group_members_2307bis(req, grp_state, el); break;@@ -815,12 +960,6 @@ sdap_process_group_members_2307bis(struct tevent_req *req, memberel->num_values = state->sysdb_dns->num_values; } else { state->count = state->check_count;
state->new_members = talloc_zero_array(state,struct sysdb_attrs *,state->count + 1);if (!state->new_members) {return ENOMEM;} ret = EBUSY;@@ -828,41 +967,35 @@ sdap_process_group_members_2307bis(struct tevent_req *req, }
static int -sdap_add_group_member_2307(struct sdap_process_group_state *state, +sdap_add_group_member_2307(struct ldb_message_element *sysdb_dns,
struct sss_domain_info *dom, const char *username){
- char *strdn;
- strdn = sysdb_user_strdn(state->sysdb_dns->values,
state->dom->name, username);- if (!strdn) {
- sysdb_dns->values[sysdb_dns->num_values].data =
(uint8_t *) talloc_strdup(sysdb_dns->values, username);- if (sysdb_dns->values[sysdb_dns->num_values].data == NULL) { return ENOMEM; }
- state->sysdb_dns->values[state->sysdb_dns->num_values].data =
(uint8_t *) strdn;- state->sysdb_dns->values[state->sysdb_dns->num_values].length =
strlen(strdn);- state->sysdb_dns->num_values++;
sysdb_dns->values[sysdb_dns->num_values].length =
strlen(username);sysdb_dns->num_values++;
return EOK;
}
static int sdap_process_missing_member_2307(struct sdap_process_group_state *state,
char *member_name, bool *in_transaction,time_t now)
char *member_name, time_t now){
- int ret, sret;
- int ret; TALLOC_CTX *tmp_ctx; const char *filter; const char *username;
- const char *user_dn; size_t count; struct ldb_message **msgs = NULL; static const char *attrs[] = { SYSDB_NAME, NULL };
- if (!in_transaction) return EINVAL;
- tmp_ctx = talloc_new(NULL); if (!tmp_ctx) return ENOMEM;
@@ -870,7 +1003,7 @@ sdap_process_missing_member_2307(struct sdap_process_group_state *state, filter = talloc_asprintf(tmp_ctx, "(%s=%s)", SYSDB_NAME_ALIAS, member_name); if (!filter) { ret = ENOMEM;
goto fail;
goto done;}
ret = sysdb_search_users(tmp_ctx, state->sysdb, filter,
@@ -881,74 +1014,51 @@ sdap_process_missing_member_2307(struct sdap_process_group_state *state, if (count != 1) { DEBUG(1, ("More than one entry with this alias?\n")); ret = EIO;
goto fail;
goto done; } /* fill username with primary name */ username = ldb_msg_find_attr_as_string(msgs[0], SYSDB_NAME, NULL);
goto done;- } else if (ret != EOK && ret != ENOENT) {
if (username == NULL) {ret = EINVAL;DEBUG(SSSDBG_MINOR_FAILURE, ("Inconsistent sysdb: user ""without primary name?\n"));goto done;}user_dn = sysdb_user_strdn(tmp_ctx, state->dom->name, username);if (username == NULL) {return ENOMEM;}ret = sdap_add_group_member_2307(state->sysdb_dns, state->dom, user_dn);if (ret != EOK) {DEBUG(SSSDBG_OP_FAILURE, ("Could not add group member %s\n", username));}- } else if (ret == ENOENT || count == 0) {
/* The entry really does not exist, add a ghost */DEBUG(SSSDBG_TRACE_FUNC, ("Adding a ghost entry\n"));ret = sdap_add_group_member_2307(state->ghost_dns, state->dom, member_name);if (ret != EOK) {DEBUG(SSSDBG_OP_FAILURE, ("Could not add group member %s\n", member_name));}- } else { ret = EIO;
goto fail;}
username = member_name;
/* The entry really does not exist, add a fake entry */
DEBUG(7, ("Adding a dummy entry\n"));
if (!*in_transaction) {
ret = sysdb_transaction_start(state->sysdb);if (ret != EOK) {DEBUG(1, ("Cannot start sysdb transaction: [%d]: %s\n",ret, strerror(ret)));return ret;}*in_transaction = true;}
ret = sysdb_add_fake_user(state->sysdb, username, NULL, now);
if (ret != EOK) {
DEBUG(1, ("Cannot store fake user entry: [%d]: %s\n",ret, strerror(ret)));goto fail;}
/*
* Convert the just received DN into the corresponding sysdb DN* for saving into member attribute of the group*/done:
- ret = sdap_add_group_member_2307(state, username);
- if (ret != EOK) {
DEBUG(1, ("Could not add group member %s\n", username));goto fail;- }
- talloc_free(tmp_ctx);
- return EOK;
-fail:
- talloc_free(tmp_ctx);
- if (*in_transaction) {
sret = sysdb_transaction_cancel(state->sysdb);if (sret == EOK) {*in_transaction = false;} else {DEBUG(0, ("Unable to cancel transaction! [%d][%s]\n",sret, strerror(sret)));}- } return ret;
}
static int sdap_process_group_members_2307(struct sdap_process_group_state *state,
struct ldb_message_element *memberel)
struct ldb_message_element *memberel,struct ldb_message_element *ghostel){ struct ldb_message *msg;
- bool in_transaction = false; char *member_name;
- char *userdn; int ret;
- errno_t sret; time_t now; int i;
@@ -968,7 +1078,12 @@ sdap_process_group_members_2307(struct sdap_process_group_state *state, */ DEBUG(7, ("Member already cached in sysdb: %s\n", member_name));
ret = sdap_add_group_member_2307(state, member_name);
userdn = sysdb_user_strdn(state->sysdb_dns, state->dom->name, member_name);if (userdn == NULL) {return ENOMEM;}ret = sdap_add_group_member_2307(state->sysdb_dns, state->dom, userdn); if (ret != EOK) { DEBUG(1, ("Could not add member %s into sysdb\n", member_name)); goto done;@@ -978,8 +1093,7 @@ sdap_process_group_members_2307(struct sdap_process_group_state *state, DEBUG(7, ("member #%d (%s): not found in sysdb\n", i, member_name));
ret = sdap_process_missing_member_2307(state, member_name,&in_transaction, now);
ret = sdap_process_missing_member_2307(state, member_name, now); if (ret != EOK) { DEBUG(1, ("Error processing missing member #%d (%s):\n", i, member_name));@@ -992,30 +1106,15 @@ sdap_process_group_members_2307(struct sdap_process_group_state *state, } }
- /* sdap_process_missing_member_2307 starts transaction */
- if (in_transaction) {
ret = sysdb_transaction_commit(state->sysdb);if (ret) {DEBUG(2, ("Cannot commit sysdb transaction\n"));goto done;}in_transaction = false;- }
- ret = EOK; talloc_free(memberel->values); memberel->values = talloc_steal(state->group, state->sysdb_dns->values); memberel->num_values = state->sysdb_dns->num_values;
- talloc_free(ghostel->values);
- ghostel->values = talloc_steal(state->group, state->ghost_dns->values);
- ghostel->num_values = state->ghost_dns->num_values;
done:
- if (in_transaction) {
/* If the transaction is still active here, we need to cancel it */sret = sysdb_transaction_cancel(state->sysdb);if (sret != EOK) {DEBUG(0, ("Unable to cancel transaction! [%d][%s]\n",sret, strerror(sret)));}- } return ret;
}
@@ -1029,8 +1128,7 @@ static void sdap_process_group_members(struct tevent_req *subreq) struct sdap_process_group_state *state = tevent_req_data(req, struct sdap_process_group_state); struct ldb_message_element *el;
- struct ldb_dn *dn;
- char* dn_string;
uint8_t* name_string;
state->check_count--; DEBUG(9, ("Members remaining: %d\n", state->check_count));
@@ -1055,31 +1153,12 @@ static void sdap_process_group_members(struct tevent_req *subreq) goto next; }
- /*
* Convert the just received DN into the corresponding sysdb DN* for later usage by sdap_save_groups()*/- dn = sysdb_user_dn(state->sysdb, state, state->dom->name,
(char*)el[0].values[0].data);- if (!dn) {
tevent_req_error(req, ENOMEM);return;- }
- dn_string = ldb_dn_alloc_linearized(state->group, dn);
- if (!dn_string) {
tevent_req_error(req, ENOMEM);return;- }
- state->sysdb_dns->values[state->sysdb_dns->num_values].data =
(uint8_t*)dn_string;- state->sysdb_dns->values[state->sysdb_dns->num_values].length =
strlen(dn_string);- state->sysdb_dns->num_values++;
- state->new_members[state->member_idx] = usr_attrs[0];
- state->member_idx++;
- name_string = el[0].values[0].data;
- state->ghost_dns->values[state->ghost_dns->num_values].data =
talloc_steal(state->ghost_dns->values, name_string);- state->ghost_dns->values[state->ghost_dns->num_values].length =
strlen((char *)name_string);- state->ghost_dns->num_values++;
next: if (ret) { @@ -1110,21 +1189,13 @@ next: }
if (state->check_count == 0) {
ret = sdap_save_users(state, state->sysdb,state->dom, state->opts,state->new_members, state->count, NULL);if (ret) {DEBUG(2, ("Failed to store users.\n"));tevent_req_error(req, ret);return;}/* * To avoid redundant sysdb lookups, populate the "member" attribute * of the group entry with the sysdb DNs of the members. */ ret = sysdb_attrs_get_el(state->group,state->opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name, &el);
state->opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name,&el); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to get the group member attribute [%d]: %s\n",@@ -1134,6 +1205,14 @@ next: } el->values = talloc_steal(state->group, state->sysdb_dns->values); el->num_values = state->sysdb_dns->num_values;
ret = sysdb_attrs_get_el(state->group, SYSDB_GHOST, &el);if (ret != EOK) {tevent_req_error(req, ret);return;}el->values = talloc_steal(state->group, state->ghost_dns->values);el->num_values = state->ghost_dns->num_values; DEBUG(9, ("Processed Group - Done\n")); tevent_req_done(req);@@ -1446,7 +1525,8 @@ static void sdap_get_groups_process(struct tevent_req *subreq) DEBUG(9, ("Saving groups without members first " "to allow unrolling of nested groups.\n")); ret = sdap_save_groups(state, state->sysdb, state->dom, state->opts,
state->groups, state->count, false, NULL);
state->groups, state->count, false,NULL, NULL); if (ret) { DEBUG(2, ("Failed to store groups.\n")); tevent_req_error(req, ret);@@ -1497,7 +1577,7 @@ static void sdap_get_groups_done(struct tevent_req *subreq) DEBUG(9, ("All groups processed\n"));
ret = sdap_save_groups(state, state->sysdb, state->dom, state->opts,
state->groups, state->count, true,
state->groups, state->count, true, NULL, &state->higher_usn); if (ret) { DEBUG(2, ("Failed to store groups.\n"));@@ -1530,10 +1610,12 @@ int sdap_get_groups_recv(struct tevent_req *req, return EOK; }
-static errno_t sdap_nested_group_populate_users(struct sysdb_ctx *sysdb, +static errno_t sdap_nested_group_populate_users(TALLOC_CTX *mem_ctx,
struct sysdb_ctx *sysdb, struct sdap_options *opts, struct sysdb_attrs **users,
int num_users);
int num_users,hash_table_t **_ghosts);static void sdap_nested_done(struct tevent_req *subreq) { @@ -1546,6 +1628,7 @@ static void sdap_nested_done(struct tevent_req *subreq) bool in_transaction = false; struct sysdb_attrs **users = NULL; struct sysdb_attrs **groups = NULL;
- hash_table_t *ghosts; struct tevent_req *req = tevent_req_callback_data(subreq, struct tevent_req); struct sdap_get_groups_state *state = tevent_req_data(req,
@@ -1608,14 +1691,15 @@ static void sdap_nested_done(struct tevent_req *subreq) } in_transaction = true;
- ret = sdap_nested_group_populate_users(state->sysdb, state->opts,
users, user_count);
ret = sdap_nested_group_populate_users(state, state->sysdb, state->opts,
users, user_count, &ghosts);if (ret != EOK) { goto fail; }
ret = sdap_save_groups(state, state->sysdb, state->dom, state->opts,
groups, group_count, false, &state->higher_usn);
groups, group_count, false, ghosts,&state->higher_usn);@@ -1641,10 +1725,12 @@ fail: tevent_req_error(req, ret); }
-static errno_t sdap_nested_group_populate_users(struct sysdb_ctx *sysdb, +static errno_t sdap_nested_group_populate_users(TALLOC_CTX *mem_ctx,
struct sysdb_ctx *sysdb, struct sdap_options *opts, struct sysdb_attrs **users,
int num_users)
int num_users,hash_table_t **_ghosts){ int i; errno_t ret, sret; @@ -1659,24 +1745,36 @@ static errno_t sdap_nested_group_populate_users(struct sysdb_ctx *sysdb, const char *sysdb_name; struct sysdb_attrs *attrs; static const char *search_attrs[] = { SYSDB_NAME, NULL };
- hash_table_t *ghosts;
- hash_key_t key;
- hash_value_t value; size_t count;
- time_t now;
if (_ghosts == NULL) {
return EINVAL;}
if (num_users == 0) { /* Nothing to do if there are no users */
*_ghosts = NULL; return EOK;}
tmp_ctx = talloc_new(NULL); if (!tmp_ctx) return ENOMEM;
ret = sss_hash_create(tmp_ctx, num_users, &ghosts);
if (ret != HASH_SUCCESS) {
ret = ENOMEM;goto done;}
ret = sysdb_transaction_start(sysdb); if (ret) { DEBUG(1, ("Failed to start transaction!\n")); goto done; }
- now = time(NULL); for (i = 0; i < num_users; i++) { ret = sysdb_attrs_primary_name(sysdb, users[i], opts->user_map[SDAP_AT_USER_NAME].name,
@@ -1744,15 +1842,17 @@ static errno_t sdap_nested_group_populate_users(struct sysdb_ctx *sysdb, ret = sysdb_set_user_attr(sysdb, sysdb_name, attrs, SYSDB_MOD_REP); if (ret != EOK) goto done; }
/* If the entry does not exist add a fake user record */ret = sysdb_add_fake_user(sysdb, username, original_dn, now);if (ret != EOK) {DEBUG(1, ("Cannot store fake user entry, ignoring: [%d]: %s\n",ret, strerror(ret)));continue;} else {DEBUG(9, ("Added incomplete user %s!\n", username));
else{key.type = HASH_KEY_STRING;key.str = discard_const(original_dn);value.type = HASH_VALUE_PTR;value.ptr = discard_const(username);ret = hash_enter(ghosts, &key, &value);if (ret != HASH_SUCCESS) {ret = ENOMEM;goto done;} }@@ -1764,13 +1864,16 @@ static errno_t sdap_nested_group_populate_users(struct sysdb_ctx *sysdb,
ret = EOK;done:
- talloc_zfree(tmp_ctx); if (ret != EOK) { sret = sysdb_transaction_cancel(sysdb); if (sret != EOK) { DEBUG(2, ("Could not cancel transaction\n")); }
*_ghosts = NULL;- } else {
*_ghosts = talloc_steal(mem_ctx, ghosts);- talloc_zfree(tmp_ctx); return ret;
}
@@ -2242,7 +2345,6 @@ sdap_nested_group_check_cache(TALLOC_CTX *mem_ctx, struct ldb_message **msgs = NULL; char *member_dn; uint64_t expiration;
- uint64_t create_time; uid_t user_uid; time_t now = time(NULL); static const char *attrs[] = { SYSDB_CACHE_EXPIRE, SYSDB_UIDNUM,
-- 1.7.7.6
From dd53be03778ab9e804d286752375292b65dc0f69 Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 04:47:23 -0400 Subject: [PATCH 03/10] Ghost members - support in proxy provider
src/providers/proxy/proxy_id.c | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/providers/proxy/proxy_id.c b/src/providers/proxy/proxy_id.c index 8a8c7ca80d1b24e53c3d55d06564e719a069642a..6c78848267ff76aa1b7e1eca9fbc36ded58ca3b0 100644 --- a/src/providers/proxy/proxy_id.c +++ b/src/providers/proxy/proxy_id.c @@ -489,6 +489,7 @@ done:
static errno_t proxy_process_missing_users(struct sysdb_ctx *sysdb,
struct sysdb_attrs *group_attrs, struct group *grp, time_t now);static int save_group(struct sysdb_ctx *sysdb, struct sss_domain_info *dom, @@ -530,8 +531,8 @@ static int save_group(struct sysdb_ctx *sysdb, struct sss_domain_info *dom, goto done; }
/* Create fake users if they don't already exist */ret = proxy_process_missing_users(sysdb, grp, now);
/* Create ghost users */ret = proxy_process_missing_users(sysdb, attrs, grp, now); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("Could not add missing members\n")); goto done;@@ -612,6 +613,7 @@ done: }
static errno_t proxy_process_missing_users(struct sysdb_ctx *sysdb,
struct sysdb_attrs *group_attrs, struct group *grp, time_t now){ @@ -636,15 +638,15 @@ static errno_t proxy_process_missing_users(struct sysdb_ctx *sysdb, talloc_zfree(msg); continue; } else if (ret == ENOENT) {
/* No entry for this user. Create a fake user */
/* No entry for this user. Create a ghost user */ DEBUG(SSSDBG_TRACE_LIBS,
("Member [%s] not cached, creating fake user entry\n",
("Member [%s] not cached, creating ghost user entry\n", grp->gr_mem[i]));
ret = sysdb_add_fake_user(sysdb, grp->gr_mem[i], NULL, now);
ret = sysdb_attrs_add_string(group_attrs, SYSDB_GHOST, grp->gr_mem[i]); if (ret != EOK) { DEBUG(SSSDBG_MINOR_FAILURE,
("Cannot store fake user entry: [%d]: %s\n",
("Cannot store ghost user entry: [%d]: %s\n", ret, strerror(ret))); goto done; }-- 1.7.7.6
From c75d6a16569fe6250b3b3659bc96961c2fdcde8e Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 04:49:16 -0400 Subject: [PATCH 04/10] Ghost members - modifications in sysdb
Deleted sysdb_add_fake_user(): This function is no longer used.
Modified sysdb_add_user(): When user object is added to sysdb, it is important to iterate over all groups that might have its name or any of its aliases as ghost member and replace this ghost membership by a real one. This will eliminate duplicite memberships.
src/db/sysdb.h | 5 - src/db/sysdb_ops.c | 256 ++++++++++++++++++++++++++++++++++------------------ 2 files changed, 167 insertions(+), 94 deletions(-)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 9e7d3c5ca2b584ac306891cbec34d63132649e0f..85dde2c4908b9c502921a82558ff09329c4052ca 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -583,11 +583,6 @@ int sysdb_add_user(struct sysdb_ctx *sysdb, int cache_timeout, time_t now);
-int sysdb_add_fake_user(struct sysdb_ctx *sysdb,
const char *name,const char *original_dn,time_t now);/* Add group (only basic attrs and w/o checks) */ int sysdb_add_basic_group(struct sysdb_ctx *sysdb, const char *name, gid_t gid); diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index bedd8cfa292cfdab2ba20260ff641730a41610b5..1bfb6c926ca73530be34d9cb6b2713a6f1e1a0d6 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -866,9 +866,16 @@ int sysdb_add_user(struct sysdb_ctx *sysdb, { TALLOC_CTX *tmp_ctx; struct ldb_message *msg;
- struct ldb_message **groups;
- struct ldb_message_element *alias_el;
- size_t group_count = 0; struct sysdb_attrs *id_attrs;
- const char *group_attrs[] = {SYSDB_NAME, SYSDB_GHOST, NULL};
- struct ldb_dn *tmpdn;
- const char *userdn;
- char *filter; uint32_t id;
- int ret;
int ret, i, j;
struct sss_domain_info *domain = sysdb->domain;
@@ -977,6 +984,105 @@ int sysdb_add_user(struct sysdb_ctx *sysdb,
ret = sysdb_set_user_attr(sysdb, name, attrs, SYSDB_MOD_REP);
- /* remove all ghost users */
- filter = talloc_asprintf(tmp_ctx, "(|(%s=%s)", SYSDB_GHOST, name);
- if (!filter) {
ret = ENOMEM;goto done;- }
- ret = sysdb_attrs_get_el(attrs, SYSDB_NAME_ALIAS, &alias_el);
- if (ret != EOK) {
goto done;- }
- for (i = 0; i < alias_el->num_values; i++) {
filter = talloc_asprintf_append(filter, "(%s=%s)",SYSDB_GHOST, alias_el->values[i].data);if (filter == NULL) {ret = ENOMEM;goto done;}- }
- filter = talloc_asprintf_append(filter, ")");
- if (filter == NULL) {
ret = ENOMEM;goto done;- }
- tmpdn = sysdb_user_dn(sysdb, tmp_ctx, sysdb->domain->name, name);
- if (!tmpdn) {
ERROR_OUT(ret, ENOMEM, done);- }
- userdn = ldb_dn_get_linearized(tmpdn);
- if (!userdn) {
ERROR_OUT(ret, EINVAL, done);- }
- tmpdn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb,
SYSDB_TMPL_GROUP_BASE, sysdb->domain->name);- if (!tmpdn) {
ret = ENOMEM;goto done;- }
- /* We need to find all groups that contain this object as a ghost user
* and replace the ghost user there by actual member record* Note that this object can be referred to either by its name or any* of its aliases*/- ret = sysdb_search_entry(tmp_ctx, sysdb, tmpdn, LDB_SCOPE_SUBTREE, filter,
group_attrs, &group_count, &groups);- if (ret != EOK && ret != ENOENT) {
goto done;- }
- for (i = 0; i < group_count; i++) {
msg = ldb_msg_new(tmp_ctx);if (!msg) {ERROR_OUT(ret, ENOMEM, done);}msg->dn = groups[i]->dn;ret = ldb_msg_add_empty(msg, SYSDB_MEMBER, SYSDB_MOD_ADD, NULL);if (ret != LDB_SUCCESS) {ERROR_OUT(ret, ENOMEM, done);}ret = ldb_msg_add_fmt(msg, SYSDB_MEMBER, "%s", userdn);if (ret != LDB_SUCCESS) {ERROR_OUT(ret, EINVAL, done);}ret = ldb_msg_add_empty(msg, SYSDB_GHOST, SYSDB_MOD_DEL, NULL);if (ret != LDB_SUCCESS) {ERROR_OUT(ret, ENOMEM, done);}ret = ldb_msg_add_fmt(msg, SYSDB_GHOST, "%s", name);if (ret != LDB_SUCCESS) {ERROR_OUT(ret, EINVAL, done);}/* Delete aliases from the ghost attribute as well */for (j = 0; j < alias_el->num_values; j++) {ret = ldb_msg_add_fmt(msg, SYSDB_GHOST, "%s",(char *)alias_el->values[j].data);if (ret != LDB_SUCCESS) {ERROR_OUT(ret, EINVAL, done);}}ret = ldb_modify(sysdb->ldb, msg);ret = sysdb_error_to_errno(ret);if (ret != EOK) {goto done;}talloc_zfree(msg);- }
- ret = EOK;
done: if (ret == EOK) { ret = ldb_transaction_commit(sysdb->ldb); @@ -989,76 +1095,6 @@ done: return ret; }
-int sysdb_add_fake_user(struct sysdb_ctx *sysdb,
const char *name,const char *original_dn,time_t now)-{
- TALLOC_CTX *tmp_ctx;
- struct ldb_message *msg;
- int ret;
- tmp_ctx = talloc_new(NULL);
- if (!tmp_ctx) {
return ENOMEM;- }
- msg = ldb_msg_new(tmp_ctx);
- if (!msg) {
ERROR_OUT(ret, ENOMEM, done);- }
- /* user dn */
- msg->dn = sysdb_user_dn(sysdb, msg, sysdb->domain->name, name);
- if (!msg->dn) {
ERROR_OUT(ret, ENOMEM, done);- }
- ret = add_string(msg, LDB_FLAG_MOD_ADD, SYSDB_OBJECTCLASS, SYSDB_USER_CLASS);
- if (ret) goto done;
- ret = add_string(msg, LDB_FLAG_MOD_ADD, SYSDB_NAME, name);
- if (ret) goto done;
- if (!now) {
now = time(NULL);- }
- ret = add_ulong(msg, LDB_FLAG_MOD_ADD, SYSDB_CREATE_TIME,
(unsigned long) now);- if (ret) goto done;
- /* set last login so that the fake entry does not get cleaned up
* immediately */- ret = add_ulong(msg, LDB_FLAG_MOD_ADD, SYSDB_LAST_LOGIN,
(unsigned long) now);- if (ret) return ret;
- ret = add_ulong(msg, LDB_FLAG_MOD_ADD, SYSDB_LAST_UPDATE,
(unsigned long) now);- if (ret) goto done;
- ret = add_ulong(msg, LDB_FLAG_MOD_ADD, SYSDB_CACHE_EXPIRE,
(unsigned long) now-1);- if (ret) goto done;
- if (original_dn) {
ret = add_string(msg, LDB_FLAG_MOD_ADD,SYSDB_ORIG_DN, original_dn);if (ret) goto done;- }
- ret = ldb_add(sysdb->ldb, msg);
- ret = sysdb_error_to_errno(ret);
-done:
- if (ret != EOK) {
DEBUG(6, ("Error: %d (%s)\n", ret, strerror(ret)));- }
- talloc_zfree(tmp_ctx);
- return ret;
-}
/* =Add-Basic-Group-NO-CHECKS============================================= */
int sysdb_add_basic_group(struct sysdb_ctx *sysdb, @@ -2213,8 +2249,13 @@ int sysdb_delete_user(struct sysdb_ctx *sysdb, const char *name, uid_t uid) { TALLOC_CTX *tmp_ctx;
const char *attrs[] = {SYSDB_GHOST, NULL};
size_t msg_count;
char *filter;
struct ldb_message **msgs; struct ldb_message *msg; int ret;
int i;
tmp_ctx = talloc_new(NULL); if (!tmp_ctx) {
@@ -2228,34 +2269,71 @@ int sysdb_delete_user(struct sysdb_ctx *sysdb, ret = sysdb_search_user_by_uid(tmp_ctx, sysdb, uid, NULL, &msg); }
- if (ret) {
goto fail;- }
- if (ret == EOK) {
if (name && uid) {/* verify name/gid match */const char *c_name;uint64_t c_uid;
- if (name && uid) {
/* verify name/gid match */const char *c_name;uint64_t c_uid;
c_name = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);c_uid = ldb_msg_find_attr_as_uint64(msg, SYSDB_UIDNUM, 0);if (c_name == NULL || c_uid == 0) {DEBUG(2, ("Attribute is missing but this should never happen!\n"));ret = EFAULT;goto fail;}if (strcmp(name, c_name) || uid != c_uid) {/* this is not the entry we are looking for */ret = EINVAL;goto fail;}}
c_name = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);c_uid = ldb_msg_find_attr_as_uint64(msg, SYSDB_UIDNUM, 0);if (c_name == NULL || c_uid == 0) {DEBUG(2, ("Attribute is missing but this should never happen!\n"));ret = EFAULT;
ret = sysdb_delete_entry(sysdb, msg->dn, false);if (ret) { goto fail; }
if (strcmp(name, c_name) || uid != c_uid) {/* this is not the entry we are looking for */ret = EINVAL;
- } else if (ret == ENOENT && name != NULL) {
/* Perhaps a ghost user? */filter = talloc_asprintf(tmp_ctx, "(%s=%s)", SYSDB_GHOST, name);if (filter == NULL) {ret = ENOMEM; goto fail; }
}
ret = sysdb_delete_entry(sysdb, msg->dn, false);
if (ret) {
ret = sysdb_search_groups(tmp_ctx, sysdb, filter, attrs, &msg_count, &msgs);if (ret != EOK) {goto fail;}for (i = 0; i < msg_count; i++) {msg = ldb_msg_new(tmp_ctx);if (!msg) {ERROR_OUT(ret, ENOMEM, fail);}msg->dn = msgs[i]->dn;ret = ldb_msg_add_empty(msg, SYSDB_GHOST, SYSDB_MOD_DEL, NULL);if (ret != LDB_SUCCESS) {ERROR_OUT(ret, ENOMEM, fail);}ret = ldb_msg_add_fmt(msg, SYSDB_GHOST, "%s", name);if (ret != LDB_SUCCESS) {ERROR_OUT(ret, EINVAL, fail);}ret = ldb_modify(sysdb->ldb, msg);ret = sysdb_error_to_errno(ret);if (ret != EOK) {goto fail;}talloc_zfree(msg);}} else { goto fail; }
talloc_zfree(tmp_ctx); return EOK;
-- 1.7.7.6
From cf117c91cb64d05d8b2958d185e96a39772b23f4 Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 04:50:36 -0400 Subject: [PATCH 05/10] Ghost members - modifications in memberof plugin
src/ldb_modules/memberof.c | 47 ++++++++++++++++++++++++++++++++++++++----- 1 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/src/ldb_modules/memberof.c b/src/ldb_modules/memberof.c index 4fc46fa84529ac1850cdb588ddf69be0f60e8238..f0b5b72ed11ce71aec8ccb6e7a7cbd2d02e9566e 100644 --- a/src/ldb_modules/memberof.c +++ b/src/ldb_modules/memberof.c @@ -28,6 +28,7 @@ #include "dhash.h"
#define DB_MEMBER "member" +#define DB_GHOST "ghost" #define DB_MEMBEROF "memberof" #define DB_MEMBERUID "memberuid" #define DB_NAME "name" @@ -191,7 +192,8 @@ static int mbof_append_muop(TALLOC_CTX *memctx, int *_num_muops, int flags, struct ldb_dn *parent,
const char *name)
const char *name,const char *element_name){ struct mbof_memberuid_op *muops = *_muops; int num_muops = *_num_muops; @@ -229,7 +231,7 @@ static int mbof_append_muop(TALLOC_CTX *memctx, if (!op->el) { return LDB_ERR_OPERATIONS_ERROR; }
op->el->name = talloc_strdup(op->el, DB_MEMBERUID);
op->el->name = talloc_strdup(op->el, element_name); if (!op->el->name) { return LDB_ERR_OPERATIONS_ERROR; }@@ -534,7 +536,8 @@ static int mbof_add_callback(struct ldb_request *req, static int mbof_next_add(struct mbof_add_operation *addop) { static const char *attrs[] = { DB_OC, DB_NAME,
DB_MEMBER, DB_MEMBEROF, NULL };
DB_MEMBER, DB_GHOST,DB_MEMBEROF, NULL };@@ -789,7 +792,8 @@ static int mbof_add_operation(struct mbof_add_operation *addop) ret = mbof_append_muop(add_ctx, &add_ctx->muops, &add_ctx->num_muops, LDB_FLAG_MOD_ADD,
parents->dns[i], name);
parents->dns[i], name,DB_MEMBERUID); if (ret != LDB_SUCCESS) { return ret; }@@ -806,6 +810,35 @@ static int mbof_add_operation(struct mbof_add_operation *addop) return ret; }
- el = ldb_msg_find_element(addop->entry, DB_GHOST);
- if (el) {
for (i = 0; i < el->num_values; i++) {/* add memberuid to all group's parents */for (j = 0; j < parents->num; j++) {ret = mbof_append_muop(add_ctx, &add_ctx->muops,&add_ctx->num_muops,LDB_FLAG_MOD_ADD,parents->dns[j],(char *)el->values[i].data,DB_GHOST);if (ret != LDB_SUCCESS) {return ret;}}/* now add memberuid to the group itself */ret = mbof_append_muop(add_ctx, &add_ctx->muops,&add_ctx->num_muops,LDB_FLAG_MOD_ADD,addop->entry_dn,(char *)el->values[i].data,DB_GHOST);if (ret != LDB_SUCCESS) {return ret;}}- }
- /* we are done with the entry now */ talloc_free(addop->entry); addop->entry = NULL;
@@ -2071,7 +2104,8 @@ static int mbof_del_mod_entry(struct mbof_del_operation *delop) ret = mbof_append_muop(del_ctx, &del_ctx->muops, &del_ctx->num_muops, LDB_FLAG_MOD_DELETE,
diff[i], name);
diff[i], name,DB_MEMBERUID); if (ret != LDB_SUCCESS) { return ret; }@@ -2311,7 +2345,8 @@ static int mbof_del_fill_muop(struct mbof_del_ctx *del_ctx, ret = mbof_append_muop(del_ctx, &del_ctx->muops, &del_ctx->num_muops, LDB_FLAG_MOD_DELETE,
valdn, name);
valdn, name,DB_MEMBERUID); if (ret != LDB_SUCCESS) { return ret; }-- 1.7.7.6
From 169772136d2c1f9cafdcec2dd30e5ad144e0834d Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 14:08:00 -0400 Subject: [PATCH 06/10] Ghost members - sysdb upgrade routine
It is remotely possible to have sysdb in an inconsistent state that might need upgrade. Consider scenario when user asks for group information. Some fake users are added as a part of this operation. Before users can be fully resolved and stored properly, SSSD is shut down and upgrade is performed.
In this case we need to go over all fake user records (uidNumber=0) and replace each of them with ghost record in all group objects that are stated in its memberof attribute.
src/db/sysdb.c | 7 ++ src/db/sysdb_private.h | 4 +- src/db/sysdb_upgrade.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 1 deletions(-)
diff --git a/src/db/sysdb.c b/src/db/sysdb.c index 41c677d7e5fe16ed2799384c5c5744f92d13d64c..fd76f73c3d4f48fbaff362477aa207e3ac260eba 100644 --- a/src/db/sysdb.c +++ b/src/db/sysdb.c @@ -1061,6 +1061,13 @@ int sysdb_domain_init_internal(TALLOC_CTX *mem_ctx, } }
if (strcmp(version, SYSDB_VERSION_0_10) == 0) {ret = sysdb_upgrade_10(sysdb, &version);if (ret != EOK) {goto done;}}/* The version should now match SYSDB_VERSION. * If not, it means we didn't match any of the * known older versions. The DB might bediff --git a/src/db/sysdb_private.h b/src/db/sysdb_private.h index a162cbba82bba8fef1e57ab2774d156307ead2cd..0fb7316b48b5c53be0198f719673080275c34b81 100644 --- a/src/db/sysdb_private.h +++ b/src/db/sysdb_private.h @@ -23,6 +23,7 @@ #ifndef __INT_SYS_DB_H__ #define __INT_SYS_DB_H__
+#define SYSDB_VERSION_0_11 "0.11" #define SYSDB_VERSION_0_10 "0.10" #define SYSDB_VERSION_0_9 "0.9" #define SYSDB_VERSION_0_8 "0.8" @@ -34,7 +35,7 @@ #define SYSDB_VERSION_0_2 "0.2" #define SYSDB_VERSION_0_1 "0.1"
-#define SYSDB_VERSION SYSDB_VERSION_0_10 +#define SYSDB_VERSION SYSDB_VERSION_0_11
#define SYSDB_BASE_LDIF \ "dn: @ATTRIBUTES\n" \ @@ -104,6 +105,7 @@ int sysdb_upgrade_06(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_07(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_08(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_09(struct sysdb_ctx *sysdb, const char **ver); +int sysdb_upgrade_10(struct sysdb_ctx *sysdb, const char **ver);
int add_string(struct ldb_message *msg, int flags, const char *attr, const char *value); diff --git a/src/db/sysdb_upgrade.c b/src/db/sysdb_upgrade.c index 4fe6afb3e49e9dbc4d459338d198b4d2a715997d..d9b68d4aa6e0398189d8f272d7ac0104c8f347ab 100644 --- a/src/db/sysdb_upgrade.c +++ b/src/db/sysdb_upgrade.c @@ -1169,3 +1169,150 @@ done: talloc_free(tmp_ctx); return ret; }
+int sysdb_upgrade_10(struct sysdb_ctx *sysdb, const char **ver) +{
- TALLOC_CTX *tmp_ctx;
- int ret;
- struct ldb_result *res;
- struct ldb_message *msg;
- struct ldb_message *user;
- struct ldb_message_element *memberof_el;
- const char *name;
- struct ldb_dn *basedn;
- const char *filter = "(&(objectClass=user)(!(uidNumber=*)))";
- const char *attrs[] = { "name", "memberof", NULL };
- int i, j;
- tmp_ctx = talloc_new(NULL);
- if (tmp_ctx == NULL) {
return ENOMEM;- }
- basedn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb, SYSDB_TMPL_USER_BASE,
sysdb->domain->name);- if (basedn == NULL) {
ret = EIO;goto done;- }
- DEBUG(SSSDBG_CRIT_FAILURE, ("UPGRADING DB TO VERSION %s\n", SYSDB_VERSION_0_11));
- ret = ldb_transaction_start(sysdb->ldb);
- if (ret != LDB_SUCCESS) {
ret = EIO;goto done;- }
- ret = ldb_search(sysdb->ldb, tmp_ctx, &res, basedn, LDB_SCOPE_SUBTREE,
attrs, "%s", filter);- if (ret != LDB_SUCCESS) {
ret = EIO;goto done;- }
- for (i = 0; i < res->count; i++) {
user = res->msgs[i];memberof_el = ldb_msg_find_element(user, "memberof");name = ldb_msg_find_attr_as_string(user, "name", NULL);if (name == NULL) {ret = EIO;goto done;}for (j = 0; j < memberof_el->num_values; j++) {msg = ldb_msg_new(tmp_ctx);if (msg == NULL) {ret = ENOMEM;goto done;}msg->dn = ldb_dn_from_ldb_val(tmp_ctx, sysdb->ldb, &memberof_el->values[j]);if (msg->dn == NULL) {ret = ENOMEM;goto done;}if (!ldb_dn_validate(msg->dn)) {DEBUG(SSSDBG_MINOR_FAILURE, ("DN validation failed during ""upgrade: [%s]\n",memberof_el->values[j].data));talloc_zfree(msg);continue;}ret = ldb_msg_add_empty(msg, "ghost", LDB_FLAG_MOD_ADD, NULL);if (ret != LDB_SUCCESS) {ret = ENOMEM;goto done;}ret = ldb_msg_add_string(msg, "ghost", name);if (ret != LDB_SUCCESS) {ret = ENOMEM;goto done;}ret = ldb_msg_add_empty(msg, "member", LDB_FLAG_MOD_DELETE, NULL);if (ret != LDB_SUCCESS) {ret = ENOMEM;goto done;}ret = ldb_msg_add_string(msg, "member", ldb_dn_get_linearized(user->dn));if (ret != LDB_SUCCESS) {ret = ENOMEM;goto done;}ret = ldb_modify(sysdb->ldb, msg);talloc_zfree(msg);if (ret != LDB_SUCCESS) {ret = sysdb_error_to_errno(ret);goto done;}}ret = ldb_delete(sysdb->ldb, user->dn);if (ret != LDB_SUCCESS) {ret = sysdb_error_to_errno(ret);goto done;}/* conversion done, upgrade version number */msg = ldb_msg_new(tmp_ctx);if (!msg) {ret = ENOMEM;goto done;}msg->dn = ldb_dn_new(tmp_ctx, sysdb->ldb, SYSDB_BASE);if (!msg->dn) {ret = ENOMEM;goto done;}ret = ldb_msg_add_empty(msg, "version", LDB_FLAG_MOD_REPLACE, NULL);if (ret != LDB_SUCCESS) {ret = ENOMEM;goto done;}ret = ldb_msg_add_string(msg, "version", SYSDB_VERSION_0_11);if (ret != LDB_SUCCESS) {ret = ENOMEM;goto done;}ret = ldb_modify(sysdb->ldb, msg);if (ret != LDB_SUCCESS) {ret = sysdb_error_to_errno(ret);goto done;}- }
- ret = EOK;
+done:
- ret = finish_upgrade(ret, sysdb->ldb, SYSDB_VERSION_0_11, ver);
- talloc_free(tmp_ctx);
- return ret;
+}
1.7.7.6
From ea12ef7f52ec959f83370b22a1b108a342461fe0 Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 04:51:08 -0400 Subject: [PATCH 07/10] Ghost members - NSS responder changes
Since there are two attributes storing information about user memberships of the group we have to include both of them in results.
This will apply only for objects that have ghost members (i.e. they contain the SYSDB_GHOST attribute). If an object has this attribute, values of this attribute are not projected to the memberuid attribute.
src/responder/nss/nsssrv_cmd.c | 238 +++++++++++++++++++++++++--------------- 1 files changed, 149 insertions(+), 89 deletions(-)
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index f36a9a322ab92144c93b8cb9041d7a28515cc85d..22ee617daf1968fa8ef0fd310e609db142fdb16a 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -1667,6 +1667,136 @@ done: #define MNUM_ROFFSET sizeof(uint32_t) #define STRS_ROFFSET 2*sizeof(uint32_t)
+static int fill_members(struct sss_packet *packet,
struct sss_domain_info *dom,struct nss_ctx *nctx,struct ldb_message_element *el,size_t *_rzero,size_t *_rsize,int *_memnum)+{
- int i, ret = EOK;
- int memnum = *_memnum;
- size_t rzero= *_rzero;
- size_t rsize = *_rsize;
- char *tmpstr;
- struct sized_string name;
- const char *namefmt = nctx->rctx->names->fq_fmt;
- TALLOC_CTX *tmp_ctx = NULL;
- size_t delim;
- size_t dom_len;
- uint8_t *body;
- size_t blen;
- const char *domain = dom->name;
- bool add_domain = dom->fqnames;
- if (add_domain) {
delim = 1;dom_len = strlen(domain);- } else {
delim = 0;dom_len = 0;- }
- tmp_ctx = talloc_new(NULL);
- if (tmp_ctx == NULL) {
return ENOMEM;- }
- sss_packet_get_body(packet, &body, &blen);
- for (i = 0; i < el->num_values; i++) {
tmpstr = sss_get_cased_name(tmp_ctx, (char *)el->values[i].data,dom->case_sensitive);if (tmpstr == NULL) {DEBUG(SSSDBG_CRIT_FAILURE,("sss_get_cased_name failed, skipping\n"));continue;}if (nctx->filter_users_in_groups) {ret = sss_ncache_check_user(nctx->ncache,nctx->neg_timeout,dom, tmpstr);if (ret == EEXIST) {DEBUG(SSSDBG_TRACE_FUNC, ("Group [%s] member [%s@%s] filtered out!"" (negative cache)\n",(char *)&body[rzero+STRS_ROFFSET],tmpstr, domain));continue;}}to_sized_string(&name, tmpstr);if (add_domain) {ret = sss_packet_grow(packet, name.len + delim + dom_len);} else {ret = sss_packet_grow(packet, name.len);}if (ret != EOK) {goto done;}sss_packet_get_body(packet, &body, &blen);if (add_domain) {ret = snprintf((char *)&body[rzero + rsize],name.len + delim + dom_len,namefmt, name, domain);if (ret >= (name.len + delim + dom_len)) {/* need more space,* got creative with the print format ? */int t = ret - name.len + delim + dom_len + 1;ret = sss_packet_grow(packet, t);if (ret != EOK) {goto done;}sss_packet_get_body(packet, &body, &blen);delim += t;/* retry */ret = snprintf((char *)&body[rzero + rsize],name.len + delim + dom_len,namefmt, name, domain);}if (ret != name.len + delim + dom_len - 1) {DEBUG(SSSDBG_OP_FAILURE, ("Failed to generate a fully qualified name"" for member [%s@%s] of group [%s]!"" Skipping\n", name.str, domain,(char *)&body[rzero+STRS_ROFFSET]));/* reclaim space */ret = sss_packet_shrink(packet,name.len + delim + dom_len);if (ret != EOK) {goto done;}continue;}} else {memcpy(&body[rzero + rsize], name.str, name.len);}if (add_domain) {rsize += name.len + delim + dom_len;} else {rsize += name.len;}memnum++;- }
+done:
- *_memnum = memnum;
- *_rzero = rzero;
- *_rsize = rsize;
- talloc_zfree(tmp_ctx);
- return ret;
+}
static int fill_grent(struct sss_packet *packet, struct sss_domain_info *dom, struct nss_ctx *nctx, @@ -1687,7 +1817,6 @@ static int fill_grent(struct sss_packet *packet, size_t delim; size_t dom_len; int i = 0;
- int j = 0; int ret, num, memnum; size_t rzero, rsize; bool add_domain = dom->fqnames;
@@ -1821,99 +1950,30 @@ static int fill_grent(struct sss_packet *packet, memcpy(&body[rzero+STRS_ROFFSET + fullname.len], pwfield.str, pwfield.len);
memnum = 0; el = ldb_msg_find_element(msg, SYSDB_MEMBERUID); if (el) {
memnum = 0;for (j = 0; j < el->num_values; j++) {tmpstr = sss_get_cased_name(tmp_ctx, (char *)el->values[j].data,dom->case_sensitive);if (tmpstr == NULL) {DEBUG(SSSDBG_CRIT_FAILURE,("sss_get_cased_name failed, skipping\n"));continue;}if (nctx->filter_users_in_groups) {ret = sss_ncache_check_user(nctx->ncache,nctx->neg_timeout,dom, tmpstr);if (ret == EEXIST) {DEBUG(6, ("Group [%s] member [%s@%s] filtered out!"" (negative cache)\n",(char *)&body[rzero+STRS_ROFFSET],tmpstr, domain));continue;}}to_sized_string(&name, tmpstr);if (add_domain) {ret = sss_packet_grow(packet, name.len + delim + dom_len);} else {ret = sss_packet_grow(packet, name.len);}if (ret != EOK) {num = 0;goto done;}sss_packet_get_body(packet, &body, &blen);if (add_domain) {ret = snprintf((char *)&body[rzero + rsize],name.len + delim + dom_len,namefmt, name, domain);if (ret >= (name.len + delim + dom_len)) {/* need more space,* got creative with the print format ? */int t = ret - name.len + delim + dom_len + 1;ret = sss_packet_grow(packet, t);if (ret != EOK) {num = 0;goto done;}sss_packet_get_body(packet, &body, &blen);delim += t;/* retry */ret = snprintf((char *)&body[rzero + rsize],name.len + delim + dom_len,namefmt, name, domain);}if (ret != name.len + delim + dom_len - 1) {DEBUG(1, ("Failed to generate a fully qualified name"" for member [%s@%s] of group [%s]!"" Skipping\n", name.str, domain,(char *)&body[rzero+STRS_ROFFSET]));/* reclaim space */ret = sss_packet_shrink(packet,name.len + delim + dom_len);if (ret != EOK) {num = 0;goto done;}continue;}} else {memcpy(&body[rzero + rsize], name.str, name.len);}if (add_domain) {rsize += name.len + delim + dom_len;} else {rsize += name.len;}memnum++;
ret = fill_members(packet, dom, nctx, el, &rzero, &rsize, &memnum);if (ret != EOK) {num = 0;goto done; }
if (memnum) {/* set num of members */SAFEALIGN_SET_UINT32(&body[rzero+MNUM_ROFFSET], memnum, NULL);
sss_packet_get_body(packet, &body, &blen);}el = ldb_msg_find_element(msg, SYSDB_GHOST);if (el) {ret = fill_members(packet, dom, nctx, el, &rzero, &rsize, &memnum);if (ret != EOK) {num = 0;goto done; }sss_packet_get_body(packet, &body, &blen);}if (memnum) {/* set num of members */SAFEALIGN_SET_UINT32(&body[rzero+MNUM_ROFFSET], memnum, NULL); } num++;-- 1.7.7.6
From 92857f4e0366a9626eaf501d7331e8c633ee5f28 Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 05:13:48 -0400 Subject: [PATCH 08/10] Ghost members - removed sdap_check_aliases()
This function is no longer necessary because we don't have fake user entries any more. The original purpose of this function was to check if there are fake user entries for particular user and, if yes, to update its membership.
src/providers/ldap/sdap_async.c | 108 ---------------------------- src/providers/ldap/sdap_async.h | 6 -- src/providers/ldap/sdap_async_initgroups.c | 7 -- src/providers/ldap/sdap_async_users.c | 6 -- 4 files changed, 0 insertions(+), 127 deletions(-)
diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c index a8a12c3d390a4ebee0dca81d6610be9fe240a4a6..b59f8e9f11209f911057ec981ad69f0a8cab5f4e 100644 --- a/src/providers/ldap/sdap_async.c +++ b/src/providers/ldap/sdap_async.c @@ -2100,114 +2100,6 @@ bool sdap_has_deref_support(struct sdap_handle *sh, struct sdap_options *opts) return false; }
-errno_t sdap_check_aliases(struct sysdb_ctx *sysdb,
struct sysdb_attrs *user_attrs,struct sss_domain_info *dom,struct sdap_options *opts,bool steal_memberships)-{
- errno_t ret;
- const char **aliases = NULL;
- const char *name = NULL;
- struct ldb_message *msg;
- TALLOC_CTX *tmp_ctx = NULL;
- char **parents;
- uid_t alias_uid, uid;
- int i;
- tmp_ctx = talloc_new(NULL);
- if (!tmp_ctx) return ENOMEM;
- ret = sysdb_attrs_primary_name(sysdb, user_attrs,
opts->user_map[SDAP_AT_USER_NAME].sys_name,&name);- if (ret != EOK) {
DEBUG(SSSDBG_TRACE_INTERNAL, ("Could not get the primary name\n"));goto done;- }
- ret = sysdb_attrs_get_uint32_t(user_attrs,
opts->user_map[SDAP_AT_USER_UID].sys_name,&uid);- if (ret != EOK) {
DEBUG(SSSDBG_TRACE_INTERNAL, ("Could not get UID\n"));goto done;- }
- ret = sysdb_attrs_get_aliases(tmp_ctx, user_attrs, name,
!dom->case_sensitive, &aliases);- if (ret != EOK) {
DEBUG(SSSDBG_TRACE_INTERNAL, ("Failed to get the alias list\n"));goto done;- }
- for (i = 0; aliases[i]; i++) {
/* In RFC2307 schema, another group might be referencing user* using secondary name, so there might be fake users in the cache* from a previous getgr call */ret = sysdb_search_user_by_name(tmp_ctx, sysdb,aliases[i], NULL, &msg);if (ret && ret != ENOENT) {DEBUG(SSSDBG_TRACE_INTERNAL, ("Error searching the cache\n"));goto done;} else if (ret == ENOENT) {DEBUG(SSSDBG_TRACE_INTERNAL,("No user with primary name same as alias %s\n", aliases[i]));continue;}alias_uid = ldb_msg_find_attr_as_uint64(msg, SYSDB_UIDNUM, 0);if (alias_uid) {if (alias_uid == uid) {DEBUG(SSSDBG_TRACE_INTERNAL,("User already cached, skipping\n"));continue;}DEBUG(SSSDBG_FATAL_FAILURE,("Cache contains non-fake user with same name ""as alias %s\n", aliases[i]));ret = EIO;goto done;}DEBUG(SSSDBG_TRACE_FUNC, ("%s is a fake user\n", aliases[i]));if (steal_memberships) {/* Get direct sysdb parents */ret = sysdb_get_direct_parents(tmp_ctx, sysdb, dom,SYSDB_MEMBER_USER,aliases[i], &parents);if (ret) {DEBUG(SSSDBG_FATAL_FAILURE,("Could not get direct parents for %s: %d [%s]\n",aliases[i], ret, strerror(ret)));goto done;}ret = sysdb_update_members(sysdb, name, SYSDB_MEMBER_USER,(const char *const *) parents,NULL);if (ret != EOK) {DEBUG(SSSDBG_FATAL_FAILURE,("Membership update failed [%d]: %s\n",ret, strerror(ret)));goto done;}}ret = sysdb_delete_user(sysdb, aliases[i], alias_uid);if (ret) {DEBUG(SSSDBG_FATAL_FAILURE,("Error deleting fake user %s\n", aliases[i]));goto done;}- }
- ret = EOK;
-done:
- talloc_free(tmp_ctx);
- return ret;
-}
errno_t sdap_attrs_add_ldap_attr(struct sysdb_attrs *ldap_attrs, const char *attr_name, diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h index 870f153107d91ba7fa32a6c59a9c9a20907a989d..34fb40daed5830aa9b1adae8ad7b62bb2c429bca 100644 --- a/src/providers/ldap/sdap_async.h +++ b/src/providers/ldap/sdap_async.h @@ -195,12 +195,6 @@ int sdap_deref_search_recv(struct tevent_req *req, size_t *reply_count, struct sdap_deref_attrs ***reply);
-errno_t sdap_check_aliases(struct sysdb_ctx *sysdb,
struct sysdb_attrs *user_attrs,struct sss_domain_info *dom,struct sdap_options *opts,bool steal_memberships);errno_t sdap_attrs_add_ldap_attr(struct sysdb_attrs *ldap_attrs, const char *attr_name, diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c index b883ccf938348f9f5dda4ed43bf714973072c9f9..861176000423e07b57ed3e135b717ea9ecaebbac 100644 --- a/src/providers/ldap/sdap_async_initgroups.c +++ b/src/providers/ldap/sdap_async_initgroups.c @@ -2646,13 +2646,6 @@ static void sdap_get_initgr_user(struct tevent_req *subreq)
switch (state->opts->schema_type) { case SDAP_SCHEMA_RFC2307:
ret = sdap_check_aliases(state->sysdb, state->orig_user, state->dom,state->opts, false);if (ret != EOK) {tevent_req_error(req, ret);return;}subreq = sdap_initgr_rfc2307_send(state, state->ev, state->opts, state->sysdb, state->sh, cname);diff --git a/src/providers/ldap/sdap_async_users.c b/src/providers/ldap/sdap_async_users.c index bc9e5551be32847b45eb87ff4b17572a9b44b584..dfce319b231a84bdbcded041b7e64cc0fbad942e 100644 --- a/src/providers/ldap/sdap_async_users.c +++ b/src/providers/ldap/sdap_async_users.c @@ -411,12 +411,6 @@ int sdap_save_users(TALLOC_CTX *memctx, DEBUG(9, ("User %d processed!\n", i)); }
ret = sdap_check_aliases(sysdb, users[i], dom,opts, true);if (ret) {DEBUG(2, ("Failed to check aliases for user %d. Ignoring.\n", i));}if (usn_value) { if (higher_usn) { if ((strlen(usn_value) > strlen(higher_usn)) ||-- 1.7.7.6
From 1c48c0d6c301ba7ca337503aadb26e7eb9fb08e8 Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 05:22:46 -0400 Subject: [PATCH 09/10] Ghost members - modified sss_groupshow
src/tools/sss_groupshow.c | 44 ++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/src/tools/sss_groupshow.c b/src/tools/sss_groupshow.c index 764e32416b046dfc6ff2a47de37627e40b0109f0..8c15171c86b15bc63649254b61254617391bbe19 100644 --- a/src/tools/sss_groupshow.c +++ b/src/tools/sss_groupshow.c @@ -31,7 +31,7 @@
#define PADDING_SPACES 4 #define GROUP_SHOW_ATTRS { SYSDB_MEMBEROF, SYSDB_GIDNUM, \
SYSDB_MEMBER, SYSDB_NAME, \
SYSDB_MEMBER, SYSDB_GHOST, SYSDB_NAME, \ NULL }#define GROUP_SHOW_MPG_ATTRS { SYSDB_MEMBEROF, SYSDB_UIDNUM, \ SYSDB_NAME, NULL } @@ -189,8 +189,8 @@ static int parse_members(TALLOC_CTX *mem_ctx, }
*user_members = um;
- *group_members = gm;
- *num_group_members = gm_index;
- if (group_members) *group_members = gm;
- if (num_group_members) *num_group_members = gm_index; talloc_zfree(tmp_ctx); return EOK;
@@ -211,8 +211,10 @@ static int process_group(TALLOC_CTX *mem_ctx, int *num_group_members) { struct ldb_message_element *el;
- int ret;
int ret, i, j;
int count = 0; struct group_info *gi = NULL;
const char **user_members;
DEBUG(6, ("Found entry %s\n", ldb_dn_get_linearized(msg->dn)));
@@ -245,6 +247,40 @@ static int process_group(TALLOC_CTX *mem_ctx, if (ret != EOK) { goto done; }
if (gi->user_members == NULL) {count = 0;} else {for (count = 0; gi->user_members[count]; count++) ;}}
el = ldb_msg_find_element(msg, SYSDB_GHOST);
if (el) {
ret = parse_members(gi, ldb, domain, el,parent_name,&user_members,NULL, NULL);if (ret != EOK) {goto done;}if (user_members != NULL) {i = count;for (count = 0; user_members[count]; count++) ;gi->user_members = talloc_realloc(gi, gi->user_members,const char *,i + count + 1);if (gi->user_members == NULL) {ret = ENOMEM;goto done;}for (j = 0; j < count; j++, i++) {gi->user_members[i] = talloc_steal(gi->user_members,user_members[j]);}gi->user_members[i] = NULL;talloc_zfree(user_members);}}
/* list memberofs */
-- 1.7.7.6
From b684bacc88a827ff9316295231b8808fd6090ef2 Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 05:23:15 -0400 Subject: [PATCH 10/10] Ghost members - various small changes
src/providers/ldap/ldap_id_cleanup.c | 2 +- src/providers/ldap/sdap_async_groups.c | 8 +++----- src/responder/nss/nsssrv_cmd.c | 2 +- src/tests/sysdb-tests.c | 2 +- 4 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/providers/ldap/ldap_id_cleanup.c b/src/providers/ldap/ldap_id_cleanup.c index 27a86b9f676a65bb7a0e4f4f10820564f3d2c0e9..3460b8cc605c9bec7b93d0a35ce295bae108865f 100644 --- a/src/providers/ldap/ldap_id_cleanup.c +++ b/src/providers/ldap/ldap_id_cleanup.c @@ -356,7 +356,7 @@ static int cleanup_users_logged_in(hash_table_t *table, uid = ldb_msg_find_attr_as_uint64(msg, SYSDB_UIDNUM, 0); if (!uid) {
DEBUG(2, ("Entry %s has no UID Attribute, fake user perhaps?\n",
DEBUG(SSSDBG_OP_FAILURE, ("Entry %s has no UID Attribute!\n", ldb_dn_get_linearized(msg->dn))); return ENOENT;diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c index a3cf4d05606559409f3bd9ab20aeae76438a508d..32e79c32c6068c69e27d54850f13cad26eee24cf 100644 --- a/src/providers/ldap/sdap_async_groups.c +++ b/src/providers/ldap/sdap_async_groups.c @@ -2390,11 +2390,9 @@ sdap_nested_group_check_cache(TALLOC_CTX *mem_ctx,
user_uid = ldb_msg_find_attr_as_uint64(msgs[0], SYSDB_UIDNUM, 0); if (!user_uid) {
/* Refresh the fake user if he was created before cache_timeout */create_time = ldb_msg_find_attr_as_uint64(msgs[0],SYSDB_CREATE_TIME,0);expiration = create_time + dom->user_timeout;
DEBUG(SSSDBG_OP_FAILURE, ("User with no UID? Skipping\n"));ret = EIO;goto fail; } else { /* Regular user, check if we need a refresh */ expiration = ldb_msg_find_attr_as_uint64(msgs[0],diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index 22ee617daf1968fa8ef0fd310e609db142fdb16a..360f3698c1281a67de311dfc28f46c3724e78ad9 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -258,7 +258,7 @@ static int fill_pwent(struct sss_packet *packet, gid = get_gid_override(msg, dom);
if (!orig_name || !uid || !gid) {
DEBUG(2, ("Incomplete or fake user object for %s[%llu]! Skipping\n",
DEBUG(SSSDBG_OP_FAILURE, ("Incomplete user object for %s[%llu]! Skipping\n", orig_name?orig_name:"<NULL>", (unsigned long long int)uid)); continue; }diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c index bcf3f732be6f172324b0d1b455c28076a2355128..dd053b3a35265aea2ef056ded628051b10dcee74 100644 --- a/src/tests/sysdb-tests.c +++ b/src/tests/sysdb-tests.c @@ -2726,7 +2726,7 @@ START_TEST(test_odd_characters) odd_username, 10000, 10000, "","","");
- fail_unless(ret == EOK, "sysdb_add_fake_user error [%d][%s]",
fail_unless(ret == EOK, "sysdb_add_basic_user error [%d][%s]", ret, strerror(ret));
/* Retrieve */
-- 1.7.7.6
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
On Thu, May 10, 2012 at 10:57:23PM +0200, Jan Zeleny wrote:
Simo Sorce simo@redhat.com wrote:
On Fri, 2012-05-04 at 14:30 +0200, Jan Zelený wrote:
Done. We have to proceed with the approach now, as the basic concept of ghost members shifted a little bit in coparison with the original patch set. Ghost attribute now co-exists with the memberuid, they don't overlap on any level whatsoever.
One comment here. With either the previous or current code I am having some difficulty determining if the ghost attributes (ane memberuid with the previous approach) of groups are appropriately handled if ghost groups should change after a new group lookup.
Assume a user is never actually resolved, but is deleted or renamed in the LDAP server. Are we properly handling updating the relative ghost group all around ?
It would be nice to see a unit test to check this behavior.
The issue is connected to the way you implemented removing the ghost attribute for the user name (and its aliases) in patch 120. I have the impression that if by the time the real user is looked up it has changed (say it has one more alias) that the operation may fail as not ghost entry with the 'new' alias is found so the delete operation may fail due to the value not being in there.
The delete operation in that case may need to be split into a looks/match/delete operation as a fallback (doing it by default would probably kill a bit performances so perhaps a fallback case is better).
Simo.
Ok, here goes another set. This issue should be addressed now. I didn't make the test suite part but I tested the behavior and it works as expected.
The other issue (member count returned to client) is addressed as well.
Jan
Sorry it took so long to get back to the patchset.
Can you rebase the patches on top of current master?
Can you move the src/providers/ldap/sdap_async_groups.c hunk from the last patch to the LDAP patch? The LDAP patch doesn't compile without it (I tried to do a partial review but couldn't compile the patches unless the last one was applied)
I rebased the patchset, I just forgot to send it. This one also contains the change you requested.
Jan
From 48a09ace701e3de727912accd399187e052e795b Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 04:45:29 -0400 Subject: [PATCH 01/10] Ghost members - add the ghost attribute to sysdb
src/db/sysdb.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h index a0d176ec7f9c04b467caf7b2c38ffc6fb263497b..9e7d3c5ca2b584ac306891cbec34d6 3132649e0f 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -68,6 +68,7 @@
#define SYSDB_MEMBER "member" #define SYSDB_MEMBERUID "memberUid"
+#define SYSDB_GHOST "ghost"
#define SYSDB_POSIX "isPosix" #define SYSDB_USER_CATEGORY "userCategory" #define SYSDB_HOST_CATEGORY "hostCategory"
@@ -167,6 +168,7 @@
NULL}#define SYSDB_GRSRC_ATTRS {SYSDB_NAME, SYSDB_GIDNUM, \
SYSDB_MEMBERUID, \
SYSDB_GHOST, \ SYSDB_DEFAULT_ATTRS, \ NULL}#define SYSDB_GRPW_ATTRS {SYSDB_NAME, SYSDB_UIDNUM, \
From 96039ae40cd4c61bd2f78f8a23c3b706cbf90749 Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 04:46:33 -0400 Subject: [PATCH 02/10] Ghost members - support in LDAP provider
The original approach was to store name and original DN in an object in sysdb. When later referenced as member of a group, it was retrieved by its original DN and the correct information about its sysdb DN was stored in the group object which referenced it.
The new approach doesn't use fake user objects, therefore this information has to be reached differently when constructing group memberships. The approach is to store all users to a hash table where original DN is used as the key and username as value. When constructing group memberships, the name is retrieved from this hash table instead of sysdb. This hash table is constructed when retrieving user objects from LDAP server - if the user is not present in sysdb, it is automatically stored in the hash table.
Another situation is for rfc2307. Because there is no nesting there, we can construct the SYSDB_GHOST attribute directly and therefore don't need a hash table of ghost users.
src/providers/ldap/sdap_async_groups.c | 472 +++++++++++++++++++------------- 1 files changed, 287 insertions(+), 185 deletions(-)
diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c index 361525037eb270462251fe03d0c5e1df63de73f4..a3cf4d05606559409f3bd9ab20aeae 76438a508d 100644 --- a/src/providers/ldap/sdap_async_groups.c +++ b/src/providers/ldap/sdap_async_groups.c @@ -96,12 +96,17 @@ static int sdap_fill_memberships(struct sysdb_attrs *group_attrs,
struct sysdb_ctx *ctx, struct sdap_options *opts, struct sss_domain_info *domain,
hash_table_t *ghosts, struct ldb_val *values, int num_values){
struct ldb_message_element *el;
- int i, j;
struct ldb_message_element *ghostel;
int i, j, k;
int ret;
errno_t hret;
hash_key_t key;
hash_value_t value;
ret = sysdb_attrs_get_el(group_attrs, SYSDB_MEMBER, &el); if (ret) {
@@ -116,29 +121,76 @@ static int sdap_fill_memberships(struct sysdb_attrs *group_attrs,
goto done; }
- for (i = 0, j = el->num_values; i < num_values; i++) {
- ret = sysdb_attrs_get_el(group_attrs, SYSDB_GHOST, &ghostel);
- if (ret) {
goto done;- }
/* sync search entry with this as origDN */ret = sdap_find_entry_by_origDN(el->values, ctx, domain,(char *)values[i].data,(char **)&el->values[j].data);
- if (ghostel->num_values == 0) {
/* Element was probably newly created, look for "member" again*/ + ret = sysdb_attrs_get_el(group_attrs, SYSDB_MEMBER, &el);
if (ret != EOK) {
if (ret != ENOENT) {
goto done;}- }
- /* Just allocate both big enough to contain all members for now */
- ghostel->values = talloc_realloc(group_attrs, ghostel->values,
struct ldb_val, + ghostel->num_values + num_values); + if (!ghostel->values) {
ret = ENOMEM;goto done;- }
- j = el->num_values;
- k = ghostel->num_values;
- for (i = 0; i < num_values; i++) {
if (ghosts == NULL) {hret = HASH_ERROR_KEY_NOT_FOUND;} else {key.type = HASH_KEY_STRING;key.str = (char *)values[i].data;hret = hash_lookup(ghosts, &key, &value);}if (hret == HASH_ERROR_KEY_NOT_FOUND) {/* sync search entry with this as origDN */ret = sdap_find_entry_by_origDN(el->values, ctx, domain,(char *)values[i].data,(char**)&el->values[j].data); + if (ret != EOK) {
/* This should never return ENOENT* -> fail if it does*/ goto done; }
DEBUG(7, (" member #%d (%s): not found!\n",i, (char *)values[i].data));} else { DEBUG(7, (" member #%d (%s): [%s]\n", i, (char *)values[i].data, (char *)el->values[j].data)); el->values[j].length = strlen((char *)el->values[j].data); j++;
} else if (hret != HASH_SUCCESS) {ret = EFAULT;goto done;} else {DEBUG(SSSDBG_TRACE_FUNC, (" member #%d (%s): ghost!\n",i, (char *)values[i].data));ghostel->values[k].data = (uint8_t*)talloc_strdup(ghostel->values, + value.ptr); + if (ghostel->values[k].data == NULL) {
ret = ENOMEM;goto done;}ghostel->values[k].length = strlen((char*)ghostel->values[k].data); + k++;
} } el->num_values = j;
ghostel->num_values = k;
ret = EOK;
done: @@ -186,6 +238,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
struct sss_domain_info *dom, struct sysdb_attrs *attrs, bool populate_members,
hash_table_t *ghosts, char **_usn_value, time_t now){
@@ -193,12 +246,14 @@ static int sdap_save_group(TALLOC_CTX *memctx,
struct sysdb_attrs *group_attrs; const char *name = NULL; gid_t gid;
- int ret;
int ret, cnt, i;
char *usn_value = NULL; TALLOC_CTX *tmpctx = NULL; bool posix_group; bool use_id_mapping = dp_opt_get_bool(opts->basic, SDAP_ID_MAPPING); char *sid_str;
hash_key_t key;
hash_value_t value;
tmpctx = talloc_new(memctx); if (!tmpctx) {
@@ -321,16 +376,58 @@ static int sdap_save_group(TALLOC_CTX *memctx,
if (populate_members) { struct ldb_message_element *el1;
struct ldb_message_element *gh; ret = sysdb_attrs_get_el(attrs, opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name, &el1); if (ret != EOK) { goto fail; }ret = sysdb_attrs_get_el(group_attrs, SYSDB_MEMBER, &el); if (ret != EOK) { goto fail; } el->values = el1->values; el->num_values = el1->num_values;ret = sysdb_attrs_get_el(attrs, SYSDB_GHOST, &gh);if (ret != EOK) {goto fail;}ret = sysdb_attrs_get_el(group_attrs, SYSDB_GHOST, &el);if (ret != EOK) {goto fail;}el->values = gh->values;el->num_values = gh->num_values;/* Now process RFC2307bis ghost hash table */if (ghosts != NULL) {cnt = el->num_values + el1->num_values;el->values = talloc_realloc(attrs, el->values, structldb_val, + cnt);
if (el->values == NULL) {ret = ENOMEM;goto fail;}for (i = 0; i < el1->num_values; i++) {key.type = HASH_KEY_STRING;key.str = (char *)el1->values[i].data;ret = hash_lookup(ghosts, &key, &value);if (ret == HASH_ERROR_KEY_NOT_FOUND) {continue;} else if (ret != HASH_SUCCESS) {ret = EFAULT;goto fail;}el->values[el->num_values].data = (uint8_t*)talloc_strdup(el->values, value.ptr); + if (el->values[el->num_values].data == NULL) {
ret = ENOMEM;goto fail;}el->values[el->num_values].length = strlen(value.ptr)+1;el->num_values++;}}}
ret = sdap_save_all_names(name, attrs, !dom->case_sensitive, group_attrs);
@@ -373,6 +470,7 @@ static int sdap_save_grpmem(TALLOC_CTX *memctx,
struct sdap_options *opts, struct sss_domain_info *dom, struct sysdb_attrs *attrs,
hash_table_t *ghosts, time_t now){
struct ldb_message_element *el;@@ -405,6 +503,7 @@ static int sdap_save_grpmem(TALLOC_CTX *memctx,
} ret = sdap_fill_memberships(group_attrs, ctx, opts, dom,
ghosts, el->values, el->num_values); if (ret) { goto fail;@@ -434,6 +533,7 @@ static int sdap_save_groups(TALLOC_CTX *memctx,
struct sysdb_attrs **groups, int num_groups, bool populate_members,
hash_table_t *ghosts, char **_usn_value){
TALLOC_CTX *tmpctx;@@ -487,7 +587,8 @@ static int sdap_save_groups(TALLOC_CTX *memctx,
/* if 2 pass savemembers = false */ ret = sdap_save_group(tmpctx, sysdb, opts, dom, groups[i],
populate_members, &usn_value, now);
populate_members, ghosts,&usn_value, now); /* Do not fail completely on errors. * Just report the failure to save and go on */@@ -520,7 +621,8 @@ static int sdap_save_groups(TALLOC_CTX *memctx,
for (i = 0; i < nsaved_groups; i++) {
ret = sdap_save_grpmem(tmpctx, sysdb, opts, dom,saved_groups[i], now); + ret = sdap_save_grpmem(tmpctx, sysdb, opts, dom, saved_groups[i], + ghosts, now);
/* Do not fail completely on errors. * Just report the failure to save and go on */ if (ret) {@@ -557,13 +659,12 @@ struct sdap_process_group_state {
struct sysdb_ctx *sysdb; struct sysdb_attrs *group;
struct sysdb_attrs **new_members;
struct ldb_message_element* sysdb_dns;
struct ldb_message_element* ghost_dns;
char **queued_members; int queue_len; const char **attrs; const char *filter;
size_t member_idx;
size_t queue_idx; size_t count; size_t check_count;
@@ -578,7 +679,32 @@ static int sdap_process_group_members_2307bis(struct tevent_req *req,
struct sdap_process_group_state *state, struct ldb_message_element *memberel);static int sdap_process_group_members_2307(struct sdap_process_group_state *state,
struct ldb_message_element*memberel); + struct ldb_message_element *memberel, + struct ldb_message_element *ghostel); + +static errno_t sdap_process_group_create_dns(TALLOC_CTX *mem_ctx,
size_t num_values,struct ldb_message_element**_dns) +{
- struct ldb_message_element *dns;
- dns = talloc(mem_ctx, struct ldb_message_element);
- if (dns == NULL) {
return ENOMEM;- }
- dns->num_values = 0;
- dns->values = talloc_array(dns, struct ldb_val,
num_values);- if (dns->values == NULL) {
talloc_zfree(dns);return ENOMEM;- }
- *_dns = dns;
- return EOK;
+}
struct tevent_req *sdap_process_group_send(TALLOC_CTX *memctx,
struct tevent_context *ev,@@ -590,6 +716,7 @@ struct tevent_req *sdap_process_group_send(TALLOC_CTX *memctx,
bool enumeration){
struct ldb_message_element *el;
struct ldb_message_element *ghostel;
struct sdap_process_group_state *grp_state; struct tevent_req *req = NULL; const char **attrs;
@@ -621,8 +748,6 @@ struct tevent_req *sdap_process_group_send(TALLOC_CTX *memctx,
grp_state->sysdb = sysdb; grp_state->group = group; grp_state->check_count = 0;
grp_state->new_members = NULL;
grp_state->member_idx = 0;
grp_state->queue_idx = 0; grp_state->queued_members = NULL; grp_state->queue_len = 0;
@@ -644,27 +769,47 @@ struct tevent_req *sdap_process_group_send(TALLOC_CTX *memctx,
goto done; }
- grp_state->sysdb_dns = talloc(grp_state, struct
ldb_message_element); - if (!grp_state->sysdb_dns) {
talloc_zfree(req);return NULL;- }
- grp_state->sysdb_dns->values = talloc_array(grp_state, struct
ldb_val, - el->num_values); - if (!grp_state->sysdb_dns->values) {
talloc_zfree(req);return NULL;- }
- grp_state->sysdb_dns->num_values = 0;
- ret = sysdb_attrs_get_el(group,
SYSDB_GHOST,&ghostel);- if (ret) {
goto done;- }
- if (ghostel->num_values == 0) {
/* Element was probably newly created, look for "member" again*/ + ret = sysdb_attrs_get_el(group,
opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name, + &el);
if (ret != EOK) {goto done;}}
ret = sdap_process_group_create_dns(grp_state, el->num_values,
&grp_state->sysdb_dns);if (ret != EOK) {
goto done;}
ret = sdap_process_group_create_dns(grp_state, el->num_values,
&grp_state->ghost_dns);if (ret != EOK) {
goto done;}
switch (opts->schema_type) {
case SDAP_SCHEMA_RFC2307:
ret = sdap_process_group_members_2307(grp_state, el);
ret = sdap_process_group_members_2307(grp_state, el,ghostel);
break; case SDAP_SCHEMA_IPA_V1: case SDAP_SCHEMA_AD: case SDAP_SCHEMA_RFC2307BIS:
/* Note that this code branch will be used only if* ldap_nesting_level = 0 is set in config file*/ ret = sdap_process_group_members_2307bis(req, grp_state, el); break;@@ -815,12 +960,6 @@ sdap_process_group_members_2307bis(struct tevent_req *req,
memberel->num_values = state->sysdb_dns->num_values; } else { state->count = state->check_count;
state->new_members = talloc_zero_array(state,struct sysdb_attrs *,state->count + 1);if (!state->new_members) {return ENOMEM;} ret = EBUSY;}
@@ -828,41 +967,35 @@ sdap_process_group_members_2307bis(struct tevent_req *req,
}
static int
-sdap_add_group_member_2307(struct sdap_process_group_state *state, +sdap_add_group_member_2307(struct ldb_message_element *sysdb_dns,
struct sss_domain_info *dom, const char *username){
- char *strdn;
- strdn = sysdb_user_strdn(state->sysdb_dns->values,
state->dom->name, username);- if (!strdn) {
sysdb_dns->values[sysdb_dns->num_values].data =
(uint8_t *) talloc_strdup(sysdb_dns->values, username);if (sysdb_dns->values[sysdb_dns->num_values].data == NULL) {
return ENOMEM;}
- state->sysdb_dns->values[state->sysdb_dns->num_values].data =
(uint8_t *) strdn;- state->sysdb_dns->values[state->sysdb_dns->num_values].length =
strlen(strdn);- state->sysdb_dns->num_values++;
sysdb_dns->values[sysdb_dns->num_values].length =
strlen(username);sysdb_dns->num_values++;
return EOK;
}
static int sdap_process_missing_member_2307(struct sdap_process_group_state *state,
char *member_name, bool*in_transaction, - time_t now)
char *member_name, time_t now){
- int ret, sret;
int ret;
TALLOC_CTX *tmp_ctx; const char *filter; const char *username;
const char *user_dn;
size_t count; struct ldb_message **msgs = NULL; static const char *attrs[] = { SYSDB_NAME, NULL };
if (!in_transaction) return EINVAL;
tmp_ctx = talloc_new(NULL); if (!tmp_ctx) return ENOMEM;
@@ -870,7 +1003,7 @@ sdap_process_missing_member_2307(struct sdap_process_group_state *state,
filter = talloc_asprintf(tmp_ctx, "(%s=%s)", SYSDB_NAME_ALIAS, member_name); if (!filter) { ret = ENOMEM;
goto fail;
goto done;}
ret = sysdb_search_users(tmp_ctx, state->sysdb, filter,
@@ -881,74 +1014,51 @@ sdap_process_missing_member_2307(struct sdap_process_group_state *state,
if (count != 1) { DEBUG(1, ("More than one entry with this alias?\n")); ret = EIO;
goto fail;
goto done; } /* fill username with primary name */ username = ldb_msg_find_attr_as_string(msgs[0], SYSDB_NAME, NULL);
goto done;- } else if (ret != EOK && ret != ENOENT) {
if (username == NULL) {ret = EINVAL;DEBUG(SSSDBG_MINOR_FAILURE, ("Inconsistent sysdb: user ""without primary name?\n"));goto done;}user_dn = sysdb_user_strdn(tmp_ctx, state->dom->name, username);if (username == NULL) {return ENOMEM;}ret = sdap_add_group_member_2307(state->sysdb_dns, state->dom,user_dn); + if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, ("Could not add group member %s\n",username)); + }
- } else if (ret == ENOENT || count == 0) {
/* The entry really does not exist, add a ghost */DEBUG(SSSDBG_TRACE_FUNC, ("Adding a ghost entry\n"));ret = sdap_add_group_member_2307(state->ghost_dns, state->dom,member_name); + if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, ("Could not add group member %s\n",member_name)); + }
} else {
ret = EIO;
goto fail;}
username = member_name;
/* The entry really does not exist, add a fake entry */
DEBUG(7, ("Adding a dummy entry\n"));
if (!*in_transaction) {
ret = sysdb_transaction_start(state->sysdb);if (ret != EOK) {DEBUG(1, ("Cannot start sysdb transaction: [%d]: %s\n",ret, strerror(ret)));return ret;}*in_transaction = true;}
ret = sysdb_add_fake_user(state->sysdb, username, NULL, now);
if (ret != EOK) {
DEBUG(1, ("Cannot store fake user entry: [%d]: %s\n",ret, strerror(ret)));goto fail;}
/*
* Convert the just received DN into the corresponding sysdb DN* for saving into member attribute of the group*/done:
ret = sdap_add_group_member_2307(state, username);
if (ret != EOK) {
DEBUG(1, ("Could not add group member %s\n", username));goto fail;}
talloc_free(tmp_ctx);
return EOK;
-fail:
talloc_free(tmp_ctx);
if (*in_transaction) {
sret = sysdb_transaction_cancel(state->sysdb);if (sret == EOK) {*in_transaction = false;} else {DEBUG(0, ("Unable to cancel transaction! [%d][%s]\n",sret, strerror(sret)));}}
return ret;
}
static int sdap_process_group_members_2307(struct sdap_process_group_state *state,
struct ldb_message_element *memberel)
struct ldb_message_element *memberel,struct ldb_message_element *ghostel){
struct ldb_message *msg;
bool in_transaction = false;
char *member_name;
char *userdn;
int ret;
errno_t sret;
time_t now; int i;
@@ -968,7 +1078,12 @@ sdap_process_group_members_2307(struct sdap_process_group_state *state,
*/ DEBUG(7, ("Member already cached in sysdb: %s\n", member_name));
ret = sdap_add_group_member_2307(state, member_name);
userdn = sysdb_user_strdn(state->sysdb_dns,state->dom->name, member_name); + if (userdn == NULL) {
return ENOMEM;}ret = sdap_add_group_member_2307(state->sysdb_dns,state->dom, userdn);
if (ret != EOK) { DEBUG(1, ("Could not add member %s into sysdb\n", member_name)); goto done;@@ -978,8 +1093,7 @@ sdap_process_group_members_2307(struct sdap_process_group_state *state,
DEBUG(7, ("member #%d (%s): not found in sysdb\n", i, member_name));
ret = sdap_process_missing_member_2307(state, member_name,&in_transaction,now); + ret = sdap_process_missing_member_2307(state, member_name, now);
if (ret != EOK) { DEBUG(1, ("Error processing missing member #%d (%s):\n", i, member_name));@@ -992,30 +1106,15 @@ sdap_process_group_members_2307(struct sdap_process_group_state *state,
} }
/* sdap_process_missing_member_2307 starts transaction */
if (in_transaction) {
ret = sysdb_transaction_commit(state->sysdb);if (ret) {DEBUG(2, ("Cannot commit sysdb transaction\n"));goto done;}in_transaction = false;}
ret = EOK; talloc_free(memberel->values); memberel->values = talloc_steal(state->group, state->sysdb_dns->values); memberel->num_values = state->sysdb_dns->num_values;
- talloc_free(ghostel->values);
- ghostel->values = talloc_steal(state->group,
state->ghost_dns->values); + ghostel->num_values = state->ghost_dns->num_values;
done:
- if (in_transaction) {
/* If the transaction is still active here, we need to cancel it*/ - sret = sysdb_transaction_cancel(state->sysdb);
if (sret != EOK) {DEBUG(0, ("Unable to cancel transaction! [%d][%s]\n",sret, strerror(sret)));}}
return ret;
}
@@ -1029,8 +1128,7 @@ static void sdap_process_group_members(struct tevent_req *subreq)
struct sdap_process_group_state *state = tevent_req_data(req, struct sdap_process_group_state); struct ldb_message_element *el;
- struct ldb_dn *dn;
- char* dn_string;
uint8_t* name_string;
state->check_count--; DEBUG(9, ("Members remaining: %d\n", state->check_count));
@@ -1055,31 +1153,12 @@ static void sdap_process_group_members(struct tevent_req *subreq)
goto next; }
- /*
* Convert the just received DN into the corresponding sysdb DN* for later usage by sdap_save_groups()*/- dn = sysdb_user_dn(state->sysdb, state, state->dom->name,
(char*)el[0].values[0].data);- if (!dn) {
tevent_req_error(req, ENOMEM);return;- }
- dn_string = ldb_dn_alloc_linearized(state->group, dn);
- if (!dn_string) {
tevent_req_error(req, ENOMEM);return;- }
- state->sysdb_dns->values[state->sysdb_dns->num_values].data =
(uint8_t*)dn_string;- state->sysdb_dns->values[state->sysdb_dns->num_values].length =
strlen(dn_string);- state->sysdb_dns->num_values++;
- state->new_members[state->member_idx] = usr_attrs[0];
- state->member_idx++;
- name_string = el[0].values[0].data;
- state->ghost_dns->values[state->ghost_dns->num_values].data =
talloc_steal(state->ghost_dns->values, name_string);- state->ghost_dns->values[state->ghost_dns->num_values].length =
strlen((char *)name_string);- state->ghost_dns->num_values++;
next: if (ret) {
@@ -1110,21 +1189,13 @@ next: }
if (state->check_count == 0) {
ret = sdap_save_users(state, state->sysdb,state->dom, state->opts,state->new_members, state->count, NULL);if (ret) {DEBUG(2, ("Failed to store users.\n"));tevent_req_error(req, ret);return;}/* * To avoid redundant sysdb lookups, populate the "member" attribute * of the group entry with the sysdb DNs of the members. */ ret = sysdb_attrs_get_el(state->group,state->opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name,&el); + state->opts->group_map[SDAP_AT_GROUP_MEMBER].sys_name, + &el);
if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to get the group member attribute [%d]: %s\n",@@ -1134,6 +1205,14 @@ next: } el->values = talloc_steal(state->group, state->sysdb_dns->values); el->num_values = state->sysdb_dns->num_values;
ret = sysdb_attrs_get_el(state->group, SYSDB_GHOST, &el);if (ret != EOK) {tevent_req_error(req, ret);return;}el->values = talloc_steal(state->group,state->ghost_dns->values); + el->num_values = state->ghost_dns->num_values;
DEBUG(9, ("Processed Group - Done\n")); tevent_req_done(req); }@@ -1446,7 +1525,8 @@ static void sdap_get_groups_process(struct tevent_req *subreq)
DEBUG(9, ("Saving groups without members first " "to allow unrolling of nested groups.\n")); ret = sdap_save_groups(state, state->sysdb, state->dom, state->opts,
state->groups, state->count, false,NULL); + state->groups, state->count, false, + NULL, NULL);
if (ret) { DEBUG(2, ("Failed to store groups.\n")); tevent_req_error(req, ret);@@ -1497,7 +1577,7 @@ static void sdap_get_groups_done(struct tevent_req *subreq)
DEBUG(9, ("All groups processed\n")); ret = sdap_save_groups(state, state->sysdb, state->dom, state->opts,
state->groups, state->count, true,
state->groups, state->count, true, NULL, &state->higher_usn); if (ret) { DEBUG(2, ("Failed to store groups.\n"));@@ -1530,10 +1610,12 @@ int sdap_get_groups_recv(struct tevent_req *req,
return EOK;}
-static errno_t sdap_nested_group_populate_users(struct sysdb_ctx *sysdb, +static errno_t sdap_nested_group_populate_users(TALLOC_CTX *mem_ctx,
struct sysdb_ctx *sysdb, struct sdap_options *opts, struct sysdb_attrs **users,
int num_users);
int num_users,hash_table_t **_ghosts);static void sdap_nested_done(struct tevent_req *subreq) {
@@ -1546,6 +1628,7 @@ static void sdap_nested_done(struct tevent_req *subreq)
bool in_transaction = false; struct sysdb_attrs **users = NULL; struct sysdb_attrs **groups = NULL;
hash_table_t *ghosts;
struct tevent_req *req = tevent_req_callback_data(subreq,
struct tevent_req);struct sdap_get_groups_state *state = tevent_req_data(req,
@@ -1608,14 +1691,15 @@ static void sdap_nested_done(struct tevent_req *subreq)
} in_transaction = true;
- ret = sdap_nested_group_populate_users(state->sysdb, state->opts,
users, user_count);
- ret = sdap_nested_group_populate_users(state, state->sysdb,
state->opts, + users, user_count, &ghosts);
if (ret != EOK) { goto fail; } ret = sdap_save_groups(state, state->sysdb, state->dom, state->opts,
groups, group_count, false,&state->higher_usn); + groups, group_count, false, ghosts,
&state->higher_usn);if (ret != EOK) {
goto fail;}
@@ -1641,10 +1725,12 @@ fail: tevent_req_error(req, ret);
}
-static errno_t sdap_nested_group_populate_users(struct sysdb_ctx *sysdb, +static errno_t sdap_nested_group_populate_users(TALLOC_CTX *mem_ctx,
struct sysdb_ctx *sysdb, struct sdap_options *opts, struct sysdb_attrs **users,
int num_users)
int num_users,hash_table_t **_ghosts){
int i; errno_t ret, sret;@@ -1659,24 +1745,36 @@ static errno_t sdap_nested_group_populate_users(struct sysdb_ctx *sysdb,
const char *sysdb_name; struct sysdb_attrs *attrs; static const char *search_attrs[] = { SYSDB_NAME, NULL };
hash_table_t *ghosts;
hash_key_t key;
hash_value_t value;
size_t count;
- time_t now;
if (_ghosts == NULL) {
return EINVAL;}
if (num_users == 0) {
/* Nothing to do if there are no users */*_ghosts = NULL; return EOK;}
tmp_ctx = talloc_new(NULL); if (!tmp_ctx) return ENOMEM;
ret = sss_hash_create(tmp_ctx, num_users, &ghosts);
if (ret != HASH_SUCCESS) {
ret = ENOMEM;goto done;}
ret = sysdb_transaction_start(sysdb); if (ret) {
DEBUG(1, ("Failed to start transaction!\n")); goto done;}
now = time(NULL);
for (i = 0; i < num_users; i++) {
ret = sysdb_attrs_primary_name(sysdb, users[i], opts->user_map[SDAP_AT_USER_NAME].na me,@@ -1744,15 +1842,17 @@ static errno_t sdap_nested_group_populate_users(struct sysdb_ctx *sysdb,
ret = sysdb_set_user_attr(sysdb, sysdb_name, attrs, SYSDB_MOD_REP); if (ret != EOK) goto done; }
/* If the entry does not exist add a fake user record */ret = sysdb_add_fake_user(sysdb, username, original_dn, now);if (ret != EOK) {DEBUG(1, ("Cannot store fake user entry, ignoring: [%d]:%s\n", - ret, strerror(ret)));
continue;} else {DEBUG(9, ("Added incomplete user %s!\n", username));
else{key.type = HASH_KEY_STRING;key.str = discard_const(original_dn);value.type = HASH_VALUE_PTR;value.ptr = discard_const(username);ret = hash_enter(ghosts, &key, &value);if (ret != HASH_SUCCESS) {ret = ENOMEM;goto done;} }}
@@ -1764,13 +1864,16 @@ static errno_t sdap_nested_group_populate_users(struct sysdb_ctx *sysdb,
ret = EOK;done:
talloc_zfree(tmp_ctx);
if (ret != EOK) {
sret = sysdb_transaction_cancel(sysdb); if (sret != EOK) { DEBUG(2, ("Could not cancel transaction\n")); }
*_ghosts = NULL;} else {
*_ghosts = talloc_steal(mem_ctx, ghosts);}
talloc_zfree(tmp_ctx);
return ret;
}
@@ -2242,7 +2345,6 @@ sdap_nested_group_check_cache(TALLOC_CTX *mem_ctx,
struct ldb_message **msgs = NULL; char *member_dn; uint64_t expiration;
uint64_t create_time;
uid_t user_uid; time_t now = time(NULL); static const char *attrs[] = { SYSDB_CACHE_EXPIRE, SYSDB_UIDNUM,
From dd53be03778ab9e804d286752375292b65dc0f69 Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 04:47:23 -0400 Subject: [PATCH 03/10] Ghost members - support in proxy provider
src/providers/proxy/proxy_id.c | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/providers/proxy/proxy_id.c b/src/providers/proxy/proxy_id.c index 8a8c7ca80d1b24e53c3d55d06564e719a069642a..6c78848267ff76aa1b7e1eca9fbc36 ded58ca3b0 100644 --- a/src/providers/proxy/proxy_id.c +++ b/src/providers/proxy/proxy_id.c
@@ -489,6 +489,7 @@ done: static errno_t proxy_process_missing_users(struct sysdb_ctx *sysdb,
struct sysdb_attrs*group_attrs,
struct group *grp, time_t now);static int save_group(struct sysdb_ctx *sysdb, struct sss_domain_info *dom,
@@ -530,8 +531,8 @@ static int save_group(struct sysdb_ctx *sysdb, struct sss_domain_info *dom,
goto done; }
/* Create fake users if they don't already exist */ret = proxy_process_missing_users(sysdb, grp, now);
/* Create ghost users */ret = proxy_process_missing_users(sysdb, attrs, grp, now); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("Could not add missing members\n")); goto done;@@ -612,6 +613,7 @@ done: }
static errno_t proxy_process_missing_users(struct sysdb_ctx *sysdb,
struct sysdb_attrs*group_attrs,
struct group *grp, time_t now){
@@ -636,15 +638,15 @@ static errno_t proxy_process_missing_users(struct sysdb_ctx *sysdb,
talloc_zfree(msg); continue; } else if (ret == ENOENT) {
/* No entry for this user. Create a fake user */
/* No entry for this user. Create a ghost user */ DEBUG(SSSDBG_TRACE_LIBS,
("Member [%s] not cached, creating fake user entry\n",
("Member [%s] not cached, creating ghost userentry\n",
grp->gr_mem[i]));
ret = sysdb_add_fake_user(sysdb, grp->gr_mem[i], NULL, now);
ret = sysdb_attrs_add_string(group_attrs, SYSDB_GHOST,grp->gr_mem[i]);
if (ret != EOK) { DEBUG(SSSDBG_MINOR_FAILURE,
("Cannot store fake user entry: [%d]: %s\n",
("Cannot store ghost user entry: [%d]: %s\n", ret, strerror(ret))); goto done; }From c75d6a16569fe6250b3b3659bc96961c2fdcde8e Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 04:49:16 -0400 Subject: [PATCH 04/10] Ghost members - modifications in sysdb
Deleted sysdb_add_fake_user(): This function is no longer used.
Modified sysdb_add_user(): When user object is added to sysdb, it is important to iterate over all groups that might have its name or any of its aliases as ghost member and replace this ghost membership by a real one. This will eliminate duplicite memberships.
src/db/sysdb.h | 5 - src/db/sysdb_ops.c | 256 ++++++++++++++++++++++++++++++++++------------------ 2 files changed, 167 insertions(+), 94 deletions(-)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 9e7d3c5ca2b584ac306891cbec34d63132649e0f..85dde2c4908b9c502921a82558ff09 329c4052ca 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -583,11 +583,6 @@ int sysdb_add_user(struct sysdb_ctx *sysdb,
int cache_timeout, time_t now);-int sysdb_add_fake_user(struct sysdb_ctx *sysdb,
const char *name,const char *original_dn,time_t now);/* Add group (only basic attrs and w/o checks) */ int sysdb_add_basic_group(struct sysdb_ctx *sysdb,
const char *name, gid_t gid);diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index bedd8cfa292cfdab2ba20260ff641730a41610b5..1bfb6c926ca73530be34d9cb6b2713 a6f1e1a0d6 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -866,9 +866,16 @@ int sysdb_add_user(struct sysdb_ctx *sysdb,
{
TALLOC_CTX *tmp_ctx; struct ldb_message *msg;
struct ldb_message **groups;
struct ldb_message_element *alias_el;
size_t group_count = 0;
struct sysdb_attrs *id_attrs;
const char *group_attrs[] = {SYSDB_NAME, SYSDB_GHOST, NULL};
struct ldb_dn *tmpdn;
const char *userdn;
char *filter;
uint32_t id;
- int ret;
int ret, i, j;
struct sss_domain_info *domain = sysdb->domain;
@@ -977,6 +984,105 @@ int sysdb_add_user(struct sysdb_ctx *sysdb,
ret = sysdb_set_user_attr(sysdb, name, attrs, SYSDB_MOD_REP);
- /* remove all ghost users */
- filter = talloc_asprintf(tmp_ctx, "(|(%s=%s)", SYSDB_GHOST, name);
- if (!filter) {
ret = ENOMEM;goto done;- }
- ret = sysdb_attrs_get_el(attrs, SYSDB_NAME_ALIAS, &alias_el);
- if (ret != EOK) {
goto done;- }
- for (i = 0; i < alias_el->num_values; i++) {
filter = talloc_asprintf_append(filter, "(%s=%s)",SYSDB_GHOST,alias_el->values[i].data); + if (filter == NULL) {
ret = ENOMEM;goto done;}- }
- filter = talloc_asprintf_append(filter, ")");
- if (filter == NULL) {
ret = ENOMEM;goto done;- }
- tmpdn = sysdb_user_dn(sysdb, tmp_ctx, sysdb->domain->name, name);
- if (!tmpdn) {
ERROR_OUT(ret, ENOMEM, done);- }
- userdn = ldb_dn_get_linearized(tmpdn);
- if (!userdn) {
ERROR_OUT(ret, EINVAL, done);- }
- tmpdn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb,
SYSDB_TMPL_GROUP_BASE, sysdb->domain->name);- if (!tmpdn) {
ret = ENOMEM;goto done;- }
- /* We need to find all groups that contain this object as a ghost
user + * and replace the ghost user there by actual member record +
- Note that this object can be referred to either by its name or any
* of its aliases*/- ret = sysdb_search_entry(tmp_ctx, sysdb, tmpdn, LDB_SCOPE_SUBTREE,
filter, + group_attrs, &group_count, &groups); + if (ret != EOK && ret != ENOENT) {
goto done;- }
- for (i = 0; i < group_count; i++) {
msg = ldb_msg_new(tmp_ctx);if (!msg) {ERROR_OUT(ret, ENOMEM, done);}msg->dn = groups[i]->dn;ret = ldb_msg_add_empty(msg, SYSDB_MEMBER, SYSDB_MOD_ADD, NULL);if (ret != LDB_SUCCESS) {ERROR_OUT(ret, ENOMEM, done);}ret = ldb_msg_add_fmt(msg, SYSDB_MEMBER, "%s", userdn);if (ret != LDB_SUCCESS) {ERROR_OUT(ret, EINVAL, done);}ret = ldb_msg_add_empty(msg, SYSDB_GHOST, SYSDB_MOD_DEL, NULL);if (ret != LDB_SUCCESS) {ERROR_OUT(ret, ENOMEM, done);}ret = ldb_msg_add_fmt(msg, SYSDB_GHOST, "%s", name);if (ret != LDB_SUCCESS) {ERROR_OUT(ret, EINVAL, done);}/* Delete aliases from the ghost attribute as well */for (j = 0; j < alias_el->num_values; j++) {ret = ldb_msg_add_fmt(msg, SYSDB_GHOST, "%s",(char *)alias_el->values[j].data);if (ret != LDB_SUCCESS) {ERROR_OUT(ret, EINVAL, done);}}ret = ldb_modify(sysdb->ldb, msg);ret = sysdb_error_to_errno(ret);if (ret != EOK) {goto done;}talloc_zfree(msg);- }
- ret = EOK;
done: if (ret == EOK) {
ret = ldb_transaction_commit(sysdb->ldb);@@ -989,76 +1095,6 @@ done: return ret;
}
-int sysdb_add_fake_user(struct sysdb_ctx *sysdb,
const char *name,const char *original_dn,time_t now)-{
- TALLOC_CTX *tmp_ctx;
- struct ldb_message *msg;
- int ret;
- tmp_ctx = talloc_new(NULL);
- if (!tmp_ctx) {
return ENOMEM;- }
- msg = ldb_msg_new(tmp_ctx);
- if (!msg) {
ERROR_OUT(ret, ENOMEM, done);- }
- /* user dn */
- msg->dn = sysdb_user_dn(sysdb, msg, sysdb->domain->name, name);
- if (!msg->dn) {
ERROR_OUT(ret, ENOMEM, done);- }
- ret = add_string(msg, LDB_FLAG_MOD_ADD, SYSDB_OBJECTCLASS,
SYSDB_USER_CLASS); - if (ret) goto done;
- ret = add_string(msg, LDB_FLAG_MOD_ADD, SYSDB_NAME, name);
- if (ret) goto done;
- if (!now) {
now = time(NULL);- }
- ret = add_ulong(msg, LDB_FLAG_MOD_ADD, SYSDB_CREATE_TIME,
(unsigned long) now);- if (ret) goto done;
- /* set last login so that the fake entry does not get cleaned up
* immediately */- ret = add_ulong(msg, LDB_FLAG_MOD_ADD, SYSDB_LAST_LOGIN,
(unsigned long) now);- if (ret) return ret;
- ret = add_ulong(msg, LDB_FLAG_MOD_ADD, SYSDB_LAST_UPDATE,
(unsigned long) now);- if (ret) goto done;
- ret = add_ulong(msg, LDB_FLAG_MOD_ADD, SYSDB_CACHE_EXPIRE,
(unsigned long) now-1);- if (ret) goto done;
- if (original_dn) {
ret = add_string(msg, LDB_FLAG_MOD_ADD,SYSDB_ORIG_DN, original_dn);if (ret) goto done;- }
- ret = ldb_add(sysdb->ldb, msg);
- ret = sysdb_error_to_errno(ret);
-done:
- if (ret != EOK) {
DEBUG(6, ("Error: %d (%s)\n", ret, strerror(ret)));- }
- talloc_zfree(tmp_ctx);
- return ret;
-}
/* =Add-Basic-Group-NO-CHECKS============================================= */
int sysdb_add_basic_group(struct sysdb_ctx *sysdb,
@@ -2213,8 +2249,13 @@ int sysdb_delete_user(struct sysdb_ctx *sysdb,
const char *name, uid_t uid){
TALLOC_CTX *tmp_ctx;
const char *attrs[] = {SYSDB_GHOST, NULL};
size_t msg_count;
char *filter;
struct ldb_message **msgs;
struct ldb_message *msg; int ret;
int i;
tmp_ctx = talloc_new(NULL); if (!tmp_ctx) {
@@ -2228,34 +2269,71 @@ int sysdb_delete_user(struct sysdb_ctx *sysdb,
ret = sysdb_search_user_by_uid(tmp_ctx, sysdb, uid, NULL, &msg); }
- if (ret) {
goto fail;- }
- if (ret == EOK) {
if (name && uid) {/* verify name/gid match */const char *c_name;uint64_t c_uid;
- if (name && uid) {
/* verify name/gid match */const char *c_name;uint64_t c_uid;
c_name = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);c_uid = ldb_msg_find_attr_as_uint64(msg, SYSDB_UIDNUM, 0);if (c_name == NULL || c_uid == 0) {DEBUG(2, ("Attribute is missing but this should neverhappen!\n")); + ret = EFAULT;
goto fail;}if (strcmp(name, c_name) || uid != c_uid) {/* this is not the entry we are looking for */ret = EINVAL;goto fail;}}
c_name = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);c_uid = ldb_msg_find_attr_as_uint64(msg, SYSDB_UIDNUM, 0);if (c_name == NULL || c_uid == 0) {DEBUG(2, ("Attribute is missing but this should neverhappen!\n")); - ret = EFAULT;
ret = sysdb_delete_entry(sysdb, msg->dn, false);if (ret) { goto fail; }
if (strcmp(name, c_name) || uid != c_uid) {/* this is not the entry we are looking for */ret = EINVAL;
} else if (ret == ENOENT && name != NULL) {
/* Perhaps a ghost user? */filter = talloc_asprintf(tmp_ctx, "(%s=%s)", SYSDB_GHOST, name);if (filter == NULL) {ret = ENOMEM; goto fail; }
}
ret = sysdb_delete_entry(sysdb, msg->dn, false);
if (ret) {
ret = sysdb_search_groups(tmp_ctx, sysdb, filter, attrs,&msg_count, &msgs); + if (ret != EOK) {
goto fail;}for (i = 0; i < msg_count; i++) {msg = ldb_msg_new(tmp_ctx);if (!msg) {ERROR_OUT(ret, ENOMEM, fail);}msg->dn = msgs[i]->dn;ret = ldb_msg_add_empty(msg, SYSDB_GHOST, SYSDB_MOD_DEL,NULL); + if (ret != LDB_SUCCESS) {
ERROR_OUT(ret, ENOMEM, fail);}ret = ldb_msg_add_fmt(msg, SYSDB_GHOST, "%s", name);if (ret != LDB_SUCCESS) {ERROR_OUT(ret, EINVAL, fail);}ret = ldb_modify(sysdb->ldb, msg);ret = sysdb_error_to_errno(ret);if (ret != EOK) {goto fail;}talloc_zfree(msg);}} else {
goto fail;}
talloc_zfree(tmp_ctx); return EOK;
From cf117c91cb64d05d8b2958d185e96a39772b23f4 Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 04:50:36 -0400 Subject: [PATCH 05/10] Ghost members - modifications in memberof plugin
src/ldb_modules/memberof.c | 47 ++++++++++++++++++++++++++++++++++++++----- 1 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/src/ldb_modules/memberof.c b/src/ldb_modules/memberof.c index 4fc46fa84529ac1850cdb588ddf69be0f60e8238..f0b5b72ed11ce71aec8ccb6e7a7cbd 2d02e9566e 100644 --- a/src/ldb_modules/memberof.c +++ b/src/ldb_modules/memberof.c @@ -28,6 +28,7 @@
#include "dhash.h"
#define DB_MEMBER "member"
+#define DB_GHOST "ghost"
#define DB_MEMBEROF "memberof" #define DB_MEMBERUID "memberuid" #define DB_NAME "name"
@@ -191,7 +192,8 @@ static int mbof_append_muop(TALLOC_CTX *memctx,
int *_num_muops, int flags, struct ldb_dn *parent,
const char *name)
const char *name,const char *element_name){
struct mbof_memberuid_op *muops = *_muops; int num_muops = *_num_muops;@@ -229,7 +231,7 @@ static int mbof_append_muop(TALLOC_CTX *memctx,
if (!op->el) { return LDB_ERR_OPERATIONS_ERROR; }
op->el->name = talloc_strdup(op->el, DB_MEMBERUID);
op->el->name = talloc_strdup(op->el, element_name); if (!op->el->name) { return LDB_ERR_OPERATIONS_ERROR; }@@ -534,7 +536,8 @@ static int mbof_add_callback(struct ldb_request *req,
static int mbof_next_add(struct mbof_add_operation *addop) {
static const char *attrs[] = { DB_OC, DB_NAME,
DB_MEMBER, DB_MEMBEROF, NULL };
DB_MEMBER, DB_GHOST,DB_MEMBEROF, NULL };struct ldb_context *ldb; struct ldb_request *req; struct mbof_add_ctx *add_ctx;
@@ -789,7 +792,8 @@ static int mbof_add_operation(struct mbof_add_operation *addop)
ret = mbof_append_muop(add_ctx, &add_ctx->muops, &add_ctx->num_muops, LDB_FLAG_MOD_ADD,
parents->dns[i], name);
parents->dns[i], name,DB_MEMBERUID); if (ret != LDB_SUCCESS) { return ret; }@@ -806,6 +810,35 @@ static int mbof_add_operation(struct mbof_add_operation *addop)
return ret; }
el = ldb_msg_find_element(addop->entry, DB_GHOST);
if (el) {
for (i = 0; i < el->num_values; i++) {/* add memberuid to all group's parents */for (j = 0; j < parents->num; j++) {ret = mbof_append_muop(add_ctx, &add_ctx->muops,&add_ctx->num_muops,LDB_FLAG_MOD_ADD,parents->dns[j],(char *)el->values[i].data,DB_GHOST);if (ret != LDB_SUCCESS) {return ret;}}/* now add memberuid to the group itself */ret = mbof_append_muop(add_ctx, &add_ctx->muops,&add_ctx->num_muops,LDB_FLAG_MOD_ADD,addop->entry_dn,(char *)el->values[i].data,DB_GHOST);if (ret != LDB_SUCCESS) {return ret;}}}
/* we are done with the entry now */ talloc_free(addop->entry); addop->entry = NULL;
@@ -2071,7 +2104,8 @@ static int mbof_del_mod_entry(struct mbof_del_operation *delop)
ret = mbof_append_muop(del_ctx, &del_ctx->muops, &del_ctx->num_muops, LDB_FLAG_MOD_DELETE,
diff[i], name);
diff[i], name,DB_MEMBERUID); if (ret != LDB_SUCCESS) { return ret; }@@ -2311,7 +2345,8 @@ static int mbof_del_fill_muop(struct mbof_del_ctx *del_ctx,
ret = mbof_append_muop(del_ctx, &del_ctx->muops, &del_ctx->num_muops, LDB_FLAG_MOD_DELETE,
valdn, name);
valdn, name,DB_MEMBERUID); if (ret != LDB_SUCCESS) { return ret; }From 169772136d2c1f9cafdcec2dd30e5ad144e0834d Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 14:08:00 -0400 Subject: [PATCH 06/10] Ghost members - sysdb upgrade routine
It is remotely possible to have sysdb in an inconsistent state that might need upgrade. Consider scenario when user asks for group information. Some fake users are added as a part of this operation. Before users can be fully resolved and stored properly, SSSD is shut down and upgrade is performed.
In this case we need to go over all fake user records (uidNumber=0) and replace each of them with ghost record in all group objects that are stated in its memberof attribute.
src/db/sysdb.c | 7 ++ src/db/sysdb_private.h | 4 +- src/db/sysdb_upgrade.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 1 deletions(-)
diff --git a/src/db/sysdb.c b/src/db/sysdb.c index 41c677d7e5fe16ed2799384c5c5744f92d13d64c..fd76f73c3d4f48fbaff362477aa207 e3ac260eba 100644 --- a/src/db/sysdb.c +++ b/src/db/sysdb.c @@ -1061,6 +1061,13 @@ int sysdb_domain_init_internal(TALLOC_CTX *mem_ctx,
} }
if (strcmp(version, SYSDB_VERSION_0_10) == 0) {ret = sysdb_upgrade_10(sysdb, &version);if (ret != EOK) {goto done;}}/* The version should now match SYSDB_VERSION. * If not, it means we didn't match any of the * known older versions. The DB might bediff --git a/src/db/sysdb_private.h b/src/db/sysdb_private.h index a162cbba82bba8fef1e57ab2774d156307ead2cd..0fb7316b48b5c53be0198f71967308 0275c34b81 100644 --- a/src/db/sysdb_private.h +++ b/src/db/sysdb_private.h @@ -23,6 +23,7 @@
#ifndef __INT_SYS_DB_H__ #define __INT_SYS_DB_H__
+#define SYSDB_VERSION_0_11 "0.11"
#define SYSDB_VERSION_0_10 "0.10" #define SYSDB_VERSION_0_9 "0.9" #define SYSDB_VERSION_0_8 "0.8"
@@ -34,7 +35,7 @@
#define SYSDB_VERSION_0_2 "0.2" #define SYSDB_VERSION_0_1 "0.1"
-#define SYSDB_VERSION SYSDB_VERSION_0_10 +#define SYSDB_VERSION SYSDB_VERSION_0_11
#define SYSDB_BASE_LDIF \
"dn: @ATTRIBUTES\n" \@@ -104,6 +105,7 @@ int sysdb_upgrade_06(struct sysdb_ctx *sysdb, const char **ver);
int sysdb_upgrade_07(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_08(struct sysdb_ctx *sysdb, const char **ver); int sysdb_upgrade_09(struct sysdb_ctx *sysdb, const char **ver);
+int sysdb_upgrade_10(struct sysdb_ctx *sysdb, const char **ver);
int add_string(struct ldb_message *msg, int flags,
const char *attr, const char *value);diff --git a/src/db/sysdb_upgrade.c b/src/db/sysdb_upgrade.c index 4fe6afb3e49e9dbc4d459338d198b4d2a715997d..d9b68d4aa6e0398189d8f272d7ac01 04c8f347ab 100644 --- a/src/db/sysdb_upgrade.c +++ b/src/db/sysdb_upgrade.c
@@ -1169,3 +1169,150 @@ done: talloc_free(tmp_ctx); return ret;
}
+int sysdb_upgrade_10(struct sysdb_ctx *sysdb, const char **ver) +{
- TALLOC_CTX *tmp_ctx;
- int ret;
- struct ldb_result *res;
- struct ldb_message *msg;
- struct ldb_message *user;
- struct ldb_message_element *memberof_el;
- const char *name;
- struct ldb_dn *basedn;
- const char *filter = "(&(objectClass=user)(!(uidNumber=*)))";
- const char *attrs[] = { "name", "memberof", NULL };
- int i, j;
- tmp_ctx = talloc_new(NULL);
- if (tmp_ctx == NULL) {
return ENOMEM;- }
- basedn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb, SYSDB_TMPL_USER_BASE,
sysdb->domain->name);- if (basedn == NULL) {
ret = EIO;goto done;- }
- DEBUG(SSSDBG_CRIT_FAILURE, ("UPGRADING DB TO VERSION %s\n",
SYSDB_VERSION_0_11)); +
- ret = ldb_transaction_start(sysdb->ldb);
- if (ret != LDB_SUCCESS) {
ret = EIO;goto done;- }
- ret = ldb_search(sysdb->ldb, tmp_ctx, &res, basedn,
LDB_SCOPE_SUBTREE, + attrs, "%s", filter);
- if (ret != LDB_SUCCESS) {
ret = EIO;goto done;- }
- for (i = 0; i < res->count; i++) {
user = res->msgs[i];memberof_el = ldb_msg_find_element(user, "memberof");name = ldb_msg_find_attr_as_string(user, "name", NULL);if (name == NULL) {ret = EIO;goto done;}for (j = 0; j < memberof_el->num_values; j++) {msg = ldb_msg_new(tmp_ctx);if (msg == NULL) {ret = ENOMEM;goto done;}msg->dn = ldb_dn_from_ldb_val(tmp_ctx, sysdb->ldb,&memberof_el->values[j]); + if (msg->dn == NULL) {
ret = ENOMEM;goto done;}if (!ldb_dn_validate(msg->dn)) {DEBUG(SSSDBG_MINOR_FAILURE, ("DN validation failedduring " + "upgrade: [%s]\n", + memberof_el->values[j].data)); + talloc_zfree(msg);
continue;}ret = ldb_msg_add_empty(msg, "ghost", LDB_FLAG_MOD_ADD,NULL); + if (ret != LDB_SUCCESS) {
ret = ENOMEM;goto done;}ret = ldb_msg_add_string(msg, "ghost", name);if (ret != LDB_SUCCESS) {ret = ENOMEM;goto done;}ret = ldb_msg_add_empty(msg, "member", LDB_FLAG_MOD_DELETE,NULL); + if (ret != LDB_SUCCESS) {
ret = ENOMEM;goto done;}ret = ldb_msg_add_string(msg, "member",ldb_dn_get_linearized(user->dn)); + if (ret != LDB_SUCCESS) {
ret = ENOMEM;goto done;}ret = ldb_modify(sysdb->ldb, msg);talloc_zfree(msg);if (ret != LDB_SUCCESS) {ret = sysdb_error_to_errno(ret);goto done;}}ret = ldb_delete(sysdb->ldb, user->dn);if (ret != LDB_SUCCESS) {ret = sysdb_error_to_errno(ret);goto done;}/* conversion done, upgrade version number */msg = ldb_msg_new(tmp_ctx);if (!msg) {ret = ENOMEM;goto done;}msg->dn = ldb_dn_new(tmp_ctx, sysdb->ldb, SYSDB_BASE);if (!msg->dn) {ret = ENOMEM;goto done;}ret = ldb_msg_add_empty(msg, "version", LDB_FLAG_MOD_REPLACE,NULL); + if (ret != LDB_SUCCESS) {
ret = ENOMEM;goto done;}ret = ldb_msg_add_string(msg, "version", SYSDB_VERSION_0_11);if (ret != LDB_SUCCESS) {ret = ENOMEM;goto done;}ret = ldb_modify(sysdb->ldb, msg);if (ret != LDB_SUCCESS) {ret = sysdb_error_to_errno(ret);goto done;}- }
- ret = EOK;
+done:
- ret = finish_upgrade(ret, sysdb->ldb, SYSDB_VERSION_0_11, ver);
- talloc_free(tmp_ctx);
- return ret;
+}
From ea12ef7f52ec959f83370b22a1b108a342461fe0 Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 04:51:08 -0400 Subject: [PATCH 07/10] Ghost members - NSS responder changes
Since there are two attributes storing information about user memberships of the group we have to include both of them in results.
This will apply only for objects that have ghost members (i.e. they contain the SYSDB_GHOST attribute). If an object has this attribute, values of this attribute are not projected to the memberuid attribute.
src/responder/nss/nsssrv_cmd.c | 238 +++++++++++++++++++++++++--------------- 1 files changed, 149 insertions(+), 89 deletions(-)
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index f36a9a322ab92144c93b8cb9041d7a28515cc85d..22ee617daf1968fa8ef0fd310e609d b142fdb16a 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c
@@ -1667,6 +1667,136 @@ done: #define MNUM_ROFFSET sizeof(uint32_t) #define STRS_ROFFSET 2*sizeof(uint32_t)
+static int fill_members(struct sss_packet *packet,
struct sss_domain_info *dom,struct nss_ctx *nctx,struct ldb_message_element *el,size_t *_rzero,size_t *_rsize,int *_memnum)+{
- int i, ret = EOK;
- int memnum = *_memnum;
- size_t rzero= *_rzero;
- size_t rsize = *_rsize;
- char *tmpstr;
- struct sized_string name;
- const char *namefmt = nctx->rctx->names->fq_fmt;
- TALLOC_CTX *tmp_ctx = NULL;
- size_t delim;
- size_t dom_len;
- uint8_t *body;
- size_t blen;
- const char *domain = dom->name;
- bool add_domain = dom->fqnames;
- if (add_domain) {
delim = 1;dom_len = strlen(domain);- } else {
delim = 0;dom_len = 0;- }
- tmp_ctx = talloc_new(NULL);
- if (tmp_ctx == NULL) {
return ENOMEM;- }
- sss_packet_get_body(packet, &body, &blen);
- for (i = 0; i < el->num_values; i++) {
tmpstr = sss_get_cased_name(tmp_ctx, (char *)el->values[i].data,dom->case_sensitive);if (tmpstr == NULL) {DEBUG(SSSDBG_CRIT_FAILURE,("sss_get_cased_name failed, skipping\n"));continue;}if (nctx->filter_users_in_groups) {ret = sss_ncache_check_user(nctx->ncache,nctx->neg_timeout,dom, tmpstr);if (ret == EEXIST) {DEBUG(SSSDBG_TRACE_FUNC, ("Group [%s] member [%s@%s]filtered out!" + " (negative cache)\n",
(char *)&body[rzero+STRS_ROFFSET],tmpstr, domain));continue;}}to_sized_string(&name, tmpstr);if (add_domain) {ret = sss_packet_grow(packet, name.len + delim + dom_len);} else {ret = sss_packet_grow(packet, name.len);}if (ret != EOK) {goto done;}sss_packet_get_body(packet, &body, &blen);if (add_domain) {ret = snprintf((char *)&body[rzero + rsize],name.len + delim + dom_len,namefmt, name, domain);if (ret >= (name.len + delim + dom_len)) {/* need more space,* got creative with the print format ? */int t = ret - name.len + delim + dom_len + 1;ret = sss_packet_grow(packet, t);if (ret != EOK) {goto done;}sss_packet_get_body(packet, &body, &blen);delim += t;/* retry */ret = snprintf((char *)&body[rzero + rsize],name.len + delim + dom_len,namefmt, name, domain);}if (ret != name.len + delim + dom_len - 1) {DEBUG(SSSDBG_OP_FAILURE, ("Failed to generate a fullyqualified name" + " for member [%s@%s] of group [%s]!" + " Skipping\n", name.str, domain,
(char *)&body[rzero+STRS_ROFFSET]));/* reclaim space */ret = sss_packet_shrink(packet,name.len + delim + dom_len);if (ret != EOK) {goto done;}continue;}} else {memcpy(&body[rzero + rsize], name.str, name.len);}if (add_domain) {rsize += name.len + delim + dom_len;} else {rsize += name.len;}memnum++;- }
+done:
- *_memnum = memnum;
- *_rzero = rzero;
- *_rsize = rsize;
- talloc_zfree(tmp_ctx);
- return ret;
+}
static int fill_grent(struct sss_packet *packet,
struct sss_domain_info *dom, struct nss_ctx *nctx,@@ -1687,7 +1817,6 @@ static int fill_grent(struct sss_packet *packet,
size_t delim; size_t dom_len; int i = 0;
int j = 0;
int ret, num, memnum; size_t rzero, rsize; bool add_domain = dom->fqnames;
@@ -1821,99 +1950,30 @@ static int fill_grent(struct sss_packet *packet,
memcpy(&body[rzero+STRS_ROFFSET + fullname.len], pwfield.str, pwfield.len);
memnum = 0; el = ldb_msg_find_element(msg, SYSDB_MEMBERUID); if (el) {
memnum = 0;for (j = 0; j < el->num_values; j++) {tmpstr = sss_get_cased_name(tmp_ctx, (char*)el->values[j].data, - dom->case_sensitive); - if (tmpstr == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE,("sss_get_cased_name failed, skipping\n"));continue;}if (nctx->filter_users_in_groups) {ret = sss_ncache_check_user(nctx->ncache,nctx->neg_timeout,dom, tmpstr);if (ret == EEXIST) {DEBUG(6, ("Group [%s] member [%s@%s] filteredout!" - " (negative cache)\n",
(char *)&body[rzero+STRS_ROFFSET],tmpstr, domain));continue;}}to_sized_string(&name, tmpstr);if (add_domain) {ret = sss_packet_grow(packet, name.len + delim +dom_len); - } else {
ret = sss_packet_grow(packet, name.len);}if (ret != EOK) {num = 0;goto done;}sss_packet_get_body(packet, &body, &blen);if (add_domain) {ret = snprintf((char *)&body[rzero + rsize],name.len + delim + dom_len,namefmt, name, domain);if (ret >= (name.len + delim + dom_len)) {/* need more space,* got creative with the print format ? */int t = ret - name.len + delim + dom_len + 1;ret = sss_packet_grow(packet, t);if (ret != EOK) {num = 0;goto done;}sss_packet_get_body(packet, &body, &blen);delim += t;/* retry */ret = snprintf((char *)&body[rzero + rsize],name.len + delim + dom_len,namefmt, name, domain);}if (ret != name.len + delim + dom_len - 1) {DEBUG(1, ("Failed to generate a fully qualifiedname" - " for member [%s@%s] of group [%s]!" - " Skipping\n", name.str, domain, - (char *)&body[rzero+STRS_ROFFSET])); - /* reclaim space */
ret = sss_packet_shrink(packet,name.len + delim +dom_len); - if (ret != EOK) {
num = 0;goto done;}continue;}} else {memcpy(&body[rzero + rsize], name.str, name.len);}if (add_domain) {rsize += name.len + delim + dom_len;} else {rsize += name.len;}memnum++;
ret = fill_members(packet, dom, nctx, el, &rzero, &rsize,&memnum); + if (ret != EOK) {
num = 0;goto done; }
if (memnum) {/* set num of members */SAFEALIGN_SET_UINT32(&body[rzero+MNUM_ROFFSET], memnum,NULL); + sss_packet_get_body(packet, &body, &blen);
}el = ldb_msg_find_element(msg, SYSDB_GHOST);if (el) {ret = fill_members(packet, dom, nctx, el, &rzero, &rsize,&memnum); + if (ret != EOK) {
num = 0;goto done; }sss_packet_get_body(packet, &body, &blen);}if (memnum) {/* set num of members */SAFEALIGN_SET_UINT32(&body[rzero+MNUM_ROFFSET], memnum,NULL);
} num++;From 92857f4e0366a9626eaf501d7331e8c633ee5f28 Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 05:13:48 -0400 Subject: [PATCH 08/10] Ghost members - removed sdap_check_aliases()
This function is no longer necessary because we don't have fake user entries any more. The original purpose of this function was to check if there are fake user entries for particular user and, if yes, to update its membership.
src/providers/ldap/sdap_async.c | 108 ---------------------------- src/providers/ldap/sdap_async.h | 6 -- src/providers/ldap/sdap_async_initgroups.c | 7 -- src/providers/ldap/sdap_async_users.c | 6 -- 4 files changed, 0 insertions(+), 127 deletions(-)
diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c index a8a12c3d390a4ebee0dca81d6610be9fe240a4a6..b59f8e9f11209f911057ec981ad69f 0a8cab5f4e 100644 --- a/src/providers/ldap/sdap_async.c +++ b/src/providers/ldap/sdap_async.c @@ -2100,114 +2100,6 @@ bool sdap_has_deref_support(struct sdap_handle *sh, struct sdap_options *opts)
return false;}
-errno_t sdap_check_aliases(struct sysdb_ctx *sysdb,
struct sysdb_attrs *user_attrs,struct sss_domain_info *dom,struct sdap_options *opts,bool steal_memberships)-{
- errno_t ret;
- const char **aliases = NULL;
- const char *name = NULL;
- struct ldb_message *msg;
- TALLOC_CTX *tmp_ctx = NULL;
- char **parents;
- uid_t alias_uid, uid;
- int i;
- tmp_ctx = talloc_new(NULL);
- if (!tmp_ctx) return ENOMEM;
- ret = sysdb_attrs_primary_name(sysdb, user_attrs,
opts->user_map[SDAP_AT_USER_NAME].sys_name, - &name);
- if (ret != EOK) {
DEBUG(SSSDBG_TRACE_INTERNAL, ("Could not get the primaryname\n")); - goto done;
- }
- ret = sysdb_attrs_get_uint32_t(user_attrs,
opts->user_map[SDAP_AT_USER_UID].sys_name, - &uid);
- if (ret != EOK) {
DEBUG(SSSDBG_TRACE_INTERNAL, ("Could not get UID\n"));goto done;- }
- ret = sysdb_attrs_get_aliases(tmp_ctx, user_attrs, name,
!dom->case_sensitive, &aliases);- if (ret != EOK) {
DEBUG(SSSDBG_TRACE_INTERNAL, ("Failed to get the aliaslist\n")); - goto done;
- }
- for (i = 0; aliases[i]; i++) {
/* In RFC2307 schema, another group might be referencing user* using secondary name, so there might be fake users in thecache - * from a previous getgr call */
ret = sysdb_search_user_by_name(tmp_ctx, sysdb,aliases[i], NULL, &msg);if (ret && ret != ENOENT) {DEBUG(SSSDBG_TRACE_INTERNAL, ("Error searching thecache\n")); - goto done;
} else if (ret == ENOENT) {DEBUG(SSSDBG_TRACE_INTERNAL,("No user with primary name same as alias %s\n",aliases[i])); - continue;
}alias_uid = ldb_msg_find_attr_as_uint64(msg, SYSDB_UIDNUM, 0);if (alias_uid) {if (alias_uid == uid) {DEBUG(SSSDBG_TRACE_INTERNAL,("User already cached, skipping\n"));continue;}DEBUG(SSSDBG_FATAL_FAILURE,("Cache contains non-fake user with same name ""as alias %s\n", aliases[i]));ret = EIO;goto done;}DEBUG(SSSDBG_TRACE_FUNC, ("%s is a fake user\n", aliases[i]));if (steal_memberships) {/* Get direct sysdb parents */ret = sysdb_get_direct_parents(tmp_ctx, sysdb, dom,SYSDB_MEMBER_USER,aliases[i], &parents);if (ret) {DEBUG(SSSDBG_FATAL_FAILURE,("Could not get direct parents for %s: %d [%s]\n",aliases[i], ret, strerror(ret)));goto done;}ret = sysdb_update_members(sysdb, name, SYSDB_MEMBER_USER,(const char *const *) parents,NULL);if (ret != EOK) {DEBUG(SSSDBG_FATAL_FAILURE,("Membership update failed [%d]: %s\n",ret, strerror(ret)));goto done;}}ret = sysdb_delete_user(sysdb, aliases[i], alias_uid);if (ret) {DEBUG(SSSDBG_FATAL_FAILURE,("Error deleting fake user %s\n", aliases[i]));goto done;}- }
- ret = EOK;
-done:
- talloc_free(tmp_ctx);
- return ret;
-}
errno_t sdap_attrs_add_ldap_attr(struct sysdb_attrs *ldap_attrs,
const char *attr_name,diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h index 870f153107d91ba7fa32a6c59a9c9a20907a989d..34fb40daed5830aa9b1adae8ad7b62 bb2c429bca 100644 --- a/src/providers/ldap/sdap_async.h +++ b/src/providers/ldap/sdap_async.h @@ -195,12 +195,6 @@ int sdap_deref_search_recv(struct tevent_req *req,
size_t *reply_count, struct sdap_deref_attrs ***reply);-errno_t sdap_check_aliases(struct sysdb_ctx *sysdb,
struct sysdb_attrs *user_attrs,struct sss_domain_info *dom,struct sdap_options *opts,bool steal_memberships);errno_t sdap_attrs_add_ldap_attr(struct sysdb_attrs *ldap_attrs,
const char *attr_name,diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c index b883ccf938348f9f5dda4ed43bf714973072c9f9..861176000423e07b57ed3e135b717e a9ecaebbac 100644 --- a/src/providers/ldap/sdap_async_initgroups.c +++ b/src/providers/ldap/sdap_async_initgroups.c @@ -2646,13 +2646,6 @@ static void sdap_get_initgr_user(struct tevent_req *subreq)
switch (state->opts->schema_type) { case SDAP_SCHEMA_RFC2307:
ret = sdap_check_aliases(state->sysdb, state->orig_user,state->dom, - state->opts, false);
if (ret != EOK) {tevent_req_error(req, ret);return;}subreq = sdap_initgr_rfc2307_send(state, state->ev, state->opts, state->sysdb, state->sh, cname);diff --git a/src/providers/ldap/sdap_async_users.c b/src/providers/ldap/sdap_async_users.c index bc9e5551be32847b45eb87ff4b17572a9b44b584..dfce319b231a84bdbcded041b7e64c c0fbad942e 100644 --- a/src/providers/ldap/sdap_async_users.c +++ b/src/providers/ldap/sdap_async_users.c @@ -411,12 +411,6 @@ int sdap_save_users(TALLOC_CTX *memctx,
DEBUG(9, ("User %d processed!\n", i)); }
ret = sdap_check_aliases(sysdb, users[i], dom,opts, true);if (ret) {DEBUG(2, ("Failed to check aliases for user %d.Ignoring.\n", i)); - }
if (usn_value) { if (higher_usn) { if ((strlen(usn_value) > strlen(higher_usn)) ||From 1c48c0d6c301ba7ca337503aadb26e7eb9fb08e8 Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 05:22:46 -0400 Subject: [PATCH 09/10] Ghost members - modified sss_groupshow
src/tools/sss_groupshow.c | 44 ++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/src/tools/sss_groupshow.c b/src/tools/sss_groupshow.c index 764e32416b046dfc6ff2a47de37627e40b0109f0..8c15171c86b15bc63649254b612546 17391bbe19 100644 --- a/src/tools/sss_groupshow.c +++ b/src/tools/sss_groupshow.c @@ -31,7 +31,7 @@
#define PADDING_SPACES 4 #define GROUP_SHOW_ATTRS { SYSDB_MEMBEROF, SYSDB_GIDNUM, \
SYSDB_MEMBER, SYSDB_NAME, \
SYSDB_MEMBER, SYSDB_GHOST, SYSDB_NAME, \ NULL }#define GROUP_SHOW_MPG_ATTRS { SYSDB_MEMBEROF, SYSDB_UIDNUM, \
SYSDB_NAME, NULL }@@ -189,8 +189,8 @@ static int parse_members(TALLOC_CTX *mem_ctx,
} *user_members = um;
- *group_members = gm;
- *num_group_members = gm_index;
if (group_members) *group_members = gm;
if (num_group_members) *num_group_members = gm_index;
talloc_zfree(tmp_ctx); return EOK;
@@ -211,8 +211,10 @@ static int process_group(TALLOC_CTX *mem_ctx,
int *num_group_members){
struct ldb_message_element *el;
- int ret;
int ret, i, j;
int count = 0;
struct group_info *gi = NULL;
const char **user_members;
DEBUG(6, ("Found entry %s\n", ldb_dn_get_linearized(msg->dn)));
@@ -245,6 +247,40 @@ static int process_group(TALLOC_CTX *mem_ctx,
if (ret != EOK) { goto done; }
if (gi->user_members == NULL) {count = 0;} else {for (count = 0; gi->user_members[count]; count++) ;}}
el = ldb_msg_find_element(msg, SYSDB_GHOST);
if (el) {
ret = parse_members(gi, ldb, domain, el,parent_name,&user_members,NULL, NULL);if (ret != EOK) {goto done;}if (user_members != NULL) {i = count;for (count = 0; user_members[count]; count++) ;gi->user_members = talloc_realloc(gi, gi->user_members,const char *,i + count + 1);if (gi->user_members == NULL) {ret = ENOMEM;goto done;}for (j = 0; j < count; j++, i++) {gi->user_members[i] = talloc_steal(gi->user_members,user_members[j]);}gi->user_members[i] = NULL;talloc_zfree(user_members);}}
/* list memberofs */
From b684bacc88a827ff9316295231b8808fd6090ef2 Mon Sep 17 00:00:00 2001 From: Jan Zeleny jzeleny@redhat.com Date: Mon, 23 Apr 2012 05:23:15 -0400 Subject: [PATCH 10/10] Ghost members - various small changes
src/providers/ldap/ldap_id_cleanup.c | 2 +- src/providers/ldap/sdap_async_groups.c | 8 +++----- src/responder/nss/nsssrv_cmd.c | 2 +- src/tests/sysdb-tests.c | 2 +- 4 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/providers/ldap/ldap_id_cleanup.c b/src/providers/ldap/ldap_id_cleanup.c index 27a86b9f676a65bb7a0e4f4f10820564f3d2c0e9..3460b8cc605c9bec7b93d0a35ce295 bae108865f 100644 --- a/src/providers/ldap/ldap_id_cleanup.c +++ b/src/providers/ldap/ldap_id_cleanup.c @@ -356,7 +356,7 @@ static int cleanup_users_logged_in(hash_table_t *table,
uid = ldb_msg_find_attr_as_uint64(msg, SYSDB_UIDNUM, 0); if (!uid) {
DEBUG(2, ("Entry %s has no UID Attribute, fake user perhaps?\n",
DEBUG(SSSDBG_OP_FAILURE, ("Entry %s has no UID Attribute!\n", ldb_dn_get_linearized(msg->dn))); return ENOENT;}
diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c index a3cf4d05606559409f3bd9ab20aeae76438a508d..32e79c32c6068c69e27d54850f13ca d26eee24cf 100644 --- a/src/providers/ldap/sdap_async_groups.c +++ b/src/providers/ldap/sdap_async_groups.c @@ -2390,11 +2390,9 @@ sdap_nested_group_check_cache(TALLOC_CTX *mem_ctx,
user_uid = ldb_msg_find_attr_as_uint64(msgs[0], SYSDB_UIDNUM, 0); if (!user_uid) {
/* Refresh the fake user if he was created beforecache_timeout */ - create_time = ldb_msg_find_attr_as_uint64(msgs[0],
SYSDB_CREATE_TIME,0);expiration = create_time + dom->user_timeout;
DEBUG(SSSDBG_OP_FAILURE, ("User with no UID? Skipping\n"));ret = EIO;goto fail; } else { /* Regular user, check if we need a refresh */ expiration = ldb_msg_find_attr_as_uint64(msgs[0],diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index 22ee617daf1968fa8ef0fd310e609db142fdb16a..360f3698c1281a67de311dfc28f46c 3724e78ad9 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -258,7 +258,7 @@ static int fill_pwent(struct sss_packet *packet,
gid = get_gid_override(msg, dom); if (!orig_name || !uid || !gid) {
DEBUG(2, ("Incomplete or fake user object for %s[%llu]!Skipping\n", + DEBUG(SSSDBG_OP_FAILURE, ("Incomplete user object for %s[%llu]! Skipping\n",
orig_name?orig_name:"<NULL>", (unsigned long long int)uid)); continue; }diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c index bcf3f732be6f172324b0d1b455c28076a2355128..dd053b3a35265aea2ef056ded62805 1b10dcee74 100644 --- a/src/tests/sysdb-tests.c +++ b/src/tests/sysdb-tests.c @@ -2726,7 +2726,7 @@ START_TEST(test_odd_characters)
odd_username, 10000, 10000, "","","");
- fail_unless(ret == EOK, "sysdb_add_fake_user error [%d][%s]",
fail_unless(ret == EOK, "sysdb_add_basic_user error [%d][%s]",
ret, strerror(ret));/* Retrieve */
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
On Wed, 2012-05-23 at 12:03 +0200, Jan Zelený wrote:
On Thu, May 10, 2012 at 10:57:23PM +0200, Jan Zeleny wrote:
The sysdb upgrade script will segfault if any users in the database are lacking memberOf links. This can happen if a user was requested via getpwnam() or getpwuid() without an initgroups() call.
The attach patch should be squashed into patch 122.
Additionally, I was trying to do some real-world comparisons but I wasn't seeing any improvement (this was against an RFC 2307 server, not RFC2307bis or FreeIPA, so that may be relevant).
Can you verify whether we expect to see improvement on such systems, or only for RFC2307bis?
On Wed, 2012-05-23 at 12:03 +0200, Jan Zelený wrote:
On Thu, May 10, 2012 at 10:57:23PM +0200, Jan Zeleny wrote:
The sysdb upgrade script will segfault if any users in the database are lacking memberOf links. This can happen if a user was requested via getpwnam() or getpwuid() without an initgroups() call.
The attach patch should be squashed into patch 122.
Nice catch, thank you
Additionally, I was trying to do some real-world comparisons but I wasn't seeing any improvement (this was against an RFC 2307 server, not RFC2307bis or FreeIPA, so that may be relevant).
Can you verify whether we expect to see improvement on such systems, or only for RFC2307bis?
I don't know why we shouldn't expect any improvements for RFC2307. Can you post me the scenario you tested?
Thanks Jan
On Thu, 2012-05-24 at 08:19 +0200, Jan Zelený wrote:
On Wed, 2012-05-23 at 12:03 +0200, Jan Zelený wrote:
On Thu, May 10, 2012 at 10:57:23PM +0200, Jan Zeleny wrote:
The sysdb upgrade script will segfault if any users in the database are lacking memberOf links. This can happen if a user was requested via getpwnam() or getpwuid() without an initgroups() call.
The attach patch should be squashed into patch 122.
Nice catch, thank you
Additionally, I was trying to do some real-world comparisons but I wasn't seeing any improvement (this was against an RFC 2307 server, not RFC2307bis or FreeIPA, so that may be relevant).
Can you verify whether we expect to see improvement on such systems, or only for RFC2307bis?
I don't know why we shouldn't expect any improvements for RFC2307. Can you post me the scenario you tested?
Replied privately with confidential data.
On Thu, 2012-05-24 at 07:29 -0400, Stephen Gallagher wrote:
On Thu, 2012-05-24 at 08:19 +0200, Jan Zelený wrote:
On Wed, 2012-05-23 at 12:03 +0200, Jan Zelený wrote:
On Thu, May 10, 2012 at 10:57:23PM +0200, Jan Zeleny wrote:
The sysdb upgrade script will segfault if any users in the database are lacking memberOf links. This can happen if a user was requested via getpwnam() or getpwuid() without an initgroups() call.
The attach patch should be squashed into patch 122.
Nice catch, thank you
Additionally, I was trying to do some real-world comparisons but I wasn't seeing any improvement (this was against an RFC 2307 server, not RFC2307bis or FreeIPA, so that may be relevant).
Can you verify whether we expect to see improvement on such systems, or only for RFC2307bis?
I don't know why we shouldn't expect any improvements for RFC2307. Can you post me the scenario you tested?
Replied privately with confidential data.
Ok, we sorted out that the lack of performance improvement was due to high latency to the LDAP server and not actually the ghost changes. When I moved to a local LDAP server, performance has actually improved by approximately 300%.
So, with squashing in my patch for the upgrades, I call this an ack.
Attaching all the patches (with the upgrade patch squashed in) to make pushing easier.
On Wed, 2012-05-30 at 11:14 -0400, Stephen Gallagher wrote:
On Thu, 2012-05-24 at 07:29 -0400, Stephen Gallagher wrote:
On Thu, 2012-05-24 at 08:19 +0200, Jan Zelený wrote:
On Wed, 2012-05-23 at 12:03 +0200, Jan Zelený wrote:
On Thu, May 10, 2012 at 10:57:23PM +0200, Jan Zeleny wrote:
The sysdb upgrade script will segfault if any users in the database are lacking memberOf links. This can happen if a user was requested via getpwnam() or getpwuid() without an initgroups() call.
The attach patch should be squashed into patch 122.
Nice catch, thank you
Additionally, I was trying to do some real-world comparisons but I wasn't seeing any improvement (this was against an RFC 2307 server, not RFC2307bis or FreeIPA, so that may be relevant).
Can you verify whether we expect to see improvement on such systems, or only for RFC2307bis?
I don't know why we shouldn't expect any improvements for RFC2307. Can you post me the scenario you tested?
Replied privately with confidential data.
Ok, we sorted out that the lack of performance improvement was due to high latency to the LDAP server and not actually the ghost changes. When I moved to a local LDAP server, performance has actually improved by approximately 300%.
So, with squashing in my patch for the upgrades, I call this an ack.
Attaching all the patches (with the upgrade patch squashed in) to make pushing easier.
Pushed to master.
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Jan Zelený -
Simo Sorce -
Stephen Gallagher