Nmap Development mailing list archives
Re: [NSE] rpc library
From: Djalal Harouni <tixxdz () gmail com>
Date: Mon, 12 Apr 2010 01:37:31 +0100
On 2010-04-10 23:36:06 +0100, Djalal Harouni wrote:
On 2010-04-08 22:10:38 +0200, Patrik Karlsson wrote:Hi Djalal, Once again, thanks for the patch, I appreciate it! I think it brings some great improvement to the error handling and I like the changes you've done in regard of how the Comm class is used. I also think there are some good points in your TODO (specifically the NFSv4 support and authentication). If you want to help out looking in to these let me know.yes I am interested in NFSv4, I will give it a try in the next days.Here are a few comments in regard to the patch: * Make sure that all function names and parameters follow the same naming convention as used in the library. I know this is somewhat messy, looking at the NSE libraries and scripts as a whole. I have certainly introduced my share of the mess too changing conventions between scripts. That said, I think you should keep to the same naming convention as already used by a script or library to keep it consistent.ok.* The parameters "program" and "version" in the Comm:new method are never used. When creating a Comm instance like this Comm:new({program='mountd', version=mountd.version}) the program and version parameters are passed as a table in parameter o. While the function could take additional parameters, the current implementation does nothing with them.I will use a more basic model.* I'm not particularly happy with the set_rpcinfo function as it does not really match up with its description and also because it's never called outside of the constructor/new method. I guess the reason for this code not being inside of the constructor is that it could then be returning nil on failure which isn't great. I therefore suggest you to move this code out to the connect method instead and catch any version mismatch there and return an error.Yeh, the idea was: since the lib uses the OO paradigm, perhaps creating simple facilities functions to change the propreities of an rpc Comm object: SetProgram (changes the program name and the id), SetVersion etc. I will create a new function: ChkVersion() used in the Connect() and in the SetVersion() (I didn't forget that this is Lua :) ).* The RPC_Registry table is somewhat superfluous and in my opinion the code becomes more difficult to read this way.the RPC_registry table is useful when adding new rpc services and to avoid hard coding in functions which is not flexible. perhaps a new name for the table ? And what about this: - patches for nfs-* and rpcinfo scripts, to use "return stdnse.format_output(false, data)" on errors - separate code: NFS code in nfs.lua ... etc, all these libraries will require("rpc") to handle rpc communications, Portmap class and RpcInfo functions must stay in rpc.lua NB: I will send you the new version ASAP, so we can move to other features, thx.
hi, attached are patches for rpc.lua lib, rpcinfo.nse and nfs-*.nse scripts 1) rpc.lua lib - better Comm:new() error handling. - two new methodes: ChkVersion(): to check if the lib support the rpc protocol version, and SetVersion() to set rpc versions. - fixed some left open sockets. 2) nse scripts which use the rpc.lua lib: - better error output for rpcinfo.nse and nfs-*.nse scripts ex:./nmap -p111 -PN -n -d1 --script scripts/nfs-dirlist.nse --script-args="nfs.version=4" 192.168.100.101 PORT STATE SERVICE REASON 111/tcp open rpcbind syn-ack | nfs-dirlist: | /mnt/nfs/files | ERROR: rpc.Helper.Dir: RPC library does not support: nfs version 4 Reminder: when adding NFSv4 support we must update the nfs and mountd versions mismatch check. I think that the code is clean now, thx.
//Patrik On 6 apr 2010, at 01.52, Djalal Harouni wrote:attached are two files: rpc.lua.diff and rpc.lua.new (new lib) new rpc.lua (code cleaned): - Comm Class/object will store/handle all rpc program infos and low level network operations. - Comm object is passed to NFS, Mount, Portmap functions as a parameter (they don't need to have a local copy). - added some debug output, nfs and mount versions mismatch check. - added NFS and Mount stat codes support with msg descriptions, perhaps useful in the futur: nfs vulnerabilitu check (check nfs code, check permissions issues, ressource limits ... etc)!!, i don't know if there is a tool that let as check the common POSIX ACLs issues over NFS (NFSv4 is a good condidate) or mount options: filesystems with block special devices enabled ... etc, perhaps patrik's nfs-acls.nse script is a good start. notes: - new protocols who may use the rpc.lua, must be added to the RPC_version table. - all the nse scripts wich uses the rpc.Helper class functions, must use this code to handle errors: --- return stdnse.format_output(false, data) --- so we need probably simple patches for nfs-* and rpcinfo scripts. todo: - separate code: NFS code in nfs.lua ... etc, all these libraries will require("rpc") to handle rpc communications, Portmap class and RpcInfo functions must stay in rpc.lua - complete the NFS procedures. - add others filetypes support. - add NFSv4 support. - add more rpc stuff: error code, authentification ... etc - ... NB: patrik has already implemented this sutff, this is simply a re-design. thx patrik. feedbacks ? On 2010-04-02 20:03:40 +0200, Patrik Karlsson wrote:I've commited this as r17146. I ended up patching only the ReadDir function as the StatFs function is only available for version less than 3 where the file handle length is fixed at 32 octets. Thanks for the bug report Djalal! //Patrik On 2 apr 2010, at 17.49, Djalal Harouni wrote:BTW patrik if you can commit these changes (nfsv3 fhandles correction) it will be cool so i can work on a clean svn copy. On 2010-04-02 16:41:47 +0100, Djalal Harouni wrote:On 2010-04-02 17:32:08 +0200, Patrik Karlsson wrote:Hi Djalal, On 2 apr 2010, at 17.23, Djalal Harouni wrote:On 2010-04-02 17:10:00 +0200, Patrik Karlsson wrote:I'm having some trouble applying this patch against the rpc.lua in svn. A portion of the patch gets rejected and my diff/patch skills are limited. Can anyone share some diff/patch magic or can you send me a patch against svn Djalal?hi patrick i just send you the patch nfsv3_rpc.lua.diff against r17144 and it's attached to, for the second patch i will probably add some more error handling and reporting as suggested by david: http://seclists.org/nmap-dev/2010/q1/922 thxThis patch looks very small compared to the first you sent me which contained both the NFS3 file handle fixes, comm class re-design and improved error handling. I had a quick look at the comm re-design in the previous patch you sent, and it looked quite good. Will it be part of the second patch you mentioned?yes, i must rewrite the error handling code.Oh, and the self.version > 2 patch is incorrect as NFSv3 does not have the STATFS procedure, the procedure 17 has been replaced with READDIRPLUS in v3.ah sorry.-- Djalal http://dzcore.wordpress.com <nfsv3_rpc.lua.diff>_______________________________________________ Sent through the nmap-dev mailing list http://cgi.insecure.org/mailman/listinfo/nmap-dev Archived at http://seclists.org/nmap-dev///Patrik -- Patrik Karlsson http://www.cqure.net http://www.twitter.com/nevdull77-- Djalal http://dzcore.wordpress.com-- Djalal http://dzcore.wordpress.com _______________________________________________ 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-- Djalal http://dzcore.wordpress.com <rpc.lua.diff><rpc.lua.new>_______________________________________________ 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-- Djalal http://dzcore.wordpress.com
-- Djalal http://dzcore.wordpress.com
Attachment:
rpc.lua.diff
Description:
Attachment:
rpc.lua
Description:
Attachment:
rpcinfo.nse.diff
Description:
Attachment:
nfs-dirlist.nse.diff
Description:
Attachment:
nfs-showmounts.nse.diff
Description:
Attachment:
nfs-statfs.nse.diff
Description:
Attachment:
nfs-acls.nse.diff
Description:
_______________________________________________ Sent through the nmap-dev mailing list http://cgi.insecure.org/mailman/listinfo/nmap-dev Archived at http://seclists.org/nmap-dev/
Current thread:
- Re: [PATCH] nselib rpc.lua Patrik Karlsson (Apr 01)
- <Possible follow-ups>
- Re: [PATCH] nselib rpc.lua Patrik Karlsson (Apr 02)
- Re: [PATCH] nselib rpc.lua Djalal Harouni (Apr 02)
- Re: [PATCH] nselib rpc.lua Patrik Karlsson (Apr 02)
- Re: [PATCH] nselib rpc.lua Djalal Harouni (Apr 02)
- Re: [PATCH] nselib rpc.lua Djalal Harouni (Apr 02)
- Re: [PATCH] nselib rpc.lua Patrik Karlsson (Apr 02)
- Re: [PATCH] nselib rpc.lua Djalal Harouni (Apr 05)
- Re: [PATCH] nselib rpc.lua Patrik Karlsson (Apr 08)
- Re: [NSE] rpc library Djalal Harouni (Apr 10)
- Re: [NSE] rpc library Djalal Harouni (Apr 11)
- Re: [NSE] rpc library David Fifield (Apr 15)
- Re: [NSE] rpc library Djalal Harouni (Apr 16)
- Re: [NSE] rpc library Djalal Harouni (Apr 17)
- Re: [NSE] rpc library Patrik Karlsson (Apr 18)
- Re: [NSE] rpc library Djalal Harouni (Apr 18)
- Re: [NSE] rpc library David Fifield (Apr 21)
- Re: [NSE] rpc library; trusted inputs? David Fifield (Apr 21)
- Re: [NSE] rpc library; trusted inputs? Djalal Harouni (Apr 26)
- Re: [NSE] rpc library; trusted inputs? David Fifield (Apr 26)
- Re: [PATCH] nselib rpc.lua Djalal Harouni (Apr 02)
- Re: [NSE] rpc library; errors during nfsd startup David Fifield (Apr 21)