> -----Original Message-----
> From: Neelakanta Reddy [mailto:reddy.neelaka...@oracle.com]
> Sent: den 1 augusti 2014 17:57
> To: Hans Feldt; Anders Björnerstedt; ramesh.bet...@oracle.com; 
> mahesh.va...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 3 of 3] immnd: add support for client authorization [#938]
> 
> Hi Hans,
> 
> Reviewed and tested the patch.
> following are the comments:
> 
> 1. IMM readme must be updated with the #938 functionality
[Hans] sure
> 
> 2. when IMMA client connects and if IMMA client exits ( before
> MDS_INSTALL ) then
> 
> if (mds_process_info_get(mds_dest) == NULL) {
> +               MDS_PROCESS_INFO *info = malloc(sizeof(MDS_PROCESS_INFO));
> 
> 
> allocated memory is not freed.
[Hans] I don't see how this can be resolved, can we ignore it for now?

> 
> Regards,
> Neel.
> 
> On Thursday 03 July 2014 02:48 PM, Hans Feldt wrote:
> >   osaf/libs/agents/saf/imma/imma_init.c      |   5 +++++
> >   osaf/libs/common/immsv/include/immsv_evt.h |   3 +++
> >   osaf/services/saf/immsv/immnd/immnd_cb.h   |   2 +-
> >   osaf/services/saf/immsv/immnd/immnd_evt.c  |  29 
> > ++++++++++++++++++++++-------
> >   osaf/services/saf/immsv/immnd/immnd_main.c |   9 +++++++++
> >   osaf/services/saf/immsv/immnd/immnd_mds.c  |   3 +++
> >   6 files changed, 43 insertions(+), 8 deletions(-)
> >
> >
> > This patch adds coarse grained (on/off) IMM access control.
> >
> > immnd configure MDS to enable the "secutil" server side feature. imma 
> > configures
> > MDS to enable the "secutil" client side feature.
> >
> > This causes the server side to receive pid, gid & uid in all received 
> > messages
> > from local clients.
> >
> > Authorization
> >
> > When handling the initialize message from either an OM or OI client, 
> > membership
> > of a configured linux group is checked using a white list approach:
> >
> > 1) If the uid of the client is 0 (superuser) access is allowed.
> >
> > 2) If the gid of the client is the same as the immnd server process, access 
> > is
> > allowed (for example other opensaf processes).
> >
> > 3) Otherwise the list of members of a configured group is scanned looking 
> > for a
> > match with the client username. If the client is member of group, access is
> > allowed and logic proceeds as normal.
> >
> > If not member, initialization returns SA_AIS_ERR_ACCESS_DENIED to the 
> > client.
> >
> > diff --git a/osaf/libs/agents/saf/imma/imma_init.c 
> > b/osaf/libs/agents/saf/imma/imma_init.c
> > --- a/osaf/libs/agents/saf/imma/imma_init.c
> > +++ b/osaf/libs/agents/saf/imma/imma_init.c
> > @@ -24,6 +24,7 @@
> >   
> > ******************************************************************************/
> >
> >   #define _GNU_SOURCE
> > +#include <configmake.h>
> >   #include <string.h>
> >
> >   #include "imma.h"
> > @@ -266,6 +267,10 @@ unsigned int imma_startup(NCSMDS_SVC_ID
> >             goto done;
> >     }
> >
> > +   const char *name = PKGLOCALSTATEDIR "/immnd.sock";
> > +   setenv("MDS_SOCK_SERVER_NAME", name, 1);
> > +   putenv("MDS_SOCK_SERVER_CONNECT=YES");
> > +
> >     if ((rc = ncs_agents_startup()) != NCSCC_RC_SUCCESS) {
> >             TRACE_3("Agents_startup failed");
> >             goto done;
> > diff --git a/osaf/libs/common/immsv/include/immsv_evt.h 
> > b/osaf/libs/common/immsv/include/immsv_evt.h
> > --- a/osaf/libs/common/immsv/include/immsv_evt.h
> > +++ b/osaf/libs/common/immsv/include/immsv_evt.h
> > @@ -280,6 +280,9 @@ typedef struct immsv_send_info {
> >     MDS_SENDTYPES stype;    /* Send type */
> >     MDS_SYNC_SND_CTXT ctxt; /* MDS Opaque context */
> >     uint8_t mSynReqCount;
> > +   pid_t pid;
> > +   uid_t uid;
> > +   gid_t gid;
> >   } IMMSV_SEND_INFO;
> >
> >   typedef struct immsv_fevs {
> > diff --git a/osaf/services/saf/immsv/immnd/immnd_cb.h 
> > b/osaf/services/saf/immsv/immnd/immnd_cb.h
> > --- a/osaf/services/saf/immsv/immnd/immnd_cb.h
> > +++ b/osaf/services/saf/immsv/immnd/immnd_cb.h
> > @@ -48,7 +48,6 @@ typedef struct immnd_immom_client_node {
> >     SaImmHandleT imm_app_hdl;       /* index for the client tree */
> >     MDS_DEST agent_mds_dest;        /* mds dest of the agent */
> >     SaVersionT version;
> > -   SaUint32T client_pid;   /*Used to recognize loader */
> >     IMMSV_SEND_INFO tmpSinfo;       /*needed for replying to
> >                                        syncronousrequests */
> >
> > @@ -171,6 +170,7 @@ typedef struct immnd_cb_tag {
> >     NCS_SEL_OBJ usr1_sel_obj;       /* Selection object for USR1 signal 
> > events */
> >     SaSelectionObjectT amf_sel_obj; /* Selection Object for AMF events */
> >     int nid_started;        /* true if started by NID */
> > +   const char *admin_group_name;  // linux group name for admins
> >   } IMMND_CB;
> >
> >   /* CB prototypes */
> > diff --git a/osaf/services/saf/immsv/immnd/immnd_evt.c 
> > b/osaf/services/saf/immsv/immnd/immnd_evt.c
> > --- a/osaf/services/saf/immsv/immnd/immnd_evt.c
> > +++ b/osaf/services/saf/immsv/immnd/immnd_evt.c
> > @@ -25,6 +25,7 @@
> >   
> > ******************************************************************************/
> >
> >   #define _GNU_SOURCE
> > +#include <osaf_secutil.h>
> >   #include "immnd.h"
> >   #include "immsv_api.h"
> >   #include "ncssysf_mem.h"
> > @@ -715,15 +716,15 @@ static uint32_t immnd_evt_proc_imm_init(
> >     int pbe_pid =  (!load_pid && (cb->pbePid > 0))?(cb->pbePid):0;
> >
> >     if (load_pid > 0) {
> > -           if (evt->info.initReq.client_pid == load_pid) {
> > +           if (sinfo->pid == load_pid) {
> >                     TRACE_2("Loader attached, pid: %u", load_pid);
> >             } else {
> >                     TRACE_2("Rejecting OM client attach during loading, pid 
> > %u != %u",
> > -                           evt->info.initReq.client_pid, load_pid);
> > +                           sinfo->pid , load_pid);
> >                     error = SA_AIS_ERR_TRY_AGAIN;
> >                     goto agent_rsp;
> >             }
> > -   } else if (evt->info.initReq.client_pid == cb->preLoadPid) {
> > +   } else if (sinfo->pid == cb->preLoadPid) {
> >             LOG_IN("2PBE Pre-loader attached");
> >     } else if (load_pid < 0) {
> >             TRACE_2("Rejecting OM client attach. Waiting for loading or 
> > sync to complete");
> > @@ -731,6 +732,22 @@ static uint32_t immnd_evt_proc_imm_init(
> >             goto agent_rsp;
> >     }
> >
> > +   /* allow access using white list approach */
> > +   if (sinfo->uid == 0) {
> > +           TRACE("superuser");
> > +   } else if (getgid() == sinfo->gid) {
> > +           TRACE("same group");
> > +   } else if ((immnd_cb->admin_group_name != NULL) &&
> > +            (osaf_user_is_member_of_group(sinfo->uid, 
> > immnd_cb->admin_group_name) == true)) {
> > +           TRACE("configured group");
> > +   } else {
> > +           syslog(LOG_AUTH, "access denied, uid:%d, pid:%d", sinfo->uid, 
> > sinfo->pid);
> > +           TRACE_2("access denied, uid:%d, pid:%d, group_name:%s", 
> > sinfo->uid, sinfo->pid,
> > +                   immnd_cb->admin_group_name);
> > +           error = SA_AIS_ERR_ACCESS_DENIED;
> > +           goto agent_rsp;
> > +   }
> > +
> >     cl_node = calloc(1, sizeof(IMMND_IMM_CLIENT_NODE));
> >     if (cl_node == NULL) {
> >             LOG_ER("IMMND - Client Alloc Failed");
> > @@ -756,7 +773,6 @@ static uint32_t immnd_evt_proc_imm_init(
> >
> >     cl_node->agent_mds_dest = sinfo->dest;
> >     cl_node->version = evt->info.initReq.version;
> > -   cl_node->client_pid = evt->info.initReq.client_pid;
> >     cl_node->sv_id = (isOm) ? NCSMDS_SVC_ID_IMMA_OM : NCSMDS_SVC_ID_IMMA_OI;
> >
> >     if (immnd_client_node_add(cb, cl_node) != NCSCC_RC_SUCCESS) {
> > @@ -769,10 +785,10 @@ static uint32_t immnd_evt_proc_imm_init(
> >     TRACE_2("Added client with id: %llx <node:%x, count:%u>",
> >             cl_node->imm_app_hdl, cb->node_id, (SaUint32T)clientId);
> >
> > -   if (sync_pid && (cl_node->client_pid == sync_pid)) {
> > +   if (sync_pid && (sinfo->pid == sync_pid)) {
> >             TRACE_2("Sync agent attached, pid: %u", sync_pid);
> >             cl_node->mIsSync = 1;
> > -   } else  if (pbe_pid && (cl_node->client_pid == pbe_pid) && !isOm && 
> > !(cl_node->mIsPbe)) {
> > +   } else  if (pbe_pid && (sinfo->pid == pbe_pid) && !isOm && 
> > !(cl_node->mIsPbe)) {
> >             LOG_NO("Persistent Back End OI attached, pid: %u", pbe_pid);
> >             cl_node->mIsPbe = 1;
> >     }
> > @@ -2042,7 +2058,6 @@ static uint32_t immnd_evt_proc_imm_resur
> >       cl_node->imm_app_hdl = m_IMMSV_PACK_HANDLE(clientId, cb->node_id);
> >       cl_node->agent_mds_dest=sinfo->dest;
> >       /*cl_node->version= .. TODO correct version (not used today)*/
> > -    cl_node->client_pid = 0; /* TODO correct PID (not important here) */
> >       cl_node->sv_id = (isOm)?NCSMDS_SVC_ID_IMMA_OM:NCSMDS_SVC_ID_IMMA_OI;
> >
> >       if (immnd_client_node_add(cb,cl_node) != NCSCC_RC_SUCCESS)
> > diff --git a/osaf/services/saf/immsv/immnd/immnd_main.c 
> > b/osaf/services/saf/immsv/immnd/immnd_main.c
> > --- a/osaf/services/saf/immsv/immnd/immnd_main.c
> > +++ b/osaf/services/saf/immsv/immnd/immnd_main.c
> > @@ -115,11 +115,20 @@ static uint32_t immnd_initialize(char *p
> >     if (getenv("SA_AMF_COMPONENT_NAME") == NULL)
> >             immnd_cb->nid_started = 1;
> >
> > +   const char *name = PKGLOCALSTATEDIR "/immnd.sock";
> > +   setenv("MDS_SOCK_SERVER_NAME", name, 1);
> > +   putenv("MDS_SOCK_SERVER_CREATE=YES");
> > +
> >     if (ncs_agents_startup() != NCSCC_RC_SUCCESS) {
> >             LOG_ER("ncs_agents_startup FAILED");
> >             goto done;
> >     }
> >
> > +   /* unset so that forked processes (e.g. loader) does not create MDS 
> > server */
> > +   unsetenv("MDS_SOCK_SERVER_CREATE");
> > +
> > +   immnd_cb->admin_group_name = getenv("IMM_ADMIN_GROUP_NAME");
> > +
> >     /* Initialize immnd control block */
> >     immnd_cb->ha_state = SA_AMF_HA_ACTIVE;
> >     immnd_cb->cli_id_gen = 1;
> > diff --git a/osaf/services/saf/immsv/immnd/immnd_mds.c 
> > b/osaf/services/saf/immsv/immnd/immnd_mds.c
> > --- a/osaf/services/saf/immsv/immnd/immnd_mds.c
> > +++ b/osaf/services/saf/immsv/immnd/immnd_mds.c
> > @@ -507,6 +507,9 @@ static uint32_t immnd_mds_rcv(IMMND_CB *
> >     pEvt->sinfo.ctxt = rcv_info->i_msg_ctxt;
> >     pEvt->sinfo.dest = rcv_info->i_fr_dest;
> >     pEvt->sinfo.to_svc = rcv_info->i_fr_svc_id;
> > +   pEvt->sinfo.pid = rcv_info->pid;
> > +   pEvt->sinfo.uid = rcv_info->uid;
> > +   pEvt->sinfo.gid = rcv_info->gid;
> >     if (rcv_info->i_rsp_reqd) {
> >             pEvt->sinfo.stype = MDS_SENDTYPE_SNDRSP;
> >     }


------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to