Discussion:
[PATCH] Updated Add detailed authentication logging for NTLM authentication.
Gary Lockyer
2017-03-02 03:48:54 UTC
Permalink
Produce detailed authentication logs for NTLM authentication, of both
successful and unsuccessful attempts. Patch includes changes to ensure
that all the required fields are passed through to the logging routines.

Updated to:
log successful authorizations
log password type for authentications
replace talloc_steal with talloc_move in
source4/smbd/service_named_pipe.c
separate the auth logging into it's own file


Subsequent patches will log kerberos authentication and
produce machine parsable json log entries.
Andrew Bartlett
2017-03-02 03:57:14 UTC
Permalink
Post by Gary Lockyer
Produce detailed authentication logs for NTLM authentication, of both
successful and unsuccessful attempts.  Patch includes changes to
ensure
that all the required fields are passed through to the logging
routines.
log successful authorizations
log password type for authentications
        replace talloc_steal with talloc_move in
            source4/smbd/service_named_pipe.c
separate the auth logging into it's own file
Subsequent patches will log kerberos authentication and
produce machine parsable json log entries.
Thanks Gary. We also need to remember password changes! (The always
forgotten attack vector :-)

Thanks!
--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team https://samba.org
Samba Development and Support, Catalyst IT
https://catalyst.net.nz/services/samba
Stefan Metzmacher
2017-03-02 11:43:48 UTC
Permalink
Post by Andrew Bartlett
Post by Gary Lockyer
Produce detailed authentication logs for NTLM authentication, of both
successful and unsuccessful attempts. Patch includes changes to ensure
that all the required fields are passed through to the logging routines.
log successful authorizations
log password type for authentications
replace talloc_steal with talloc_move in
source4/smbd/service_named_pipe.c
separate the auth logging into it's own file
Subsequent patches will log kerberos authentication and
produce machine parsable json log entries.
Thanks Gary. We also need to remember password changes! (The always
forgotten attack vector :-)
I haven't looked very closely, but is some places I had the impression
that a later patch fixes a problem in a former patch.
In order to understand the flow better, it would be useful to have this
sorted out and every single commit complies and is supposed to
work without crashing with NULL pointer deferences.

Typically it's better to pass new unused arguments from the top,
before they get used at the bottom.

I guess you also need to be prepared that a dcerpc bind negotiates no
presentation context, it's possible to use a dummy uuid and just
do the authentication.

We may also want to distinguish between the different LogonSamLogon
levels (interactive vs. network at least).

I'm also wondering why the log functions gets the user supplied info
but not the full auth_session_info.

The output format should also be flexible, we can't guarantee that
we'll generate the exact output forever.

metze
Andrew Bartlett
2017-03-02 22:16:46 UTC
Permalink
Post by Stefan Metzmacher
Post by Gary Lockyer
Produce detailed authentication logs for NTLM authentication, of both
successful and unsuccessful attempts.  Patch includes changes to
ensure
that all the required fields are passed through to the logging routines.
log successful authorizations
log password type for authentications
        replace talloc_steal with talloc_move in
            source4/smbd/service_named_pipe.c
separate the auth logging into it's own file
Subsequent patches will log kerberos authentication and
produce machine parsable json log entries.
Thanks Gary.  We also need to remember password changes!  (The
always
forgotten attack vector :-)
I haven't looked very closely, but is some places I had the
impression
that a later patch fixes a problem in a former patch.
In order to understand the flow better, it would be useful to have this
sorted out and every single commit complies and is supposed to
work without crashing with NULL pointer deferences.
Thanks. Each commit does compile (we have been using your rebase/x
make -j trick), and they are meant to operate, but we will look again
for any more cases we need to implement.
Post by Stefan Metzmacher
Typically it's better to pass new unused arguments from the top,
before they get used at the bottom.
I guess you also need to be prepared that a dcerpc bind negotiates no
presentation context, it's possible to use a dummy uuid and just
do the authentication.
Very interesting. I'll cover that - but what happens in Samba then,
can the user re-use the authentication on another UUID? (it is
important to know this for auditing.)
Post by Stefan Metzmacher
We may also want to distinguish between the different LogonSamLogon
levels (interactive vs. network at least).
A very good idea. We have an unused parameter that would be ideal for
that.
Post by Stefan Metzmacher
I'm also wondering why the log functions gets the user supplied info
but not the full auth_session_info.
At the point that we see the user_supplied_info, we don't get the
auth_session_info, this is generated later. Because of the differences
between the source3 and source4 auth subsystems, I can't even pass in
the user_info_dc etc, so we just pick out some key parameters.
Post by Stefan Metzmacher
The output format should also be flexible, we can't guarantee that
we'll generate the exact output forever.
Indeed! This work is just the first step, designed to be practical to
read, the full task from our clients calls for JSON (one one line),
which is the next step now we have the plumbing in place.

