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

[Patrik Karlsson <patrik () cqure net>]

On Feb 17, 2011, at 00:31 , Chris Woodbury wrote:

> On Sat, Feb 12, 2011 at 6:43 AM, Patrik Karlsson <patrik () cqure net> wrote:
> > * 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.
>
> Seeing your changes, I agree that it seems cumbersome to have to run a
> separate script just to do discovery. As I see it now, I think it
> would make the most sense to implement all of the discovery
> functionality in mssql.lua instead of in ms-sql-discover, and to have
> discovery be done on a per-host basis, within mssql.lua, and be
> triggered by whichever ms-sql-* script runs first. The main changes
> would be that discovery_performed would have to be tracked per-host,
> and Helper.GetInstanceByName() would check to see if discovery had
> been performed already (right now, if you run multiple scripts, each
> one does SSRP discovery).

There might be a problem with this approach. As ms-sql-discover runs it uses the information retrieved from the browser 
to determine what tcp ports exist.
WIth this information it then sets each of the ports as being fingerprinted as ms-sql-s. When the next script runs the 
portrule is triggered for each of these ports.
Unfortunately, I don't think there's currently a way to first connect to the browser, mark the ports as ms-sql-s and 
then trigger the portrule within the same script.
One way could be to place the discovery code within the hostrule or portrule function, however doing this causes the 
following stacktrace:

stack traceback:
        ./nse_main.lua:654: in function <./nse_main.lua:653>
        [C]: in function 'socket_lock'
        [string "local connect, socket_lock = ...;..."]:3: in function 'connect'

Another way is going down the path of forced dependencies, which I believe has been discussed in the past.

> While we're on that subject, how about
> renaming GetInstanceByName() to something like GetTargetInstances()?
> Depending on the circumstances and script arguments, it might be
> getting the instance by name, by port, by pipe name, or all that are
> available.

Sounds good to me.

> > In my personal opinion, I think that may be a little bit too complicated and I'm leaning more towards a instance 
> > oriented approach rather than a port oriented.
>
> I agree; this makes a lot of sense.
>
> > Taking ms-sql-query as an example I see the following use cases:
> > 1. I want to run a query against a default instance running on port 1433
> > This currently isn't a problem at all as the portrule will match port 1433 and run the query.
> >
> > 2. I want to run a query against an instance registered with the browser called INST1 running on port 1435
> > This is problematic as the portrule won't match unless port 1435 is detected as ms-sql-s which currently requires a 
> > version scan running with a version intensity of 8.
> > A quick test running this against a system here takes 64 seconds which is kind of a pain to do each time you want to 
> > run a query against that instance.
> > Using the combination of ms-sql-discover and ms-sql-query this could be achieved through the following command:
> > sudo ./nmap -sT -sU -p T:1435,U:1434 10.0.200.111 --script ms-sql-discover,ms-sql-query 
> > --script-args=mssql.scanned-ports-only
> >
> > 3. I want to run a query against an instance on port 1435, that isn't registered with the browser
> > This case has the same problem with service detection as described in the second case.
> > The ms-sql-discover script fails to detect the port as sql server as it's running on a non default port and the 
> > browsers not responding.
> > If I'm not totally off here there's currently no other way to query an instance on this port other than going 
> > through the tedious service scan.
> >
> > While this port oriented approach gives the user good control over what Nmap is doing I think it requires the user 
> > to know quite a lot of how instances work and are resolved.
>
> Good point. I was focusing too much on use case #4, where I want to do
> versioning and run, say, ms-sql-empty-password against all of the SQL
> Servers in a Class B.

I would like to achieve this as well in a easy way. I think the mssql.instance="all" should provide us with what we 
need here.

> > Another potential problem would be limiting the execution of a script to a single instance in the case of named 
> > pipes as they would be accessed over the same port. I haven't been able to verify this yet though.
>
> Yeah, specifying by port won't work here, since we could have multiple
> instances available via named pipes. It's awkward, but we might just
> need to have a mssql.instance-pipename script argument.

