[illumos-Advocates] [RTI] 1121 smbsrv should use SPNEGO (outbound authentication)

[Gordon Ross]

> I'm reviewing:
> 
> mlsvc_client.c: line 229.  Why the XXX ?

Oh, that was a comment to help explain to reviewers what happened
there with the ndr_rpc_get_heap() call.  I meant to remove it.

> mlsvc_client.c: line 183, 193, 240... others?  You are using
> syslog(LOG_DEBUG) here.  Is that really appropriate?  Does it match
> existing practice in this code?  I'm especially concerned if this
> winds
> up in a library, rather than in application code.  There are other
> error
> paths in this code that don't fire off a syslog() event.  (In general,
> for developer debugging, I prefer dtrace or its ilk, and I try to
> reserve syslog for things that need administrator attention.)

Yes, a lot of the smbsrv libraries use syslog(LOG_DEBUG, ...)
One of the current diagnostic practices for smbsrv is to change
the *.err to *.debug in syslog.conf and re-run whatever was
not working as expected.  These syslog calls basically replace
similar ones that were in the libsmbrdr code this obsoletes.

I too generally prefer dtrace, but I don't want to break the
current diagnostic methods people are using.

Looking at those syslogs again, I think the one on 182
(after smbrdr_ctx_new) should be LOG_ERR.  It means that
something is wrong with the configuration that we use to
connect to a domain controller, so some administrative
action is needed to correct it.

The other syslog(LOG_DEBUG...) calls will be rare, so
I don't think there's any harm leaving them.

> mlsvc_client.c: line 549.  Why this change?

This function is called via the clnt->xa_exchange member
over in libmlrpc:ndr_client.c where the return value is
just zero or -error code.  The old code was "confused"
about this return value :)

> smbrdr_glue.c: line 177. This really feels like something that should
> be
> done via dtrace fbt provider.

Well, as explained above, this is currently the easiest
way to find out why (i.e.) join domain doesn't work.
(enable *.debug in syslog.conf)

The bigger-picture problem with error reporting in this
code is that these operations all happen from inside smbd,
where we can't directly report things to the user as we
might wish that we could.  What I'm doing next will help
with that, so these could become call-backs to a caller
provided "log this error" function that displays it to
the user or whatever.  For now, it's syslog.


> With that reply, the rest of the changes look good.

Thanks for having a look.

Gordon