Thank you so much for taking the time to look at and think about this
work!

Andrew Bartlett
--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team https://samba.org
Samba Development and Support, Catalyst IT
https://catalyst.net.nz/services/samba
Andrew Bartlett
2017-03-03 00:36:28 UTC
Permalink
Post by Stefan Metzmacher
Post by Gary Lockyer
Produce detailed authentication logs for NTLM authentication,
of
both
successful and unsuccessful attempts.  Patch includes changes to
ensure
that all the required fields are passed through to the logging routines.
log successful authorizations
log password type for authentications
        replace talloc_steal with talloc_move in
            source4/smbd/service_named_pipe.c
separate the auth logging into it's own file
Subsequent patches will log kerberos authentication and
produce machine parsable json log entries.
Thanks Gary.  We also need to remember password changes!  (The
always
forgotten attack vector :-)
I haven't looked very closely, but is some places I had the
impression
that a later patch fixes a problem in a former patch.
In order to understand the flow better, it would be useful to have this
sorted out and every single commit complies and is supposed to
work without crashing with NULL pointer deferences.
Thanks.  Each commit does compile (we have been using your rebase/x
make -j trick), and they are meant to operate, but we will look again
for any more cases we need to implement. 
We found a few mixups. One thing that helps us is that the
tsocket_address and DEBUG functions all handle NULL gracefully, so we
think we are OK with this patch ordering. However we will re-check.
Post by Stefan Metzmacher
Typically it's better to pass new unused arguments from the top,
before they get used at the bottom.
I guess you also need to be prepared that a dcerpc bind negotiates no
presentation context, it's possible to use a dummy uuid and just
do the authentication.
Very interesting.  I'll cover that - but what happens in Samba then,
can the user re-use the authentication on another UUID? (it is
important to know this for auditing.)
I think the current auth-logging-ntlm branch handles that, but I do
wish the source4 code was simpler: The source3 code just rejects a
bind without a valid table to map to.
Post by Stefan Metzmacher
We may also want to distinguish between the different LogonSamLogon
levels (interactive vs. network at least).
A very good idea.  We have an unused parameter that would be ideal
for
that. 
Done!

Gary will continue on this to add the JSON, but I'm really pleased with
the progress and hopefully we can get this much in to the tree soon.
We now handle logging anonymous ldap and un-authenticated dce/rpc
authorizations!

Andrew Bartlett
--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team https://samba.org
Samba Development and Support, Catalyst IT
https://catalyst.net.nz/services/samba
Jeremy Allison
2017-03-03 01:13:22 UTC
Permalink
Post by Andrew Bartlett
Gary will continue on this to add the JSON, but I'm really pleased with
the progress and hopefully we can get this much in to the tree soon.
We now handle logging anonymous ldap and un-authenticated dce/rpc
authorizations!
That's great work Andrew, thanks ! We've long needed good logging
on the AD-DC side of things - it will help expand enterprise
Samba-AD deployments greatly once people can log authentications.

Jeremy.
Andrew Bartlett
2017-03-03 05:39:01 UTC
Permalink
Post by Jeremy Allison
Post by Andrew Bartlett
Gary will continue on this to add the JSON, but I'm really pleased with
the progress and hopefully we can get this much in to the tree
soon. 
We now handle logging anonymous ldap and un-authenticated dce/rpc
authorizations!
That's great work Andrew, thanks !
Thanks!
Post by Jeremy Allison
We've long needed good logging
on the AD-DC side of things - it will help expand enterprise
Samba-AD deployments greatly once people can log authentications.
Indeed :-)

BTW, Because I only ever want to do this once, I've worked with Gary to
also cover the file server side of things.  

In short, this won't be an AD-only feature :-)

