9:22 a.m.
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
11:01 a.m.
Stephen Gallagher <sgallagh(a)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
8:41 a.m.
Stephen Gallagher <sgallagh(a)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
9 a.m.
On Wed, 2012-04-25 at 15:41 +0200, Jan Zelený wrote:
> Stephen Gallagher <sgallagh(a)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.
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
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.
--
Simo Sorce * Red Hat, Inc * New York
10:04 a.m.
On Wed, 2012-04-25 at 15:41 +0200, Jan Zelený wrote:
> > Stephen Gallagher <sgallagh(a)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.
>
> 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
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
8:34 a.m.
> Stephen Gallagher <sgallagh(a)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
I'm sending patch set rebased on the current master.
Jan
9:11 a.m.
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
1:44 p.m.
Updated patch set in the attachment
Stephen Gallagher <sgallagh(a)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
5:56 a.m.
On Wed, May 02, 2012 at 08:44:20PM +0200, Jan Zeleny wrote:
Updated patch set in the attachment
Stephen Gallagher <sgallagh(a)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
7:30 a.m.
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(a)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
8:29 a.m.
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.
--
Simo Sorce * Red Hat, Inc * New York
10:12 a.m.
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 * Red Hat, Inc * New York
3:57 p.m.
Simo Sorce <simo(a)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
4:37 a.m.
On Thu, May 10, 2012 at 10:57:23PM +0200, Jan Zeleny wrote:
Simo Sorce <simo(a)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(a)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(a)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++;
}
}
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, 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);
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].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(a)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(a)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(a)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 };
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;
}
--
1.7.7.6
From 169772136d2c1f9cafdcec2dd30e5ad144e0834d Mon Sep 17 00:00:00
2001
From: Jan Zeleny <jzeleny(a)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 be
diff --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(a)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(a)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(a)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(a)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(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
5:03 a.m.
On Thu, May 10, 2012 at 10:57:23PM +0200, Jan Zeleny wrote:
> Simo Sorce <simo(a)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(a)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(a)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, 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);
>
> 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(a)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 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;
>
> }
>
> From c75d6a16569fe6250b3b3659bc96961c2fdcde8e Mon Sep 17 00:00:00 2001
> From: Jan Zeleny <jzeleny(a)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 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;
>
> From cf117c91cb64d05d8b2958d185e96a39772b23f4 Mon Sep 17 00:00:00 2001
> From: Jan Zeleny <jzeleny(a)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(a)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 be
>
> diff --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 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;
> +}
>
> From ea12ef7f52ec959f83370b22a1b108a342461fe0 Mon Sep 17 00:00:00 2001
> From: Jan Zeleny <jzeleny(a)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 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++;
>
> From 92857f4e0366a9626eaf501d7331e8c633ee5f28 Mon Sep 17 00:00:00 2001
> From: Jan Zeleny <jzeleny(a)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 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..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(a)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(a)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 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..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(a)lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
--
Thank you
Jan Zeleny
Red Hat Software Engineer
Brno, Czech Republic
12:41 p.m.
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?
1:19 a.m.
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
6:29 a.m.
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.
10:14 a.m.
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.
2:51 p.m.
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.
4341
days inactive
4379
days old
sssd-devel@lists.fedorahosted.org
20 comments
4 participants
participants (4)
-
Jakub Hrozek -
Jan Zelený -
Simo Sorce -
Stephen Gallagher