Quantcast

[PATCH] Updated Add detailed authentication logging for NTLM authentication.

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Updated Add detailed authentication logging for NTLM authentication.

Samba - samba-technical mailing list
On Thu, 2017-03-16 at 21:25 +1300, Andrew Bartlett via samba-technical
wrote:

> On Tue, 2017-03-14 at 06:10 +1300, Andrew Bartlett via samba-
> technical
> wrote:
> > On Mon, 2017-03-13 at 09:05 +0100, Stefan Metzmacher via samba-
> > technical wrote:
> > > Hi Gary,
> > >
> > > > Updated to use jansson for the JSON generation, removing the
> > > > glib
> > > > dependencies. We're planning to get the tests written tomorrow,
> > > > which
> > > > will finish this piece of work off.
> > > >
> > > > Samples of the new log lines below, line breaks and indent
> > > > added
> > > > for
> > > > clarity.
> > > >
> > > > Authorization
> > > >
> > > > Human Readable
> > > > Successful AuthZ: [DCE/RPC,ncacn_np]
> > > > user [NT AUTHORITY]\[SYSTEM] [S-1-5-18]
> > > > at [Mon, 13 Mar 2017 16:17:57 NZDT]
> > > > Remote host [ipv6::::0] local host [ipv6::::0]
> > >
> > > Can we get the hires=true timestamp here as well?
> > >
> > >
> > > I think we've learned our lesson of having pytalloc_Object
> > > as a public structure. Please don't make TeventContext_Object
> > > public...
> > >
> > > pytevent_Context_AsTeventContext() should be a function.
> > > In addition we should have a pytevent_Context_Check() function,
> > > which will also be used within pytevent_Context_AsTeventContext()
> > > before casting/dereferencing the struct elements.
> >
> > That means adding a whole pytevent-util like we have with pytalloc
> > and
> > pyldb.  I'm not sure it is worth it - the alternative is to just
> > extend
> > pymessaging to have an tevent_loop_once() wrapper waiting for one
> > message.  
>
> Just to let you know, while it is still in our branch, I'll drop the
> pytalloc changes tomorrow in favour of a loop_once() in pymessaging. 
>
> We have made some massive progress towards merging this in the past
> few
> days, and I've even got a prototype of Kerberos KDC logging
> included. 

I've added KDC logging now.

> We now have a working pymessaging layer (it didn't work before as a
> server), and we use it to collect messages, formatted as JSON, about
> every auth and authZ event.  
>
> We use that to ensure we get the right messages, with the right
> details, for a given action (eg bind to ncacn_np, AS-REQ).  These
> will
> be Samba's most tested DEBUG() statements anywhere :-)
>
> (We assert - by hand-waving - that if the correct message was sent
> over
> the message bus that the DEBUG line probably worked out OK as well). 
>
> Already this has found one case in the ntvfs file server where local
> and remote addresses were swapped before we even started. 
>
> If tomorrow goes well, we may have something to review over the
> weekend!

Friday didn't work out (naturally), but the curious may wish to inspect

git://git.catalyst.net.nz/samba.git auth-logging

I've also used the testsuite to test the earlier revisions to ensure
operation as well as compilation while we introduce the new parameters.

Thanks,

Andrew Bartlett

--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Updated Add detailed authentication logging for NTLM authentication.

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Thu, 2017-03-16 at 21:25 +1300, Andrew Bartlett via samba-technical
wrote:

> On Tue, 2017-03-14 at 06:10 +1300, Andrew Bartlett via samba-
> technical
> wrote:
> > On Mon, 2017-03-13 at 09:05 +0100, Stefan Metzmacher via samba-
> > technical wrote:
> > > Hi Gary,
> > >
> > > > Updated to use jansson for the JSON generation, removing the
> > > > glib
> > > > dependencies. We're planning to get the tests written tomorrow,
> > > > which
> > > > will finish this piece of work off.
> > > >
> > > > Samples of the new log lines below, line breaks and indent
> > > > added
> > > > for
> > > > clarity.
> > > >
> > > > Authorization
> > > >
> > > > Human Readable
> > > > Successful AuthZ: [DCE/RPC,ncacn_np]
> > > > user [NT AUTHORITY]\[SYSTEM] [S-1-5-18]
> > > > at [Mon, 13 Mar 2017 16:17:57 NZDT]
> > > > Remote host [ipv6::::0] local host [ipv6::::0]
> > >
> > > Can we get the hires=true timestamp here as well?
> > >
> > >
> > > I think we've learned our lesson of having pytalloc_Object
> > > as a public structure. Please don't make TeventContext_Object
> > > public...
> > >
> > > pytevent_Context_AsTeventContext() should be a function.
> > > In addition we should have a pytevent_Context_Check() function,
> > > which will also be used within pytevent_Context_AsTeventContext()
> > > before casting/dereferencing the struct elements.
> >
> > That means adding a whole pytevent-util like we have with pytalloc
> > and
> > pyldb.  I'm not sure it is worth it - the alternative is to just
> > extend
> > pymessaging to have an tevent_loop_once() wrapper waiting for one
> > message.  
>
> Just to let you know, while it is still in our branch, I'll drop the
> pytalloc changes tomorrow in favour of a loop_once() in pymessaging. 
>
> We have made some massive progress towards merging this in the past
> few
> days, and I've even got a prototype of Kerberos KDC logging included.
>  
>
> We now have a working pymessaging layer (it didn't work before as a
> server), and we use it to collect messages, formatted as JSON, about
> every auth and authZ event.  
>
> We use that to ensure we get the right messages, with the right
> details, for a given action (eg bind to ncacn_np, AS-REQ).  These
> will
> be Samba's most tested DEBUG() statements anywhere :-)