Finally, I do wish to thank Gary for his incredible patience watching
over my shoulder as I fly around the code-base setting up the hooks
with him. 

Andrew Bartlett
Post by Jeremy Allison
Jeremy.
--
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
Samba Developer, Catalyst IT http://catalyst.net.nz/services/samba
Gary Lockyer
2017-03-05 18:36:06 UTC
Permalink
Post by Andrew Bartlett
Gary will continue on this to add the JSON, but I'm really pleased with
the progress and hopefully we can get this much in to the tree soon.
We now handle logging anonymous ldap and un-authenticated dce/rpc
authorizations!
To generate the json log lines I'm planning on using glib-json
https://developer.gnome.org/json-glib/stable/, rather than rolling my own.

Doing this however does add dependencies on
* json-glib
* gio
* gobject
* glib

And requires compiling with -pthread

Is this a reasonable approach?

Gary.
Andreas Schneider
2017-03-10 10:43:38 UTC
Permalink
Post by Gary Lockyer
Post by Andrew Bartlett
Gary will continue on this to add the JSON, but I'm really pleased with
the progress and hopefully we can get this much in to the tree soon.
We now handle logging anonymous ldap and un-authenticated dce/rpc
authorizations!
To generate the json log lines I'm planning on using glib-json
https://developer.gnome.org/json-glib/stable/, rather than rolling my own.
Doing this however does add dependencies on
* json-glib
* gio
* gobject
* glib
And requires compiling with -pthread
Is this a reasonable approach?
I'm against glib.


Take a look at:

https://github.com/json-c/json-c/wiki



