rpc/auth: Avoid NULL dereference in rpcsvc_auth_request_init()

Code section is bogus!
------------------------------------------
370:       if (!auth->authops->request_init)
371:              ret = auth->authops->request_init (req, auth->authprivate);
------------------------------------------

Seems to have been never been used historically since
logically above code has never been true to actually execute
"authops->request_init() --> auth_glusterfs_{v2,}_request_init()"

On top of that under "rpcsvc_request_init()"
verf.flavour and verf.datalen are initialized from what is
provided through 'callmsg'.
------------------------------------------
        req->verf.flavour = rpc_call_verf_flavour (callmsg);
        req->verf.datalen = rpc_call_verf_len (callmsg);

        /* AUTH */
        rpcsvc_auth_request_init (req);
        return req;
------------------------------------------

So the code in 'auth_glusterfs_{v2,}_request_init()'
performing this operation will over-write the original
flavour and datalen.

------------------------------------------
      if (!req)
                return -1;
        memset (req->verf.authdata, 0, GF_MAX_AUTH_BYTES);
        req->verf.datalen = 0;
        req->verf.flavour = AUTH_NULL;
------------------------------------------

Refactoring the whole code into a more understandable version
and also avoiding a potential NULL dereference

Change-Id: I1a430fcb4d26de8de219bd0cb3c46c141649d47d
BUG: 1049735
Signed-off-by: Harshavardhana <harsha@harshavardhana.net>
Reviewed-on: http://review.gluster.org/6591
Reviewed-by: Santosh Pradhan <spradhan@redhat.com>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>

6 files changed, 24 insertions, 46 deletions

diff --git a/rpc/rpc-lib/src/auth-glusterfs.c b/rpc/rpc-lib/src/auth-glusterfs.c
index db488434c..48871ffb3 100644
--- a/rpc/rpc-lib/src/auth-glusterfs.c
+++ b/rpc/rpc-lib/src/auth-glusterfs.c
@@ -50,12 +50,6 @@ ret:
 int
 auth_glusterfs_request_init (rpcsvc_request_t *req, void *priv)
 {
-        if (!req)
-                return -1;
-        memset (req->verf.authdata, 0, GF_MAX_AUTH_BYTES);
-        req->verf.datalen = 0;
-        req->verf.flavour = AUTH_NULL;
-
         return 0;
 }
 
@@ -172,12 +166,6 @@ ret:
 int
 auth_glusterfs_v2_request_init (rpcsvc_request_t *req, void *priv)
 {
-        if (!req)
-                return -1;
-        memset (req->verf.authdata, 0, GF_MAX_AUTH_BYTES);
-        req->verf.datalen = 0;
-        req->verf.flavour = AUTH_NULL;
-
         return 0;
 }
 
diff --git a/rpc/rpc-lib/src/auth-null.c b/rpc/rpc-lib/src/auth-null.c
index ebdcc8ff8..b030341ab 100644
--- a/rpc/rpc-lib/src/auth-null.c
+++ b/rpc/rpc-lib/src/auth-null.c
@@ -22,15 +22,6 @@
 int
 auth_null_request_init (rpcsvc_request_t *req, void *priv)
 {
-        if (!req)
-                return -1;
-
-        memset (req->cred.authdata, 0, GF_MAX_AUTH_BYTES);
-        req->cred.datalen = 0;
-
-        memset (req->verf.authdata, 0, GF_MAX_AUTH_BYTES);
-        req->verf.datalen = 0;
-
         return 0;
 }
 
diff --git a/rpc/rpc-lib/src/auth-unix.c b/rpc/rpc-lib/src/auth-unix.c
index fa5f0576e..27351f669 100644
--- a/rpc/rpc-lib/src/auth-unix.c
+++ b/rpc/rpc-lib/src/auth-unix.c
@@ -24,12 +24,6 @@
 int
 auth_unix_request_init (rpcsvc_request_t *req, void *priv)
 {
-        if (!req)
-                return -1;
-        memset (req->verf.authdata, 0, GF_MAX_AUTH_BYTES);
-        req->verf.datalen = 0;
-        req->verf.flavour = AUTH_NULL;
-
         return 0;
 }
 
