SSSD / sssd

A daemon to manage identity, authentication and authorization for centrally-managed systems.
https://sssd.io
GNU General Public License v3.0
588 stars 237 forks source link

ad_gpo_ndr has a hard-coded parser for Samba security descriptors that changed in Samba 4.20 #7395

Open abartlet opened 3 months ago

abartlet commented 3 months ago

src/providers/ad/ad_gpo_ndr.c has:

/*

You should be aware that on the Samba side of things struct security_descriptor has changed struct security_ace has new elements [switch_is(type), subcontext(0), subcontext_size(ndr_subcontext_size_of_ace_coda(r, size, n dr->flags))] security_ace_coda coda;

This was added in Samba 4.20 by @douglasbagnall

Either sssd should entirely own the security descriptor, run the validation itself etc, and not reference Samba code at all, or it should use Samba code to parse it, possibly by requesting an API that can be maintained.

sumit-bose commented 3 months ago

Hi,

thanks for making us aware.

Are there any concerns to make the ndr_{push|pull|print|size}_security_descriptor() functions available in a public library similar to e.g. libndr-krb5pac.so?

Or would it be a way forward if SSSD would generate the needed code with pidl and the related IDL files from the Samba source tree at build time?

bye, Sumit

abartlet commented 3 months ago

Much of the new security descriptor parsing code is manual - there is a new coda which you may wish to handle (used in AD claims) and a massive new language for it. It would not be applicable to GPOs, but some other use cases might be interesting in the future. So just grabbing the PIDL output would just give undefined references.

Long term, I think proposing a new public library for SSSD with as generic an interface as possible with the complexity handled and tested on the Samba side (against opaque structures) would be the best approach, but in the short term you need to send in a MR to make public the things you need.

joakim-tjernlund commented 1 month ago

samba 4.20.3 was just released, I wonder if this issue is still present in recent sssd?