Andreas
--
Andreas Schneider GPG-ID: CC014E3D
Samba Team ***@samba.org
www.samba.org
Alexander Bokovoy
2017-03-10 11:04:16 UTC
Permalink
Post by Andreas Schneider
Post by Gary Lockyer
Post by Andrew Bartlett
Gary will continue on this to add the JSON, but I'm really pleased with
the progress and hopefully we can get this much in to the tree soon.
We now handle logging anonymous ldap and un-authenticated dce/rpc
authorizations!
To generate the json log lines I'm planning on using glib-json
https://developer.gnome.org/json-glib/stable/, rather than rolling my own.
Doing this however does add dependencies on
* json-glib
* gio
* gobject
* glib
And requires compiling with -pthread
Is this a reasonable approach?
I'm against glib.
https://github.com/json-c/json-c/wiki
I'd suggest jansson: https://github.com/akheron/jansson
We use it in JOSE (https://github.com/latchset/jose) and it also has
very good documentation: https://jansson.readthedocs.io/en/latest/index.html
--
/ Alexander Bokovoy
Andrew Bartlett
2017-03-10 21:03:49 UTC
Permalink
Post by Alexander Bokovoy
Post by Andreas Schneider
Post by Gary Lockyer
Post by Andrew Bartlett
Gary will continue on this to add the JSON, but I'm really pleased with
the progress and hopefully we can get this much in to the tree soon.
We now handle logging anonymous ldap and un-authenticated dce/rpc
authorizations!
To generate the json log lines I'm planning on using glib-json
https://developer.gnome.org/json-glib/stable/, rather than
rolling my own.
Doing this however does add dependencies on
* json-glib
* gio
* gobject
* glib
And requires compiling with -pthread
Is this a reasonable approach?
I'm against glib.
Andreas:

Can you please elaborate? Before we start re-working the code I would
appreciate understanding the technical objection. To be clear: The
JSON auditing is optional based on the availability of the dependency..
Post by Alexander Bokovoy
Post by Andreas Schneider
https://github.com/json-c/json-c/wiki
I'd suggest jansson: https://github.com/akheron/jansson
We use it in JOSE (https://github.com/latchset/jose) and it also has
very good documentation: https://jansson.readthedocs.io/en/latest/ind
ex.html
Thanks. We found it difficult to locate many packages that were
suitable, maintained and actually packaged.

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
Andreas Schneider
2017-03-11 08:57:52 UTC
Permalink
Post by Andrew Bartlett
Post by Alexander Bokovoy
Post by Andreas Schneider
Post by Gary Lockyer
On Fri, Mar 03, 2017 at 01:36:28PM +1300, Andrew Bartlett
Post by Andrew Bartlett
Gary will continue on this to add the JSON, but I'm really pleased with
the progress and hopefully we can get this much in to the tree soon.
We now handle logging anonymous ldap and un-authenticated dce/rpc
authorizations!
To generate the json log lines I'm planning on using glib-json
https://developer.gnome.org/json-glib/stable/, rather than rolling my own.
Doing this however does add dependencies on
* json-glib
* gio
* gobject
* glib
And requires compiling with -pthread
Is this a reasonable approach?
I'm against glib.
Can you please elaborate? Before we start re-working the code I would
appreciate understanding the technical objection. To be clear: The
JSON auditing is optional based on the availability of the dependency..
As stated above json-glib pulls in serveral g* libraries. jannson doesn't have
any dependencies. I'm for the smaller library!


Also it looks like jansson is in a really good shape, check:

Parsing JSON is a minefield
http://seriot.ch/parsing_json.php
Post by Andrew Bartlett
Post by Alexander Bokovoy
Post by Andreas Schneider
https://github.com/json-c/json-c/wiki
I'd suggest jansson: https://github.com/akheron/jansson
We use it in JOSE (https://github.com/latchset/jose) and it also has
very good documentation: https://jansson.readthedocs.io/en/latest/ind
ex.html
Yes, jansson really looks nice and seems to be on all distros.



Andreas
Gary Lockyer
2017-03-10 01:42:04 UTC
Permalink
Revised patch with work to date, rebased on the current master.

We're aiming to add tests and wrap things up early next week. Any
review/comments greatly appreciated.

Gary.
Gary Lockyer
2017-03-10 02:41:50 UTC
Permalink
Another revision, got the rebase slightly wrong.
Post by Gary Lockyer
Revised patch with work to date, rebased on the current master.
We're aiming to add tests and wrap things up early next week. Any
review/comments greatly appreciated.
Gary.
Gary Lockyer via samba-technical
2017-03-13 03:39:41 UTC
Permalink
Review/comments apreciated.

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]

JSON
JSON Authorization: {
"timestamp": "2017-03-13T16:17:57.446508+1300",
"Authorization": {
"version": {"major": 1, "minor": 0},
"remoteAddress": "ipv6::::0",
"authType": "ncacn_np",
"transportProtection": "SMB",
"localAddress": "ipv6::::0",
"sid": "S-1-5-18",
"serviceDescription": "DCE/RPC",
"domain": "NT AUTHORITY",
"account": "SYSTEM",
"logonServer": null,
"accountFlags": "0x00000000"
},
"type": "Authorization"
}

Authentication

Human Readable
Auth: [SamLogon,network] user [ADDOMAIN]\[Administrator]
at [Mon, 13 Mar 2017 16:18:39 NZDT]
with [ntlmv2] status [NT_STATUS_OK]
workstation [schannel0] remote host [ipv4:127.0.0.11:15408]
became [ADDOMAIN]\[Administrator]
S-1-5-21-2041978683-1917359319-4010618550-500.
local host [ipv4:127.0.0.30:445]
NETLOGON computer [schannel0] trust account [schannel0]

JSON

JSON Authentication: {
"timestamp": "2017-03-13T16:18:39.477569+1300",
"type": "Authentication",
"Authentication": {
"version": {"major": 1, "minor": 0},
"clientAccount": "Administrator",
"status": "NT_STATUS_OK",
"mappedDomain": "ADDOMAIN",
"remoteAddress": "ipv4:127.0.0.11:15408",
"passwordType": "ntlmv2",
"netlogonComputer": "schannel0",
"becameAccount": "Administrator",
"localAddress": "ipv4:127.0.0.30:445",
"authDescription": "network",
"becameDomain": "ADDOMAIN",
"serviceDescription": "SamLogon",
"clientDomain": "ADDOMAIN",
"workstation": "schannel0",
"mappedAccount": "Administrator",
"trustAccount": "schannel0"
}
}
Post by Gary Lockyer
Another revision, got the rebase slightly wrong.
Post by Gary Lockyer
Revised patch with work to date, rebased on the current master.
We're aiming to add tests and wrap things up early next week. Any
review/comments greatly appreciated.
Gary.
Stefan Metzmacher via samba-technical
2017-03-13 08:05:32 UTC
Permalink
Hi Gary,
Post by Gary Lockyer via samba-technical
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.

I haven't looked at the rest yet...

metze
Andrew Bartlett via samba-technical
2017-03-13 17:10:45 UTC
Permalink
On Mon, 2017-03-13 at 09:05 +0100, Stefan Metzmacher via samba-
Post by Stefan Metzmacher via samba-technical
Hi Gary,
Post by Gary Lockyer via samba-technical
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.  

I wanted to do this in good python, plugging the bits together, but it
is becoming more work than makes sense...
Post by Stefan Metzmacher via samba-technical
I haven't looked at the rest yet...
Thanks for the comments.

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
Andrew Bartlett via samba-technical
2017-03-16 08:25:05 UTC
Permalink
On Tue, 2017-03-14 at 06:10 +1300, Andrew Bartlett via samba-technical
Post by Andrew Bartlett via samba-technical
On Mon, 2017-03-13 at 09:05 +0100, Stefan Metzmacher via samba-
Post by Stefan Metzmacher via samba-technical
Hi Gary,
Post by Gary Lockyer via samba-technical
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 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!

BTW, While I'll document this JSON auth message target as a developer
feature for now, I'm sure someone will find some really neat way to use
it. It may also be a good way to test other 'impossible to test' bits
of Samba.

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
Andrew Bartlett via samba-technical
2017-03-20 07:38:25 UTC
Permalink
On Thu, 2017-03-16 at 21:25 +1300, Andrew Bartlett via samba-technical
Post by Andrew Bartlett via samba-technical
On Tue, 2017-03-14 at 06:10 +1300, Andrew Bartlett via samba-
technical
Post by Andrew Bartlett via samba-technical
On Mon, 2017-03-13 at 09:05 +0100, Stefan Metzmacher via samba-
Post by Stefan Metzmacher via samba-technical
Hi Gary,
Post by Gary Lockyer via samba-technical
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.
Post by Andrew Bartlett via samba-technical
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
Andrew Bartlett via samba-technical
2017-03-22 22:45:49 UTC
Permalink
On Thu, 2017-03-16 at 21:25 +1300, Andrew Bartlett via samba-technical
Post by Andrew Bartlett via samba-technical
On Tue, 2017-03-14 at 06:10 +1300, Andrew Bartlett via samba-
technical
Post by Andrew Bartlett via samba-technical
On Mon, 2017-03-13 at 09:05 +0100, Stefan Metzmacher via samba-
Post by Stefan Metzmacher via samba-technical
Hi Gary,
Post by Gary Lockyer via samba-technical
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
Jeremy Allison via samba-technical
2017-03-23 01:48:08 UTC
Permalink
Post by Andrew Bartlett via samba-technical
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
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.
Andrew Bartlett via samba-technical
2017-03-23 02:35:35 UTC
Permalink
On Thu, Mar 23, 2017 at 11:45:49AM +1300, Andrew Bartlett via samba-
Post by Andrew Bartlett via samba-technical
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
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
Andrew Bartlett via samba-technical
2017-03-27 20:06:32 UTC
Permalink
On Thu, 2017-03-23 at 15:35 +1300, Andrew Bartlett via samba-technical
Post by Andrew Bartlett via samba-technical
On Thu, Mar 23, 2017 at 11:45:49AM +1300, Andrew Bartlett via samba-
Post by Andrew Bartlett via samba-technical
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/h
ea
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
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 ;-)
The auth-logging patch is still 500k, but I attach it here for the
record. This is without the patches it depends on, so can't be pushed
on it's own, but should allow some review.

We are sorting out some last issues, but it is almost ready.

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
Stefan Metzmacher
2017-03-03 09:31:10 UTC
Permalink
Hi Andrew,
Post by Andrew Bartlett
Post by Stefan Metzmacher
I haven't looked very closely, but is some places I had the
impression
that a later patch fixes a problem in a former patch.
In order to understand the flow better, it would be useful to have this
sorted out and every single commit complies and is supposed to
work without crashing with NULL pointer deferences.
Thanks. Each commit does compile (we have been using your rebase/x
make -j trick), and they are meant to operate, but we will look again
for any more cases we need to implement.
The main place was the netlogon server changes where one commit
moves some code up (maybe dereferencinf the wrong union arm)
and later down again,

Maybe 'x make -j test TESTS="..."' with a subset of relevant tests
would be good. Just add an smb_panic(__location__) to each of the
relevant code path, SMB1 auth with and without SPNEGO in both servers
and SMB2 auth in both server, RPC bind in all servers, LDAP simple and
sasl bind
in the server, both ntlm_auth tools. If you do that one at a time with
the full patchset,
you should easily find the few relevant tests to trigger each code path.
Post by Andrew Bartlett
Post by Stefan Metzmacher
Typically it's better to pass new unused arguments from the top,
before they get used at the bottom.
I guess you also need to be prepared that a dcerpc bind negotiates no
presentation context, it's possible to use a dummy uuid and just
do the authentication.
Very interesting. I'll cover that - but what happens in Samba then,
can the user re-use the authentication on another UUID? (it is
important to know this for auditing.)
It's similar as with SMB sessions vs. with multiple tree connects.
In both cases I guess you'd need a separate audit event for the
resource access with a given auth_session_info. This is also
important for DCERPC over smb where you don't have DCERPC authentication
but still access a particular interface with the inherited
auth_session_info from the transport connection.

I guess the auth_description should be just something like
"DCERPC(auth_type=%d,auth_level=%d)" instead of using the
interface name.

For SMB we may also want to have the selected protocol dialect
in the auth_description. I think that is important information.

Maybe it's also useful to have logon_server in the audit log.
Maybe also the user_flags (thinks like NETLOGON_GRACE_LOGON would be
important)
or even more stuff (except the session keys) from the netr_Sam*Info*
structures.

We should also try to make our server much better in filling
the netr_SamBaseInfo details.

But testing with Windows 2008R2 just showed that the NETLOGON_GRACE_LOGON
is always 0, even for logons with the old password.
Post by Andrew Bartlett
Post by Stefan Metzmacher
We may also want to distinguish between the different LogonSamLogon
levels (interactive vs. network at least).
A very good idea. We have an unused parameter that would be ideal for
that.
I thought you whould use use auth_descriptions like "SamLogon(Interactive)",
"SamLogon(InteractiveTransitive)"...
Post by Andrew Bartlett
Post by Stefan Metzmacher
I'm also wondering why the log functions gets the user supplied info
but not the full auth_session_info.
At the point that we see the user_supplied_info, we don't get the
auth_session_info, this is generated later. Because of the differences
between the source3 and source4 auth subsystems, I can't even pass in
the user_info_dc etc, so we just pick out some key parameters.
Ok.
Post by Andrew Bartlett
Post by Stefan Metzmacher
The output format should also be flexible, we can't guarantee that
we'll generate the exact output forever.
Indeed! This work is just the first step, designed to be practical to
read, the full task from our clients calls for JSON (one one line),
which is the next step now we have the plumbing in place.
Yes, but still the JSON needs to flexible and not written in stone,
I'm not sure but maybe it would be good to a some kind of version number.

metze
Andrew Bartlett
2017-03-03 18:45:11 UTC
Permalink
Post by Stefan Metzmacher
Hi Andrew,
Post by Stefan Metzmacher
I haven't looked very closely, but is some places I had the impression
that a later patch fixes a problem in a former patch.
In order to understand the flow better, it would be useful to
have
this
sorted out and every single commit complies and is supposed to
work without crashing with NULL pointer deferences.
Thanks.  Each commit does compile (we have been using your rebase/x
make -j trick), and they are meant to operate, but we will look again
for any more cases we need to implement. 
The main place was the netlogon server changes where one commit
moves some code up (maybe dereferencinf the wrong union arm)
and later down again,
I'll look at that one again. We thought we had fixed that one! :-)
Post by Stefan Metzmacher
Maybe 'x make -j test TESTS="..."' with a subset of relevant tests
would be good. Just add an smb_panic(__location__) to each of the
relevant code path, SMB1 auth with and without SPNEGO in both servers
and SMB2 auth in both server, RPC bind in all servers, LDAP simple and
sasl bind
in the server, both ntlm_auth tools. If you do that one at a time with
the full patchset,
you should easily find the few relevant tests to trigger each code path.
Good point.
Post by Stefan Metzmacher
Post by Stefan Metzmacher
Typically it's better to pass new unused arguments from the top,
before they get used at the bottom.
I guess you also need to be prepared that a dcerpc bind
negotiates no
presentation context, it's possible to use a dummy uuid and just
do the authentication.
Very interesting.  I'll cover that - but what happens in Samba
then,
can the user re-use the authentication on another UUID? (it is
important to know this for auditing.)
It's similar as with SMB sessions vs. with multiple tree connects.
In both cases I guess you'd need a separate audit event for the
resource access with a given auth_session_info. This is also
important for DCERPC over smb where you don't have DCERPC
authentication
but still access a particular interface with the inherited
auth_session_info from the transport connection.
I've caught that in the DCE/RPC bind without authentication I think,
but clearly I need some help.

Should I do the logging at the iface->bind() call instead/additionally?
Post by Stefan Metzmacher
I guess the auth_description should be just something like
"DCERPC(auth_type=%d,auth_level=%d)" instead of using the
interface name.
I can change the service name back to dcerpc if you prefer. I'm trying
not to build compound strings with printf here, because we want to have
individual details in each element in the json.  

Also, while in-scope for the project if we have time at the end, we
have not yet tackled logging the level of protection.

(If you can point me at where we know we will be doing SMB encryption
it will help to plumb a distinct transport_protection up the stack,
because it isn't seen in what GENSEC negotiates - boo, hiss).

The auth_type part is logged already as the auth description, where we
know if krb5, ntlmssp or schannel was used.
Post by Stefan Metzmacher
For SMB we may also want to have the selected protocol dialect
in the auth_description. I think that is important information.
Can you point me at where I get that string? Again, this would be the
service (the auth_description is for strings like "ncacn_np", "ntlmssp"
or "krb5", "network" or "interactive" - so far - eg how the user
authenticated).

Note that I would prefer to incorrectly log everything as "cifs" than
have 10 (and growing) different strings an admin later has to filter on
to know they a auditing file server access. That is why I limited it
to just SMB and SMB2. This isn't the one and only debug line in Samba!
:-)
Post by Stefan Metzmacher
Maybe it's also useful to have logon_server in the audit log.
Maybe also the user_flags (thinks like NETLOGON_GRACE_LOGON would be
important)
or even more stuff (except the session keys) from the netr_Sam*Info*
structures.
The plan is to make the JSON reasonably verbose, and easy to extend.  

Gary has got started on that, hopefully we can get in a base and then
we can add some more. I similarly thought that the logon server would
be a handy thing to log.
Post by Stefan Metzmacher
We should also try to make our server much better in filling
the netr_SamBaseInfo details.
But testing with Windows 2008R2 just showed that the
NETLOGON_GRACE_LOGON
is always 0, even for logons with the old password.
Fun :-)
Post by Stefan Metzmacher
Post by Stefan Metzmacher
We may also want to distinguish between the different
LogonSamLogon
levels (interactive vs. network at least).
A very good idea.  We have an unused parameter that would be ideal
for
that. 
I thought you whould use use auth_descriptions like
"SamLogon(Interactive)",
"SamLogon(InteractiveTransitive)"...
I can use the full names if you like. The SamLogon is already recorded
as the service.
Post by Stefan Metzmacher
Post by Stefan Metzmacher
I'm also wondering why the log functions gets the user supplied info
but not the full auth_session_info.
At the point that we see the user_supplied_info, we don't get the
auth_session_info, this is generated later.  Because of the
differences
between the source3 and source4 auth subsystems, I can't even pass in
the user_info_dc etc, so we just pick out some key parameters. 
Ok.
Post by Stefan Metzmacher
The output format should also be flexible, we can't guarantee that
we'll generate the exact output forever.
Indeed!  This work is just the first step, designed to be practical
to
read, the full task from our clients calls for JSON (one one line),
which is the next step now we have the plumbing in place.
Yes, but still the JSON needs to flexible and not written in stone,
I'm not sure but maybe it would be good to a some kind of version number.
That's what we are aiming for. We can add a version, but because it is
an extensible structure, it isn't really needed, because we can just
add more elements.

Gary spent much time at Catalyst working on a project for parsing log
entries and storing them in a database, so he knows and is in contact
with those who have lost much hair to regular expressions over horrible
log lines. So I have pretty high hopes that this will be one of the
better ones ;-)

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