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

[Chris Woodbury <woodbusy () gmail com>]

Patrik-

Actually, I don't think that's the case now anyway. ms-sql-discover doesn't
run as a port-script, and I believe the only non-standard ports it will
connect to are ones it gets from the SQL Server Browser, or the
mssql.instance-port argument.

In any case, I think I prefer your named probes proposal, or a force
argument, in this case.

-chris

On Wed, Mar 2, 2011 at 10:45 AM, Patrik Karlsson <patrik () cqure net> wrote:

> On Feb 28, 2011, at 19:55 , Chris Woodbury wrote:
>
> > Patrik-
> >
> > I had a discussion with one of my colleagues this weekend which left me
> wondering whether there is any need now for the ms-sql-discover script.
> Originally, of course, it did all of the discover, but since we ditched that
> approach a while ago, I feel that it's basically just ms-sql-info without
> the accurate version detection (in fact, you could get the same
> functionality with nmap -sSU -p T:445,U:1434 --script ms-sql-info
> --script-args mssql.scanned-ports-only). At the same time, I think it's
> potentially confusing to users as to what it is and whether it needs to be
> used. So, unless you feel otherwise, I say let's delete it.
>
> It provides us with the possibility to run against a different port than
> 1433 without providing the instance-port argument.
> Essentially what named probes or the force argument would.
>
> Eg:
> ./nmap -p 1435 192.168.56.101 --script ms-sql-discover,ms-sql-query
>
> Don't know if it's worth keeping for that purpose though. Probably not?
>
> //Patrik
>
> > -chris
> > On Sat, Feb 26, 2011 at 4:44 PM, Patrik Karlsson <patrik () cqure net>
> wrote:
> > On Feb 26, 2011, at 21:06 , Chris Woodbury wrote:
> >
> > > Thanks for catching the unintended change in -brute. I was testing
> something
> > > out and forgot to revert before I updated the documentation. I should
> do a
> > > better job of checking my patches. Everything else sounds good, though;
> > > let's merge it!
> >
> > Ok, it's merged into trunk. Hopefully, without braking anything.
> >
> >
> > > -chris
> > >
> > > On Sat, Feb 26, 2011 at 4:04 AM, Patrik Karlsson <patrik () cqure net>
> wrote:
> > > > On Feb 25, 2011, at 23:31 , Chris Woodbury wrote:
> > > >
> > > > > On Fri, Feb 25, 2011 at 1:53 PM, Patrik Karlsson <patrik () cqure net>
> > > > wrote:
> > > > > On Feb 25, 2011, at 00:15 , Chris Woodbury wrote:
> > > > > > Based on all of that, I have a few comments:
> > > > > > * Now that we have instance-name and instance-port, I'm not sure
> that
> > > > it's intuitive that the way to hit all instances is by specifying
> > > > instance-name=all, especially considering the case where there are
> instances
> > > > we don't know the names of. How about having a script argument like
> > > > mssql.all-instances?
> > > > > I've added a instance-all argument to align better with the other
> names.
> > > > Does that make sense?
> > > > > Good idea.
> > > > >
> > > > > > * I really like the idea that it's now possible to use SQL Server as
> a
> > > > proxy for doing Windows password testing. However, I think we should
> put up
> > > > some hurdles for the user. Virtually all the Windows networks I've
> worked
> > > > with have had account lockout policies in place, and I'm concerned
> that
> > > > someone could accidentally end up brute-forcing a domain and locking
> out a
> > > > bunch of accounts. I think we ought to add a script argument to enable
> it.
> > > > > At first, I didn't understand the problem here, as you previously
> wrote
> > > > that you modified the script to abort once it detected a lockout.
> > > > > When I tested it I noticed what happened, and that there's no way of
> > > > detecting that an account has been locked out based on the error
> message
> > > > returned by the server.
> > > > > I agree that there's a risk of locking out a number of accounts,
> > > > potentially the whole domain, if you run the script without knowing
> what
> > > > your doing or how it works.
> > > > > Either we document this with a few exclamation marks, or we simply
> fail
> > > > to start the script (if Windows authentication is used) and return an
> > > > explanation why and instructions how to proceed.
> > > > > We could use an argument that tells the script how many attempts to
> > > > perform for each account, if unset and the authentication method is
> set to
> > > > Windows, the script won't start.
> > > > > How about that?
> > > > >
> > > > > Actually, since you mention it, it might be nice to have an "only do
> this
> > > > many attempts" argument for SQL Server authentication as well, since
> those
> > > > can also have lockouts. But maybe that's just something the user
> should
> > > > control with the password database (incidentally, the brute script now
> tries
> > > > the username as the password - should that be configurable?).
> > > > > Anyway, I'd be fine with doing something like that for Windows at
> least,
> > > > or maybe just a ms-sql-brute.brute-windows-accounts argument, so that
> it's
> > > > explicit. I think either would be fine, as long as it's an extra step
> that
> > > > is required and is only applicable for Windows authentication.
> > > > I think controlling the amount of tries should be done by the number
> of
> > > > entries in the password database.
> > > > Since we detect account lockouts when sql authentication is used, we
> don't
> > > > need an additional hurdle here in my opinion.
> > > >
> > > > I've made the following changes to the ms-sql-brute script:
> > > > * If the script is started ONLY with the mssql.domain argument, it
> returns
> > > > the following message:
> > > > | ms-sql-brute:
> > > > |   Windows authentication was enabled but the argument
> > > > |   ms-sql-brute.brute-windows-accounts was not given. As there is
> > > > currently no
> > > > |   way of detecting accounts being locked out when Windows
> authentication
> > > > is
> > > > |   used, make sure that the amount entries in the password list
> > > > |_  (passdb argument) are at least 2 entries below the lockout
> threshold.
> > > > When the ms-sql-brute.brute-windows-accounts argument is given it runs
> as
> > > > expected.
> > > > Regarding changes to the ms-sql-brute script I would like to re-write
> it so
> > > > that it uses the brute library I wrote a while back. But let's merge
> this
> > > > first :)
> > > >
> > > > > > Let me know what you think about all of that. In the meantime, I'm
> > > > going to do some documentation.
> > > > > Great!
> > > > >
> > > > > I took a shot at this in the attached patch; let me know what you
> think.
> > > > There's also a small bugfix in there in mssql.lua, where I changed a
> "host"
> > > > to "instance.host", which fixed a bug for
> broadcast-ms-sql-discover.nse
> > > > I've applied it but it failed on the ms-sql-discover script, so I did
> that
> > > > one manually.
> > > > Also it removed the change to ms-sql-brute which makes it work
> together
> > > > with Windows authentication, so I added that again.
> > > >
> > > > I've done a bunch of testing too and unless there's something else, I
> think
> > > > we should try to merge this?
> > > >
> > > > //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/
> >
> > --
> > Patrik Karlsson
> > http://www.cqure.net
> > http://www.twitter.com/nevdull77
>
> --
> 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/
_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/