Menu
▾
▴
[devel] [PATCH 0 of 3] Review Request for IMM: Protect non long DN capable OIs/appliers in callbacks [#1204]
|
From: Anders B. <and...@er...> - 2014-11-16 19:39:44
|
Summary: IMM: Detect long DN/RDN/SaNameT-attr for ccb-callbacks [#1204]
Review request for Trac Ticket(s): 1204
Peer Reviewer(s): Neel; Zoran
Pull request to:
Affected branch(es): 4.5; default(4.6)
Development branch: 4.5
--------------------------------
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 n
Samples n
Tests n
Other n
Comments (indicate scope for each "y" above):
---------------------------------------------
changeset 31583db04df533aa044b977c7e30d755e86c119e
Author: Anders Bjornerstedt <and...@er...>
Date: Sun, 16 Nov 2014 17:42:27 +0100
IMM: Detect long DN/parentDN, long RDN, long SaNameT attr for create
callback [#1204]
The IMMND server will screen the contents of every ccbObjectCreate for long
DNs or long RDN. If any long DN or RDN is detected, then the message sent to
the client (for the callback) will be a new (node local) message type. The
new message type conveys the same semantics as the old create-callback
message type but also flags that the callback has values that are not
compatible with any OI/applier that is not long DN capable.
The imma library uses this info to check if the client is long DN capable.
If the OI/applier is not capable. the lib does not generate the create
callback. Instead the library bounces internally with error directly to the
server. The CCB will fail (get aborted). For appliers, the dispatch loop is
broken and the handle is invalidated. THis is necessary because the applier
can not reply to the server. IF the main OI is long DN capable, or there is
no main OI, then the CCB can succeed in commit, yet the create callback
never reaches the applier. To avoid a silent breach of contract with such an
applier, the handle is instead stale-marked and BAD_HANDLE returned to the
applier.
This patch also adds a check that prevents long DNs from being created
before upgrade to 4.5 is complete (protocoll45Allowed()). The risk is small
but in theory some application could try to generate a long DN object while
the upgrade to 4.5 is in progress. Without this added check, the create
would succeed on upgraded nodes and fail on not yet upgraded nodes, casuing
the imm database to be inter-node inconsistent.
changeset df45842ca379c9844849e6668335acfcd544ba13
Author: Anders Bjornerstedt <and...@er...>
Date: Sun, 16 Nov 2014 18:57:53 +0100
IMM: Protect non long DN capable clients in modify callback [#1204]
Same fix as for the create callback but here for the modify callback. The
RDN issue is not relevant for the modify callback because the RDN attribute
will never be part of such a callback, it can not be modified.
The IMMND server will screen the contents of every ccbObjectModify for long
DNs. If any long DN is detected, then the message sent to the client (for
the callback) will be a new node local message type. Two cases need to be
caught. a) A modify of an SaNameT attribute trying to set a long DN. b) The
object itself may have been created with a long DN whith no OI attached.
Thus we have to detect the long DN case also. This is also relevant to
protect incapable appliers even if the OI is capable.
The new message type conveys the same semantics as the old modify-callback
message type but adds the information that the callback has values that are
not compatible with an OI that is not long DN capable.
The imma library uses this fact to check if the client is long DN capable.
If not it does not generate the callback. Instead the library bounces
internally with error directly to the server. The CCB will fail (get
aborted). For appliers, the dispatch loop is broken and the handle is
invalidated. This is necessary because the applier can not reply to the
server. IF the main OI is long DN capable, or there is no main OI, then the
CCB can succeed in commit, yet the create callback never reaches the
applier. To avoid a silent breach of contract with such an applier, the
handle is stale-marked and BAD_HANDLE returned to the applier.
changeset 2b324c94c741164c4ae235c93a9de616f379f5fc
Author: Anders Bjornerstedt <and...@er...>
Date: Sun, 16 Nov 2014 20:25:29 +0100
IMM: Protect non long DN capable clients in delete callback [#1204]
Same fix as for the create and modify callbacks but here for delete-cb. The
only thing relevant for the delete callback is the object DN.
The IMMND server will screen the DN of every ccbObjectDelete for having a
long DN. If this is detected, then the message sent to the client (for the
callback) will be a new node local message type. This is also relevant to
protect incapable appliers even if the OI is capable.
The new message type conveys the same semantics as the old delete-callback
message type but adds the information that the callback has a DN that is not
compatible with an OI that is not long DN capable.
The imma library uses this fact to check if the client is long DN capable.
If not it does not generate the callback. Instead the library bounces
internally with error directly to the server. The CCB will fail (get
aborted). For appliers, the dispatch loop is broken and the handle is
invalidated. This is necessary because the applier can not reply to the
server. If the main OI is long DN capable, or there is no main OI, then the
CCB can succeed in commit, yet the delete callback never reaches the
applier. To avoid a silent breach of contract with such an applier, the
handle is stale-marked and BAD_HANDLE returned to the applier.
Complete diffstat:
------------------
osaf/libs/agents/saf/imma/imma_proc.c | 208 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------
osaf/libs/agents/saf/imma/imma_proc.h | 1 +
osaf/libs/common/immsv/immsv_evt.c | 24 ++++++--
osaf/libs/common/immsv/include/immsv_evt.h | 3 +
osaf/services/saf/immsv/immnd/ImmModel.cc | 205 +++++++++++++++++++++++++++++++++++++++++++++----------------------------
osaf/services/saf/immsv/immnd/ImmModel.hh | 9 ++-
osaf/services/saf/immsv/immnd/immnd_evt.c | 42 +++++++++++---
osaf/services/saf/immsv/immnd/immnd_init.h | 6 +-
8 files changed, 362 insertions(+), 136 deletions(-)
Testing Commands:
-----------------
CAn be tested using immcfg on the om side and immapplier for the OI and applier roles.
immapplier by default does not have support for long DNs enabled, allowing it to be used
to test the negative cases. Support for long names is enabled by setting the environment
variable:
export SA_ENABLE_EXTENDED_NAMES=1
Testing, Expected Results:
--------------------------
Long DNs or long RDN attribute values (only relevant for ccbObjectCreate) shall not
reach the appliaiton callback. Instead the callback bounces back in the linrary to the server with
error.
Appliers can not reply with error, but a "silent treatment" would be a breach of the
API contract. Appliers not capable of long dNS that receiv a long DN callback will
be exited from the dispatch function with BAD_HANDLE. Te handle has been marked as stale
and exposed. This may seem harsh but I see no other solution. An applier should not
attach as applier to a class or object, if that applier is not capable of long DNs,
yet the object or some instances of the class contain long DNs.
Special applier and PBE are assumed to be long DN capable.
Conditions of Submission:
-------------------------
Ack from Neel.
Arch Built Started Linux distro
-------------------------------------------
mips n n
mips64 n n
x86 n n
x86_64 n n
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.
|
