RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 855121 - ricci: harmful handling of XML entities
Summary: ricci: harmful handling of XML entities
Keywords:
Status: CLOSED DUPLICATE of bug 855112
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: ricci
Version: 6.3
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: rc
: ---
Assignee: Jan Pokorný [poki]
QA Contact: Cluster QE
URL:
Whiteboard:
: 949566 (view as bug list)
Depends On: 858386
Blocks: 855112 855117 855120
TreeView+ depends on / blocked
 
Reported: 2012-09-06 18:19 UTC by Jan Pokorný [poki]
Modified: 2019-07-11 07:36 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of: 855112
Environment:
Last Closed: 2014-03-27 16:43:21 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 245025 1 None None None 2021-01-20 06:05:38 UTC
Red Hat Bugzilla 726065 0 medium CLOSED cman_tool -r version fails to validate cluster.conf file if it contains an ampersand '&' 2021-02-22 00:41:40 UTC
Red Hat Bugzilla 883900 1 None None None 2021-01-20 06:05:38 UTC
Red Hat Knowledge Base (Solution) 198233 0 None None None Never

Internal Links: 245025 726065 883900

Description Jan Pokorný [poki] 2012-09-06 18:19:45 UTC
+++ This bug was initially created as a clone of Bug #855112 +++

In connection to case discussed in the public cluster ML [1], we need
to make sure the user input that makes its way into XML (either as
cluster.conf or XML-formatted requests towards ricci) is XML-ready.
That is, the "unsafe" characters are encoded as XML entities.

Otherwise, such requests have no effect and unfortunately, neither
ricci provides a sensible diagnostics about this.  Example of such
input is the password for the initial authentization against ricci.

---

Specifically on the ricci's side, we need to be prepared for this
escaped input in order to convert it back where suitable (e.g.,
the mentioned case of authentication password).

---

[1] https://www.redhat.com/archives/linux-cluster/2012-August/msg00135.html

Comment 1 Jan Pokorný [poki] 2012-09-07 18:20:36 UTC
In fact, this seems to be an non-issue for ricci side as lon as
only &, <, >, ' and " are considered.

Then, only client-side work would be needed.

Comment 2 Jan Pokorný [poki] 2012-09-10 13:43:45 UTC
FWIW, a sort of unit test for the discussed XML entities un-/escaping
in ricci si now in upstream [2].  It was used to check that those
5 characters from [comment 1] are treated correctly.

[2] http://git.fedorahosted.org/cgit/conga.git/commit/?id=35c7af4

Comment 3 Jan Pokorný [poki] 2012-09-11 10:27:36 UTC
In fact, there is an issue with proactive skipping of already
present XML entities when performing the escaping as a part of
cooking the externalized XML.  Taking the circumstances into
account, we need to make sure there is not such extra treatment
and initial '&' of these entities is escaped as usual.

I proposed a (roughly tested) patch in (temporary) branch upstream:
http://git.fedorahosted.org/cgit/conga.git/commit/?h=bz855121

The (sort of) unit test is committed separately into the master:
http://git.fedorahosted.org/cgit/conga.git/commit/?id=35c7af4

Using this test (see also the commit message for the fix):

BEFORE:

ricci/test/xml_escape$ make check
./test-xmlescape | sed "s|\(.*\)|\1\n|g" | tee -a /dev/stderr | \
    ./test-xmlescape -i | sed "s|\(.*\)|\1\n|g"
foo&amp;bar
foo&amp;bar
foo&bar

Original information got changed within escape-unescape cycle.

AFTER:

$ make check
./test-xmlescape | sed "s|\(.*\)|\1\n|g" | tee -a /dev/stderr | \
    ./test-xmlescape -i | sed "s|\(.*\)|\1\n|g"
foo&amp;bar
foo&amp;amp;bar
foo&amp;bar

Information preserved.

Comment 5 Jan Pokorný [poki] 2012-09-11 15:50:11 UTC
Re [comment 3]:
This change may not be so important from common user-defined values
standpoint, but it becomes actual when the ricci password is generated
by some kind of PRNG and the resulting string, possibly containg
sequences like "&lt;", is just copied around whenever needed, without
any attention to its content.
This is the situation the patch helps to prevent.

Comment 6 Jan Pokorný [poki] 2012-09-18 20:27:52 UTC
In fact, the things are not got right on multiple levels,
"keen escaping" discussed in [comment 3] being one them.

I did a little experiment taking ricci codebase and playing with
common/XML.cpp and copying resulting ricci, ricci-worker, and modcluster
binaries in place of the ones from ricci-0.16.2-55.el6.x86_64.
Whenever un-/escaped string differed from its original, this was logged
into syslog for debugging purposes (see bellow).

