Ack with comment inlined below.
Only tested for non-interference.
Not tested for effectiveness.

Hans Feldt wrote:
>  osaf/services/saf/immsv/immnd/immnd_evt.c |  67 
> ++++++++++++++++++++++--------
>  1 files changed, 48 insertions(+), 19 deletions(-)
>
>
> SMF running as non root cannot change saImmRepositoryInit which is a normal
> upgrade use case. Same for changing longDnsAllowed but which is one shot
> change.
>
> Reason is IMM now only allows root (uid=0) to modify service objects.
>
> This is changed so that members of the same group as immnd, for example smfd
> is allowed to change the mentioned attributes. Change of access control 
> related
> attributes is still restricted to root.
>
> 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
> @@ -38,7 +38,7 @@
>  #define IMMND_SEARCH_BUNDLE_SIZE ((MDS_DIRECT_BUF_MAXSIZE / 100) * 90)   
>  #define IMMND_MAX_SEARCH_RESULT (IMMND_SEARCH_BUNDLE_SIZE / 300)  
>  
> -static SaAisErrorT immnd_fevs_local_checks(IMMND_CB *cb, IMMSV_FEVS 
> *fevsReq, uid_t uid);
> +static SaAisErrorT immnd_fevs_local_checks(IMMND_CB *cb, IMMSV_FEVS 
> *fevsReq, const IMMSV_SEND_INFO *sinfo);
>  static uint32_t immnd_evt_proc_cb_dump(IMMND_CB *cb);
>  static uint32_t immnd_evt_proc_imm_init(IMMND_CB *cb, IMMND_EVT *evt, 
> IMMSV_SEND_INFO *sinfo, SaBoolT isOm);
>  static uint32_t immnd_evt_proc_imm_finalize(IMMND_CB *cb, IMMND_EVT *evt, 
> IMMSV_SEND_INFO *sinfo, SaBoolT isOm);
> @@ -2835,7 +2835,7 @@ static uint32_t immnd_evt_proc_fevs_forw
>       }
>  
>       if(newMsg) {
> -             error = immnd_fevs_local_checks(cb, &(evt->info.fevsReq), 
> (sinfo)?(sinfo->uid):0);
> +             error = immnd_fevs_local_checks(cb, &(evt->info.fevsReq), 
> sinfo);
>               if(error != SA_AIS_OK) {
>                       /*Fevs request will NOT be forwarded to IMMD.
>                         Return directly with error or OK for idempotent 
> requests.
> @@ -3048,7 +3048,8 @@ static uint32_t immnd_evt_proc_fevs_forw
>    nodes and not propagated over fevs, because sync clients may not yet
>    have synced the implementer setting and thus reject the idempotent case. 
>  */
> -static SaAisErrorT immnd_fevs_local_checks(IMMND_CB *cb, IMMSV_FEVS 
> *fevsReq, uid_t uid)
> +static SaAisErrorT immnd_fevs_local_checks(IMMND_CB *cb, IMMSV_FEVS *fevsReq,
> +             const IMMSV_SEND_INFO *sinfo)
>  {
>       SaAisErrorT error = SA_AIS_OK;
>       osafassert(fevsReq);
> @@ -3101,27 +3102,55 @@ static SaAisErrorT immnd_fevs_local_chec
>               goto unpack_failure;
>       }
>  
> +     /* Some checks below use sinfo. On the node where the request 
> originated,
> +      * the sinfo pointer is valid. On other nodes that receives the same 
> through
> +      * FEVS, sinfo is NULL. So the value of sinfo can be used if checks are 
> to
> +      * be done only on the originating node.
> +      */
>   
The fevs_local_checks (this function) is only invoked at the node where 
the request originated.
So the part in the above comment about "done only on the originating 
node" does not really make sense.
The problem is rather that asynchronous requests dont have sinfo.
So access control can not be done for asynchronous requests.
There only relevant one I can think of is the asynchronous 
admin-operation.-invoke.
But we dont perform access control for the synchronous variant of 
admin-op (yet) so it should not
be a problem (yet).

/AndersBj

> +
>       switch (frwrd_evt.info.immnd.type) {
>  
>       case IMMND_EVT_A2ND_OBJ_MODIFY:
> -             if((strcmp(frwrd_evt.info.immnd.info.objModify.objectName.buf, 
> OPENSAF_IMM_OBJECT_DN) == 0) ||
> -                     
> (strcmp(frwrd_evt.info.immnd.info.objModify.objectName.buf, 
> "safRdn=immManagement,safApp=safImmService") == 0))
> -             {
> -                     /* Modifications to:
> -                        opensafImm=opensafImm,safApp=safImmService
> -                        or:
> -                        safRdn=immManagement,safApp=safImmService
> -                        are only allowed for root users.
> +             if ((sinfo != NULL) &&
> +                     
> ((strcmp(frwrd_evt.info.immnd.info.objModify.objectName.buf,
> +                             OPENSAF_IMM_OBJECT_DN) == 0) ||
> +                     
> (strcmp(frwrd_evt.info.immnd.info.objModify.objectName.buf,
> +                             "safRdn=immManagement,safApp=safImmService") == 
> 0))) {
> +                     /* Modifications to IMM service objects are only 
> allowed for root
> +                      * users and same group as me. Except for access 
> control settings
> +                      * which are only allowed by root.
>                       */
> -                     if(uid) {
> -                             struct passwd *pwd = getpwuid(uid);
> +                     if ((sinfo->uid > 0) && (sinfo->gid != getgid())) {
> +                             struct passwd *pwd = getpwuid(sinfo->uid);
>                               if (pwd != NULL) {
> -                                     syslog(LOG_AUTH, "Modifications to imm 
> service objects denied for %s(uid=%d)",
> -                                             pwd->pw_name, uid);
> +                                     syslog(LOG_AUTH,
> +                                             "Modifications to imm service 
> objects denied for %s(uid=%d)",
> +                                             pwd->pw_name, sinfo->uid);
>                               }
>                               error = SA_AIS_ERR_ACCESS_DENIED;
>                               goto done;
> -                     }
> +                     } else if (sinfo->uid > 0) {
> +                             // non root and same group as me, disallow 
> access control changes
> +                             const IMMSV_ATTR_MODS_LIST *attrMod =
> +                                     
> frwrd_evt.info.immnd.info.objModify.attrMods;
> +                             while (attrMod != NULL) {
> +                                     if 
> ((strcmp(attrMod->attrValue.attrName.buf,
> +                                                     
> OPENSAF_IMM_ACCESS_CONTROL_MODE) == 0) ||
> +                                             
> (strcmp(attrMod->attrValue.attrName.buf,
> +                                                     
> OPENSAF_IMM_AUTHORIZED_GROUP) == 0)) {
> +                                             struct passwd *pwd = 
> getpwuid(sinfo->uid);
> +                                             if (pwd != NULL)
> +                                                     syslog(LOG_AUTH,
> +                                                             "change of %s 
> denied for %s(uid=%d)",
> +                                                             
> attrMod->attrValue.attrName.buf, pwd->pw_name,
> +                                                             sinfo->uid);
> +                                             error = 
> SA_AIS_ERR_ACCESS_DENIED;
> +                                             goto done;
> +                                     }
> +                                     attrMod = attrMod->next;
> +                             }
> +                     } else
> +                             ; // modifications by root are OK
>               }
>               /* intentional fall through. */
>       case IMMND_EVT_A2ND_OBJ_CREATE:
> @@ -3349,11 +3378,11 @@ static SaAisErrorT immnd_fevs_local_chec
>                         Because of the very special and powerful nature of 
> this operation, only
>                         root users should be allowed to use it, when acces 
> control is enabled.
>                       */
> -                     if(uid) {
> -                             struct passwd *pwd = getpwuid(uid);
> +                     if ((sinfo != NULL) && (sinfo->uid > 0)) {
> +                             struct passwd *pwd = getpwuid(sinfo->uid);
>                               if (pwd != NULL) {
>                                       syslog(LOG_AUTH, 
> "saImmOmAdminOwnerClear denied for %s(uid=%d)",
> -                                             pwd->pw_name, uid);
> +                                             pwd->pw_name, sinfo->uid);
>                               }
>                               error = SA_AIS_ERR_ACCESS_DENIED;
>                               goto done;
>   


------------------------------------------------------------------------------
Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer
Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports
Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper
Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer
http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to