[PATCH] Add special users to Samba4 (bugzilla #4918)

[Andrew Kroeger]

Andrew Bartlett wrote:
>> This last patch is the one that I anticipate the most feedback on.
>> Given the special handling of the foreignSecurityPrincipal objectClass
>> that appears to be special-cased for CN=ForeignSecurityPrincipals, I
>> needed to add another special case for CN=WellKnows Security Principals.
>>  There may be a more general way to handle this, but I couldn't find any
>> existing code that looked like it would help.(ldb_dn_canonical_string
> 
> Could we do this a bit more generically:  If no objectSid is present,
> and the CN can be parsed as a SID, then use that, otherwise just apply
> the template?

Sounds good to me.  I assume:
- If objectSid is present, go forward with the case I have coded for and
emit an error if anything goes wrong.
- If objectSid is not present, and the CN cannot be parsed as an SID,
emti an error just as the original code does.

When you say "just apply the template", what does applying the template
actually do?  That was part of the original code that I copied into the
new function I created without understanding what it actually does.
When should/should not the template be applied?

> 
>> I also anticipate there may be issues with the following:
>> - The use of ldb_dn_canonical_string() to get the DN to differentiate
>> this special case from the other handling of foreignSecurityPrincipal.
>> - The use of strstr() to look for a given substring in the DN.
>> - My code makes a case-sensitive comparision, but I don't know if the
>> DNs are case-sensitive.
> 
> They are not case sensitive or insensitive, it depends on the parts...
> 
> The function you were looking for is:
> /* Determine if dn is below base, in the ldap tree.  Used for
>  * evaluating a subtree search.
>  * 0 if they match, otherwise non-zero
>  */
> 
> int ldb_dn_compare_base(struct ldb_dn *base, struct ldb_dn *dn)
> 
> (regardless, I think we can avoid doing that, and make it compleatly
> generic). 

Agreed.

I assume not, but are you aware of any other cases of the
foreignSecurityPrincipal objectClass that would require any additional
special or one-off handling?

>> - The samldb_fill_wellKnown_foreignSecurityPrincipal_object() function I
>> created copies the input ldb_message and then calls
>> samdb_copy_template().  I don't know if any of that is necessary, as I
>> don't yet fully understand what they do :)  I do know that my code fails
>> if msg2 is not set, so evidently what I am trying to do depends on the
>> additional processing code that happens later in samdb_add() after the
>> check for msg2 == NULL.
> 
> Indeed, the code you copied this from is complex.  It is critical co
> copy the msg into msg2, and to return it with:
> 
> 	ret = samldb_notice_sid(module, msg2, sid);
> 	if (ret == 0) {
> 		talloc_steal(msg, msg2);
> 		*ret_msg = msg2;
> 	}

OK.  You got me with that one...

The new function I wrote does not call samldb_notice_sid(), but I see
the call in the original function I copied the code from.  Can you
elaborate on what both the samldb_notice_sid() and talloc_steal() calls
accomplish?

> 
>> I welcome any and all feedback regarding what I have put together.
> 
> This looks like a really useful patch.  I think if the wellKnown case
> would be best dealt with in the core
> samldb_fill_foreignSecurityPrincipal_object() code, but very well done
> for chasing this so far!
> 
> Do you want to try that?

Absolutely.  I'm on it.

I know you're working on preparations, release notes, etc. for the first
alpha release.  What sort of timeline are you looking at?  Although it
is by no means critical for an alpha, I think there would definitely be
an "eye candy" benefit for the alpha by having the "psuedo users" appear
when someone attempts to manipulate the security attributes of
files/directories.  As long as the alpha release is not looming on the
horizon, I'd like to work with you to get this in before the alpha
release.  Is this feasible?

Thanks,

Andrew Kroeger