The Samba-Bugzilla – Bug 9779
add UPN enumeration to passdb internal API
Last modified: 2013-07-24 18:27:50 UTC
These patches add support to enumerate UPN suffixes, a list of DNS domains associated with Samba forest. Currently only FreeIPA 3.2 is going to use these since FreeIPA implements KDC that Windows AD DC will be able to contact for cross-realm trust. The same will be added later to Samba AD DC, once cross-realm trust support is implemented.
Created attachment 8749 [details]
fix wafsamba for creating non-empty ABI files in specific case for PASSDB
Created attachment 8750 [details]
Filter out internal ldapsam init function
Created attachment 8751 [details]
Add support to enumerate UPN suffixes
Created attachment 8752 [details]
Use passdb UPN suffixes enumeration in _netr_GetForestTrustInformation
Before this goes in -- can we get a bit more background of this, together with a sample implementation?
I did try to explain and gave links to the implementation in my original mail to samba-technical@ -- https://lists.samba.org/archive/samba-technical/2013-April/091345.html
Here is it:
find set of patches to add support to enumerate UPN suffixes
associated with a forest smbd is serving.
This is needed for the cases like FreeIPA cross-realm forest trusts where
additional DNS domains may be associated with the realm. The patchset
exposes additional DNS domains to a requester (AD DC) via
netr_GetForestTrustInformation and netr_DsRGetForestTrustInformation. In
the latter case this is done only for a condition when trusted_domain_name
is NULL, as described in MS-NRPC and in source4/torture/rpc/netlogon.c.
FreeIPA's implementation for pdb_enum_upn_suffixes() is here:
Here is a screenshot of how it looks like from a Windows Server 2012 side:
Note that suffixes are disabled by default when imported, this is normal
In order to implement same for Samba AD DC code we would need to define
some schema and tree location to store the suffixes. In FreeIPA case we use
domainRelatedObject object class and associatedDomain attribute to store
list of domains. These are standard LDAP object class and attribute and
they could be reused.
Same applies to ldapsam module, we need to define where and how store the
Currently source4/torture/rpc/forest_trust.c expects in
test_validate_trust() that only primary domain's DNS name is returned by
netr_GetForestTrustInformation(). This will need to change when support for
UPN suffixes is added in Samba AD DC code.
pdb_set_upn_suffixes() was added from a perspective of making possible to
modify the suffixes from 'net' utility.
Thanks for the explanation. I find it weird that we create APIs in Samba that Samba does not use itself but only external projects use. If I'm the only one finding that a bit strange then put it in.
Karolin, please add the patches to v4-0-test. Thanks!
I'm planning on adding the same support to Samba AD DC. In fact, you can see it in the proposal already.
The code for ldapsam can be borrowed almost one-to-one from ipasam.
Hopefully, I'll have something to demonstrate before SambaXP.
Hang on a minute. This patchset has:
-#define PASSDB_INTERFACE_VERSION 20
+#define PASSDB_INTERFACE_VERSION 21
so it's changing an ABI mid-release cycle.
Now there aren't any passdb modules that I know of that are external to Samba, so this might be an ok thing to do, but can we have a policy discussion on it on samba-technical first please ?
Specifically, I'd like to see a general question of whether there are any external PASSDB ABI users (non-Samba) out there, and if not if we should reclassify this as an internal interface we can change mid release cycle instead of a separate one.
Note I'm not saying this is a bad thing to do, just that we need more discussion on it.
Oh hang on a min, does FreeIPA qualify as an external PASSDB user ? Is it the only one ?
I'm going to bounce this back temporarily to Alexander until we discuss this.
I agree with Jeremy and Volker:
It came as a slight surprise to me when the passdb api was made public, and as far as I know, the freeipa is the only external user so far (and the api was published _for_ freeipa, if I got that right..).
I do also feel bad about doing ABI changes in a stable release:
I thought that this is something that we usually don't do.
So discussion seems necessary here.
To my knowledge FreeIPA is the only external passdb API user, starting from 3.0 release (autumn 2012), when cross-realm trusts to AD were implemented. We are currently preparing 3.2 which relies on this API extension.
When proposing this patchset I followed Michael's way for sid_check_object_is_for_passdb() which was also proposed to 4.0 a week before final 4.0 version (on Dec 3, 2012), albeight without explicit PASSDB version bump. I remember we discussed it at the time and decided to not raise soname and PASSDB version.
For FreeIPA Andreas and I are closely tracking Samba changes and coordinate binary builds in Fedora/RHEL 6 so API version bump is not an issue. Currently, there are no other platforms where FreeIPA server side is built, though we are working with Ubuntu developers on that.
I'd like to see the code in 4.0 to simplify maintenance. It is well-contained change and affects only components that are aware of the changing API.
Can you kickstart the discussion on samba-technical so we can see if there are other users of the PASSDB API we are not currently aware of. Most users and vendors don't track our bug database that closely.
So, in days since April 11th when I posted request to samba-technical, nobody responded to it. Should this be treated as confirmation of my understanding that passdb users outside Samba are now limited to FreeIPA?
Moving to 4.0.6. As we agreed to push these patches to 4.0 tree during SambaXP and 4.0.6 tree was frozen, please commit these to the next version.
Re-assign to Jeremy. Please act before June 25th as the tree will be frozen again.
This is still not in 4.0.7!
Re-assigning to Karolin for inclusion in 4.0.next and 4.1.0.
Pushed to autobuild-v4-0-test.
0001-wafsamba-fix-samba_abi-for-default-catch-all-case.patch does not apply on current v4-1-test. Please provide an updated patchset.
Sorry Karolin, I think these patchsets are already in 4.1.0.
So this just needs pushing to the 4.0.next branch.
Sorry for the confusion.
Pushed to v4-0-test.
Closing out bug report.