Following conditional macros were used to enable/disable particular
parts of code:

INVERT: whether to use unescaping of the entities (i.e., use invert_chars
function); studying the behavior of xmllib2 tells that this is completely
unnecessary as this is done automatically (at least as far as attribute
values are concerned)

KEENESCAPE: whether to use original code or mimic the fix from [comment 3]

For this purpose, in relation to [comment 3], I put following XML snippet
into cluster.conf on the examined node:
> <fencedevices>
>   <fencedevice agent="fence_apc" ipaddr="localhost" login="root"
>                name="apcpowerswitch" passwd="foo&amp;amp;bar"/>
> </fencedevices>
then from the machine outside the cluster, I ran:
> ccs-overly -d -h mynode1 --getconf
which I used to verify for correctness of password form corresponding
to the snippet above (marked as CCS-RESULT below).

---

RICCI-COMPILE: INVERT=1 KEENESCAPE=1
ricci: ricci-xml: foo&amp;bar -> foo&bar
ricci[660]: Executing '/usr/libexec/ricci/ricci-worker
            -f /var/lib/ricci/queue/1168596506'
modcluster: ricci-xml: foo&amp;bar -> foo&bar
modcluster: ricci-xml: foo&bar => foo&amp;bar
ricci-worker: ricci-xml: foo&bar => foo&amp;bar
ricci: ricci-xml: foo&bar => foo&amp;bar

CCS-RESULT: foo&amp;bar

---

RICCI-COMPILE: INVERT=1 KEENESCAPE=0
ricci: ricci-xml: foo&amp;bar -> foo&bar
ricci[1926]: Executing '/usr/libexec/ricci/ricci-worker
             -f /var/lib/ricci/queue/1778838431'
modcluster: ricci-xml: foo&amp;bar -> foo&bar
modcluster: ricci-xml: foo&bar => foo&amp;bar
ricci-worker: ricci-xml: foo&bar => foo&amp;bar
ricci: ricci-xml: foo&bar => foo&amp;bar

CCS-RESULT: foo&amp;bar

---

RICCI-COMPILE: INVERT=0 KEENESCAPE=1
ricci[3163]: Executing '/usr/libexec/ricci/ricci-worker
             -f /var/lib/ricci/queue/521582756'
ricci-worker: ricci-xml: foo&bar => foo&amp;bar
ricci: ricci-xml: foo&bar => foo&amp;bar

CCS-RESULT: foo&amp;bar

---

RICCI-COMPILE: INVERT=0 KEENESCAPE=0
ricci[4309]: Executing '/usr/libexec/ricci/ricci-worker
             -f /var/lib/ricci/queue/1490636628'
modcluster: ricci-xml: foo&amp;bar => foo&amp;amp;bar
ricci-worker: ricci-xml: foo&amp;bar => foo&amp;amp;bar
ricci: ricci-xml: foo&amp;bar => foo&amp;amp;bar

CCS-RESULT: foo&amp;amp;bar

---

Based on this, the conclusion is:

1/ patch proposed in [comment 3] is half-way to a desired behavior

2/ there is no need for and it is even harmful to unescape entities
   (call invert_chars) as the attribute values provided by xmllib2
   are already unescaped ("raw" as opposed to "mangled for XML well-
   -formedness")

3/ modcluster package (clustermon BZ component) is also affected
   -> [bug 858386]

Action items: 2/ and 3/ (will solve these shortly)

Comment 7 Jan Pokorný [poki] 2012-09-24 18:17:55 UTC
Based on [comment 6], I committed another changeset into referred branch
bz855121.

In addition, the version of top-level protocol was bumped to 1.0 -> 1.1.
Provided that it is likely more doable to update client components then
server one in usual deployments (n.b. very often different machines),
this allows for backward-compatible workaround of buggy versions of
ricci.  These are to be implemented as part of already opened bugs:

- ccs:      [bug 855117]
- ccs_sync: [bug 855120]
- luci:     [bug 855112]

Comment 11 Chris Feist 2012-10-12 18:31:11 UTC
Bumping to 6.5 since we're still waiting on an acked patch.

Comment 14 Shane Bradley 2013-09-17 14:09:01 UTC
*** Bug 949566 has been marked as a duplicate of this bug. ***

Comment 17 Fabio Massimo Di Nitto 2014-03-27 16:43:21 UTC

*** This bug has been marked as a duplicate of bug 855112 ***


Note You need to log in before you can comment on or make changes to this bug.