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

[Patrik Karlsson <patrik () cqure net>]

On 15 feb 2011, at 22.17, Chris Woodbury wrote:

> On Sat, Feb 12, 2011 at 6:43 AM, Patrik Karlsson <patrik () cqure net> wrote:
> > I've been able to do some testing and here's some initial feedback:
>
> Thanks for the quick turnaround and all the feedback! Sorry for my own
> delay responding.

No problem.

> > * To me, it was not obvious that the ms-sql-discover script needs to be run in order to get any result from the 
> > ms-sql-info script.
> >  As the dependencies are not "forced", and there's currently no way of achieving this, it requires the user to 
> > specify both scripts each time which isn't very clear.
> >  I would personally prefer being able to run just ms-sql-info to be able to retrieve the version information from 
> > the instances.
>
> Good point about the dependency not being clear. It probably makes
> sense to include ms-sql-info in with the other scripts in terms of how
> we handle the various modes of operation (i.e. has a discover script
> been run or not, what ports have been scanned, etc.).
>
> > * The TCP port is not printed by neither ms-sql-discover or ms-sql-info even though it's in the SSRP string. I'm 
> > including a patch for this [1].
>
> This was a stupid mistake on my part, brought on by a last-minute
> change (for some reason, I thought that doing a["b.c"] would get
> around the issue of having to check whether b exists (since you'll get
> an error indexing nil when you do a.b.c). It does of course get around
> that issue, but it doesn't actually work.). Anyway, let's just revert
> that whole last-minute change and go back to a.b.c-style indexing, to
> be consistent with the rest of the use of SqlServerInstanceInfo and
> SqlServerVersionInfo. Patch for this: [1]

This patch failed to apply against ms-sql-discover for some reason, but I made the necessary changes.

> > * When attempting to connect over named pipes, for some reason, I keep getting ERROR: 18456. I'm obvioulsy doing 
> > something wrong here.
>
> Login error 18456 is "invalid username or password" (i.e. a normal
> failed login attempt). I'll have a patch in a little bit that returns
> a meaningful message for login errors (as well as fixing a few other
> little things).
> Incidentally, if you're getting error 18452, that's "User is not
> associated with a trusted connection", which probably means that the
> instance doesn't support SQL Server authentication.

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?

> > * I've seen the following error a few times, not sure what triggers it:
> > |   [10.0.200.111\MSSQLSERVER]
> > |     TCP: Connect failed, Named Pipes: Already connected via TCP
>
> This happens when there is a TCP port and a named pipe for the
> instance, but the TCP port can't be reached (e.g. it's firewalled, or
> the instance is stopped). Patch for this: [2]

Ok, sounds like it may have been the problem because I kept restarting some instances to test the different behavior.
I've applied this patch.

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

> -chris
>
> These are patches against /nmap-exp/patrik/nmap-mssql/ :
> [1] tcp_port.patch
> [2] already_connected.patch
> <already_connected.patch><tcp_port.patch>



//Patrik
--
Patrik Karlsson
http://www.cqure.net
http://www.twitter.com/nevdull77





_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/