Re: [NSE] rpc library

[Djalal Harouni <tixxdz () gmail com>]

On 2010-04-18 10:54:05 +0200, Patrik Karlsson wrote:
> Hi Djalal,
>
> I've looked through the code and I'm happy with the changes. Great work!
> Good catch with the version mismatch checks in the Dir and GetAttributes functions!
yeh but there is a small bug that was introduced when I was redoing the
changes in the last patch, so if you can Patrik change this:

-- all the version mismatch checks:

return status, string.format("versions mismatch, nfs v%d - mount
v%d",  nfs_comm.version, mnt_comm.version)

will be:

return false, string.format("versions mismatch, nfs v%d - mount
v%d",  nfs_comm.version, mnt_comm.version)


replace status with false.

I hope that we didn't forget anything.

> I made some changes to shorten the error messages returned by the Helper class back to the scripts:
>
> | nfs-acls:  
> |   /tmp
> |     ERROR: rpc.Helper.GetAttributes: Mount query failed: Permission denied.
> |   /home/storage/backup
> |     uid: 0; gid: 0; mode: drwxr-xr-x (755)
> |   /home
> |_    uid: 0; gid: 0; mode: drwxr-xr-x (755)
>
> becomes
>
> | nfs-acls:  
> |   /tmp
> |     ERROR: Mount failed: Permission denied.
> |   /home/storage/backup
> |     uid: 0; gid: 0; mode: drwxr-xr-x (755)
> |   /home
> |_    uid: 0; gid: 0; mode: drwxr-xr-x (755)
>
> The long error messages that helps us track down where the error occurred is printed as a debug message instead.
Yes this is better.

> I'm attaching this patch and unless David or anyone else has any additional thoughts I say we commit the changes.

> //Patrik
thx Patrik and David, pls don't forget the previous NSE scripts diffs to
get advantage of the new error handling.

> On 17 apr 2010, at 17.21, Djalal Harouni wrote:
> > An other simplified description of the patches (I hope):
> >
> > 1) RPC library:
> > - Comm class re-design:
> >
> >  - Added Connect(), Disconnect(), ChkProgram(), ChkVersion() and SetVersion() methodes.
> >  This methodes let us to handle network connections and to store and check RPC based programs infos.
> >
> >  - All RPC programs (Portmap/rpcbind, NFS, Mount etc) informations are stored in the Comm object:
> >    program name string, program id, used version, socket, remote IP PORT and protocol.
> >
> >  - Portmap, NFS and Mount functions must use the Comm object as a parameter in order to perform network operations, 
> > all the Connect() and Disconnect() stuff for each Class was removed, duplicate code. 
> >
> >
> >  Simple picture :)
> >  - Helper functions, ex: rpc.Helper.RpcInfo()
> >     |       Creates the Comm object and makes the connection.
> >     |       The Comm object have all the RPC infos (socket, program name etc)
> >     |
> >        |    ex: rpc.Helper.RpcInfo()
> >     |         comm = Comm:new('rpcbind', 2) -- we have some infos
> >     |         comm:Connect(host, port) -- we have the additional network infos
> >     |
> >     |         portmap:Dump(comm) -- call the *Internal* Portmap.Dump function
> >     |                            -- pass comm object as a parameter
> >     |                            -- *this* is the change
> >     |
> >     |         comm:Disconnect() -- disconnect and return result after that
> >     |
> >  - Portmap/NFS Internal functions, ex: Portmap:Dump()
> >     |       The Comm object is passed as a prameter by the high level functions.
> >     |       Perform encode/decode and network operations
> >     |
> >     |       ex: portmap:Dump()
> >     |         comm:EncodePacket()
> >     |         comm:SendPacket()
> >     |         comm:ReceivePacket()
> >     |         ...
> >     |         return final data to the rpc.Helper.RpcInfo()
> >
> > Conclusion about the Comm re-design:
> >  - The Comm object is created by the high level *Helpers* functions and passed to the *Internal* ones.
> >  - New *Internal* RPC Protocols or Procedures functions don't need to *worry* about network operations or RPC 
> > infos, they also don't need to have local copies of the Comm object, it is used as a parameter and this is more 
> > *flexible* for new code.
> >  NB: the Comm object did already handle the encode/decode operations.
> >
> >
> > Other changes to the RPC lib
> > - better error handling:
> >  - table of error messages and stat codes for NFS v1,2,3
> >  - table of error messages and stat codes for Mount
> >  - return the complete error description:  "rpc.Helper.function: internal_function: error message"
> >  - debug output for RPC error procedures and decoding operations.
> >
> > - fixed some left open sockets
> > - added nfs and mount version mismatch check
> > ...
> >
> >
> > 2) NSE Scripts:
> > - better error handling.
> >
> >
> > I must remind you that some scripts/libs (including these ones) trust remote inputs!!! 
> > I have found this thread: http://seclists.org/nmap-dev/2009/q3/210 which is a good start.
> >
> > I hope it's clear :)
> >
> > > David Fifield
> >
> > -- 
> > Djalal
> > http://dzcore.wordpress.com
> > <rpc.lua.diff><rpcinfo.nse.diff><nfs-showmounts.nse.diff><nfs-dirlist.nse.diff><nfs-acls.nse.diff><nfs-statfs.nse.diff>_______________________________________________
> > 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
_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/