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
