[PATCH] Implement msDS-RevealedUsers for RODC auditing

Stefan Metzmacher metze at samba.org
Thu Mar 9 09:50:53 UTC 2017


Hi Garming,

> Here are some patches to implement the msDS-RevealedUsers attribute for
> RODCs. The typical behaviour is that when an RODC replicates passwords,
> the user whose secrets were revealed (to this less privileged domain
> controller) are recorded against the RODC using this attribute.
> 
> There are a few changes required in order to correctly handled
> multi-valued binary linked attributes (which should also not be modified):
> 
>   * Handling duplicated backlinks pointing to the same object (as the
>     forward links are repeated DNs with different binary portions)
>   * Improve dbcheck handling against these links
>   * Restricting modification, through previously unimplemented
>     restriction of systemOnly attributes
> 
> There's a number of tests now written for the auditing behaviour, as
> well as a number of fixes to bugs in the overall RODC (which were found
> through the testing). 
> 
> Any thoughts would be appreciated.

I think some of the commits can be squashed, e.g.
  getncchanges: Add a comment for new function
  getncchanges: Reorder parameters in filter_attrs for ...
and
  objectclass_attrs: Add comments linking back to ADTS

Regarding "getncchanges: include object SID in tokenGroups
 calculation for repl secret"

Shouldn't it be better to pass a list of attributes
to samdb_result_sid_array_ndr()? we may also need to
include the sIDHistory attribute in addition to
the "objectSid" attribute.

It seems the primary group is already part of tokenGroups,
and I'm not sure if sIDHistory will also be part of it.

All in all it looks like we basically need a similar logic
compared to authsam_make_user_info_dc, maybe we can
share more code and have these calculation just in
one code path, by making use of tokenGroups* in all places
and a relatively simple wrapper to build the full sid list.

If it turns out to be too complex to unify this at least
add a comment that refers to sIDHistory, so that I'll find
that place in a git grep later, when I finish this commit
https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=513a727a3d5755b3a7453387577690c76348d739

We should make sure your ldb commit, will be part of the
next ldb release, please let Andrew include it in his
index patchset.

This are my overall comments, I'll let someone else do
a deep review...

Thanks!
metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170309/219f8783/signature.sig>


More information about the samba-technical mailing list