Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1016 Script to extract iLO server info from /xmldata?item=all #1082

Closed
wants to merge 4 commits into from

Conversation

Projects
None yet
6 participants
@rajeevrmenon97
Copy link

rajeevrmenon97 commented Dec 6, 2017

I'm new to nmap. I'm trying to start contributing by attempting the fix of an issue labeled easy. This is my first NSE script. I am trying to resolve issue #1016 . This script extracts hardware info on HP iLO board from /xmldata?item=all.

@dmiller-nmap
Copy link

dmiller-nmap left a comment

Thanks for this cool script! Let's get it cleaned up and ready to merge.

local slaxml = require "slaxml"
local stdnse = require "stdnse"

portrule = function(host,port)

This comment has been minimized.

Copy link
@dmiller-nmap

dmiller-nmap Jan 22, 2018

You should be able to use the shortport.http portrule for this script.

info['cUUID '] = getTag(dom,"cUUID")
info['ILOType '] = getTag(dom,"PN")
info['ILOFirmware'] = getTag(dom,"FWRI")
info['Serial No '] = getTag(dom,"SN")

This comment has been minimized.

Copy link
@dmiller-nmap

dmiller-nmap Jan 22, 2018

Even though justified text fields look nice, they throw off machine parsing in XML. Please do not pad output with whitespace.


action = function(host,port)
local response = http.get(host,port,"/xmldata?item=all")
if response["status"] == "404"

This comment has been minimized.

Copy link
@dmiller-nmap

dmiller-nmap Jan 22, 2018

Shouldn't this be if response.status ~= "200"? Non-iLO servers may return other status codes like 401, 403, etc.

return
end
local domtable = slaxml.parseDOM(response["body"],{stripWhitespace=true})
return stdnse.format_output(true, parseXML(domtable))

This comment has been minimized.

Copy link
@dmiller-nmap

dmiller-nmap Jan 22, 2018

The stdnse.format_output function is deprecated. Please use NSE structured output instead.

--| Mac Address : 11:22:33:44:55:66
--| Description : iLO 4
--| IP Address : Unknown
--|_ Status : Disabled

This comment has been minimized.

Copy link
@dmiller-nmap

dmiller-nmap Jan 22, 2018

An example of a Lua table that represents this info for structured output (see other notes) would be:

{
  ["ILO Type"] = "Integrated Lights-Out 4 (iLO 4)",
  ["Serial"] = "ILOXXXXXXXXXXX",
  ["NICs"] = {
    ["NIC 1"] = {
      ["MAC Address"] = "12:34:56:78:9a:bc",
      ["Status"] = "OK"
    },
    ["NIC 2"] = {...},
  },
}

Ensure that the elements are displayed in the same order every time by using output_table structures.

count = count + 1
for k,m in ipairs(n.kids) do
if m.name == "DESCRIPTION" then
info["Description"] = m.kids[1].value

This comment has been minimized.

Copy link
@dmiller-nmap

dmiller-nmap Jan 22, 2018

If a field (usually IPADDR) is empty, this will crash, since m.kids[1] is nil. Check that #m.kids >= 1 and that m.kids[1].type == "text" before checking m.name.

@rajeevrmenon97 rajeevrmenon97 force-pushed the rajeevrmenon97:master branch from 4f556cd to 768a0ae Jan 23, 2018

@rajeevrmenon97

This comment has been minimized.

Copy link
Author

rajeevrmenon97 commented Jan 23, 2018

@dmiller-nmap I've made the requested changes. Thanks for reviewing my first script.

@tpltnt

This comment has been minimized.

Copy link

tpltnt commented Jul 16, 2018

Hi there. I wrote a script to analyse the XML output of this script. You can find it here.

@rajeevrmenon97

This comment has been minimized.

Copy link
Author

rajeevrmenon97 commented Aug 3, 2018

@dmiller-nmap can you merge the changes. This is my first contribution to nmap :D

@tpltnt

This comment has been minimized.

Copy link

tpltnt commented Sep 20, 2018

@dmiller-nmap your requests seem to have been addressed. Is this now ready for merging ?

@jeffmcjunkin

This comment has been minimized.

Copy link

jeffmcjunkin commented Jan 2, 2019

Ping on @tpltnt 's behalf. This looks like a very useful NSE script, I'd love to see it make mainline.

@rajeevrmenon97

This comment has been minimized.

Copy link
Author

rajeevrmenon97 commented Jan 4, 2019

@dmiller-nmap pls consider merging

@tpltnt

This comment has been minimized.

Copy link

tpltnt commented Apr 7, 2019

ping on behalf of @jeffmcjunkin and @rajeevrmenon97

@rajeevrmenon97

This comment has been minimized.

Copy link
Author

rajeevrmenon97 commented Apr 7, 2019

I was really excited to start contributing to nmap. This was my first PR. I'm really disappointed at this not getting accepted and that's why I stopped working on this project.

@tpltnt

This comment has been minimized.

Copy link

tpltnt commented Apr 7, 2019

I can see your point @rajeevrmenon97

@cldrn

This comment has been minimized.

Copy link
Member

cldrn commented Apr 8, 2019

Hello,

After testing and applying Nmap's coding standards (https://secwiki.org/w/Nmap/Code_Standards) I committed your script in r37613. We always appreciate the help and even the needed nudges from time to time.

As you probably know, lately the development efforts have been focused on NPcap but this does not mean NSE is forgotten. Keep contributing and I will try to help as much as time allows me!

@rajeevrmenon97

This comment has been minimized.

Copy link
Author

rajeevrmenon97 commented Apr 8, 2019

Thanks for committing my script. I made this NSE almost a year ago. I'll start looking into NPcap.

@rajeevrmenon97

This comment has been minimized.

Copy link
Author

rajeevrmenon97 commented Apr 8, 2019

What do you mean by committed to r37613? When will it be merged?

@fyodor

This comment has been minimized.

Copy link

fyodor commented Apr 11, 2019

Thanks @cldrn . And yes, Npcap has been taking almost all of our time lately, but we're super close to the big 1.0 release and then we're going to turn our attention back to Nmap proper. Of course all of our Npcap work is done to benefit Nmap too. It's just low-level infrastructure rather than shiny new user-visible features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.