Re: [NSE] Several changes to mssql.lua and SQL Server scripts

[Chris Woodbury <woodbusy () gmail com>]

p.s. A bit of commentary on login_error_message.patch: I realized that
Helper.LoginWithDetail() was redundant, that I could just pass the
login error code as a third return from Login() instead of making a
whole other function for it. So, I merged them back into a single
function. Let me know if you think otherwise or have any other
thoughts.

-chris

On Wed, Feb 16, 2011 at 1:26 AM, Chris Woodbury <woodbusy () gmail com> wrote:
> On Tue, Feb 15, 2011 at 3:54 PM, Patrik Karlsson <patrik () cqure net> wrote:
> > I got named pipes running after a while as a realized I needed to use proper credentials for the smb library.
> > A patch for a clearer error message here, like you mentioned, would be great.
> > Maybe the error should give the user a hint about passing the proper credentials using the smbuser and smbpass 
> > arguments?
>
> That's a good idea. It's now on my to-do list.
>
> > > So, that was the easy stuff. Let me process the bigger issues you
> > > raised, and I'll get back to you.
> > Before you do, be sure to check out the changes I made to the code in nmap-mssql.
> > The initial commit is your code, but in the following commits I've made a few changes that change the way some 
> > things worked, mainly:
> > * the ms-sql-info will now return results, even though the ms-sql-discover script wasn't run.
> > * the ms-sql-discover script will now fingerprint the ports discovered through the browser as ms-sql-s so that the 
> > portrule will be triggered in each of the scripts.
> > * added the mssql.instance argument, that allows connecting by instance name or to 'all' instances.
> > * added the mssql.protocol argument through which named pipes or tcp can be forced on a specific instance.
> > * added integrated authentication which may be forced by using the mssql.domain argument.
> > * All scripts will now run against a single instance, with the exception of ms-sql-info, unless forced by setting 
> > the mssql.instance to all.
> >
> > I think that's all. Let me know what you think about these changes.
>
> OK, I'm still working my way through your additions, but I have a
> couple patches that I want to send out before I get too backed up. I
> love that you added support for integrated Windows auth (I was
> planning to do that as well!). That will really take these to the next
> level. I also like the mssql.protocol argument, to give the user some
> more control there.
>
> Patches:
> 1) login_error_message.patch - As promised before, here's a patch to
> give some more detail on login errors
> 2) ntlmssp_auth_offsets.patch - I was excited to try out NTLM auth,
> but it didn't work for me at first. This patch makes the offsets match
> the actual lengths of the strings in the NTLMSSP data.
> 3) move_mssql_protocol_to_tdsstream.patch - Would it make sense to
> move the mssql.protocol enforcement directly into TDSStream, instead
> of Helper?
> 4) Also, can you apply the patches to smb.lua in
> http://seclists.org/nmap-dev/2011/q1/501 ? These are needed for named
> pipe support.
> 5) ms-sql-discover.nse, line 137:
> - if ( instance.port.number ) then
> + if ( instance.port and instance.port.number ) then
> 6) named_pipe_login.patch - This patch should be applied AFTER all the
> others. It works around an issue I discovered, where SQL Server often
> (but not always, and never for the "sa" account) disconnects the named
> pipe immediately if there's a failed login. Thus, instead of reading
> the login error code, we get a Receive() error. This patch treats a
> Receive() error at that point during login as an invalid
> username/password.
>
> I tested those patches against your nmap-exp branch, so you shouldn't
> have any problems applying them this time.
>
> -chris
_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/