[patch] clean up sid when writing to tdb

[Philipp Gesang]

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.

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

- copySid() too preserves extra SubAuth’s.

In any case, Windows happily includes extra SubAuth’s when
copying or converting to string whereas Samba’s sid_copy() and
dom_sid_string() do drop them.

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


Best,
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/3ee4cece/signature.sig>