diff --git a/rpc/rpc-lib/src/rpcsvc-auth.c b/rpc/rpc-lib/src/rpcsvc-auth.c
index 0ede19f74..384e4a75d 100644
--- a/rpc/rpc-lib/src/rpcsvc-auth.c
+++ b/rpc/rpc-lib/src/rpcsvc-auth.c
@@ -369,25 +369,36 @@ ret:
 
 
 int
-rpcsvc_auth_request_init (rpcsvc_request_t *req)
+rpcsvc_auth_request_init (rpcsvc_request_t *req, struct rpc_msg *callmsg)
 {
-        int                     ret = -1;
+        int32_t                 ret = 0;
         rpcsvc_auth_t           *auth = NULL;
 
-        if (!req)
-                return -1;
+        if (!req || !callmsg) {
+                ret = -1;
+                goto err;
+        }
+
+        req->cred.flavour = rpc_call_cred_flavour (callmsg);
+        req->cred.datalen = rpc_call_cred_len (callmsg);
+        req->verf.flavour = rpc_call_verf_flavour (callmsg);
+        req->verf.datalen = rpc_call_verf_len (callmsg);
 
         auth = rpcsvc_auth_get_handler (req);
-        if (!auth)
+        if (!auth) {
+                ret = -1;
                 goto err;
-        ret = 0;
+        }
+
         gf_log (GF_RPCSVC, GF_LOG_TRACE, "Auth handler: %s", auth->authname);
-        if (!auth->authops->request_init)
-                ret = auth->authops->request_init (req, auth->authprivate);
 
-	req->auxgids = req->auxgidsmall; /* reset to auxgidlarge during
-					    unsersialize if necessary */
-	req->auxgidlarge = NULL;
+        if (auth->authops->request_init)
+              ret = auth->authops->request_init (req, auth->authprivate);
+
+        /* reset to auxgidlarge during
+           unsersialize if necessary */
+        req->auxgids = req->auxgidsmall;
+        req->auxgidlarge = NULL;
 err:
         return ret;
 }
diff --git a/rpc/rpc-lib/src/rpcsvc.c b/rpc/rpc-lib/src/rpcsvc.c
index d19a3ca0c..69db8b70b 100644
--- a/rpc/rpc-lib/src/rpcsvc.c
+++ b/rpc/rpc-lib/src/rpcsvc.c
@@ -367,13 +367,7 @@ rpcsvc_request_init (rpcsvc_t *svc, rpc_transport_t *trans,
          * been copied into the required sections of the req structure,
          * we just need to fill in the meta-data about it now.
          */
-        req->cred.flavour = rpc_call_cred_flavour (callmsg);
-        req->cred.datalen = rpc_call_cred_len (callmsg);
-        req->verf.flavour = rpc_call_verf_flavour (callmsg);
-        req->verf.datalen = rpc_call_verf_len (callmsg);
-
-        /* AUTH */
-        rpcsvc_auth_request_init (req);
+        rpcsvc_auth_request_init (req, callmsg);
         return req;
 }
 
diff --git a/rpc/rpc-lib/src/rpcsvc.h b/rpc/rpc-lib/src/rpcsvc.h
index 28ec93e11..30a969b11 100644
--- a/rpc/rpc-lib/src/rpcsvc.h
+++ b/rpc/rpc-lib/src/rpcsvc.h
@@ -553,7 +553,7 @@ struct rpcsvc_auth_list {
 };
 
 extern int
-rpcsvc_auth_request_init (rpcsvc_request_t *req);
+rpcsvc_auth_request_init (rpcsvc_request_t *req, struct rpc_msg *callmsg);
 
 extern int
 rpcsvc_auth_init (rpcsvc_t *svc, dict_t *options);