[patch] clean up sid when writing to tdb

[Philipp Gesang]

-<| Quoting Philipp Gesang <philipp.gesang at intra2net.com>, on Monday, 2018-10-15 11:55:34 AM |>-
> Hi Volker, hi Andrew,
> 
> thanks for having a look.
> 
> -<| Quoting Andrew Bartlett via samba-technical <abartlet at samba.org>, on Saturday, 2018-10-13 07:55:18 PM |>-
> > On Fri, 2018-10-12 at 18:26 +0200, Volker Lendecke via samba-technical
> > wrote:
> > > On Fri, Oct 12, 2018 at 05:04:32PM +0200, Philipp Gesang via samba-technical wrote:
> > > > Hey guys,
> > > > 
> > > > I’d appreciate feedback on the attached patch.
> > > 
> > > Shouldn't we just store the required bytes determined with
> > > ndr_size_dom_sid() and ndr_push/pull_dom_sid? This has the obvious
> > > upgrade issues, but maybe we can handle that somehow.
> 
> How much bug-for-bug compat with Windows is the goal here?
> I’ve had the SID handling checked by kind colleagues who are
> more versed in Windowsy things. These are the observations:
> 
> - Roundtrip conversion (string → pSID / CSid → string) preserves
>   extraneous SubAuth’s beyond what the SubAuthCount indicates.

Scratch that, this was due to a miscommunication.

> - isValidSid() will reject a version != 1, but it won’t object to
>   additional SubAuth’s.

This holds.

> - copySid() too preserves extra SubAuth’s.

Also not the case. Sorry for the noise.

> > Volker,
> > 
> > You are quite correct, pushing a buffer is a really bad idea.  The structure is:
> > 
> > struct dom_sid {
> >         uint8_t sid_rev_num;
> >         int8_t num_auths;/* [range(0,15)] */
> >         uint8_t id_auth[6];
> >         uint32_t sub_auths[15];
> > }
> > 
> > and so the sub_auths would be endian-dependant. 
> > 
> > Not that we get much swapping of tdb files between hosts, but we are
> > meant to be able.  
> > 
> > Note, while not directly relevant here, we will have to be more careful
> > about this if we make broader use of lmdb, which is host endian
> > dependent per
> > https://blog.separateconcerns.com/2016-04-03-lmdb-format.html
>
> > Now as to how to fix this while being backward and forward compatible
> > and portable across byte orders?  Trickier. 

Storing sids the “string format syntax” is endian-independent.

Philipp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20181015/e1a22f20/signature.sig>