Re: [NSE] NSE HTTP library

[Sven Klemm <sven () c3d2 de>]

Thomas Buchanan wrote:
> Great to see this library being developed.  It seems like it will be
> very useful.  I'm not quite sure about the way the library handles
> multiple headers.  It looks like if it encounters multiple instances of
> the same header, it combines their values into a comma separated list.
> But there's nothing that prevents a server from returning a header value
> that includes a comma, which could lead to ambiguous results.  Maybe
> instead of creating a comma separated list, you could create a table
> that holds each individual header value.  The caller would have to check
> the type of the return value, which has it's own drawbacks, but it does
> eliminate ambiguity.  Perhaps you have a better idea, or perhaps I'm not
> reading the code correctly.

No you read the code correctly. I got the idea from the RFC as
multiple headers with the same name must be combinable into one line:

from http://tools.ietf.org/html/rfc2616#section-4.2:
 Multiple message-header fields with the same field-name MAY be
 present in a message if and only if the entire field-value for that
 header field is defined as a comma-separated list [i.e., #(values)].
 It MUST be possible to combine the multiple header fields into one
 "field-name: field-value" pair, without changing the semantics of the
 message, by appending each subsequent field-value to the first, each
 separated by a comma. The order in which header fields with the same
 field-name are received is therefore significant to the
 interpretation of the combined field value, and thus a proxy MUST NOT
 change the order of these field values when a message is forwarded.

I would prefer always returning a string to avoid having to check for
the type as caller. What do you think about changing the separator to
newline? This should introduce no ambiguities.

> I noticed this in testing your patch to HTTPAuth.nse, it doesn't look
> like it correctly handles multiple authentication headers.  For example,
> one server I tested sent this back as part of the header:
>
> Server: Microsoft-IIS/5.1
> Date: Fri, 18 Jan 2008 22:05:34 GMT
> WWW-Authenticate: Negotiate
> WWW-Authenticate: NTLM
> WWW-Authenticate: Basic realm="Here be dragons!"
>
> With your patch to the script, the output read:
> 80/tcp   open   http
> |  HTTP Auth: HTTP Service requires authentication
> |_   Auth type: Negotiate, realm = Here be dragons!
>
> It should be (output from unpatched script):
> 80/tcp   open   http
> |  HTTP Auth: HTTP Service requires authentication
> |    Auth type: Negotiate
> |    Auth type: NTLM
> |_   Auth type: Basic, realm = Here be dragons!
>
> I was trying to come up with a way to properly determine if multiple
> auth headers were present, but as indicated, parsing a comma separated
> list could lead to ambiguous results.  For example, if the
> authentication type is Digest, additional information is provided by the
> server in a comma separated list, such as the domain, nonce values, etc.

You are right my patch did not handle multiple WWW-Authenticate lines.

As header fieldnames are case insensitive it's probably also a good
idea to lowercase them in the library to get consistent results.

> Anyway, not trying to discourage you from your development efforts, just
> wanted to give you a little feedback.  I'm planning to try and port my
> UPnP script over to your http library this weekend, and I'll try to let
> you know how that goes.

Thanks for your oppinion. I've attached a new version of the http
library that separates different lines of the same header by newline
and lowercases the header fieldnames and a new version of the HTTPAuth
patch which properly handles multiple WWW-Authenticate headers.

Cheers,
Sven

-- 
Sven Klemm
http://cthulhu.c3d2.de/~sven/

-- See nmaps COPYING for licence
module(...,package.seeall)

require 'stdnse'
require 'url'

--
-- http.get( host, port, path )
-- http.request( host, port, request )
-- http.get_url( url )
--
-- host may either be a string or table
-- port may either be a number or a table
--
-- the format of the return value is a table with the following structure:
-- {status = 200, header = {}, body ="<html>...</html>"}
-- the header table has an entry for each received header with the header name being the key
-- the table also has an entry named "status" which contains the http status code of the request
-- in case of an error status is nil


-- fetch relative URL with get request
get = function( host, port, path )
  local hostname = host
  if type(host) == 'table' then
    hostname = ( host.name ~= '' and host.name ) or host.ip
  end

  local data
  data = "GET "..path.." HTTP/1.1\r\n"
  data = data .. "Host: "..hostname.."\r\n"
  data = data .. "User-Agent: Nmap NSE\r\n"
  data = data .. "Connection: close\r\n\r\n"

  return request( host, port, data )
end

-- fetch URL with get request
get_url = function( u )
  local parsed = url.parse( u )
  local port = {}

  port.service = parsed.scheme
  port.number = parsed.port

  if not port.number then
    if parsed.scheme == 'https' then
      port.number = 443
    else
      port.number = 80
    end
  end

  local path = parsed.path or "/"
  if parsed.query then
    path = path .. "?" .. parsed.query
  end

  return get( parsed.host, port, path )
end

-- send http request and return the result as table
-- host may be a table or the hostname
-- port may be a table or the portnumber
request = function( host, port, data )

  if type(host) == 'table' then
    host = ( host.name ~= '' and host.name ) or host.ip
  end

  local protocol = 'tcp'
  if type(port) == 'table' then
    if nmap.have_ssl() and ( port.service == 'https' or ( port.version and port.version.service_tunnel == 'ssl' ) ) then
      protocol = 'ssl'
    end
    port = port.number
  end

  local socket = nmap.new_socket()
  socket:connect( host, port, protocol )
  socket:send( data )

  local buffer = stdnse.make_buffer( socket, "\r?\n" )

  local status, line, result, _
  local header, body = {}, {}
  -- header loop
  while true do
    status, line = buffer()
    if (not status or line == "") then break end
    table.insert(header,line)
  end

  result = {status=nil,header={},body=""}

  -- build nicer table for header
  for key, value in pairs( header ) do
    if key == 1 then
      local code
      _, _, code = string.find( value, "HTTP/%d\.%d (%d+)")
      result.status = tonumber(code)
    else
      _, _, key, value = string.find( value, "(.+): (.*)" )
      if key and value then
        key = key:lower()
        value = value:gsub( '[\r\n]+', '' )
        if result.header[key] then
          result.header[key] = result.header[key] .. '\n' .. value
        else
          result.header[key] = value
        end
      end
    end
  end

  -- body loop
  while true do
    status, line = buffer()
    if (not status) then break end
    table.insert(body,line)
  end

  socket:close()
  result.body = table.concat( body, "\n" )

  return result

end

Index: scripts/HTTPAuth.nse
===================================================================
--- scripts/HTTPAuth.nse        (revision 6740)
+++ scripts/HTTPAuth.nse        (working copy)
@@ -14,74 +14,26 @@
 categories = {"intrusive"}
 
 require "shortport"
+require "http"
 
-portrule = shortport.port_or_service({80, 8080}, "http")
+portrule = shortport.port_or_service({80, 443, 8080}, {"http","https"})
 
 action = function(host, port)
-       local socket
-       local catch = function()
-               socket:close()
-       end
 
-       local try = nmap.new_try(catch)
-
-       local get_http_headers = function(dst, dst_port, query_string)
-               socket = nmap.new_socket()
-
-               try(socket:connect(dst, dst_port))
-               try(socket:send(query_string))
-
-               local response = ""
-               local lines
-               local status
-
-               while true do
-                       status, lines = socket:receive_lines(1)
-
-                       if not status then
-                               break
-                       end
-
-                       response = response .. lines
-               end
-
-               try(socket:close())
-
-               local tags = {"(.-)<![Dd][Oo][Cc][Tt][Yy][Pp][Ee]", "(.-)<[Hh][Tt][Mm][Ll]", "(.-)<[Hh][Ee][Aa][Dd]", 
"(.-)<[Bb][Oo][Dd][Yy]"}
-               local hdrs
-
-               for I = 1, #tags do
-                       hdrs = string.match(response, tags[I])
-                       if hdrs ~= nil and hdrs ~= response and hdrs ~= "" then
-                               return hdrs
-                       end
-               end
-
-               return response
-       end
-
-       local auth
-       local value
        local realm
        local scheme
        local result
        local basic = false
+       local query
 
-       local query = "GET / HTTP/1.1\r\n"
-       query = query .. "Accept: */*\r\n"
-       query = query .. "Accept-Language: en\r\n"
-       query = query .. "User-Agent: Nmap NSE\r\n"
-       query = query .. "Connection: close\r\n"
-       query = query .. "Host: " .. host.ip .. ":" .. port.number .. "\r\n\r\n"
+       local answer = http.get( host, port, "/" )
 
-       local headers = get_http_headers(host.ip, port.number, query)
-
        --- check for 401 response code
-       auth = string.match(headers, "HTTP/1.- 401")
-       if auth ~= nil then
+       if answer.status == 401 then
                result = "HTTP Service requires authentication\n"
                -- loop through any WWW-Authenticate: headers to determine valid authentication schemes
-               for value in string.gmatch(headers, "[Aa]uthenticate:(.-)\n") do
+               local header = answer.header['www-authenticate']
+               for value in header:gmatch( "[^\n]+" ) do
                        result = result .. "  Auth type: "
                        scheme, realm = string.match(value, "(%a+).-[Rr]ealm=\"(.-)\"")
                        if scheme == "Basic" then
@@ -104,11 +56,8 @@
                query = query .. "Connection: close\r\n"
                query = query .. "Host: " .. host.ip .. ":" .. port.number .. "\r\n\r\n"
 
-               auth = ""
-               headers = get_http_headers(host.ip, port.number, query)
-
-               auth = string.match(headers, "HTTP/1.- 40[013]")
-               if auth == nil then
+               answer = http.request(host, port, query)
+               if answer.status ~= 401 and answer.status ~= 403 then
                        result = result .. "  HTTP server may accept user=\"admin\" with blank password for Basic 
authentication\n"
                end
 
@@ -120,11 +69,8 @@
                query = query .. "Connection: close\r\n"
                query = query .. "Host: " .. host.ip .. ":" .. port.number .. "\r\n\r\n"
 
-               auth = ""
-               headers = get_http_headers(host.ip, port.number, query)
-
-               auth = string.match(headers, "HTTP/1.- 40[013]")
-               if auth == nil then
+               answer = http.request(host, port, query)
+               if answer.status ~= 401 and answer.status ~= 403 then
                        result = result .. "  HTTP server may accept user=\"admin\" with password=\"admin\" for Basic 
authentication\n"
                end
        end

Attachment: signature.asc
Description: OpenPGP digital signature

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