That could be useful too, but for the moment you can get around this using -p 445 and specifying the instance name 
using mssql.instance.
The library would still default to tcp, if available though, but you can override this choice by using the 
mssql.protocol argument.

> > Taking a instance oriented approach, where it's up to Nmap (or the script really) to determine the correct ports to 
> > use, the commands for the above use cases would end up like this instead:
> >
> > 1. No change
> > 2. ./nmap -p 1435 --script ms-sql-query 10.0.200.111 --script-args mssql.instance-name='SQLEXPRESS'
> > 3. ./nmap -p 1435 --script ms-sql-query 10.0.200.111 --script-args mssql.instance-port=1435
> >
> > I realize that the port argument in the 3rd example is redundant/misleading/strange, but unfortunately necessary as 
> > we're running as a host rule at this point.
> > I guess that use case 3 could also be achieved through Martin Swende's proposed force patch [3] that would force the 
> > script to run against the port supplied using -p.
>
> I'm not completely convinced one way or the other, but I think I might
> prefer having an argument like mssql.force (we'd need a better name,
> though).

I agree.

> It does seem a little goofy/redundant to be putting the port
> number in twice, and I can't think of a situation where you would be
> running an Nmap scan against several ports, but only want the SQL
> Server scripts to target one of them. But, on the other hand, maybe
> it's clearer and more consistent to use instance-port to specify it.
> Either way, I think the plumbing in mssql will be the same, so it's
> just a matter of the user interface.
>
> Our problem would go away, of course, if Martin Swende's patch were
> applied. That would be the easiest approach for us :)

Yes, I agree, don't know what remains in order to push that in though.
An alternative way would be to implement functionality that would allow us to launch a specific service probe based on 
the name.
This would allow for quick service detection of ports that we suspect/know are a specific service.

> > The patch also allows the instance-name to be "all", which will result in all instance being processed by the script.
>
> This is good. The default behavior is conservative, but it's not
> cumbersome to get it to run against everything, so we're still in good
> shape for use case #4.
>
> I think we would also want to allow a list of instance names to be
> given, for added flexibility.

I agree, this should be a small patch I think.

> > To me, and perhaps only me?, this approach is clearer even though it will connect to ports outside of the given -p 
> > scope. Comments, ideas and suggestions are most welcome.
> >
> > [1] ms-sql-tcp-port.patch
> > [2] http://seclists.org/nmap-dev/2011/q1/325
> > [3] http://seclists.org/nmap-dev/2010/q4/543
> > [4] ms-sql-connect-by-instance-name.patch
> >
> > //Patrik
>
> While I'm at it, there are a few other ideas that I wanted to throw out there:
> * It seems to me that we're going to end up with 2 or 3 classes of SQL
> Server scripts (e.g. discovery, per-instance), each of which will have
> the same criteria for when they run. I think it would make sense,
> then, to put some functions in mssql.Helper that would generate the
> hostrules and portrules, and have the scripts just call those (e.g.
> hostrule = mssql.Helper.get_standard_hostrule() ).

Sounds good.

> * At some point, I think we ought to put some top-level documentation
> into mssql.lua that gives an overview of the SQL Server scripts and
> lays out the different classes and how the mssql arguments affect them
> (e.g. ms-sql-info runs against all instances by default, doesn't use
> mssql.username).

I think that's probably a good idea, considering how much more functionality is going into the library at the moment.

> * Am I wrong, or is it unnecessary to have 1433 in the portrule? Is it
> possible to have a situation in which there is a SQL Server available
> on 1433 and to have Nmap not classify it as "ms-sql-s"?

The 1433 will allow the script to run even though no version/service scanning has been performed.

> Anyway, I think we're headed in the right direction. Let me know your
> thoughts. In the meantime, I'll start working on some of this.

Great! I might have some time over to lend a hand.
> -chris



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