We think we are finally ready to have this branch pushed to master.  

I'm currently testing if we can disconnect the tests from the messaging
bugs I found while we debate API preferences.

If we can split it, or otherwise get past this, we hope to push as much
possible soon, so if you or anybody else would like to review it that
would be most helpful!

git://git.catalyst.net.nz/samba.git auth-logging

http://git.catalyst.net.nz/gitweb?p=samba.git;a=shortlog;h=refs/heads/a
uth-logging

We didn't ask for formal review/push yet because we wanted to address
the concern you raised early, that we might not work between some of
the early commits.  We have written an extensive test suite, and run
that between all the commits that change APIs via:

git rebase -i
x make test TESTS=auth_log

Therefore we are now confident we do not have any of the feared issues
there.

This is a massive improvement in Samba's suitability for as an AD DC,
and I'm very proud of the work done here by Gary in particular.

Thanks,

Andrew Bartlett


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Updated Add detailed authentication logging for NTLM authentication.

Samba - samba-technical mailing list
On Thu, Mar 23, 2017 at 11:45:49AM +1300, Andrew Bartlett via samba-technical wrote:

>
> We think we are finally ready to have this branch pushed to master.  
>
> I'm currently testing if we can disconnect the tests from the messaging
> bugs I found while we debate API preferences.
>
> If we can split it, or otherwise get past this, we hope to push as much
> possible soon, so if you or anybody else would like to review it that
> would be most helpful!
>
> git://git.catalyst.net.nz/samba.git auth-logging
>
> http://git.catalyst.net.nz/gitweb?p=samba.git;a=shortlog;h=refs/heads/a
> uth-logging
>
> We didn't ask for formal review/push yet because we wanted to address
> the concern you raised early, that we might not work between some of
> the early commits.  We have written an extensive test suite, and run
> that between all the commits that change APIs via:
>
> git rebase -i
> x make test TESTS=auth_log
>
> Therefore we are now confident we do not have any of the feared issues
> there.
>
> This is a massive improvement in Samba's suitability for as an AD DC,
> and I'm very proud of the work done here by Gary in particular.

Sounds good ! Can you post it as a patchset to the list as
well, or is it too big ?

Jeremy.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Updated Add detailed authentication logging for NTLM authentication.

Samba - samba-technical mailing list
On Wed, 2017-03-22 at 18:48 -0700, Jeremy Allison wrote:

> On Thu, Mar 23, 2017 at 11:45:49AM +1300, Andrew Bartlett via samba-
> technical wrote:
> >
> > We think we are finally ready to have this branch pushed to master.
> >  
> >
> > I'm currently testing if we can disconnect the tests from the
> > messaging
> > bugs I found while we debate API preferences.
> >
> > If we can split it, or otherwise get past this, we hope to push as
> > much
> > possible soon, so if you or anybody else would like to review it
> > that
> > would be most helpful!
> >
> > git://git.catalyst.net.nz/samba.git auth-logging
> >
> > http://git.catalyst.net.nz/gitweb?p=samba.git;a=shortlog;h=refs/hea
> > ds/a
> > uth-logging
> >
> > We didn't ask for formal review/push yet because we wanted to
> > address
> > the concern you raised early, that we might not work between some
> > of
> > the early commits.  We have written an extensive test suite, and
> > run
> > that between all the commits that change APIs via:
> >
> > git rebase -i
> > x make test TESTS=auth_log
> >
> > Therefore we are now confident we do not have any of the feared
> > issues
> > there. 
> >
> > This is a massive improvement in Samba's suitability for as an AD
> > DC,
> > and I'm very proud of the work done here by Gary in particular. 
>
> Sounds good ! Can you post it as a patchset to the list as
> well, or is it too big ?

It is huge (500k) partly because it builds on metze's branch, thanks to
his rebase-for-hire services ;-)

Thanks,

Andrew Bartlett


12
Loading...