Bug 9779 - add UPN enumeration to passdb internal API
add UPN enumeration to passdb internal API
Status: RESOLVED FIXED
Product: Samba 4.0
Classification: Unclassified
Component: DCE-RPCs and pipes
4.0.6
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-04-10 08:26 UTC by Alexander Bokovoy
Modified: 2013-07-24 18:27 UTC (History)
4 users (show)

See Also:


Attachments
fix wafsamba for creating non-empty ABI files in specific case for PASSDB (1.74 KB, patch)
2013-04-10 08:27 UTC, Alexander Bokovoy
asn: review+
Details
Filter out internal ldapsam init function (1.13 KB, patch)
2013-04-10 08:28 UTC, Alexander Bokovoy
asn: review+
Details
Add support to enumerate UPN suffixes (5.74 KB, patch)
2013-04-10 08:28 UTC, Alexander Bokovoy
asn: review+
Details
Use passdb UPN suffixes enumeration in _netr_GetForestTrustInformation (5.98 KB, patch)
2013-04-10 08:29 UTC, Alexander Bokovoy
asn: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Bokovoy 2013-04-10 08:26: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.
Comment 1 Alexander Bokovoy 2013-04-10 08:27:38 UTC
Created attachment 8749 [details]
fix wafsamba for creating non-empty ABI files in specific case for PASSDB
Comment 2 Alexander Bokovoy 2013-04-10 08:28:07 UTC
Created attachment 8750 [details]
Filter out internal ldapsam init function
Comment 3 Alexander Bokovoy 2013-04-10 08:28:34 UTC
Created attachment 8751 [details]
Add support to enumerate UPN suffixes
Comment 4 Alexander Bokovoy 2013-04-10 08:29:13 UTC
Created attachment 8752 [details]
Use passdb UPN suffixes enumeration in _netr_GetForestTrustInformation
Comment 5 Volker Lendecke 2013-04-10 08:35:47 UTC
Before this goes in -- can we get a bit more background of this, together with a sample implementation?
Comment 6 Alexander Bokovoy 2013-04-10 08:42:29 UTC
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:

at
https://git.samba.org/?p=ab/samba.git/.git;a=shortlog;h=refs/heads/s3-passdb please
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:
http://git.fedorahosted.org/cgit/freeipa.git/commit/?id=cc56723151c9ebf58d891e85617319d861af14a4

Here is a screenshot of how it looks like from a Windows Server 2012 side:
http://abbra.fedorapeople.org/.paste/win2012-multiple-suffixes.png

Note that suffixes are disabled by default when imported, this is normal
Windows behavior.

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
suffixes.

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.
Comment 7 Volker Lendecke 2013-04-10 13:30:04 UTC
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.
Comment 8 Andreas Schneider 2013-04-10 13:32:49 UTC
Karolin, please add the patches to v4-0-test. Thanks!
Comment 9 Alexander Bokovoy 2013-04-10 13:50:49 UTC
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.
Comment 10 Jeremy Allison 2013-04-10 22:45:44 UTC
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.

Cheers,

Jeremy.
Comment 11 Michael Adam 2013-04-10 22:55:11 UTC
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.
Comment 12 Alexander Bokovoy 2013-04-11 04:06:11 UTC
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.
Comment 13 Jeremy Allison 2013-04-11 18:15:45 UTC
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.

Thanks !

Jeremy.
Comment 14 Alexander Bokovoy 2013-04-29 16:07:42 UTC
So, in days since April 11th when I posted request to samba-technical[1], nobody responded to it. Should this be treated as confirmation of my understanding that passdb users outside Samba are now limited to FreeIPA?

[1] https://lists.samba.org/archive/samba-technical/2013-April/091520.html
Comment 15 Alexander Bokovoy 2013-06-07 08:36:38 UTC
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.
Comment 16 Alexander Bokovoy 2013-06-19 09:25:49 UTC
Re-assign to Jeremy. Please act before June 25th as the tree will be frozen again.
Comment 17 Andreas Schneider 2013-07-02 13:33:25 UTC
This is still not in 4.0.7!
Comment 18 Jeremy Allison 2013-07-18 17:59:12 UTC
Re-assigning to Karolin for inclusion in 4.0.next and 4.1.0.

Jeremy.
Comment 19 Karolin Seeger 2013-07-18 18:06:10 UTC
Pushed to autobuild-v4-0-test.
Comment 20 Karolin Seeger 2013-07-18 18:06:53 UTC
0001-wafsamba-fix-samba_abi-for-default-catch-all-case.patch does not apply on current v4-1-test. Please provide an updated patchset.
Thanks!
Comment 21 Jeremy Allison 2013-07-18 18:37:41 UTC
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.

Jeremy.
Comment 22 Karolin Seeger 2013-07-24 18:27:50 UTC
Pushed to v4-0-test.
Closing out bug report.

Thanks!