> -----Original Message-----
> From: Neelakanta Reddy [mailto:[email protected]]
> Sent: den 1 augusti 2014 17:57
> To: Hans Feldt; Anders Björnerstedt; [email protected];
> [email protected]
> Cc: [email protected]
> 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
