Re: [NSE] rpc library

[Patrik Karlsson <patrik () cqure net>]

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!

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.
I'm attaching this patch and unless David or anyone else has any additional thoughts I say we commit the changes.

//Patrik

Attachment: rpc-shorter-errors.lua.diff
Description:

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





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