Bug 1441669

Summary: cibadmin can return a CIB not conforming to the schema if deny ACLs are in use
Product: Red Hat Enterprise Linux 8 Reporter: Tomas Jelinek <tojeline>
Component: pacemakerAssignee: Ken Gaillot <kgaillot>
Status: CLOSED WONTFIX QA Contact: cluster-qe <cluster-qe>
Severity: high Docs Contact:
Priority: low    
Version: 8.3CC: cluster-maint, mnovacek
Target Milestone: rc   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-12-01 07:28:30 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Tomas Jelinek 2017-04-12 12:26:26 UTC
Description of problem:
Cibadmin can return a CIB not conforming to the schema if deny ACLs are in use.


Version-Release number of selected component (if applicable):
pacemaker-1.1.16-6.el7.x86_64


How reproducible:
always, easily


Steps to Reproduce:
[root@rh73-node1:~]# pcs resource create dummy ocf:pacemaker:Dummy
[root@rh73-node1:~]# pcs acl user create user1
[root@rh73-node1:~]# pcs acl role create role1
[root@rh73-node1:~]# pcs acl permission add role1 write xpath /
[root@rh73-node1:~]# pcs acl permission add role1 deny id dummy
[root@rh73-node1:~]# pcs acl role assign role1 user user1
[root@rh73-node1:~]# pcs acl enable
[root@rh73-node1:~]# cibadmin -Q --scope acls
<acls>
  <acl_target id="user1">
    <role id="role1"/>
  </acl_target>
  <acl_role id="role1">
    <acl_permission id="role1-write" kind="write" xpath="/"/>
    <acl_permission id="role1-deny" kind="deny" reference="dummy"/>
  </acl_role>
</acls>

# now switch to user1
[user1@rh73-node1 ~]$ pcs cluster cib > cib.xml
[user1@rh73-node1 ~]$ crm_verify --verbose --xml-file cib.xml
cib.xml:57: element acl_permission: validity error : IDREF attribute reference references an unknown ID "dummy"
   error: main: CIB did not pass DTD/schema validation
Errors found during check: config not valid
[user1@rh73-node1 ~]$ crm_verify --verbose --live-check
element acl_permission: validity error : IDREF attribute reference references an unknown ID "dummy"
   error: main: CIB did not pass DTD/schema validation
Errors found during check: config not valid


Actual results:
Cibadmin returns a CIB which does not conform to the schema.


Expected results:
Cibadmin always returns a CIB which does conform to the schema.
When filtering out pieces of CIB based on the configured ACLs, everything referencing the filtered out pieces should be filtered out as well.


Additional info:
The problem is dummy resource got removed based on the configured ACLs but the ACLs referencing dummy resource were left in the CIB. Similar situation happens when dummy resource is referenced in constraints. And there is more as Ken pointed out at [bug 1441332 comment 1].


Consequences:
Pcs cannot in any way work witch such a CIB. Pcs is fetching the CIB via cibadmin (even when the CIB is in a file) and cibadmin just returns an error:

[user1@rh73-node1 ~]$ pcs -f cib.xml resource --full --debug
Running: /usr/sbin/cibadmin -l -Q
Return Value: 203
--Debug Output Start--
cibadmin: Connection to local file 'cib.xml' failed: Update does not conform to the configured schema
Signon to CIB failed: Update does not conform to the configured schema
Init failed, could not perform requested operations
--Debug Output End--

Error: unable to get cib
Error: unable to get cib

Comment 3 Ken Gaillot 2017-04-12 17:38:58 UTC
This is a thorny problem, but we can probably come up with something better than we have now.

There isn't a solution to cibadmin providing a partial (possibly invalid) CIB to users with limited read access to the CIB. That is inherent in the concept of ACLs. However, we may be able to work around this for particular use cases.

One possibility is to have the cib daemon support a validation IPC call. crm_verify --live-check could use this, rather than retrieve the live CIB and check the result. That doesn't help using crm_verify with a file, though.

Another possibility is that cibadmin could potentially mark its output in some way as being partial, when that's the case. Then callers such as pcs could know whether it's reasonable to try to validate it.

Comment 4 Andrew Beekhof 2017-04-18 00:17:21 UTC
(In reply to Ken Gaillot from comment #3)
> This is a thorny problem, but we can probably come up with something better
> than we have now.
> 
> There isn't a solution to cibadmin providing a partial (possibly invalid)
> CIB to users with limited read access to the CIB. That is inherent in the
> concept of ACLs. However, we may be able to work around this for particular
> use cases.
> 
> One possibility is to have the cib daemon support a validation IPC call.
> crm_verify --live-check could use this, rather than retrieve the live CIB
> and check the result. That doesn't help using crm_verify with a file, though.
> 
> Another possibility is that cibadmin could potentially mark its output in
> some way as being partial, when that's the case. Then callers such as pcs
> could know whether it's reasonable to try to validate it.

I think both of these are ideas worth implementing

Comment 5 Jan Pokorný [poki] 2017-04-18 14:22:15 UTC
re [comment 3]:

> Another possibility is that cibadmin could potentially mark its
> output in some way as being partial, when that's the case. Then
> callers such as pcs could know whether it's reasonable to try to
> validate it.

Relatively easy way out building upon this idea would be, as it's
so far deemed just a task how to deal with:
- broken referential integrity when pointed-to parts are missing, and
- broken schema validity because required parts are missing,
to have "relaxed" counterparts, preferably having a unified pattern
like relaxed-pacemaker-2.7, for particular schema versions.

These would be originals modified like this
- get rid of xsd:ID and xsd:IDREF semantically encumbered datatypes,
  lexical validity can still be enforced through the regular
  expression facet of XML Schema Datatypes to be mounted there
  instead
- get rid of any hard enforcement of 1+ cardinality element occurrence
  (oneOrMore -> zeroOrMore, optional, etc.)

It should be straightforward to craft a XSL transformation for this,
allowing the "relaxed" schemas to be ad-hoc generated when building
pacemaker.  With slight changes, it would be compatible with
crm_verify, and there would be valid document in the eyes of pcs.
"Relax" schemas would still be frowned upon in actual resulting
CIBs for pacemaker to work with at it's most fundamental,
live-synchronized level.

Comment 6 Jan Pokorný [poki] 2017-04-19 19:58:25 UTC
re [comment 5]:
I forgot to add that, naturally, information the CIB is (potentially?)
partial due to ACL-based filtering of the tree is then expressed by
accordingly set /cib/@validate-with (like the suggested
"relaxed-pacemaker-2.7").

There's a question whether this indication will be needed for the proper
subtree results (like just "/cib/configuration" one) passing, as they
also may be made incomplete due to the filtering.  One of the solutions
would be to (ab)use processing instructions (PI) as defined in XML spec:

  <?pcmk incomplete=yes?>
  <!-- the subtree itself -->

Comment 7 michal novacek 2017-08-04 09:52:06 UTC
qa-ack+: clear reproducer in initial commit

Comment 8 Ken Gaillot 2017-10-09 17:30:16 UTC
Due to time constraints, this will not make 7.5

Comment 11 RHEL Program Management 2020-12-01 07:28:30 UTC
After evaluating this issue, there are no plans to address it further or fix it in an upcoming release.  Therefore, it is being closed.  If plans change such that this issue will be fixed in an upcoming release, then the bug can be reopened.