[PATCH] passdb: Increase ABI version to 0.28.0

[Alexander Bokovoy]

On ti, 05 maalis 2019, Volker Lendecke via samba-technical wrote:
> On Tue, Mar 05, 2019 at 02:04:51PM +0200, Alexander Bokovoy via samba-technical wrote:
> > I can deal with that if a header generated from idmap.idl would be
> > installed publicly. That's the only reason we used these functions --
> > 'struct unixid' definition is not public.
> > 
> > Attached patch allows to make it public.
> 
> Please don't add to exposing internal APIs. Libpassdb is just an API
> that RedHat chose to utilize in FreeIPA, and the more internals of
> Samba we expose the more conflicts like this will come up.
> 
> We need to enter a serious discussion to limit this to the absolute
> necessary and agree on a proper API. I don't want to just version and
> expose every implementation aspect of Samba internals.
> 
> So -- what does FreeIPA really need, and can we expose that with
> something serious, not just a random collection of dropped-in headers
> and files?
FreeIPA implements a PASSDB module which is loaded into Samba. It needs
to get access to the same API other PASSDB modules have to use.

Here is the list of functions it needs access for right now aside from
the ones already available:

enum ndr_err_code ndr_pull_trustAuthInOutBlob(struct ndr_pull *ndr, int ndr_flags, struct trustAuthInOutBlob *r); /*available in libndr-samba.so */
bool sid_check_is_builtin(const struct dom_sid *sid); /* available in libpdb.so */
/* available in libpdb.so, renamed from sid_check_is_domain() in c43505b621725c9a754f0ee98318d451b093f2ed */
bool sid_linearize(char *outbuf, size_t len, const struct dom_sid *sid); /* available in libsmbconf.so */
char *sid_string_talloc(TALLOC_CTX *mem_ctx, const struct dom_sid *sid); /* available in libsmbconf.so */
char *sid_string_dbg(const struct dom_sid *sid); /* available in libsmbconf.so */
char *escape_ldap_string(TALLOC_CTX *mem_ctx, const char *s); /* available in libsmbconf.so */
bool secrets_store(const char *key, const void *data, size_t size); /* available in libpdb.so */
void idmap_cache_set_sid2unixid(const struct dom_sid *sid, struct unixid *unix_id); /* available in libsmbconf.so */
bool E_md4hash(const char *passwd, uint8_t p16[16]); /* available in libcliauth-samba4.so */

'struct unixid' is the struct accepted by the
idmap_cache_set_sid2unixid(). Without calling this function, we
experienced a huge slowdown in past because the SIDs discovered by the
passdb module during lookups (from IPA LDAP) weren't cached and had to
be re-discovered over and over again. pdb_ldap calls to that, as well as
winbindd itself.

Most importantly, it is also a structure accepted by
sid_to_id()/id_to_sid() which PASSDB module have to implement.

secrets_store() is used to bootstrap domain SID in secrets.tdb from
IPA LDAP. This allows to avoid manual setup in the scripts. This should
be replaced by use of PDB_secrets_store_domain_sid() which is in
passdb.h already.

ndr_pull_trustAuthInOutBlob() is needed as FreeIPA ipasam module
provides trusted domains handling and sets Kerberos keys for a
cross-forest principal, unlike any other passdb module
except a glue for Samba DC (pdb_samba_dsdb).

E_md4hash() is used to generate NT hash for the trusted domain because
API calling into pdb_getsampwnam() callbacks in PASSDB expects NT hash
to be present there. I'm not sure whether this holds for the trusted
domain objects anymore, need to check.

The rest are handy functions to operate on SIDs or escape LDAP strings
in the way expected by the Samba code.
 
> Or -- can we in the future just decouple this completely? Why does a
> completely separate non-associated project need to touch purely
> internal, private Samba APIs at all?
See above -- it implements a module for Samba internal use.

I'm all for clearing this space but right now I have to struggle with
every change done in Samba around PASSDB interface. Bringing the module
back to Samba would not help either because it becomes useless without
the rest of FreeIPA deployment and cannot be tested as part of
autobuild.

I can bring the code to Samba if you really after forcing that. I also
do not think it is worth to fight around a generated idmap.h header.
After all, we are talking about an enum and a single struct that
unlikely to change for years:

enum id_type {
        ID_TYPE_NOT_SPECIFIED,
        ID_TYPE_UID,
        ID_TYPE_GID,
        ID_TYPE_BOTH
};

struct unixid {
        uint32_t id;
        enum id_type type;
}

Sumit and I tried to avoid operating ourselves on that struct and thus
we called out for Samba helper functions. However, there is no
initialization function available so we actually have this definition in
the ipasam code already to be able to write

	struct unixid id;
        unixid_from_uid(&id, uid);
        idmap_cache_set_sid2unixid(sid, &id);

Either way, even with gen_ndr/idmap.h made available, we would need to
effectively set a replacement for unixid_from_[g,u]id() ourselves which
is not a big deal.

-- 
/ Alexander Bokovoy