Ok. By "runtime configure option" I assume you simply mean a new configuration attribute in some configuration object, existing or new.
The question is which class it should reside in? In the context of just this patch, it would make sense to add it to the class for 'opensafImm=opensafImm,safApp=safImmService' which is 'OpensafImm'. The only minor issue with that, is when we add the corresponding support in other services, then one could argue that the config attribute should really be in some more generic opensaf object/class. But I dont think we have any such thing. Ticket #897 already added a config attributge to this class for OpenSAF 4.5 so you may Just add it to samples/immsv/OpensafImm_Upgrade_4.5.xml /AndersBj -----Original Message----- From: Hans Feldt Sent: den 12 augusti 2014 10:48 To: Anders Björnerstedt; [email protected]; [email protected]; [email protected] Cc: [email protected] Subject: RE: [PATCH 0 of 3] Review Request for MDS #554 & IMM #938 My idea is to: - add a configure option that will change the default behavior of access control from ON to OFF - add an IMM runtime configure option to enable access control I will work on this additional patch now. I also would like to get an opinion from TLC about the default IMM access control mode. I suggest ON. /Hans > -----Original Message----- > From: Anders Björnerstedt > Sent: den 12 augusti 2014 10:39 > To: Hans Feldt; [email protected]; [email protected]; > [email protected] > Cc: [email protected] > Subject: RE: [PATCH 0 of 3] Review Request for MDS #554 & IMM #938 > > Ack from me on this. > > I have reviewed earlier unofficial patches for this so have not scrutinized > this version. > > The only hesitation I have is if the default configure for this feature > should be *on*. > If the effects of having it on are non intrusive for legacy deployments then > default should be on. > But if deployments require a lot of addaption, when this is on, then default > should be off. > > If this is seen as the most reasonable approach, then the main thing > is to get this delivered to OpenSAF 4.5 so that at least some deployments > could start prototyping with it. > > /AndersBj > > > -----Original Message----- > From: Hans Feldt > Sent: den 3 juli 2014 11:19 > To: Anders Björnerstedt; [email protected]; > [email protected]; [email protected] > Cc: [email protected] > Subject: [PATCH 0 of 3] Review Request for MDS #554 & IMM #938 > > Summary: IMM Access control > Review request for Trac Ticket(s): 554 & 938 Peer Reviewer(s): MDS and > IMM maintainers Pull request to: <<LIST THE PERSON WITH PUSH ACCESS > HERE>> Affected branch(es): default Development branch: <<IF ANY GIVE > THE REPO URL>> > > -------------------------------- > Impacted area Impact y/n > -------------------------------- > Docs n > Build system n > RPM/packaging n > Configuration files n > Startup scripts n > SAF services y > OpenSAF services n > Core libraries y > Samples n > Tests n > Other n > > > Comments (indicate scope for each "y" above): > --------------------------------------------- > Right now access control is on by default. Probably we need to add a big > on/off configuration option. > > In the base patch there is a slight change compared to the one sent > on review before. The server socket is created before the thread is started > and handed over to the server thread. This is to eliminate possible race > conditions. > > changeset af43561fa4737833609f5374dc2dd728e48db536 > Author: Hans Feldt <[email protected]> > Date: Thu, 03 Jul 2014 10:44:30 +0200 > > base: add secutil for sending, receiving over a UNIX socket [#554] > > This patch add some infrastructure that can be used by other services to > securely get credentials for a client. > > When initialized on the server side a tiny server thread is created that > listen to a named connection oriented UNIX socket. For each incoming > connection request it calls a user specified callback and handover file > descriptor and peer credentials. > > A typical use case is for a library to create the MDS socket and then > use > the client side function to send the MDS address as a data messages. The > client credentials can be stored on the server side and verified when > needed. > > Note each received connection less message needs to be checked with the > original received MDS address so no spoofing (handle hijacking) is > happening. > > changeset 63a51ca71438fbc219cb418962cf622b8bc0c429 > Author: Hans Feldt <[email protected]> > Date: Thu, 03 Jul 2014 10:45:34 +0200 > > mds: use secutil to provide optional client authentication [#554] > > Three new environment variables are now supported: > > MDS_SOCK_SERVER_CREATE - When this variable is defined in the > environment, a > server socket will be created. Used on the server side. > > MDS_SOCK_SERVER_NAME - Name of the server socket (used to bind the > name) > > MDS_SOCK_SERVER_CONNECT - When this variable is defined in the > environment, > a client will try to connect to a server and securely exchange its MDS > address. > > Other changes: > - The MDS_CALLBACK_RECEIVE_INFO structure has added pid, uid & gid > members. > They will have valid values on the server side when the above mentioned > has > been configured properly. This is an MDS backwards compatible interface > change, data has been added. > > - A new MDS database maintained on the server side, MDS_DEST is the key. > Each entry represents a node local MDS core (process). > > changeset 960eb531b6edec24d6dec6601b465371c0dae486 > Author: Hans Feldt <[email protected]> > Date: Thu, 03 Jul 2014 10:47:48 +0200 > > immnd: add support for client authorization [#938] > > 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. > > > Added Files: > ------------ > osaf/libs/core/common/include/osaf_secutil.h > osaf/libs/core/common/osaf_secutil.c > > > Complete diffstat: > ------------------ > osaf/libs/agents/saf/imma/imma_init.c | 5 ++ > osaf/libs/common/immsv/include/immsv_evt.h | 3 + > osaf/libs/core/common/Makefile.am | 3 +- > osaf/libs/core/common/include/Makefile.am | 3 +- > osaf/libs/core/common/include/osaf_secutil.h | 83 > +++++++++++++++++++++++++++++++++++++ > osaf/libs/core/common/osaf_secutil.c | 306 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++++++++++++++++++++++++ > osaf/libs/core/include/mds_papi.h | 4 +- > osaf/libs/core/mds/include/mds_core.h | 15 ++++++ > osaf/libs/core/mds/include/mds_dt2c.h | 3 + > osaf/libs/core/mds/mds_c_api.c | 23 ++++++++++ > osaf/libs/core/mds/mds_c_db.c | 28 ++++++++++++ > osaf/libs/core/mds/mds_c_sndrcv.c | 4 +- > osaf/libs/core/mds/mds_dt_common.c | 11 ++++ > osaf/libs/core/mds/mds_main.c | 134 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 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 + > 18 files changed, 656 insertions(+), 12 deletions(-) > > > Testing Commands: > ----------------- > Build a default configured opensaf. > Start opensaf > As a normal user access IMM with e.g. immfind As root access IMM > Reconfigure opensaf, in immnd.conf add (on ubuntu "adm" > exist): > export IMM_ADMIN_GROUP_NAME=adm > Restart opensaf > run command with group adm: > sudo -g adm immfind > > Testing, Expected Results: > -------------------------- > Opensaf starts meaning other processes in the same group has access > root can use IMM A normal user cannot do e.g. immfind Unauthorized > access logged to (normally) /var/log/auth user of group adm can > access IMM > > > Conditions of Submission: > ------------------------- > ack from maintainers > > > Arch Built Started Linux distro > ------------------------------------------- > mips n n > mips64 n n > x86 n n > x86_64 y y ubuntu14.04 > powerpc n n > powerpc64 n n > > > Reviewer Checklist: > ------------------- > [Submitters: make sure that your review doesn't trigger any > checkmarks!] > > > Your checkin has not passed review because (see checked entries): > > ___ Your RR template is generally incomplete; it has too many blank entries > that need proper data filled in. > > ___ You have failed to nominate the proper persons for review and push. > > ___ Your patches do not have proper short+long header > > ___ You have grammar/spelling in your header that is unacceptable. > > ___ You have exceeded a sensible line length in your headers/comments/text. > > ___ You have failed to put in a proper Trac Ticket # into your commits. > > ___ You have incorrectly put/left internal data in your comments/files > (i.e. internal bug tracking tool IDs, product names etc) > > ___ You have not given any evidence of testing beyond basic build tests. > Demonstrate some level of runtime or other sanity testing. > > ___ You have ^M present in some of your files. These have to be removed. > > ___ You have needlessly changed whitespace or added whitespace crimes > like trailing spaces, or spaces before tabs. > > ___ You have mixed real technical changes with whitespace and other > cosmetic code cleanup changes. These have to be separate commits. > > ___ You need to refactor your submission into logical chunks; there is > too much content into a single commit. > > ___ You have extraneous garbage in your review (merge commits etc) > > ___ You have giant attachments which should never have been sent; > Instead you should place your content in a public tree to be pulled. > > ___ You have too many commits attached to an e-mail; resend as threaded > commits, or place in a public tree for a pull. > > ___ You have resent this content multiple times without a clear indication > of what has changed between each re-send. > > ___ You have failed to adequately and individually address all of the > comments and change requests that were proposed in the initial review. > > ___ You have a misconfigured ~/.hgrc file (i.e. username, email etc) > > ___ Your computer have a badly configured date and time; confusing the > the threaded patch review. > > ___ Your changes affect IPC mechanism, and you don't present any results > for in-service upgradability test. > > ___ Your changes affect user manual and documentation, your patch series > do not contain the patch that updates the Doxygen manual. ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
